- Revision
- 230133
- Author
- [email protected]
- Date
- 2018-03-31 10:32:37 -0700 (Sat, 31 Mar 2018)
Log Message
Cherry-pick r230128. rdar://problem/39057006
REGRESSION (r229828): Facebook login popup is blank
https://bugs.webkit.org/show_bug.cgi?id=184206
<rdar://problem/39057006>
Reviewed by Wenson Hsieh.
Source/WebCore:
Since r229828, we freeze the layer tree during the navigation policy check.
We freeze in WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction()
and unfreeze in WebFrameLoaderClient::didDecidePolicyForNavigationAction().
WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction() gets called
from PolicyChecker::checkNavigationPolicy() which has 3 call sites in
FrameLoader and one in DocumentLoader for redirects. The call sites in
FrameLoader were taking care of calling didDecidePolicyForNavigationAction()
on the FrameLoaderClient in their completion handler, but the DocumentLoader
call site was failing to do so. As a result, the layer tree would stay frozen.
To make this a lot less error prone, I moved the call to
WebFrameLoaderClient::didDecidePolicyForNavigationAction() to
PolicyChecker::checkNavigationPolicy(), inside the completion handler passed
to WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(). This way,
even if new code starts calling PolicyChecker::checkNavigationPolicy(), we
do not need to worry about letting the client know when the policy decision
is made.
No new tests, covered by existing redirection tests with the
new assertion I added.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy):
(WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
Source/WebKit:
Add assertion to make sure we never try to do a policy check to
a resource response while a policy check for a navigation is
pending. This assertion was being hit by several of our redirection
tests without my fix.
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230128 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: tags/Safari-606.1.11.2/Source/WebCore/ChangeLog (230132 => 230133)
--- tags/Safari-606.1.11.2/Source/WebCore/ChangeLog 2018-03-31 17:26:53 UTC (rev 230132)
+++ tags/Safari-606.1.11.2/Source/WebCore/ChangeLog 2018-03-31 17:32:37 UTC (rev 230133)
@@ -1,3 +1,92 @@
+2018-03-31 Jason Marcell <[email protected]>
+
+ Cherry-pick r230128. rdar://problem/39057006
+
+ REGRESSION (r229828): Facebook login popup is blank
+ https://bugs.webkit.org/show_bug.cgi?id=184206
+ <rdar://problem/39057006>
+
+ Reviewed by Wenson Hsieh.
+
+ Source/WebCore:
+
+ Since r229828, we freeze the layer tree during the navigation policy check.
+ We freeze in WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction()
+ and unfreeze in WebFrameLoaderClient::didDecidePolicyForNavigationAction().
+
+ WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction() gets called
+ from PolicyChecker::checkNavigationPolicy() which has 3 call sites in
+ FrameLoader and one in DocumentLoader for redirects. The call sites in
+ FrameLoader were taking care of calling didDecidePolicyForNavigationAction()
+ on the FrameLoaderClient in their completion handler, but the DocumentLoader
+ call site was failing to do so. As a result, the layer tree would stay frozen.
+
+ To make this a lot less error prone, I moved the call to
+ WebFrameLoaderClient::didDecidePolicyForNavigationAction() to
+ PolicyChecker::checkNavigationPolicy(), inside the completion handler passed
+ to WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(). This way,
+ even if new code starts calling PolicyChecker::checkNavigationPolicy(), we
+ do not need to worry about letting the client know when the policy decision
+ is made.
+
+ No new tests, covered by existing redirection tests with the
+ new assertion I added.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+
+ Source/WebKit:
+
+ Add assertion to make sure we never try to do a policy check to
+ a resource response while a policy check for a navigation is
+ pending. This assertion was being hit by several of our redirection
+ tests without my fix.
+
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230128 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-03-30 Chris Dumez <[email protected]>
+
+ REGRESSION (r229828): Facebook login popup is blank
+ https://bugs.webkit.org/show_bug.cgi?id=184206
+ <rdar://problem/39057006>
+
+ Reviewed by Wenson Hsieh.
+
+ Since r229828, we freeze the layer tree during the navigation policy check.
+ We freeze in WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction()
+ and unfreeze in WebFrameLoaderClient::didDecidePolicyForNavigationAction().
+
+ WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction() gets called
+ from PolicyChecker::checkNavigationPolicy() which has 3 call sites in
+ FrameLoader and one in DocumentLoader for redirects. The call sites in
+ FrameLoader were taking care of calling didDecidePolicyForNavigationAction()
+ on the FrameLoaderClient in their completion handler, but the DocumentLoader
+ call site was failing to do so. As a result, the layer tree would stay frozen.
+
+ To make this a lot less error prone, I moved the call to
+ WebFrameLoaderClient::didDecidePolicyForNavigationAction() to
+ PolicyChecker::checkNavigationPolicy(), inside the completion handler passed
+ to WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(). This way,
+ even if new code starts calling PolicyChecker::checkNavigationPolicy(), we
+ do not need to worry about letting the client know when the policy decision
+ is made.
+
+ No new tests, covered by existing redirection tests with the
+ new assertion I added.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+
2018-03-29 Jason Marcell <[email protected]>
Revert r229680. rdar://problem/39011568
Modified: tags/Safari-606.1.11.2/Source/WebCore/loader/FrameLoader.cpp (230132 => 230133)
--- tags/Safari-606.1.11.2/Source/WebCore/loader/FrameLoader.cpp 2018-03-31 17:26:53 UTC (rev 230132)
+++ tags/Safari-606.1.11.2/Source/WebCore/loader/FrameLoader.cpp 2018-03-31 17:32:37 UTC (rev 230133)
@@ -2923,8 +2923,6 @@
void FrameLoader::continueFragmentScrollAfterNavigationPolicy(const ResourceRequest& request, bool shouldContinue)
{
- m_client.didDecidePolicyForNavigationAction();
-
m_quickRedirectComing = false;
if (!shouldContinue)
@@ -3170,8 +3168,6 @@
// through this method already, nested; otherwise, policyDataSource should still be set.
ASSERT(m_policyDocumentLoader || !m_provisionalDocumentLoader->unreachableURL().isEmpty());
- m_client.didDecidePolicyForNavigationAction();
-
bool isTargetItem = history().provisionalItem() ? history().provisionalItem()->isTargetItem() : false;
bool urlIsDisallowed = allowNavigationToInvalidURL == AllowNavigationToInvalidURL::No && !request.url().isValid();
Modified: tags/Safari-606.1.11.2/Source/WebCore/loader/PolicyChecker.cpp (230132 => 230133)
--- tags/Safari-606.1.11.2/Source/WebCore/loader/PolicyChecker.cpp 2018-03-31 17:26:53 UTC (rev 230132)
+++ tags/Safari-606.1.11.2/Source/WebCore/loader/PolicyChecker.cpp 2018-03-31 17:32:37 UTC (rev 230133)
@@ -150,6 +150,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_frame.loader().client().didDecidePolicyForNavigationAction();
+
m_delegateIsDecidingNavigationPolicy = false;
switch (policyAction) {
Modified: tags/Safari-606.1.11.2/Source/WebKit/ChangeLog (230132 => 230133)
--- tags/Safari-606.1.11.2/Source/WebKit/ChangeLog 2018-03-31 17:26:53 UTC (rev 230132)
+++ tags/Safari-606.1.11.2/Source/WebKit/ChangeLog 2018-03-31 17:32:37 UTC (rev 230133)
@@ -1,3 +1,72 @@
+2018-03-31 Jason Marcell <[email protected]>
+
+ Cherry-pick r230128. rdar://problem/39057006
+
+ REGRESSION (r229828): Facebook login popup is blank
+ https://bugs.webkit.org/show_bug.cgi?id=184206
+ <rdar://problem/39057006>
+
+ Reviewed by Wenson Hsieh.
+
+ Source/WebCore:
+
+ Since r229828, we freeze the layer tree during the navigation policy check.
+ We freeze in WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction()
+ and unfreeze in WebFrameLoaderClient::didDecidePolicyForNavigationAction().
+
+ WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction() gets called
+ from PolicyChecker::checkNavigationPolicy() which has 3 call sites in
+ FrameLoader and one in DocumentLoader for redirects. The call sites in
+ FrameLoader were taking care of calling didDecidePolicyForNavigationAction()
+ on the FrameLoaderClient in their completion handler, but the DocumentLoader
+ call site was failing to do so. As a result, the layer tree would stay frozen.
+
+ To make this a lot less error prone, I moved the call to
+ WebFrameLoaderClient::didDecidePolicyForNavigationAction() to
+ PolicyChecker::checkNavigationPolicy(), inside the completion handler passed
+ to WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(). This way,
+ even if new code starts calling PolicyChecker::checkNavigationPolicy(), we
+ do not need to worry about letting the client know when the policy decision
+ is made.
+
+ No new tests, covered by existing redirection tests with the
+ new assertion I added.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+
+ Source/WebKit:
+
+ Add assertion to make sure we never try to do a policy check to
+ a resource response while a policy check for a navigation is
+ pending. This assertion was being hit by several of our redirection
+ tests without my fix.
+
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230128 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-03-30 Chris Dumez <[email protected]>
+
+ REGRESSION (r229828): Facebook login popup is blank
+ https://bugs.webkit.org/show_bug.cgi?id=184206
+ <rdar://problem/39057006>
+
+ Reviewed by Wenson Hsieh.
+
+ Add assertion to make sure we never try to do a policy check to
+ a resource response while a policy check for a navigation is
+ pending. This assertion was being hit by several of our redirection
+ tests without my fix.
+
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
+
2018-03-29 Jason Marcell <[email protected]>
Revert r229680. rdar://problem/39011568
Modified: tags/Safari-606.1.11.2/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (230132 => 230133)
--- tags/Safari-606.1.11.2/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2018-03-31 17:26:53 UTC (rev 230132)
+++ tags/Safari-606.1.11.2/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2018-03-31 17:32:37 UTC (rev 230133)
@@ -729,6 +729,8 @@
return;
}
+ ASSERT(!m_isDecidingNavigationPolicyDecision);
+
RefPtr<API::Object> userData;
// Notify the bundle client.
