[prev in list] [next in list] [prev in thread] [next in thread]
List: webkit-changes
Subject: [webkit-changes] [230133] tags/Safari-606.1.11.2/Source
From: jmarcell () apple ! com
Date: 2018-03-31 17:32:37
Message-ID: 20180331173237.C858510051CF () svn ! webkit ! org
[Download RAW message or body]
[Attachment #2 (text/html)]
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[230133] tags/Safari-606.1.11.2/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: \
verdana,arial,helvetica,sans-serif; font-size: 10pt; } #msg dl a { font-weight: \
bold} #msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: \
bold; } #msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: \
6px; } #logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em \
0; } #logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg \
h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; } \
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; \
} #logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: \
-1.5em; padding-left: 1.5em; } #logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em \
1em 0 1em; background: white;} #logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid \
#fa0; border-bottom: 1px solid #fa0; background: #fff; } #logmsg table th { \
text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted \
#fa0; } #logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: \
0.2em 0.5em; } #logmsg table thead th { text-align: center; border-bottom: 1px solid \
#fa0; } #logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: \
6px; } #patch { width: 100%; }
#patch h4 {font-family: \
verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, \
#patch .copfile {border:1px solid #ccc;margin:10px 0;} #patch ins \
{background:#dfd;text-decoration:none;display:block;padding:0 10px;} #patch del \
{background:#fdd;text-decoration:none;display:block;padding:0 10px;} #patch .lines, \
.info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a \
href="http://trac.webkit.org/projects/webkit/changeset/230133">230133</a></dd> \
<dt>Author</dt> <dd>jmarcell@apple.com</dd> <dt>Date</dt> <dd>2018-03-31 10:32:37 \
-0700 (Sat, 31 Mar 2018)</dd> </dl>
<h3>Log Message</h3>
<pre>Cherry-pick <a href="http://trac.webkit.org/projects/webkit/changeset/230128">r230128</a>. \
rdar://problem/39057006
REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/229828">r229828</a>): \
Facebook login popup is blank https://bugs.webkit.org/show_bug.cgi?id=184206
<rdar://problem/39057006>
Reviewed by Wenson Hsieh.
Source/WebCore:
Since <a href="http://trac.webkit.org/projects/webkit/changeset/229828">r229828</a>, \
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</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#tagsSafari6061112SourceWebCoreChangeLog">tags/Safari-606.1.11.2/Source/WebCore/ChangeLog</a></li>
<li><a href="#tagsSafari6061112SourceWebCoreloaderFrameLoadercpp">tags/Safari-606.1.11.2/Source/WebCore/loader/FrameLoader.cpp</a></li>
<li><a href="#tagsSafari6061112SourceWebCoreloaderPolicyCheckercpp">tags/Safari-606.1.11.2/Source/WebCore/loader/PolicyChecker.cpp</a></li>
<li><a href="#tagsSafari6061112SourceWebKitChangeLog">tags/Safari-606.1.11.2/Source/WebKit/ChangeLog</a></li>
<li><a href="#tagsSafari6061112SourceWebKitWebProcessWebCoreSupportWebFrameLoaderClie \
ntcpp">tags/Safari-606.1.11.2/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="tagsSafari6061112SourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: tags/Safari-606.1.11.2/Source/WebCore/ChangeLog \
(230132 => 230133)</h4> <pre class="diff"><span>
<span class="info">--- 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) </span><span class="lines">@@ -1,3 +1,92 @@
</span><ins>+2018-03-31 Jason Marcell <jmarcell@apple.com>
+
+ 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 <cdumez@apple.com>
+
+ 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):
+
</ins><span class="cx"> 2018-03-29 Jason Marcell <jmarcell@apple.com>
</span><span class="cx">
</span><span class="cx"> Revert r229680. rdar://problem/39011568
</span></span></pre></div>
<a id="tagsSafari6061112SourceWebCoreloaderFrameLoadercpp"></a>
<div class="modfile"><h4>Modified: \
tags/Safari-606.1.11.2/Source/WebCore/loader/FrameLoader.cpp (230132 => 230133)</h4> \
<pre class="diff"><span> <span class="info">--- \
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) </span><span class="lines">@@ -2923,8 +2923,6 @@
</span><span class="cx">
</span><span class="cx"> void \
FrameLoader::continueFragmentScrollAfterNavigationPolicy(const ResourceRequest& \
request, bool shouldContinue) </span><span class="cx"> {
</span><del>- m_client.didDecidePolicyForNavigationAction();
-
</del><span class="cx"> m_quickRedirectComing = false;
</span><span class="cx">
</span><span class="cx"> if (!shouldContinue)
</span><span class="lines">@@ -3170,8 +3168,6 @@
</span><span class="cx"> // through this method already, nested; otherwise, \
policyDataSource should still be set. </span><span class="cx"> \
ASSERT(m_policyDocumentLoader || \
!m_provisionalDocumentLoader->unreachableURL().isEmpty()); </span><span \
class="cx"> </span><del>- m_client.didDecidePolicyForNavigationAction();
-
</del><span class="cx"> bool isTargetItem = history().provisionalItem() ? \
history().provisionalItem()->isTargetItem() : false; </span><span class="cx">
</span><span class="cx"> bool urlIsDisallowed = allowNavigationToInvalidURL == \
AllowNavigationToInvalidURL::No && !request.url().isValid(); \
</span></span></pre></div> <a \
id="tagsSafari6061112SourceWebCoreloaderPolicyCheckercpp"></a> <div \
class="modfile"><h4>Modified: \
tags/Safari-606.1.11.2/Source/WebCore/loader/PolicyChecker.cpp (230132 => \
230133)</h4> <pre class="diff"><span>
<span class="info">--- \
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) </span><span class="lines">@@ -150,6 +150,8 @@
</span><span class="cx"> String suggestedFilename = \
action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute(); \
</span><span class="cx"> ResourceRequest requestCopy = request; </span><span \
class="cx"> 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 { </span><ins>+ \
m_frame.loader().client().didDecidePolicyForNavigationAction(); +
</ins><span class="cx"> m_delegateIsDecidingNavigationPolicy = false;
</span><span class="cx">
</span><span class="cx"> switch (policyAction) {
</span></span></pre></div>
<a id="tagsSafari6061112SourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: tags/Safari-606.1.11.2/Source/WebKit/ChangeLog \
(230132 => 230133)</h4> <pre class="diff"><span>
<span class="info">--- 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) </span><span class="lines">@@ -1,3 +1,72 @@
</span><ins>+2018-03-31 Jason Marcell <jmarcell@apple.com>
+
+ 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 <cdumez@apple.com>
+
+ 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):
+
</ins><span class="cx"> 2018-03-29 Jason Marcell <jmarcell@apple.com>
</span><span class="cx">
</span><span class="cx"> Revert r229680. rdar://problem/39011568
</span></span></pre></div>
<a id="tagsSafari6061112SourceWebKitWebProcessWebCoreSupportWebFrameLoaderClientcpp"></a>
<div class="modfile"><h4>Modified: \
tags/Safari-606.1.11.2/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp \
(230132 => 230133)</h4> <pre class="diff"><span>
<span class="info">--- \
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) </span><span class="lines">@@ -729,6 +729,8 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ ASSERT(!m_isDecidingNavigationPolicyDecision);
+
</ins><span class="cx"> RefPtr<API::Object> userData;
</span><span class="cx">
</span><span class="cx"> // Notify the bundle client.
</span></span></pre>
</div>
</div>
</body>
</html>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic