Title: [91189] trunk
Revision
91189
Author
[email protected]
Date
2011-07-18 09:48:32 -0700 (Mon, 18 Jul 2011)

Log Message

[Qt] ASSERTION FAILED in ResourceHandle::setDefersLoading causes crash
https://bugs.webkit.org/show_bug.cgi?id=62808

Source/WebCore:

The assertion in ResourceHandle::setDefersLoading assumes asynchronous
content delivery -- To resume a page, first, its main resource loader
calls setDefersLoading to resume loading the main content; then all the
sub-resource loaders calls setDefersLoading to resume sub-contents.
However, since QNetworkReplyHandler delivers content synchronously,
some new sub-resource loaders get created as soon as the main resource
loader resumed, and all these new sub-resource loaders set their
defersLoading flag to false. Then, the assertion fails for these new
sub-resource loaders when calling setDefersLoading on them. As a fix,
this path makes QNetworkReplyHandler deliver content asynchronously
when its load type is set to SynchronousLoad.

Reviewed by Benjamin Poulain.

Test: loader/load-defer-resume-crash.html

* platform/network/qt/QNetworkReplyHandler.cpp:
(WebCore::QNetworkReplyHandlerCallQueue::setDeferSignals):
* platform/network/qt/QNetworkReplyHandler.h:
(WebCore::QNetworkReplyHandler::setLoadingDeferred):

LayoutTests:

Added a test for the crash occurs when load deferring is turned off.

Reviewed by Benjamin Poulain.

* loader/load-defer-resume-crash-expected.txt: Added.
* loader/load-defer-resume-crash.html: Added.
* loader/resources/images.html: Added.
* platform/chromium/test_expectations.txt: Skip this test since the LayoutTestController::setDefersLoading is not implemented for chromium.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (91188 => 91189)


--- trunk/LayoutTests/ChangeLog	2011-07-18 16:43:27 UTC (rev 91188)
+++ trunk/LayoutTests/ChangeLog	2011-07-18 16:48:32 UTC (rev 91189)
@@ -1,3 +1,17 @@
+2011-07-18  Yi Shen  <[email protected]>
+
+        [Qt] ASSERTION FAILED in ResourceHandle::setDefersLoading causes crash
+        https://bugs.webkit.org/show_bug.cgi?id=62808
+
+        Added a test for the crash occurs when load deferring is turned off.
+
+        Reviewed by Benjamin Poulain.
+
+        * loader/load-defer-resume-crash-expected.txt: Added.
+        * loader/load-defer-resume-crash.html: Added.
+        * loader/resources/images.html: Added.
+        * platform/chromium/test_expectations.txt: Skip this test since the LayoutTestController::setDefersLoading is not implemented for chromium.
+
 2011-07-18  Young Han Lee  <[email protected]>
 
         AnimationBase asserts if a test tries to pause during the delay phase

Added: trunk/LayoutTests/loader/load-defer-resume-crash-expected.txt (0 => 91189)


--- trunk/LayoutTests/loader/load-defer-resume-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/loader/load-defer-resume-crash-expected.txt	2011-07-18 16:48:32 UTC (rev 91189)
@@ -0,0 +1,3 @@
+For the test to pass there should be no crash.
+
+

Added: trunk/LayoutTests/loader/load-defer-resume-crash.html (0 => 91189)


--- trunk/LayoutTests/loader/load-defer-resume-crash.html	                        (rev 0)
+++ trunk/LayoutTests/loader/load-defer-resume-crash.html	2011-07-18 16:48:32 UTC (rev 91189)
@@ -0,0 +1,24 @@
+<html>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+function run() {
+    var frameElement = document.createElement('iframe')
+    frameElement.setAttribute("src", "resources/images.html");
+    document.getElementById("frameDiv").appendChild(frameElement);
+    if (window.layoutTestController) {
+        layoutTestController.setDefersLoading(true);
+        setTimeout("layoutTestController.setDefersLoading(false);layoutTestController.notifyDone();",1000);
+    } else
+        alert("Deferring loads");
+}
+
+</script>
+<body _onload_='run()'>
+<p>For the test to pass there should be no crash.</p>
+<div id="frameDiv"></div>
+</body>
+</html>

Added: trunk/LayoutTests/loader/resources/images.html (0 => 91189)


--- trunk/LayoutTests/loader/resources/images.html	                        (rev 0)
+++ trunk/LayoutTests/loader/resources/images.html	2011-07-18 16:48:32 UTC (rev 91189)
@@ -0,0 +1,6 @@
+<html>
+<body>
+<img border="0" src=""
+<img border="0" src=""
+</body>
+</html>

Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (91188 => 91189)


--- trunk/LayoutTests/platform/chromium/test_expectations.txt	2011-07-18 16:43:27 UTC (rev 91188)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt	2011-07-18 16:48:32 UTC (rev 91189)
@@ -160,6 +160,7 @@
 
 // Unskip after implementing LayoutTestController::setDefersLoading and ::goBack.
 BUGWK60877 SKIP : loader/navigation-while-deferring-loads.html = FAIL
+BUGWK60877 SKIP : loader/load-defer-resume-crash.html = FAIL
 
 // Skipped until new WebSocket protocol is implemented.
 BUGWK50099 SKIP : http/tests/websocket/tests/hybi/ = PASS FAIL TIMEOUT

Modified: trunk/Source/WebCore/ChangeLog (91188 => 91189)


--- trunk/Source/WebCore/ChangeLog	2011-07-18 16:43:27 UTC (rev 91188)
+++ trunk/Source/WebCore/ChangeLog	2011-07-18 16:48:32 UTC (rev 91189)
@@ -1,3 +1,29 @@
+2011-07-18  Yi Shen  <[email protected]>
+
+        [Qt] ASSERTION FAILED in ResourceHandle::setDefersLoading causes crash
+        https://bugs.webkit.org/show_bug.cgi?id=62808
+
+        The assertion in ResourceHandle::setDefersLoading assumes asynchronous
+        content delivery -- To resume a page, first, its main resource loader
+        calls setDefersLoading to resume loading the main content; then all the
+        sub-resource loaders calls setDefersLoading to resume sub-contents.
+        However, since QNetworkReplyHandler delivers content synchronously,
+        some new sub-resource loaders get created as soon as the main resource
+        loader resumed, and all these new sub-resource loaders set their
+        defersLoading flag to false. Then, the assertion fails for these new
+        sub-resource loaders when calling setDefersLoading on them. As a fix,
+        this path makes QNetworkReplyHandler deliver content asynchronously
+        when its load type is set to SynchronousLoad.
+
+        Reviewed by Benjamin Poulain.
+
+        Test: loader/load-defer-resume-crash.html
+
+        * platform/network/qt/QNetworkReplyHandler.cpp:
+        (WebCore::QNetworkReplyHandlerCallQueue::setDeferSignals):
+        * platform/network/qt/QNetworkReplyHandler.h:
+        (WebCore::QNetworkReplyHandler::setLoadingDeferred):
+
 2011-07-18  Young Han Lee  <[email protected]>
 
         AnimationBase asserts if a test tries to pause during the delay phase

Modified: trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp (91188 => 91189)


--- trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp	2011-07-18 16:43:27 UTC (rev 91188)
+++ trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp	2011-07-18 16:48:32 UTC (rev 91189)
@@ -178,10 +178,13 @@
     flush();
 }
 
-void QNetworkReplyHandlerCallQueue::setDeferSignals(bool defer)
+void QNetworkReplyHandlerCallQueue::setDeferSignals(bool defer, bool sync)
 {
     m_deferSignals = defer;
-    flush();
+    if (sync)
+        flush();
+    else
+        QMetaObject::invokeMethod(this, "flush",  Qt::QueuedConnection);
 }
 
 void QNetworkReplyHandlerCallQueue::flush()

Modified: trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.h (91188 => 91189)


--- trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.h	2011-07-18 16:43:27 UTC (rev 91188)
+++ trunk/Source/WebCore/platform/network/qt/QNetworkReplyHandler.h	2011-07-18 16:48:32 UTC (rev 91189)
@@ -41,11 +41,12 @@
 class ResourceResponse;
 class QNetworkReplyHandler;
 
-class QNetworkReplyHandlerCallQueue {
+class QNetworkReplyHandlerCallQueue : public QObject {
+    Q_OBJECT
 public:
     QNetworkReplyHandlerCallQueue(QNetworkReplyHandler*, bool deferSignals);
     bool deferSignals() const { return m_deferSignals; }
-    void setDeferSignals(bool);
+    void setDeferSignals(bool, bool sync = false);
 
     typedef void (QNetworkReplyHandler::*EnqueuedCall)();
     void push(EnqueuedCall method);
@@ -60,7 +61,7 @@
     bool m_flushing;
     QList<EnqueuedCall> m_enqueuedCalls;
 
-    void flush();
+    Q_INVOKABLE void flush();
 };
 
 class QNetworkReplyWrapper : public QObject {
@@ -120,7 +121,7 @@
     };
 
     QNetworkReplyHandler(ResourceHandle*, LoadType, bool deferred = false);
-    void setLoadingDeferred(bool deferred) { m_queue.setDeferSignals(deferred); }
+    void setLoadingDeferred(bool deferred) { m_queue.setDeferSignals(deferred, m_loadType == SynchronousLoad); }
 
     QNetworkReply* reply() const { return m_replyWrapper ? m_replyWrapper->reply() : 0; }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to