Title: [229596] trunk/Source/WebCore
Revision
229596
Author
cdu...@apple.com
Date
2018-03-13 18:02:21 -0700 (Tue, 13 Mar 2018)

Log Message

fast/loader/_javascript_-url-iframe-remove-on-navigate.html is a flaky crash on iOS with async delegates
https://bugs.webkit.org/show_bug.cgi?id=183610

Reviewed by Youenn Fablet.

The issue was that in DocumentLoader::loadMainResource(), the call to requestMainResource() which
return null due to the load getting cancelled synchronously. If this load is the parent frame's last
pending load, then the 'load' event gets fired in the parent frame. In the test, the parent frame's
load event handler does a document.write() call which blows away the iframe. As a result, when
we return from the requestMainResource(), m_frame is null and we crash later on dereferencing it.

No new tests, covered by fast/loader/_javascript_-url-iframe-remove-on-navigate-async-delegate.html
which was crashing flakily.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::loadMainResource):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (229595 => 229596)


--- trunk/Source/WebCore/ChangeLog	2018-03-14 01:00:17 UTC (rev 229595)
+++ trunk/Source/WebCore/ChangeLog	2018-03-14 01:02:21 UTC (rev 229596)
@@ -1,3 +1,22 @@
+2018-03-13  Chris Dumez  <cdu...@apple.com>
+
+        fast/loader/_javascript_-url-iframe-remove-on-navigate.html is a flaky crash on iOS with async delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183610
+
+        Reviewed by Youenn Fablet.
+
+        The issue was that in DocumentLoader::loadMainResource(), the call to requestMainResource() which
+        return null due to the load getting cancelled synchronously. If this load is the parent frame's last
+        pending load, then the 'load' event gets fired in the parent frame. In the test, the parent frame's
+        load event handler does a document.write() call which blows away the iframe. As a result, when
+        we return from the requestMainResource(), m_frame is null and we crash later on dereferencing it.
+
+        No new tests, covered by fast/loader/_javascript_-url-iframe-remove-on-navigate-async-delegate.html
+        which was crashing flakily.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::loadMainResource):
+
 2018-03-13  Jer Noble  <jer.no...@apple.com>
 
         [iOS] Muted media playback can interrupt out-of-process audio

Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (229595 => 229596)


--- trunk/Source/WebCore/loader/DocumentLoader.cpp	2018-03-14 01:00:17 UTC (rev 229595)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp	2018-03-14 01:02:21 UTC (rev 229596)
@@ -1718,15 +1718,12 @@
 
     m_mainResource = m_cachedResourceLoader->requestMainResource(WTFMove(mainResourceRequest)).value_or(nullptr);
 
-#if ENABLE(CONTENT_EXTENSIONS)
-    if (m_mainResource && m_mainResource->errorOccurred() && m_frame->page() && m_mainResource->resourceError().domain() == ContentExtensions::WebKitContentBlockerDomain) {
-        RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Blocked by content blocker error (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
-        cancelMainResourceLoad(frameLoader()->blockedByContentBlockerError(m_request));
-        return;
-    }
-#endif
+    if (!m_mainResource) {
+        // The frame may have gone away if this load was cancelled synchronously and this was the last pending load.
+        // This is because we may have fired the load event in a parent frame.
+        if (!m_frame)
+            return;
 
-    if (!m_mainResource) {
         if (!m_request.url().isValid()) {
             RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Unable to load main resource, URL is invalid (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
             cancelMainResourceLoad(frameLoader()->client().cannotShowURLError(m_request));
@@ -1744,6 +1741,16 @@
         return;
     }
 
+    ASSERT(m_frame);
+
+#if ENABLE(CONTENT_EXTENSIONS)
+    if (m_mainResource->errorOccurred() && m_frame->page() && m_mainResource->resourceError().domain() == ContentExtensions::WebKitContentBlockerDomain) {
+        RELEASE_LOG_IF_ALLOWED("startLoadingMainResource: Blocked by content blocker error (frame = %p, main = %d)", m_frame, m_frame->isMainFrame());
+        cancelMainResourceLoad(frameLoader()->blockedByContentBlockerError(m_request));
+        return;
+    }
+#endif
+
     if (!mainResourceLoader()) {
         m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
         frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, request);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to