QNetworkReplyImpl: Protect against recursive event loops

This fixes a bug that occured together with a QProgressDialog.
The signal emission was like:
readyRead readyRead readyRead [...] readyRead finished readyRead
Now finished should be properly at the ending of this sequence.

Task-number: 256630
Reviewed-by: Thiago Macieira <thiago.macieira@nokia.com>

5 files changed, 175 insertions, 2 deletions

diff --git a/src/network/access/qnetworkreplyimpl.cpp b/src/network/access/qnetworkreplyimpl.cpp
index e8cb6ee1b3..d903d03337 100644
--- a/src/network/access/qnetworkreplyimpl.cpp
+++ b/src/network/access/qnetworkreplyimpl.cpp
@@ -54,6 +54,7 @@ QT_BEGIN_NAMESPACE
 inline QNetworkReplyImplPrivate::QNetworkReplyImplPrivate()
     : copyDevice(0), networkCache(0),
       cacheEnabled(false), cacheSaveDevice(0),
+      notificationHandlingPaused(false),
       bytesDownloaded(0), lastBytesDownloaded(-1), bytesUploaded(-1),
       state(Idle)
 {
@@ -164,9 +165,11 @@ void QNetworkReplyImplPrivate::_q_copyReadyRead()
 
     lastBytesDownloaded = bytesDownloaded;
     QVariant totalSize = cookedHeaders.value(QNetworkRequest::ContentLengthHeader);
+    pauseNotificationHandling();
     emit q->downloadProgress(bytesDownloaded,
                              totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
     emit q->readyRead();
+    resumeNotificationHandling();
 }
 
 void QNetworkReplyImplPrivate::_q_copyReadChannelFinished()
@@ -210,6 +213,9 @@ void QNetworkReplyImplPrivate::backendNotify(InternalNotifications notification)
 
 void QNetworkReplyImplPrivate::handleNotifications()
 {
+    if (notificationHandlingPaused)
+        return;
+
     NotificationQueue current = pendingNotifications;
     pendingNotifications.clear();
 
@@ -248,6 +254,22 @@ void QNetworkReplyImplPrivate::handleNotifications()
     }
 }
 
+// Do not handle the notifications while we are emitting downloadProgress
+// or readyRead
+void QNetworkReplyImplPrivate::pauseNotificationHandling()
+{
+    notificationHandlingPaused = true;
+}
+
+// Resume notification handling
+void QNetworkReplyImplPrivate::resumeNotificationHandling()
+{
+    Q_Q(QNetworkReplyImpl);
+    notificationHandlingPaused = false;
+    if (pendingNotifications.size() >= 1)
+        QCoreApplication::postEvent(q, new QEvent(QEvent::NetworkReplyUpdated));
+}
+
 void QNetworkReplyImplPrivate::createCache()
 {
     // check if we can save and if we're allowed to
@@ -318,8 +340,10 @@ void QNetworkReplyImplPrivate::consume(qint64 count)
         bytesUploaded += count;
 
     QVariant totalSize = request.header(QNetworkRequest::ContentLengthHeader);
+    pauseNotificationHandling();
     emit q->uploadProgress(bytesUploaded,
                            totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
+    resumeNotificationHandling();
 }
 
 qint64 QNetworkReplyImplPrivate::nextDownstreamBlockSize() const
@@ -367,12 +391,14 @@ void QNetworkReplyImplPrivate::feed(const QByteArray &data)
     QPointer<QNetworkReplyImpl> qq = q;
 
     QVariant totalSize = cookedHeaders.value(QNetworkRequest::ContentLengthHeader);
+    pauseNotificationHandling();
     emit q->downloadProgress(bytesDownloaded,
                              totalSize.isNull() ? Q_INT64_C(-1) : totalSize.toLongLong());
     emit q->readyRead();
 
     // hopefully we haven't been deleted here
     if (!qq.isNull()) {
+        resumeNotificationHandling();
         // do we still have room in the buffer?
         if (nextDownstreamBlockSize() > 0)
             backendNotify(QNetworkReplyImplPrivate::NotifyDownstreamReadyWrite);
@@ -409,19 +435,25 @@ void QNetworkReplyImplPrivate::finished()
     state = Finished;
     pendingNotifications.clear();
 
+    pauseNotificationHandling();
     QVariant totalSize = cookedHeaders.value(QNetworkRequest::ContentLengthHeader);
-    if (bytesDownloaded != lastBytesDownloaded || totalSize.isNull())
+    if (bytesDownloaded != lastBytesDownloaded || totalSize.isNull()) {
         emit q->downloadProgress(bytesDownloaded, bytesDownloaded);
-    if (bytesUploaded == -1 && outgoingData)
+    }
+    if (bytesUploaded == -1 && outgoingData) {
         emit q->uploadProgress(0, 0);
+    }
+    resumeNotificationHandling();
 
     completeCacheSave();
 
     // note: might not be a good idea, since users could decide to delete us
     // which would delete the backend too...
     // maybe we should protect the backend
+    pauseNotificationHandling();
     emit q->readChannelFinished();
     emit q->finished();
+    resumeNotificationHandling();
 }
 
 void QNetworkReplyImplPrivate::error(QNetworkReplyImpl::NetworkError code, const QString &errorMessage)
diff --git a/src/network/access/qnetworkreplyimpl_p.h b/src/network/access/qnetworkreplyimpl_p.h
index 32bfa4c7a6..65b3679788 100644
--- a/src/network/access/qnetworkreplyimpl_p.h
+++ b/src/network/access/qnetworkreplyimpl_p.h
@@ -129,6 +129,9 @@ public:
     void setup(QNetworkAccessManager::Operation op, const QNetworkRequest &request,
                QIODevice *outgoingData);
     void setNetworkCache(QAbstractNetworkCache *networkCache);
+
+    void pauseNotificationHandling();
+    void resumeNotificationHandling();
     void backendNotify(InternalNotifications notification);
     void handleNotifications();
     void createCache();
@@ -156,6 +159,8 @@ public:
     QIODevice *cacheSaveDevice;
 
     NotificationQueue pendingNotifications;
+    bool notificationHandlingPaused;
+
     QUrl urlForLastAuthentication;
 #ifndef QT_NO_NETWORKPROXY
     QNetworkProxy lastProxyAuthentication;
diff --git a/tests/auto/auto.pro b/tests/auto/auto.pro
index 1cdb840c8e..a215daafac 100644
--- a/tests/auto/auto.pro
+++ b/tests/auto/auto.pro
@@ -207,6 +207,7 @@ SUBDIRS += _networkselftest \
            qnetworkproxy \
 	   qnetworkrequest \
 	   qnetworkreply \
+           qnetworkaccessmanager_and_qprogressdialog \
            qnumeric \
            qobject \
            qobjectrace \
diff --git a/tests/auto/qnetworkaccessmanager_and_qprogressdialog/qnetworkaccessmanager_and_qprogressdialog.pro b/tests/auto/qnetworkaccessmanager_and_qprogressdialog/qnetworkaccessmanager_and_qprogressdialog.pro
new file mode 100644
index 0000000000..7ed5b07f7e
--- /dev/null
+++ b/tests/auto/qnetworkaccessmanager_and_qprogressdialog/qnetworkaccessmanager_and_qprogressdialog.pro
@@ -0,0 +1,5 @@
+load(qttest_p4)
+SOURCES += tst_qnetworkaccessmanager_and_qprogressdialog.cpp
+QT += network
+
+
diff --git a/tests/auto/qnetworkaccessmanager_and_qprogressdialog/tst_qnetworkaccessmanager_and_qprogressdialog.cpp b/tests/auto/qnetworkaccessmanager_and_qprogressdialog/tst_qnetworkaccessmanager_and_qprogressdialog.cpp
new file mode 100644
index 0000000000..62e4ce5038
--- /dev/null
+++ b/tests/auto/qnetworkaccessmanager_and_qprogressdialog/tst_qnetworkaccessmanager_and_qprogressdialog.cpp
@@ -0,0 +1,130 @@
+/****************************************************************************
+**
+** Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+** Contact: Nokia Corporation (qt-info@nokia.com)
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** No Commercial Usage
+** This file contains pre-release code and may not be distributed.
+** You may use this file in accordance with the terms and conditions
+** contained in the either Technology Preview License Agreement or the
+** Beta Release License Agreement.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Nokia gives you certain
+** additional rights. These rights are described in the Nokia Qt LGPL
+** Exception version 1.0, included in the file LGPL_EXCEPTION.txt in this
+** package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+** If you are unsure which license is appropriate for your use, please
+** contact the sales department at http://www.qtsoftware.com/contact.
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+
+#include <QtTest/QtTest>
+#include <QtGui>
+#include <QtCore>
+#include <QtNetwork/QNetworkAccessManager>
+#include <QtNetwork/QNetworkRequest>
+#include <QtNetwork/QNetworkReply>
+#include <qdebug.h>
+
+#include "../network-settings.h"
+
+
+class tst_QNetworkAccessManager_And_QProgressDialog : public QObject
+{
+Q_OBJECT
+private slots:
+    void downloadCheck();
+};
+
+class DownloadCheckWidget : public QWidget
+{
+    Q_OBJECT;
+public:
+    DownloadCheckWidget(QWidget *parent = 0) : QWidget(parent)
+                                    , progressDlg(this), netmanager(this)
+                                    , lateReadyRead(true)
+    {
+        progressDlg.setRange(1, 100);
+        QMetaObject::invokeMethod(this, "go", Qt::QueuedConnection);
+    }
+    bool lateReadyRead;
+public slots:
+    void go()
+    {
+        QNetworkReply *reply = netmanager.get(
+                QNetworkRequest(
+                QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/bigfile")
+                ));
+        connect(reply, SIGNAL(downloadProgress(qint64, qint64)),
+                this, SLOT(dataReadProgress(qint64, qint64)));
+        connect(reply, SIGNAL(readyRead()),
+                this, SLOT(dataReadyRead()));
+        connect(reply, SIGNAL(finished()), this, SLOT(finishedFromReply()));
+
+        progressDlg.exec();
+    }
+    void dataReadProgress(qint64 done, qint64 total)
+    {
+        QNetworkReply *reply = qobject_cast<QNetworkReply *>(sender());
+        progressDlg.setMaximum(total);
+        progressDlg.setValue(done);
+    }
+    void dataReadyRead()
+    {
+        QNetworkReply *reply = qobject_cast<QNetworkReply *>(sender());
+        lateReadyRead = true;
+    }
+    void finishedFromReply()
+    {
+        QNetworkReply *reply = qobject_cast<QNetworkReply *>(sender());
+        lateReadyRead = false;
+        reply->deleteLater();
+        QTestEventLoop::instance().exitLoop();
+    }
+
+
+private:
+    QProgressDialog progressDlg;
+    QNetworkAccessManager netmanager;
+};
+
+void tst_QNetworkAccessManager_And_QProgressDialog::downloadCheck()
+{
+    DownloadCheckWidget widget;
+    widget.show();
+    // run and exit on finished()
+    QTestEventLoop::instance().enterLoop(10);
+    QVERIFY(!QTestEventLoop::instance().timeout());
+    // run some more to catch the late readyRead() (or: to not catch it)
+    QTestEventLoop::instance().enterLoop(1);
+    QVERIFY(QTestEventLoop::instance().timeout());
+    // the following fails when a readyRead() was received after finished()
+    QVERIFY(!widget.lateReadyRead);
+}
+
+
+
+QTEST_MAIN(tst_QNetworkAccessManager_And_QProgressDialog)
+#include "tst_qnetworkaccessmanager_and_qprogressdialog.moc"