Modified: trunk/Source/WebCore/ChangeLog (150119 => 150120)
--- trunk/Source/WebCore/ChangeLog 2013-05-15 16:27:43 UTC (rev 150119)
+++ trunk/Source/WebCore/ChangeLog 2013-05-15 16:31:13 UTC (rev 150120)
@@ -1,3 +1,28 @@
+2013-05-15 Jocelyn Turcotte <[email protected]>
+
+ [Qt] Fix a crash under ~PingLoader when the QNAM on the page has been destroyed.
+ https://bugs.webkit.org/show_bug.cgi?id=116035
+
+ Reviewed by Allan Sandfeld Jensen.
+
+ The previous fix only moved the crash location from WebKit down to QNetworkReplyHttpImpl
+ which expects its QNetworkAccessManager to still be alive.
+
+ Fix it by watching the QNetworkReply's destroyed() signal and avoid the dangling pointer
+ instead. The QNetworkReply doesn't need to be aborted in this case anyway.
+
+ * platform/network/qt/QNetworkReplyHandler.cpp:
+ (WebCore::QNetworkReplyWrapper::QNetworkReplyWrapper):
+ (WebCore::QNetworkReplyWrapper::release):
+ (WebCore::QNetworkReplyWrapper::stopForwarding):
+ Rename resetConnections to stopForwarding since not all connections are related
+ to data forwarding to the client anymore.
+ (WebCore::QNetworkReplyWrapper::receiveMetaData):
+ (WebCore::QNetworkReplyWrapper::replyDestroyed):
+ (WebCore::QNetworkReplyWrapper::didReceiveFinished):
+ * platform/network/qt/QNetworkReplyHandler.h:
+ (QNetworkReplyWrapper):
+
2013-05-15 Darin Adler <[email protected]>
[Mac] Make Clipboard::declareAndWriteDragImage non-virtual
Modified: trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp (150119 => 150120)
--- trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp 2013-05-15 16:27:43 UTC (rev 150119)
+++ trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp 2013-05-15 16:31:13 UTC (rev 150120)
@@ -269,13 +269,11 @@
{
Q_ASSERT(m_reply);
- // Allow the QNetworkReply to outlive its parent QNetworkAccessManager in case the later gets destroyed before our ResourceHandle is done with it.
- m_reply->setParent(0);
-
// 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()));
+ connect(m_reply, SIGNAL(destroyed()), this, SLOT(replyDestroyed()));
}
QNetworkReplyWrapper::~QNetworkReplyWrapper()
@@ -290,7 +288,7 @@
if (!m_reply)
return 0;
- resetConnections();
+ m_reply->disconnect(this);
QNetworkReply* reply = m_reply;
m_reply = 0;
m_sniffer = nullptr;
@@ -304,10 +302,10 @@
receiveMetaData();
}
-void QNetworkReplyWrapper::resetConnections()
+void QNetworkReplyWrapper::stopForwarding()
{
if (m_reply) {
- // Disconnect all connections except the one to setFinished() slot.
+ // Disconnect all connections that might affect the ResourceHandleClient.
m_reply->disconnect(this, SLOT(receiveMetaData()));
m_reply->disconnect(this, SLOT(didReceiveFinished()));
m_reply->disconnect(this, SLOT(didReceiveReadyRead()));
@@ -318,9 +316,8 @@
void QNetworkReplyWrapper::receiveMetaData()
{
// This slot is only used to receive the first signal from the QNetworkReply object.
- resetConnections();
+ stopForwarding();
-
WTF::String contentType = m_reply->header(QNetworkRequest::ContentTypeHeader).toString();
m_encoding = extractCharsetFromMediaType(contentType);
m_advertisedMIMEType = extractMIMETypeFromMediaType(contentType);
@@ -372,6 +369,12 @@
m_reply->setProperty("_q_isFinished", true);
}
+void QNetworkReplyWrapper::replyDestroyed()
+{
+ m_reply = 0;
+ m_sniffer = nullptr;
+}
+
void QNetworkReplyWrapper::emitMetaDataChanged()
{
QueueLocker lock(m_queue);
@@ -402,7 +405,7 @@
void QNetworkReplyWrapper::didReceiveFinished()
{
// Disconnecting will make sure that nothing will happen after emitting the finished signal.
- resetConnections();
+ stopForwarding();
m_queue->push(&QNetworkReplyHandler::finish);
}
Modified: trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.h (150119 => 150120)
--- trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.h 2013-05-15 16:27:43 UTC (rev 150119)
+++ trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.h 2013-05-15 16:31:13 UTC (rev 150120)
@@ -94,9 +94,10 @@
void didReceiveReadyRead();
void receiveSniffedMIMEType();
void setFinished();
+ void replyDestroyed();
private:
- void resetConnections();
+ void stopForwarding();
void emitMetaDataChanged();
QNetworkReply* m_reply;
Modified: trunk/Source/WebKit/qt/ChangeLog (150119 => 150120)
--- trunk/Source/WebKit/qt/ChangeLog 2013-05-15 16:27:43 UTC (rev 150119)
+++ trunk/Source/WebKit/qt/ChangeLog 2013-05-15 16:31:13 UTC (rev 150120)
@@ -1,3 +1,14 @@
+2013-05-15 Jocelyn Turcotte <[email protected]>
+
+ [Qt] Fix a crash under ~PingLoader when the QNAM on the page has been destroyed.
+ https://bugs.webkit.org/show_bug.cgi?id=116035
+
+ Reviewed by Allan Sandfeld Jensen.
+
+ * tests/qwebpage/tst_qwebpage.cpp:
+ (tst_QWebPage::networkReplyParentDidntChange): Change test to match the new expectation.
+ (tst_QWebPage::destroyQNAMBeforeAbortDoesntCrash):
+
2013-05-14 Jocelyn Turcotte <[email protected]>
[Qt] Fix a crash under ~PingLoader when the QNAM on the page has been destroyed.
Modified: trunk/Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp (150119 => 150120)
--- trunk/Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp 2013-05-15 16:27:43 UTC (rev 150119)
+++ trunk/Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp 2013-05-15 16:31:13 UTC (rev 150120)
@@ -174,7 +174,8 @@
#endif
void originatingObjectInNetworkRequests();
- void networkReplyParentChanged();
+ void networkReplyParentDidntChange();
+ void destroyQNAMBeforeAbortDoesntCrash();
void testJSPrompt();
void showModalDialog();
void testStopScheduledPageRefresh();
@@ -2848,19 +2849,32 @@
QVERIFY(qobject_cast<QWebFrame*>(networkManager->requests.at(i).originatingObject()) == childFrames.at(i));
}
-void tst_QWebPage::networkReplyParentChanged()
+void tst_QWebPage::networkReplyParentDidntChange()
{
TestNetworkManager* networkManager = new TestNetworkManager(m_page);
m_page->setNetworkAccessManager(networkManager);
networkManager->requests.clear();
- // Trigger a load and check if pending QNetworkReplies have been reparented before returning to the event loop.
+ // Trigger a load and check that pending QNetworkReplies haven't been reparented before returning to the event loop.
m_view->load(QUrl("qrc:///resources/content.html"));
QVERIFY(networkManager->requests.count() > 0);
- QVERIFY(networkManager->findChildren<QNetworkReply*>().isEmpty());
+ QVERIFY(networkManager->findChildren<QNetworkReply*>().size() > 0);
}
+void tst_QWebPage::destroyQNAMBeforeAbortDoesntCrash()
+{
+ QNetworkAccessManager* networkManager = new QNetworkAccessManager;
+ m_page->setNetworkAccessManager(networkManager);
+
+ m_view->load(QUrl("qrc:///resources/content.html"));
+ delete networkManager;
+ // This simulates what PingLoader does with its QNetworkReply when it times out.
+ // PingLoader isn't attached to a QWebPage and can be kept alive
+ // for 60000 seconds (~16.7 hours) to then cancel its ResourceHandle.
+ m_view->stop();
+}
+
/**
* Test fixups for https://bugs.webkit.org/show_bug.cgi?id=30914
*
