2011-05-04 Caio Marcelo de Oliveira Filho <caio.oliveira@openbossa.org>
authorcaio.oliveira@openbossa.org <caio.oliveira@openbossa.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 May 2011 12:24:40 +0000 (12:24 +0000)
committercaio.oliveira@openbossa.org <caio.oliveira@openbossa.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 May 2011 12:24:40 +0000 (12:24 +0000)
        Reviewed by Andreas Kling.

        [Qt] Fix QNetworkReplyWrapper to not depend on QNetworkReply::isFinished() method
        https://bugs.webkit.org/show_bug.cgi?id=59070

        Applications using our API and our autotests subclass QNetworkReply as part of providing a
        custom QNetworkAccessManager. But there's an API limitation in Qt 4.7, that makes
        QNetworkReply::isFinished() always be false for these custom replies. This was fixed in Qt
        4.8, see http://bugreports.qt.nokia.com/browse/QTBUG-11737.

        The consequence is that QtWebKit cannot rely on this function. So now QNetworkReplyWrapper
        watches for the finished() signal and set a dynamic property "_q_isFinished" on the reply
        indicating that it is finished. When there's no finished signal (synchronous) we set the
        dynamic property once we get the reply.

        This fixes tst_QWebFrame::requestedUrl(), that was breaking because sniffer was not
        emitting its own finished() signal, causing QWebFrame::loadFinished() to not be emitted.

        * platform/network/qt/QNetworkReplyHandler.cpp:
        (WebCore::QNetworkReplyWrapper::QNetworkReplyWrapper):
        Connect the finished signal to the new setFinished() slot.

        (WebCore::QNetworkReplyWrapper::synchronousLoad):
        Since we don't get the finished signal for synchronous loads, set the dynamic property
        before processing it.

        (WebCore::QNetworkReplyWrapper::resetConnections):
        Do not reset the connection to setFinished().

        (WebCore::QNetworkReplyWrapper::setFinished):
        Set the dynamic property in the reply.

        (WebCore::QNetworkReplyWrapper::emitMetaDataChanged):
        (WebCore::QNetworkReplyHandler::start):
        Change to use wrapper's isFinished() instead of asking the reply directly.

        * platform/network/qt/QNetworkReplyHandler.h:
        (WebCore::QNetworkReplyWrapper::isFinished):
        Checks the dynamic property of the reply.

        * platform/network/qt/QtMIMETypeSniffer.cpp:
        (QtMIMETypeSniffer::sniff):
        Use the dynamic property to check if the reply is finished.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@85734 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp
Source/WebCore/platform/network/qt/QNetworkReplyHandler.h
Source/WebCore/platform/network/qt/QtMIMETypeSniffer.cpp

index 5c44919..5322fd3 100644 (file)
@@ -1,3 +1,49 @@
+2011-05-04  Caio Marcelo de Oliveira Filho  <caio.oliveira@openbossa.org>
+
+        Reviewed by Andreas Kling.
+
+        [Qt] Fix QNetworkReplyWrapper to not depend on QNetworkReply::isFinished() method
+        https://bugs.webkit.org/show_bug.cgi?id=59070
+
+        Applications using our API and our autotests subclass QNetworkReply as part of providing a
+        custom QNetworkAccessManager. But there's an API limitation in Qt 4.7, that makes
+        QNetworkReply::isFinished() always be false for these custom replies. This was fixed in Qt
+        4.8, see http://bugreports.qt.nokia.com/browse/QTBUG-11737.
+
+        The consequence is that QtWebKit cannot rely on this function. So now QNetworkReplyWrapper
+        watches for the finished() signal and set a dynamic property "_q_isFinished" on the reply
+        indicating that it is finished. When there's no finished signal (synchronous) we set the
+        dynamic property once we get the reply.
+
+        This fixes tst_QWebFrame::requestedUrl(), that was breaking because sniffer was not
+        emitting its own finished() signal, causing QWebFrame::loadFinished() to not be emitted.
+
+        * platform/network/qt/QNetworkReplyHandler.cpp:
+        (WebCore::QNetworkReplyWrapper::QNetworkReplyWrapper):
+        Connect the finished signal to the new setFinished() slot.
+
+        (WebCore::QNetworkReplyWrapper::synchronousLoad):
+        Since we don't get the finished signal for synchronous loads, set the dynamic property
+        before processing it.
+
+        (WebCore::QNetworkReplyWrapper::resetConnections):
+        Do not reset the connection to setFinished().
+
+        (WebCore::QNetworkReplyWrapper::setFinished):
+        Set the dynamic property in the reply.
+
+        (WebCore::QNetworkReplyWrapper::emitMetaDataChanged):
+        (WebCore::QNetworkReplyHandler::start):
+        Change to use wrapper's isFinished() instead of asking the reply directly.
+
+        * platform/network/qt/QNetworkReplyHandler.h:
+        (WebCore::QNetworkReplyWrapper::isFinished):
+        Checks the dynamic property of the reply.
+
+        * platform/network/qt/QtMIMETypeSniffer.cpp:
+        (QtMIMETypeSniffer::sniff):
+        Use the dynamic property to check if the reply is finished.
+
 2011-05-04  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Ryosuke Niwa.
index 8b055be..0c3cf50 100644 (file)
@@ -214,8 +214,10 @@ QNetworkReplyWrapper::QNetworkReplyWrapper(QNetworkReplyHandlerCallQueue* queue,
 {
     Q_ASSERT(m_reply);
 
-    connect(m_reply, SIGNAL(readyRead()), this, SLOT(receiveMetaData()));
+    // setFinished() must be the first that we connect, so isFinished() is updated when running other slots.
+    connect(m_reply, SIGNAL(finished()), this, SLOT(setFinished()));
     connect(m_reply, SIGNAL(finished()), this, SLOT(receiveMetaData()));
+    connect(m_reply, SIGNAL(readyRead()), this, SLOT(receiveMetaData()));
 }
 
 QNetworkReplyWrapper::~QNetworkReplyWrapper()
@@ -239,10 +241,20 @@ QNetworkReply* QNetworkReplyWrapper::release()
     return reply;
 }
 
+void QNetworkReplyWrapper::synchronousLoad()
+{
+    setFinished();
+    receiveMetaData();
+}
+
 void QNetworkReplyWrapper::resetConnections()
 {
-    if (m_reply)
-        m_reply->disconnect(this);
+    if (m_reply) {
+        // Disconnect all connections except the one to setFinished() slot.
+        m_reply->disconnect(this, SLOT(receiveMetaData()));
+        m_reply->disconnect(this, SLOT(didReceiveFinished()));
+        m_reply->disconnect(this, SLOT(didReceiveReadyRead()));
+    }
     QCoreApplication::removePostedEvents(this, QEvent::MetaCall);
 }
 
@@ -293,6 +305,16 @@ void QNetworkReplyWrapper::receiveSniffedMIMEType()
     emitMetaDataChanged();
 }
 
+void QNetworkReplyWrapper::setFinished()
+{
+    // Due to a limitation of QNetworkReply public API, its subclasses never get the chance to
+    // change the result of QNetworkReply::isFinished() method. So we need to keep track of the
+    // finished state ourselves. This limitation is fixed in 4.8, but we'll still have applications
+    // that don't use the solution. See http://bugreports.qt.nokia.com/browse/QTBUG-11737.
+    Q_ASSERT(!isFinished());
+    m_reply->setProperty("_q_isFinished", true);
+}
+
 void QNetworkReplyWrapper::emitMetaDataChanged()
 {
     QueueLocker lock(m_queue);
@@ -303,7 +325,7 @@ void QNetworkReplyWrapper::emitMetaDataChanged()
         m_queue->push(&QNetworkReplyHandler::forwardData);
     }
 
-    if (m_reply->isFinished()) {
+    if (isFinished()) {
         m_queue->push(&QNetworkReplyHandler::finish);
         return;
     }
@@ -640,7 +662,7 @@ void QNetworkReplyHandler::start()
 
     m_replyWrapper = new QNetworkReplyWrapper(&m_queue, reply, m_resourceHandle->shouldContentSniff() && d->m_context->mimeSniffingEnabled(), this);
 
-    if (m_loadType == SynchronousLoad && m_replyWrapper->reply()->isFinished()) {
+    if (m_loadType == SynchronousLoad) {
         m_replyWrapper->synchronousLoad();
         // If supported, a synchronous request will be finished at this point, no need to hook up the signals.
         return;
index ec4e65a..defeec4 100644 (file)
@@ -21,8 +21,9 @@
 
 #include <QObject>
 
-#include <QNetworkRequest>
 #include <QNetworkAccessManager>
+#include <QNetworkReply>
+#include <QNetworkRequest>
 
 #include "FormData.h"
 #include "QtMIMETypeSniffer.h"
@@ -70,7 +71,7 @@ public:
     QNetworkReply* reply() const { return m_reply; }
     QNetworkReply* release();
 
-    void synchronousLoad() { receiveMetaData(); }
+    void synchronousLoad();
 
     QUrl redirectionTargetUrl() const { return m_redirectionTargetUrl; }
     QString encoding() const { return m_encoding; }
@@ -80,11 +81,15 @@ public:
     bool responseContainsData() const { return m_responseContainsData; }
     bool wasRedirected() const { return m_redirectionTargetUrl.isValid(); }
 
+    // See setFinished().
+    bool isFinished() const { return m_reply->property("_q_isFinished").toBool(); }
+
 private Q_SLOTS:
     void receiveMetaData();
     void didReceiveFinished();
     void didReceiveReadyRead();
     void receiveSniffedMIMEType();
+    void setFinished();
 
 private:
     void resetConnections();
index e719b80..60ab727 100644 (file)
@@ -41,7 +41,10 @@ QtMIMETypeSniffer::QtMIMETypeSniffer(QNetworkReply* reply, const QString& advert
 
 bool QtMIMETypeSniffer::sniff()
 {
-    if (!m_reply->isFinished() && m_reply->bytesAvailable() < m_sniffer.dataSize())
+    // See QNetworkReplyWrapper::setFinished().
+    const bool isReplyFinished = m_reply->property("_q_isFinished").toBool();
+
+    if (!isReplyFinished && m_reply->bytesAvailable() < m_sniffer.dataSize())
         return false;
 
     QByteArray data = m_reply->peek(m_sniffer.dataSize());