Changeset 228922 in webkit

Timestamp:

Feb 22, 2018 10:29:26 AM (6 years ago)

Author:

Chris Dumez

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/Document/open-with-pending-load-async-policy-expected.txt (added)
• LayoutTests/fast/dom/Document/open-with-pending-load-async-policy.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (2 diffs)
• Source/WebCore/loader/PolicyChecker.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -143 +143 @@
 #include "PluginDocument.h"
 #include "PointerLockController.h"
+#include "PolicyChecker.h"
 #include "PopStateEvent.h"
 #include "ProcessingInstruction.h"
@@ -2619 +2620 @@
         }
 
+        if (m_frame->loader().policyChecker().delegateIsDecidingNavigationPolicy())
+            m_frame->loader().policyChecker().stopCheck();
         if (m_frame->loader().state() == FrameStateProvisional)
             m_frame->loader().stopAllLoaders();

trunk/Source/WebCore/loader/PolicyChecker.cpp

@@ -146 +146 @@
     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:
@@ -162 +164 @@
         ASSERT_NOT_REACHED();
     });
-    m_delegateIsDecidingNavigationPolicy = false;
 }