QAbstractHttpServer: Simplify handleNewConection/handleReadyRead

Remove usage of QObjectUserData.
Remove usage of QHash of QHttpServerRequest.

Change-Id: I8a6c44bcfefc12c841ae67562e9bbec10f6ab9bf
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

5 files changed, 44 insertions, 50 deletions

diff --git a/src/httpserver/qabstracthttpserver.cpp b/src/httpserver/qabstracthttpserver.cpp
index f21c8a9..f31e7b8 100644
--- a/src/httpserver/qabstracthttpserver.cpp
+++ b/src/httpserver/qabstracthttpserver.cpp
@@ -47,15 +47,8 @@ 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()
@@ -63,52 +56,47 @@ 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)
@@ -116,8 +104,7 @@ void QAbstractHttpServerPrivate::handleReadyRead()
                 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
index 4a0cd4b..69732ff 100644
--- a/src/httpserver/qabstracthttpserver_p.h
+++ b/src/httpserver/qabstracthttpserver_p.h
@@ -67,14 +67,9 @@ public:
     };
 #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
index 7500ede..4b0b1e1 100644
--- 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
index 3d1210c..11754b2 100644
--- a/src/httpserver/qhttpserverrequest.h
+++ b/src/httpserver/qhttpserverrequest.h
@@ -44,7 +44,7 @@ class QString;
 class QTcpSocket;
 
 class QHttpServerRequestPrivate;
-class Q_HTTPSERVER_EXPORT QHttpServerRequest : public QObjectUserData
+class Q_HTTPSERVER_EXPORT QHttpServerRequest
 {
     friend class QAbstractHttpServerPrivate;
     friend class QHttpServerResponse;
@@ -52,7 +52,7 @@ class Q_HTTPSERVER_EXPORT QHttpServerRequest : public QObjectUserData
     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
index 4f7f10e..bdb40a8 100644
--- 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://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://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)
 }