QAbstractHttpServer: Simplify handleNewConection/handleReadyRead

Remove usage of QObjectUserData. Remove usage of QHash of QHttpServerRequest. Change-Id: I8a6c44bcfefc12c841ae67562e9bbec10f6ab9bf Reviewed-by: Mårten Nordheim <[email protected]>
penguin806 · Jul 9, 2019 · fb551f9 · fb551f9
1 parent ec8a569
commit fb551f9
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 50 deletions.
diff --git a/src/httpserver/qabstracthttpserver.cpp b/src/httpserver/qabstracthttpserver.cpp
@@ -47,77 +47,64 @@ QT_BEGIN_NAMESPACE
 
 Q_LOGGING_CATEGORY(lcHttpServer, "qt.httpserver")
 
-#if !defined(QT_NO_USERDATA)
-QAtomicInt QAbstractHttpServerPrivate::userDataId = -1;
-#endif
-
 QAbstractHttpServerPrivate::QAbstractHttpServerPrivate()
 {
-#if !defined(QT_NO_USERDATA)
- userDataId.testAndSetRelaxed(-1, int(QObject::registerUserData()));
-#endif
 }
 
 void QAbstractHttpServerPrivate::handleNewConnections()
 {
  Q_Q(QAbstractHttpServer);
  auto tcpServer = qobject_cast<QTcpServer *>(q->sender());
  Q_ASSERT(tcpServer);
- while (auto *socket = tcpServer->nextPendingConnection()) {
+ while (auto socket = tcpServer->nextPendingConnection()) {
  auto request = new QHttpServerRequest; // TODO own tcp server could pre-allocate it
  http_parser_init(&request->d->httpParser, HTTP_REQUEST);
- connect(socket, &QTcpSocket::readyRead, this, &QAbstractHttpServerPrivate::handleReadyRead);
- socket->connect(socket, &QTcpSocket::disconnected, &QObject::deleteLater);
-#if !defined(QT_NO_USERDATA)
- socket->setUserData(uint(userDataId), request);
-#else
- QObject::connect(socket, &QTcpSocket::destroyed, [&]() {
- requests.remove(socket);
+
+ QObject::connect(socket, &QTcpSocket::readyRead, q_ptr,
+ [this, request, socket] () {
+ handleReadyRead(socket, request);
+ });
+
+ QObject::connect(socket, &QTcpSocket::disconnected, &QObject::deleteLater);
+ QObject::connect(socket, &QObject::destroyed, [request] () {
+ delete request;
  });
- requests.insert(socket, request);
-#endif
  }
 }
 
-void QAbstractHttpServerPrivate::handleReadyRead()
+void QAbstractHttpServerPrivate::handleReadyRead(QTcpSocket *socket,
+ QHttpServerRequest *request)
 {
  Q_Q(QAbstractHttpServer);
- auto socket = qobject_cast<QTcpSocket *>(q->sender());
  Q_ASSERT(socket);
-#if !defined(QT_NO_USERDATA)
- auto request = static_cast<QHttpServerRequest *>(socket->userData(uint(userDataId)));
-#else
- auto request = requests[socket];
-#endif
- auto &requestPrivate = request->d;
+ Q_ASSERT(request);
 
  if (!socket->isTransactionStarted())
  socket->startTransaction();
 
- if (requestPrivate->state == QHttpServerRequestPrivate::State::OnMessageComplete)
- requestPrivate->clear();
+ if (request->d->state == QHttpServerRequestPrivate::State::OnMessageComplete)
+ request->d->clear();
 
- if (!requestPrivate->parse(socket)) {
+ if (!request->d->parse(socket)) {
  socket->disconnect();
  return;
  }
 
- if (!requestPrivate->httpParser.upgrade &&
- requestPrivate->state != QHttpServerRequestPrivate::State::OnMessageComplete)
+ if (!request->d->httpParser.upgrade &&
+ request->d->state != QHttpServerRequestPrivate::State::OnMessageComplete)
  return; // Partial read
 
- if (requestPrivate->httpParser.upgrade) { // Upgrade
- const auto &headers = requestPrivate->headers;
- const auto upgradeHash = requestPrivate->headerHash(QByteArrayLiteral("upgrade"));
+ if (request->d->httpParser.upgrade) { // Upgrade
+ const auto &headers = request->d->headers;
+ const auto upgradeHash = request->d->headerHash(QByteArrayLiteral("upgrade"));
  const auto it = headers.find(upgradeHash);
  if (it != headers.end()) {
 #if defined(QT_WEBSOCKETS_LIB)
  if (it.value().second.compare(QByteArrayLiteral("websocket"), Qt::CaseInsensitive) == 0) {
  static const auto signal = QMetaMethod::fromSignal(
  &QAbstractHttpServer::newWebSocketConnection);
  if (q->isSignalConnected(signal)) {
- disconnect(socket, &QTcpSocket::readyRead,
- this, &QAbstractHttpServerPrivate::handleReadyRead);
+ QObject::disconnect(socket, &QTcpSocket::readyRead, nullptr, nullptr);
  socket->rollbackTransaction();
  websocketServer.handleConnection(socket);
  Q_EMIT socket->readyRead();

diff --git a/src/httpserver/qabstracthttpserver_p.h b/src/httpserver/qabstracthttpserver_p.h
@@ -67,14 +67,9 @@ class Q_HTTPSERVER_EXPORT QAbstractHttpServerPrivate: public QObjectPrivate
  };
 #endif // defined(QT_WEBSOCKETS_LIB)
 
-#if !defined(QT_NO_USERDATA)
- static QAtomicInt userDataId;
-#else
- QHash<QTcpSocket *, QHttpServerRequest *> requests;
-#endif
-
  void handleNewConnections();
- void handleReadyRead();
+ void handleReadyRead(QTcpSocket *socket,
+ QHttpServerRequest *request);
 };
 
 QT_END_NAMESPACE

diff --git a/src/httpserver/qhttpserverrequest.cpp b/src/httpserver/qhttpserverrequest.cpp
@@ -249,12 +249,10 @@ int QHttpServerRequestPrivate::onChunkComplete(http_parser *httpParser)
 }
 
 QHttpServerRequest::QHttpServerRequest() :
- QObjectUserData(),
  d(new QHttpServerRequestPrivate)
 {}
 
 QHttpServerRequest::QHttpServerRequest(const QHttpServerRequest &other) :
- QObjectUserData(),
  d(other.d)
 {}
 

diff --git a/src/httpserver/qhttpserverrequest.h b/src/httpserver/qhttpserverrequest.h
@@ -44,15 +44,15 @@ class QString;
 class QTcpSocket;
 
 class QHttpServerRequestPrivate;
-class Q_HTTPSERVER_EXPORT QHttpServerRequest : public QObjectUserData
+class Q_HTTPSERVER_EXPORT QHttpServerRequest
 {
  friend class QAbstractHttpServerPrivate;
  friend class QHttpServerResponse;
 
  Q_GADGET
 
 public:
- ~QHttpServerRequest() override;
+ virtual ~QHttpServerRequest();
 
  enum class Method
  {

diff --git a/tests/auto/qabstracthttpserver/tst_qabstracthttpserver.cpp b/tests/auto/qabstracthttpserver/tst_qabstracthttpserver.cpp
@@ -147,12 +147,26 @@ void tst_QAbstractHttpServer::websocket()
  auto tcpServer = new QTcpServer;
  tcpServer->listen();
  server.bind(tcpServer);
- QWebSocket socket;
- const QUrl url(QString::fromLatin1("ws:https://localhost:%1").arg(tcpServer->serverPort()));
- socket.open(url);
+ auto makeWebSocket = [this, tcpServer] () mutable {
+ auto s = new QWebSocket(QString::fromUtf8(""),
+ QWebSocketProtocol::VersionLatest,
+ this);
+ const QUrl url(QString::fromLatin1("ws:https://localhost:%1").arg(tcpServer->serverPort()));
+ s->open(url);
+ return s;
+ };
+
+ // We have to send two requests to make sure that swapping between
+ // QTcpSocket and QWebSockets works correctly
+ auto s1 = makeWebSocket();
+ auto s2 = makeWebSocket();
+
  QSignalSpy newConnectionSpy(&server, &HttpServer::newWebSocketConnection);
- QTRY_COMPARE(newConnectionSpy.count(), 1);
+ QTRY_COMPARE(newConnectionSpy.count(), 2);
+ delete server.nextPendingWebSocketConnection();
  delete server.nextPendingWebSocketConnection();
+ delete s1;
+ delete s2;
 #endif // defined(QT_WEBSOCKETS_LIB)
 }