QNAM HTTP: Pair channels with requests at a later state.

Connect a new channel only when there is no one currently available.
Only pair the request with the channel once we know if it got connected
or not. This leads to faster sending on requests as we reuse already
connected channels that became free in the meanwhile.

Task-number: QTBUG-17084
Reviewed-by: Markus Goetz
Reviewed-by: Peter Hartmann

3 files changed, 48 insertions, 24 deletions

diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp
index 35c3a67..29ae5b0 100644
--- a/src/network/access/qhttpnetworkconnection.cpp
+++ b/src/network/access/qhttpnetworkconnection.cpp
@@ -487,7 +487,7 @@ void QHttpNetworkConnectionPrivate::requeueRequest(const HttpMessagePair &pair)
     QMetaObject::invokeMethod(q, "_q_startNextRequest", Qt::QueuedConnection);
 }
 
-void QHttpNetworkConnectionPrivate::dequeueAndSendRequest(QAbstractSocket *socket)
+bool QHttpNetworkConnectionPrivate::dequeueRequest(QAbstractSocket *socket)
 {
     Q_ASSERT(socket);
 
@@ -500,8 +500,7 @@ void QHttpNetworkConnectionPrivate::dequeueAndSendRequest(QAbstractSocket *socke
             prepareRequest(messagePair);
         channels[i].request = messagePair.first;
         channels[i].reply = messagePair.second;
-        channels[i].sendRequest();
-        return;
+        return true;
     }
 
     if (!lowPriorityQueue.isEmpty()) {
@@ -511,9 +510,9 @@ void QHttpNetworkConnectionPrivate::dequeueAndSendRequest(QAbstractSocket *socke
             prepareRequest(messagePair);
         channels[i].request = messagePair.first;
         channels[i].reply = messagePair.second;
-        channels[i].sendRequest();
-        return;
+        return true;
     }
+    return false;
 }
 
 // this is called from _q_startNextRequest and when a request has been sent down a socket from the channel
@@ -784,17 +783,8 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest()
     // try to get a free AND connected socket
     for (int i = 0; i < channelCount; ++i) {
         if (!channels[i].reply && !channels[i].isSocketBusy() && channels[i].socket->state() == QAbstractSocket::ConnectedState) {
-            dequeueAndSendRequest(channels[i].socket);
-        }
-    }
-
-    // return fast if there is nothing to do
-    if (highPriorityQueue.isEmpty() && lowPriorityQueue.isEmpty())
-        return;
-    // try to get a free unconnected socket
-    for (int i = 0; i < channelCount; ++i) {
-        if (!channels[i].reply && !channels[i].isSocketBusy()) {
-            dequeueAndSendRequest(channels[i].socket);
+            if (dequeueRequest(channels[i].socket))
+                channels[i].sendRequest();
         }
     }
 
@@ -811,6 +801,21 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest()
     for (int i = 0; i < channelCount; i++)
         if (channels[i].socket->state() == QAbstractSocket::ConnectedState)
             fillPipeline(channels[i].socket);
+
+    // If there is not already any connected channels we need to connect a new one.
+    // We do not pair the channel with the request until we know if it is 
+    // connected or not. This is to reuse connected channels before we connect new once.
+    int queuedRequest = highPriorityQueue.count() + lowPriorityQueue.count();
+    for (int i = 0; i < channelCount; ++i) {
+        if (channels[i].socket->state() == QAbstractSocket::ConnectingState)
+            queuedRequest--;
+        if ( queuedRequest <=0 )
+            break;
+        if (!channels[i].reply && !channels[i].isSocketBusy() && (channels[i].socket->state() == QAbstractSocket::UnconnectedState)) {
+            channels[i].ensureConnection();
+            queuedRequest--;
+        }
+    }
 }
 
 
diff --git a/src/network/access/qhttpnetworkconnection_p.h b/src/network/access/qhttpnetworkconnection_p.h
index d4748c1..874ea22 100644
--- a/src/network/access/qhttpnetworkconnection_p.h
+++ b/src/network/access/qhttpnetworkconnection_p.h
@@ -161,7 +161,7 @@ public:
 
     QHttpNetworkReply *queueRequest(const QHttpNetworkRequest &request);
     void requeueRequest(const HttpMessagePair &pair); // e.g. after pipeline broke
-    void dequeueAndSendRequest(QAbstractSocket *socket);
+    bool dequeueRequest(QAbstractSocket *socket);
     void prepareRequest(HttpMessagePair &request);
 
     void fillPipeline(QAbstractSocket *socket);
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index 60094b0..23dd518 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -502,6 +502,7 @@ void QHttpNetworkConnectionChannel::_q_receiveReply()
 // called when unexpectedly reading a -1 or when data is expected but socket is closed
 void QHttpNetworkConnectionChannel::handleUnexpectedEOF()
 {
+    Q_ASSERT(reply);
     if (reconnectAttempts <= 0) {
         // too many errors reading/receiving/parsing the status, close the socket and emit error
         requeueCurrentlyPipelinedRequests();
@@ -524,7 +525,8 @@ bool QHttpNetworkConnectionChannel::ensureConnection()
 
     // resend this request after we receive the disconnected signal
     if (socketState == QAbstractSocket::ClosingState) {
-        resendCurrent = true;
+        if (reply)
+            resendCurrent = true;
         return false;
     }
 
@@ -646,6 +648,7 @@ bool QHttpNetworkConnectionChannel::expand(bool dataComplete)
 
 void QHttpNetworkConnectionChannel::allDone()
 {
+    Q_ASSERT(reply);
 #ifndef QT_NO_COMPRESS
     // expand the whole data.
     if (reply->d_func()->expectContent() && reply->d_func()->autoDecompress && !reply->d_func()->streamEnd) {
@@ -741,6 +744,7 @@ void QHttpNetworkConnectionChannel::allDone()
 
 void QHttpNetworkConnectionChannel::detectPipeliningSupport()
 {
+    Q_ASSERT(reply);
     // detect HTTP Pipelining support
     QByteArray serverHeaderField;
     if (
@@ -824,6 +828,7 @@ void QHttpNetworkConnectionChannel::handleStatus()
 
 bool QHttpNetworkConnectionChannel::resetUploadData()
 {
+    Q_ASSERT(reply);
     QNonContiguousByteDevice* uploadByteDevice = request.uploadByteDevice();
     if (!uploadByteDevice)
         return true;
@@ -881,7 +886,8 @@ void QHttpNetworkConnectionChannel::closeAndResendCurrentRequest()
 {
     requeueCurrentlyPipelinedRequests();
     close();
-    resendCurrent = true;
+    if (reply)
+        resendCurrent = true;
     if (qobject_cast<QHttpNetworkConnection*>(connection))
         QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
 }
@@ -983,10 +989,10 @@ void QHttpNetworkConnectionChannel::_q_connected()
     //channels[i].reconnectAttempts = 2;
     if (!pendingEncrypt) {
         state = QHttpNetworkConnectionChannel::IdleState;
+        if (!reply)
+            connection->d_func()->dequeueRequest(socket);
         if (reply)
             sendRequest();
-        else
-            close();
     }
 }
 
@@ -1040,6 +1046,9 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
     QPointer<QHttpNetworkConnection> that = connection;
     QString errorString = connection->d_func()->errorDetail(errorCode, socket, socket->errorString());
 
+    // Need to dequeu the request so that we can emit the error.
+    if (!reply)
+        connection->d_func()->dequeueRequest(socket);
     if (reply) {
         reply->d_func()->errorString = errorString;
         emit reply->finishedWithError(errorCode, errorString);
@@ -1054,7 +1063,11 @@ void QHttpNetworkConnectionChannel::_q_error(QAbstractSocket::SocketError socket
 #ifndef QT_NO_NETWORKPROXY
 void QHttpNetworkConnectionChannel::_q_proxyAuthenticationRequired(const QNetworkProxy &proxy, QAuthenticator* auth)
 {
-    connection->d_func()->emitProxyAuthenticationRequired(this, proxy, auth);
+    // Need to dequeue the request before we can emit the error.
+    if (!reply)
+        connection->d_func()->dequeueRequest(socket);
+    if (reply)
+        connection->d_func()->emitProxyAuthenticationRequired(this, proxy, auth);
 }
 #endif
 
@@ -1070,7 +1083,10 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
         return; // ### error
     state = QHttpNetworkConnectionChannel::IdleState;
     pendingEncrypt = false;
-    sendRequest();
+    if (!reply)
+        connection->d_func()->dequeueRequest(socket);
+    if (reply)
+        sendRequest();
 }
 
 void QHttpNetworkConnectionChannel::_q_sslErrors(const QList<QSslError> &errors)
@@ -1081,7 +1097,10 @@ void QHttpNetworkConnectionChannel::_q_sslErrors(const QList<QSslError> &errors)
     // Also pause the connection because socket notifiers may fire while an user
     // dialog is displaying
     connection->d_func()->pauseConnection();
-    emit reply->sslErrors(errors);
+    if (pendingEncrypt && !reply)
+        connection->d_func()->dequeueRequest(socket);
+    if (reply)
+        emit reply->sslErrors(errors);
     connection->d_func()->resumeConnection();
 }