Enhance QFileNetworkReply

It is now considered as finished immediatly after creation to avoid
an event loop spin.
Had to change the auto tests a bit to account for the new behaviour.

Reviewed-by: Peter Hartmann

5 files changed, 64 insertions, 70 deletions

diff --git a/src/network/access/qfilenetworkreply.cpp b/src/network/access/qfilenetworkreply.cpp
index 44dd9e7..8c5065c 100644
--- a/src/network/access/qfilenetworkreply.cpp
+++ b/src/network/access/qfilenetworkreply.cpp
@@ -44,35 +44,32 @@
 #include "QtCore/qdatetime.h"
 #include <QtCore/QCoreApplication>
 #include <QtCore/QFileInfo>
+#include <QDebug>
 
 QT_BEGIN_NAMESPACE
 
 QFileNetworkReplyPrivate::QFileNetworkReplyPrivate()
-    : QNetworkReplyPrivate(), realFileSize(0), finished(false)
+    : QNetworkReplyPrivate(), realFileSize(0)
 {
 }
 
-QFileNetworkReply::QFileNetworkReply(QObject *parent, const QNetworkRequest &req)
+QFileNetworkReply::~QFileNetworkReply()
+{
+}
+
+QFileNetworkReply::QFileNetworkReply(QObject *parent, const QNetworkRequest &req, const QNetworkAccessManager::Operation op)
     : QNetworkReply(*new QFileNetworkReplyPrivate(), parent)
 {
     setRequest(req);
     setUrl(req.url());
-    setOperation(QNetworkAccessManager::GetOperation);
-    QMetaObject::invokeMethod(this, "_q_startOperation", Qt::QueuedConnection);
+    setOperation(op);
     QNetworkReply::open(QIODevice::ReadOnly);
-}
 
-QFileNetworkReply::~QFileNetworkReply()
-{
-}
+    qRegisterMetaType<QNetworkReply::NetworkError>("QNetworkReply::NetworkError");
 
-// This code is mostly inspired by QNetworkAccessFileBackend
-// We also use its translation context for error messages
-void QFileNetworkReplyPrivate::_q_startOperation()
-{
-    Q_Q(QFileNetworkReply);
+    QFileNetworkReplyPrivate *d = (QFileNetworkReplyPrivate*) d_func();
 
-    QUrl url = q->url();
+    QUrl url = req.url();
     if (url.host() == QLatin1String("localhost"))
         url.setHost(QString());
 
@@ -81,81 +78,75 @@ void QFileNetworkReplyPrivate::_q_startOperation()
     if (!url.host().isEmpty()) {
         // we handle only local files
         QString msg = QCoreApplication::translate("QNetworkAccessFileBackend", "Request for opening non-local file %1").arg(url.toString());
-        q->setError(QNetworkReply::ProtocolInvalidOperationError, msg);
-        emit q->error(QNetworkReply::ProtocolInvalidOperationError);
-        doFinished();
+        setError(QNetworkReply::ProtocolInvalidOperationError, msg);
+        QMetaObject::invokeMethod(this, "error", Qt::QueuedConnection,
+            Q_ARG(QNetworkReply::NetworkError, QNetworkReply::ProtocolInvalidOperationError));
+        QMetaObject::invokeMethod(this, "finished", Qt::QueuedConnection);
         return;
     }
 #endif
     if (url.path().isEmpty())
         url.setPath(QLatin1String("/"));
-    q->setUrl(url);
+    setUrl(url);
 
 
     QString fileName = url.toLocalFile();
     if (fileName.isEmpty()) {
         fileName = url.toString(QUrl::RemoveAuthority | QUrl::RemoveFragment | QUrl::RemoveQuery);
     }
-    realFile.setFileName(fileName);
+    d->realFile.setFileName(fileName);
 
-    QFileInfo fi(realFile);
+    QFileInfo fi(d->realFile);
     if (fi.isDir()) {
         QString msg = QCoreApplication::translate("QNetworkAccessFileBackend", "Cannot open %1: Path is a directory").arg(url.toString());
-        q->setError(QNetworkReply::ContentOperationNotPermittedError, msg);
-        emit q->error(QNetworkReply::ContentOperationNotPermittedError);
-        doFinished();
+        setError(QNetworkReply::ContentOperationNotPermittedError, msg);
+        QMetaObject::invokeMethod(this, "error", Qt::QueuedConnection,
+            Q_ARG(QNetworkReply::NetworkError, QNetworkReply::ContentOperationNotPermittedError));
+        QMetaObject::invokeMethod(this, "finished", Qt::QueuedConnection);
         return;
     }
 
-    bool opened = realFile.open(QIODevice::ReadOnly | QIODevice::Unbuffered);
+    bool opened = d->realFile.open(QIODevice::ReadOnly | QIODevice::Unbuffered);
 
     // could we open the file?
     if (!opened) {
         QString msg = QCoreApplication::translate("QNetworkAccessFileBackend", "Error opening %1: %2")
-                      .arg(realFile.fileName(), realFile.errorString());
+                      .arg(d->realFile.fileName(), d->realFile.errorString());
 
-        if (realFile.exists()) {
-            q->setError(QNetworkReply::ContentAccessDenied, msg);
-            emit q->error(QNetworkReply::ContentAccessDenied);
+        if (d->realFile.exists()) {
+            setError(QNetworkReply::ContentAccessDenied, msg);
+            QMetaObject::invokeMethod(this, "error", Qt::QueuedConnection,
+                Q_ARG(QNetworkReply::NetworkError, QNetworkReply::ContentAccessDenied));
         } else {
-            q->setError(QNetworkReply::ContentNotFoundError, msg);
-            emit q->error(QNetworkReply::ContentNotFoundError);
+            setError(QNetworkReply::ContentNotFoundError, msg);
+            QMetaObject::invokeMethod(this, "error", Qt::QueuedConnection,
+                Q_ARG(QNetworkReply::NetworkError, QNetworkReply::ContentNotFoundError));
         }
-        doFinished();
+        QMetaObject::invokeMethod(this, "finished", Qt::QueuedConnection);
         return;
     }
 
-    realFileSize = fi.size();
-    q->setHeader(QNetworkRequest::LastModifiedHeader, fi.lastModified());
-    q->setHeader(QNetworkRequest::ContentLengthHeader, realFileSize);
+    d->realFileSize = fi.size();
+    setHeader(QNetworkRequest::LastModifiedHeader, fi.lastModified());
+    setHeader(QNetworkRequest::ContentLengthHeader, d->realFileSize);
 
-    emit q->metaDataChanged();
-    emit q->downloadProgress(realFileSize, realFileSize);
-    emit q->readyRead();
-    doFinished();
+    QMetaObject::invokeMethod(this, "metaDataChanged", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(this, "downloadProgress", Qt::QueuedConnection,
+        Q_ARG(qint64, d->realFileSize), Q_ARG(qint64, d->realFileSize));
+    QMetaObject::invokeMethod(this, "readyRead", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(this, "finished", Qt::QueuedConnection);
 }
 
 bool QFileNetworkReplyPrivate::isFinished() const
 {
-    return finished;
-}
-
-void QFileNetworkReplyPrivate::doFinished()
-{
-    Q_Q(QFileNetworkReply);
-    finished = true;
-    emit q->finished();
+    return true;
 }
 
-
 void QFileNetworkReply::close()
 {
     Q_D(QFileNetworkReply);
     QNetworkReply::close();
     d->realFile.close();
-
-    if (!d->finished)
-        d->doFinished();
 }
 
 void QFileNetworkReply::abort()
@@ -163,9 +154,6 @@ void QFileNetworkReply::abort()
     Q_D(QFileNetworkReply);
     QNetworkReply::close();
     d->realFile.close();
-
-    if (!d->finished)
-        d->doFinished();
 }
 
 qint64 QFileNetworkReply::bytesAvailable() const
diff --git a/src/network/access/qfilenetworkreply_p.h b/src/network/access/qfilenetworkreply_p.h
index 6f10672..125fa2e 100644
--- a/src/network/access/qfilenetworkreply_p.h
+++ b/src/network/access/qfilenetworkreply_p.h
@@ -66,7 +66,7 @@ class QFileNetworkReply: public QNetworkReply
 {
     Q_OBJECT
 public:
-    QFileNetworkReply(QObject *parent, const QNetworkRequest &req);
+    QFileNetworkReply(QObject *parent, const QNetworkRequest &req, const QNetworkAccessManager::Operation op);
     ~QFileNetworkReply();
     virtual void abort();
 
@@ -76,12 +76,9 @@ public:
     virtual bool isSequential () const;
     qint64 size() const;
 
-
     virtual qint64 readData(char *data, qint64 maxlen);
 
     Q_DECLARE_PRIVATE(QFileNetworkReply)
-    Q_PRIVATE_SLOT(d_func(), void _q_startOperation())
-
 };
 
 class QFileNetworkReplyPrivate: public QNetworkReplyPrivate
@@ -92,12 +89,7 @@ public:
     QFile realFile;
     qint64 realFileSize;
 
-    void _q_startOperation();
-
     virtual bool isFinished() const;
-    void doFinished();
-    bool finished;
-
 
     Q_DECLARE_PUBLIC(QFileNetworkReply)
 };
diff --git a/src/network/access/qnetworkaccessmanager.cpp b/src/network/access/qnetworkaccessmanager.cpp
index d27fbe7..e16aedc 100644
--- a/src/network/access/qnetworkaccessmanager.cpp
+++ b/src/network/access/qnetworkaccessmanager.cpp
@@ -687,10 +687,10 @@ QNetworkReply *QNetworkAccessManager::createRequest(QNetworkAccessManager::Opera
     // Also if the scheme is empty we consider it a file.
     // The QNetworkAccessFileBackend will right now only be used
     // for PUT or qrc://
-    if (op == QNetworkAccessManager::GetOperation
+    if ((op == QNetworkAccessManager::GetOperation || op == QNetworkAccessManager::HeadOperation)
          && (req.url().scheme() == QLatin1String("file")
              || req.url().scheme().isEmpty())) {
-        return new QFileNetworkReply(this, req);
+        return new QFileNetworkReply(this, req, op);
     }
 
     QNetworkRequest request = req;
diff --git a/src/network/access/qnetworkreply.cpp b/src/network/access/qnetworkreply.cpp
index 49a287f..0a8ea5d 100644
--- a/src/network/access/qnetworkreply.cpp
+++ b/src/network/access/qnetworkreply.cpp
@@ -239,7 +239,10 @@ QNetworkReplyPrivate::QNetworkReplyPrivate()
     \note Do not delete the object in the slot connected to this
     signal. Use deleteLater().
 
-    \sa QNetworkAccessManager::finished()
+    You can also use isFinished() to check if a QNetworkReply
+    has finished even before you receive the finished() signal.
+
+    \sa QNetworkAccessManager::finished(), isFinished()
 */
 
 /*!
diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
index 33753f1..3bf06d7 100644
--- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
@@ -838,13 +838,20 @@ void tst_QNetworkReply::stateChecking()
     QVERIFY(reply->isOpen());
     QVERIFY(reply->isReadable());
     QVERIFY(!reply->isWritable());
-    QCOMPARE(reply->errorString(), QString("Unknown error"));
+
+    // both behaviours are OK since we might change underlying behaviour again
+    if (!reply->isFinished())
+        QCOMPARE(reply->errorString(), QString("Unknown error"));
+    else
+        QVERIFY(!reply->errorString().isEmpty());
+
 
     QCOMPARE(reply->manager(), &manager);
     QCOMPARE(reply->request(), req);
     QCOMPARE(int(reply->operation()), int(QNetworkAccessManager::GetOperation));
-    QCOMPARE(reply->error(), QNetworkReply::NoError);
-    QCOMPARE(reply->isFinished(), false);
+    // error and not error are OK since we might change underlying behaviour again
+    if (!reply->isFinished())
+        QCOMPARE(reply->error(), QNetworkReply::NoError);
     QCOMPARE(reply->url(), url);
 
     reply->abort();
@@ -1151,7 +1158,8 @@ void tst_QNetworkReply::getErrors()
     QNetworkReplyPtr reply = manager.get(request);
     reply->setParent(this);     // we have expect-fails
 
-    QCOMPARE(reply->error(), QNetworkReply::NoError);
+    if (!reply->isFinished())
+        QCOMPARE(reply->error(), QNetworkReply::NoError);
 
     // now run the request:
     connect(reply, SIGNAL(finished()),
@@ -1512,6 +1520,7 @@ void tst_QNetworkReply::ioGetFromFile()
 
     QNetworkRequest request(QUrl::fromLocalFile(file.fileName()));
     QNetworkReplyPtr reply = manager.get(request);
+    QVERIFY(reply->isFinished()); // a file should immediatly be done
     DataReader reader(reply);
 
     connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()));
@@ -3170,7 +3179,9 @@ void tst_QNetworkReply::ioPostToHttpEmptyUploadProgress()
 
 void tst_QNetworkReply::lastModifiedHeaderForFile()
 {
-    QFileInfo fileInfo(SRCDIR "./bigfile");
+    QFileInfo fileInfo(SRCDIR "/bigfile");
+    QVERIFY(fileInfo.exists());
+
     QUrl url = QUrl::fromLocalFile(fileInfo.filePath());
 
     QNetworkRequest request(url);