Merge r228922 - 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.
WebKit · Mar 5, 2018 · 394524b · 394524b
1 parent 40ff8ef
commit 394524b
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 1 deletion.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2018-02-22  Chris Dumez  <cdumez@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-09  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: Object.shallowEqual always fails when comparing array property values

diff --git a/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt b/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt
@@ -0,0 +1,3 @@
+This tests that calling document.open on a document that has a pending load correctly cancels the load
+SUCCESS
+
diff --git a/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html b/LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html
@@ -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="data:text/html,Should not be seen" onload="runTest()"></iframe>')
+var oRTE = frames[0].document; 
+oRTE.open("text/html","replace");
+oRTE.write("REPLACED");
+oRTE.close();
+</script>
+</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2018-02-22  Chris Dumez  <cdumez@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  Miguel Gomez  <magomez@igalia.com>
 
         including both gl3.h and gl2.h when USE_OPENGL_ES is enabled

diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
@@ -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"
@@ -2622,6 +2623,8 @@ void Document::open(Document* responsibleDocument)
             }
         }
 
+        if (m_frame->loader().policyChecker().delegateIsDecidingNavigationPolicy())
+            m_frame->loader().policyChecker().stopCheck();
         if (m_frame->loader().state() == FrameStateProvisional)
             m_frame->loader().stopAllLoaders();
     }

diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
@@ -145,6 +145,8 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
     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 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
         }
         ASSERT_NOT_REACHED();
     });
-    m_delegateIsDecidingNavigationPolicy = false;
 }
 
 void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction&& function)