Document.open() cancels existing provisional load but not navigation policy check
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Feb 2018 18:29:26 +0000 (18:29 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Feb 2018 18:29:26 +0000 (18:29 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228922 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/PolicyChecker.cpp

index 1a7440b1a66e646b6279a5dc0bc840de372732a4..a26951b17dd5b1160acd89caab65f2f0e04736de 100644 (file)
@@ -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-22  Matt Lewis  <jlewis3@apple.com>
 
         Updated expectations for http/tests/appcache/404-resource-with-slow-main-resource.php.
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
new file mode 100644 (file)
index 0000000..851e017
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..15a7a53
--- /dev/null
@@ -0,0 +1,29 @@
+<script>\r
+if (window.testRunner) {\r
+    if (testRunner.setShouldDecideNavigationPolicyAfterDelay)\r
+        testRunner.setShouldDecideNavigationPolicyAfterDelay(true);\r
+    testRunner.dumpAsText();\r
+}\r
+    \r
+function runTest() {\r
+    var result = document.getElementById('result');\r
+    \r
+    var text = document.getElementById('iframe').contentDocument.documentElement.outerText;\r
+    if (text == 'REPLACED')\r
+        result.innerHTML = 'SUCCESS';\r
+    else\r
+        result.innerHTML = 'FAILURE - Got "' + text + '"';\r
+}\r
+\r
+</script>\r
+<body>\r
+<div>This tests that calling document.open on a document that has a pending load correctly cancels the load</div>\r
+<div id="result"></div>\r
+<script language="JavaScript" type="text/javascript">\r
+document.write('<iframe id="iframe" src="data:text/html,Should not be seen" onload="runTest()"></iframe>')\r
+var oRTE = frames[0].document; \r
+oRTE.open("text/html","replace");\r
+oRTE.write("REPLACED");\r
+oRTE.close();\r
+</script>\r
+</body>\r
index 050caa4123f9de056fb065a1e9c4445c8dd8c4c5..ad46d7765d6760ff951b906242a82d24041dea52 100644 (file)
@@ -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  Youenn Fablet  <youenn@apple.com>
 
         Add release asserts for service worker fetch and postMessage events
index 94232326e0b7fba2962f80b6e46fed1059dfb04b..167fa6a71d2327ae5a4528cef79ca1e768aafbc2 100644 (file)
 #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 @@ 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();
     }
index f1dc5f0829a97f7c56a32fcb4706c1bd3386e90b..68520a127911f16246cfaa791d37d498f9489b19 100644 (file)
@@ -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)