choke downloadProgress signals

The QNetworkReply::downloadProgress signal is intended for updating
user interface components (e.g. a progress bar).
Limit signal emissions to 10 times per second, with an additional
signal just before the finished() signal to provide the 100% progress.

For the size of download where a progress bar is necessary, this
update frequency seems sufficient.

The implementation is done by dropping signals which would be emitted
less than 100ms after the previous signal emission.

Task-number: QTBUG-20449
Change-Id: I9c2dbe16c70f3270cbf98f3c74cf9d9a3f0ab900
Reviewed-by: David Faure <faure@kde.org>
Reviewed-by: Markus Goetz <markus@woboq.com>
Reviewed-by: Martin Petersson <Martin.Petersson@nokia.com>

5 files changed, 40 insertions, 7 deletions

diff --git a/src/network/access/qnetworkreply.cpp b/src/network/access/qnetworkreply.cpp
index 3b6ec5d4b5..0e1aa1e0f2 100644
--- a/src/network/access/qnetworkreply.cpp
+++ b/src/network/access/qnetworkreply.cpp
@@ -45,6 +45,8 @@
 
 QT_BEGIN_NAMESPACE
 
+const int QNetworkReplyPrivate::progressSignalInterval = 100;
+
 QNetworkReplyPrivate::QNetworkReplyPrivate()
     : readBufferMaxSize(0),
       operation(QNetworkAccessManager::UnknownOperation),
diff --git a/src/network/access/qnetworkreply_p.h b/src/network/access/qnetworkreply_p.h
index 9c838a5539..04598fc712 100644
--- a/src/network/access/qnetworkreply_p.h
+++ b/src/network/access/qnetworkreply_p.h
@@ -57,6 +57,7 @@
 #include "qnetworkrequest_p.h"
 #include "qnetworkreply.h"
 #include "QtCore/qpointer.h"
+#include <QtCore/QElapsedTimer>
 #include "private/qiodevice_p.h"
 
 QT_BEGIN_NAMESPACE
@@ -69,6 +70,8 @@ public:
     QUrl url;
     QPointer<QNetworkAccessManager> manager;
     qint64 readBufferMaxSize;
+    QElapsedTimer downloadProgressSignalChoke;
+    const static int progressSignalInterval;
     QNetworkAccessManager::Operation operation;
     QNetworkReply::NetworkError errorCode;
     bool isFinished;
diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp
index 5be59e20ce..cf92f55cdf 100644
--- a/src/network/access/qnetworkreplyhttpimpl.cpp
+++ b/src/network/access/qnetworkreplyhttpimpl.cpp
@@ -888,6 +888,10 @@ void QNetworkReplyHttpImplPrivate::postRequest()
     delegate->moveToThread(thread);
     // This call automatically moves the uploadDevice too for the asynchronous case.
 
+    // Start timer for progress notifications
+    downloadProgressSignalChoke.start();
+
+
     // Send an signal to the delegate so it starts working in the other thread
     if (synchronous) {
         emit q->startHttpRequestSynchronously(); // This one is BlockingQueuedConnection, so it will return when all work is done
@@ -1022,8 +1026,11 @@ void QNetworkReplyHttpImplPrivate::replyDownloadData(QByteArray d)
     emit q->readyRead();
     // emit readyRead before downloadProgress incase this will cause events to be
     // processed and we get into a recursive call (as in QProgressDialog).
-    emit q->downloadProgress(bytesDownloaded,
+    if (downloadProgressSignalChoke.elapsed() >= progressSignalInterval) {
+        downloadProgressSignalChoke.restart();
+        emit q->downloadProgress(bytesDownloaded,
                              totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
+    }
 
 }
 
@@ -1181,7 +1188,10 @@ void QNetworkReplyHttpImplPrivate::replyDownloadProgressSlot(qint64 bytesReceive
     // processed and we get into a recursive call (as in QProgressDialog).
     if (bytesDownloaded > 0)
         emit q->readyRead();
-    emit q->downloadProgress(bytesDownloaded, bytesTotal);
+    if (downloadProgressSignalChoke.elapsed() >= progressSignalInterval) {
+        downloadProgressSignalChoke.restart();
+        emit q->downloadProgress(bytesDownloaded, bytesTotal);
+    }
 }
 
 void QNetworkReplyHttpImplPrivate::httpAuthenticationRequired(const QHttpNetworkRequest &request,
@@ -1623,8 +1633,11 @@ void QNetworkReplyHttpImplPrivate::_q_cacheLoadReadyRead()
 
     // This readyRead() goes to the user. The user then may or may not read() anything.
     emit q->readyRead();
-    emit q->downloadProgress(bytesDownloaded,
+    if (downloadProgressSignalChoke.elapsed() >= progressSignalInterval) {
+        downloadProgressSignalChoke.restart();
+        emit q->downloadProgress(bytesDownloaded,
                              totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
+    }
 
     // If there are still bytes available in the cacheLoadDevice then the user did not read
     // in response to the readyRead() signal. This means we have to load from the cacheLoadDevice
@@ -1863,6 +1876,8 @@ void QNetworkReplyHttpImplPrivate::finished()
 
     if (totalSize.isNull() || totalSize == -1) {
         emit q->downloadProgress(bytesDownloaded, bytesDownloaded);
+    } else {
+        emit q->downloadProgress(bytesDownloaded, totalSize.toLongLong());
     }
 
     if (bytesUploaded == -1 && (outgoingData || outgoingDataBuffer))
diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h
index 8c911779a3..b73fdf3dc7 100644
--- a/src/network/access/qnetworkreplyhttpimpl_p.h
+++ b/src/network/access/qnetworkreplyhttpimpl_p.h
@@ -188,7 +188,6 @@ public:
 
     void checkForRedirect(const int statusCode);
 
-
     // incoming from user
     QNetworkAccessManager *manager;
     QNetworkAccessManagerPrivate *managerPrivate;
diff --git a/src/network/access/qnetworkreplyimpl.cpp b/src/network/access/qnetworkreplyimpl.cpp
index 7cbbe389da..9f8a6cfb5c 100644
--- a/src/network/access/qnetworkreplyimpl.cpp
+++ b/src/network/access/qnetworkreplyimpl.cpp
@@ -144,6 +144,9 @@ void QNetworkReplyImplPrivate::_q_startOperation()
     }
 #endif
 
+    // Start timer for progress notifications
+    downloadProgressSignalChoke.start();
+
     if (backend && backend->isSynchronous()) {
         state = Finished;
         q_func()->setFinished(true);
@@ -210,8 +213,11 @@ void QNetworkReplyImplPrivate::_q_copyReadyRead()
     // emit readyRead before downloadProgress incase this will cause events to be
     // processed and we get into a recursive call (as in QProgressDialog).
     emit q->readyRead();
-    emit q->downloadProgress(bytesDownloaded,
+    if (downloadProgressSignalChoke.elapsed() >= progressSignalInterval) {
+        downloadProgressSignalChoke.restart();
+        emit q->downloadProgress(bytesDownloaded,
                              totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
+    }
     resumeNotificationHandling();
 }
 
@@ -640,8 +646,11 @@ void QNetworkReplyImplPrivate::appendDownstreamDataSignalEmissions()
     emit q->readyRead();
     // emit readyRead before downloadProgress incase this will cause events to be
     // processed and we get into a recursive call (as in QProgressDialog).
-    emit q->downloadProgress(bytesDownloaded,
+    if (downloadProgressSignalChoke.elapsed() >= progressSignalInterval) {
+        downloadProgressSignalChoke.restart();
+        emit q->downloadProgress(bytesDownloaded,
                              totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
+    }
 
     resumeNotificationHandling();
     // do we still have room in the buffer?
@@ -747,7 +756,10 @@ void QNetworkReplyImplPrivate::appendDownstreamDataDownloadBuffer(qint64 bytesRe
     // processed and we get into a recursive call (as in QProgressDialog).
     if (bytesDownloaded > 0)
         emit q->readyRead();
-    emit q->downloadProgress(bytesDownloaded, bytesTotal);
+    if (downloadProgressSignalChoke.elapsed() >= progressSignalInterval) {
+        downloadProgressSignalChoke.restart();
+        emit q->downloadProgress(bytesDownloaded, bytesTotal);
+    }
 }
 
 void QNetworkReplyImplPrivate::finished()
@@ -795,6 +807,8 @@ void QNetworkReplyImplPrivate::finished()
     pauseNotificationHandling();
     if (totalSize.isNull() || totalSize == -1) {
         emit q->downloadProgress(bytesDownloaded, bytesDownloaded);
+    } else {
+        emit q->downloadProgress(bytesDownloaded, totalSize.toLongLong());
     }
 
     if (bytesUploaded == -1 && (outgoingData || outgoingDataBuffer))