Title: [228922] trunk
Revision
228922
Author
cdu...@apple.com
Date
2018-02-22 10:29:26 -0800 (Thu, 22 Feb 2018)

Log Message

Document.open() cancels existing provisional load but not navigation policy check
https://bugs.webkit.org/show_bug.cgi?id=183012
<rdar://problem/37755831>

Reviewed by Alex Christensen.

Source/WebCore:

Test: fast/dom/Document/open-with-pending-load-async-policy.html

* dom/Document.cpp:
(WebCore::Document::open):
The existing code was calling FrameLoader::stopAllLoaders() when the loader's state
is FrameStateProvisional. The issue is that the FrameLoader's state only gets set
to FrameStateProvisional after the policy decision for the navigation is made.
This means that we fail to cancel a pending load if is still in the policy decision
stage, which can happen when the policy decision is made asynchronously. We now
also cancel such pending navigation policy checks as well.

* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
Make sure the m_delegateIsDecidingNavigationPolicy flag gets reset inside the
lambda. Otherwise, it gets reset too early when the policy decision is made
asynchronously.

LayoutTests:

Add layout test coverage.

* fast/dom/Document/open-with-pending-load-async-policy-expected.txt: Added.
* fast/dom/Document/open-with-pending-load-async-policy.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (228921 => 228922)


--- trunk/LayoutTests/ChangeLog	2018-02-22 18:22:24 UTC (rev 228921)
+++ trunk/LayoutTests/ChangeLog	2018-02-22 18:29:26 UTC (rev 228922)
@@ -1,3 +1,16 @@
+2018-02-22  Chris Dumez  <cdu...@apple.com>
+
+        Document.open() cancels existing provisional load but not navigation policy check
+        https://bugs.webkit.org/show_bug.cgi?id=183012
+        <rdar://problem/37755831>
+
+        Reviewed by Alex Christensen.
+
+        Add layout test coverage.
+
+        * fast/dom/Document/open-with-pending-load-async-policy-expected.txt: Added.
+        * fast/dom/Document/open-with-pending-load-async-policy.html: Added.
+
 2018-02-22  Matt Lewis  <jlew...@apple.com>
 
         Updated expectations for http/tests/appcache/404-resource-with-slow-main-resource.php.

Added: trunk/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt (0 => 228922)


--- trunk/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt	2018-02-22 18:29:26 UTC (rev 228922)
@@ -0,0 +1,3 @@
+This tests that calling document.open on a document that has a pending load correctly cancels the load
+SUCCESS
+

Added: trunk/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html (0 => 228922)


--- trunk/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html	2018-02-22 18:29:26 UTC (rev 228922)
@@ -0,0 +1,29 @@
+<script>
+if (window.testRunner) {
+    if (testRunner.setShouldDecideNavigationPolicyAfterDelay)
+        testRunner.setShouldDecideNavigationPolicyAfterDelay(true);
+    testRunner.dumpAsText();
+}
+    
+function runTest() {
+    var result = document.getElementById('result');
+    
+    var text = document.getElementById('iframe').contentDocument.documentElement.outerText;
+    if (text == 'REPLACED')
+        result.innerHTML = 'SUCCESS';
+    else
+        result.innerHTML = 'FAILURE - Got "' + text + '"';
+}
+
+</script>
+<body>
+<div>This tests that calling document.open on a document that has a pending load correctly cancels the load</div>
+<div id="result"></div>
+<script language="_javascript_" type="text/_javascript_">
+document.write('<iframe id="iframe" src="" not be seen" _onload_="runTest()"></iframe>')
+var oRTE = frames[0].document; 
+oRTE.open("text/html","replace");
+oRTE.write("REPLACED");
+oRTE.close();
+</script>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (228921 => 228922)


--- trunk/Source/WebCore/ChangeLog	2018-02-22 18:22:24 UTC (rev 228921)
+++ trunk/Source/WebCore/ChangeLog	2018-02-22 18:29:26 UTC (rev 228922)
@@ -1,3 +1,28 @@
+2018-02-22  Chris Dumez  <cdu...@apple.com>
+
+        Document.open() cancels existing provisional load but not navigation policy check
+        https://bugs.webkit.org/show_bug.cgi?id=183012
+        <rdar://problem/37755831>
+
+        Reviewed by Alex Christensen.
+
+        Test: fast/dom/Document/open-with-pending-load-async-policy.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::open):
+        The existing code was calling FrameLoader::stopAllLoaders() when the loader's state
+        is FrameStateProvisional. The issue is that the FrameLoader's state only gets set
+        to FrameStateProvisional after the policy decision for the navigation is made.
+        This means that we fail to cancel a pending load if is still in the policy decision
+        stage, which can happen when the policy decision is made asynchronously. We now
+        also cancel such pending navigation policy checks as well.
+
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy):
+        Make sure the m_delegateIsDecidingNavigationPolicy flag gets reset inside the
+        lambda. Otherwise, it gets reset too early when the policy decision is made
+        asynchronously.
+
 2018-02-22  Youenn Fablet  <you...@apple.com>
 
         Add release asserts for service worker fetch and postMessage events

Modified: trunk/Source/WebCore/dom/Document.cpp (228921 => 228922)


--- trunk/Source/WebCore/dom/Document.cpp	2018-02-22 18:22:24 UTC (rev 228921)
+++ trunk/Source/WebCore/dom/Document.cpp	2018-02-22 18:29:26 UTC (rev 228922)
@@ -142,6 +142,7 @@
 #include "PlugInsResources.h"
 #include "PluginDocument.h"
 #include "PointerLockController.h"
+#include "PolicyChecker.h"
 #include "PopStateEvent.h"
 #include "ProcessingInstruction.h"
 #include "PublicSuffix.h"
@@ -2618,6 +2619,8 @@
             }
         }
 
+        if (m_frame->loader().policyChecker().delegateIsDecidingNavigationPolicy())
+            m_frame->loader().policyChecker().stopCheck();
         if (m_frame->loader().state() == FrameStateProvisional)
             m_frame->loader().stopAllLoaders();
     }

Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (228921 => 228922)


--- trunk/Source/WebCore/loader/PolicyChecker.cpp	2018-02-22 18:22:24 UTC (rev 228921)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp	2018-02-22 18:29:26 UTC (rev 228922)
@@ -145,6 +145,8 @@
     String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
     ResourceRequest requestCopy = request;
     m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, [this, function = WTFMove(function), request = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename)](PolicyAction policyAction) mutable {
+        m_delegateIsDecidingNavigationPolicy = false;
+
         switch (policyAction) {
         case PolicyAction::Download:
             m_frame.loader().setOriginalURLForDownloadRequest(request);
@@ -161,7 +163,6 @@
         }
         ASSERT_NOT_REACHED();
     });
-    m_delegateIsDecidingNavigationPolicy = false;
 }
 
 void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction&& function)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to