<!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>[286479] trunk/Source/WebKit</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/286479">286479</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2021-12-02 20:37:35 -0800 (Thu, 02 Dec 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>Regression(<a href="http://trac.webkit.org/projects/webkit/changeset/283179">r283179</a>) Google Drive freezes after downloading a folder
https://bugs.webkit.org/show_bug.cgi?id=233783
<rdar://85918531>

Reviewed by Darin Adler.

When process-swapping on a navigation response due to COOP, we create a new ProvisionalPageProxy
to trigger a provisional load in a new WebProcess. In the common case, the ProvisionalPageProxy
gets committed, we process-swap and everything works fine. However, if the client decides to
convert the navigation into a download (like in the Google Drive case), then the
ProvisionalPageProxy gets destroyed without being committed. The issue is that the committed
process still thinks at this point that it is in the middle of a navigation and its layer
tree is thus frozen with reason=PageTransition. This is what makes Google Drive look frozen
after the download. This is not an issue with PSON because the process-swap happens in
decidePolicyForNavigationAction() and we tell the previous process to ignore the navigation
when we process-swap.

To address the issue, we now tell the committed process to cancel its navigation if the
ProvisionalPageProxy ends up getting destroyed without being committed. This lets the
committed process know there is no point in waiting for this navigation to happen and allows
it to unfreeze its layer tree.

* UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
(WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
* UIProcess/ProvisionalPageProxy.h:
(WebKit::ProvisionalPageProxy::isProcessSwappingOnNavigationResponse const):
(WebKit::ProvisionalPageProxy::shouldClosePreviousPageAfterCommit const): Deleted.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::commitProvisionalPage):
(WebKit::WebPageProxy::continueNavigationInNewProcess):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitUIProcessProvisionalPageProxycpp">trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp</a></li>
<li><a href="#trunkSourceWebKitUIProcessProvisionalPageProxyh">trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h</a></li>
<li><a href="#trunkSourceWebKitUIProcessWebPageProxycpp">trunk/Source/WebKit/UIProcess/WebPageProxy.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (286478 => 286479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/ChangeLog    2021-12-03 04:24:41 UTC (rev 286478)
+++ trunk/Source/WebKit/ChangeLog       2021-12-03 04:37:35 UTC (rev 286479)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2021-12-02  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r283179) Google Drive freezes after downloading a folder
+        https://bugs.webkit.org/show_bug.cgi?id=233783
+        <rdar://85918531>
+
+        Reviewed by Darin Adler.
+
+        When process-swapping on a navigation response due to COOP, we create a new ProvisionalPageProxy
+        to trigger a provisional load in a new WebProcess. In the common case, the ProvisionalPageProxy
+        gets committed, we process-swap and everything works fine. However, if the client decides to
+        convert the navigation into a download (like in the Google Drive case), then the
+        ProvisionalPageProxy gets destroyed without being committed. The issue is that the committed
+        process still thinks at this point that it is in the middle of a navigation and its layer
+        tree is thus frozen with reason=PageTransition. This is what makes Google Drive look frozen
+        after the download. This is not an issue with PSON because the process-swap happens in
+        decidePolicyForNavigationAction() and we tell the previous process to ignore the navigation
+        when we process-swap.
+
+        To address the issue, we now tell the committed process to cancel its navigation if the
+        ProvisionalPageProxy ends up getting destroyed without being committed. This lets the
+        committed process know there is no point in waiting for this navigation to happen and allows
+        it to unfreeze its layer tree.
+
+        * UIProcess/ProvisionalPageProxy.cpp:
+        (WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
+        (WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
+        * UIProcess/ProvisionalPageProxy.h:
+        (WebKit::ProvisionalPageProxy::isProcessSwappingOnNavigationResponse const):
+        (WebKit::ProvisionalPageProxy::shouldClosePreviousPageAfterCommit const): Deleted.
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::commitProvisionalPage):
+        (WebKit::WebPageProxy::continueNavigationInNewProcess):
+
</ins><span class="cx"> 2021-12-02  Rachel Ginsberg  <rginsberg@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Make enum values consisten in ApplicationManifest::Icon::Purpose
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessProvisionalPageProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (286478 => 286479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp   2021-12-03 04:24:41 UTC (rev 286478)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp      2021-12-03 04:37:35 UTC (rev 286479)
</span><span class="lines">@@ -58,7 +58,7 @@
</span><span class="cx"> #define PROVISIONALPAGEPROXY_RELEASE_LOG(channel, fmt, ...) RELEASE_LOG(channel, "%p - [pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", PID=%i, navigationID=%" PRIu64 "] ProvisionalPageProxy::" fmt, this, m_page.identifier().toUInt64(), m_webPageID.toUInt64(), m_process->processIdentifier(), m_navigationID, ##__VA_ARGS__)
</span><span class="cx"> #define PROVISIONALPAGEPROXY_RELEASE_LOG_ERROR(channel, fmt, ...) RELEASE_LOG_ERROR(channel, "%p - [pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", PID=%i, navigationID=%" PRIu64 "] ProvisionalPageProxy::" fmt, this, m_page.identifier().toUInt64(), m_webPageID.toUInt64(), m_process->processIdentifier(), m_navigationID, ##__VA_ARGS__)
</span><span class="cx"> 
</span><del>-ProvisionalPageProxy::ProvisionalPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, std::unique_ptr<SuspendedPageProxy> suspendedPage, uint64_t navigationID, bool isServerRedirect, const WebCore::ResourceRequest& request, ProcessSwapRequestedByClient processSwapRequestedByClient, bool shouldClosePreviousPageAfterCommit, API::WebsitePolicies* websitePolicies)
</del><ins>+ProvisionalPageProxy::ProvisionalPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, std::unique_ptr<SuspendedPageProxy> suspendedPage, uint64_t navigationID, bool isServerRedirect, const WebCore::ResourceRequest& request, ProcessSwapRequestedByClient processSwapRequestedByClient, bool isProcessSwappingOnNavigationResponse, API::WebsitePolicies* websitePolicies)
</ins><span class="cx">     : m_page(page)
</span><span class="cx">     , m_webPageID(suspendedPage ? suspendedPage->webPageID() : PageIdentifier::generate())
</span><span class="cx">     , m_process(WTFMove(process))
</span><span class="lines">@@ -66,7 +66,7 @@
</span><span class="cx">     , m_isServerRedirect(isServerRedirect)
</span><span class="cx">     , m_request(request)
</span><span class="cx">     , m_processSwapRequestedByClient(processSwapRequestedByClient)
</span><del>-    , m_shouldClosePreviousPageAfterCommit(shouldClosePreviousPageAfterCommit)
</del><ins>+    , m_isProcessSwappingOnNavigationResponse(isProcessSwappingOnNavigationResponse)
</ins><span class="cx"> #if PLATFORM(IOS_FAMILY)
</span><span class="cx">     , m_provisionalLoadActivity(m_process->throttler().foregroundActivity("Provisional Load"_s))
</span><span class="cx"> #endif
</span><span class="lines">@@ -117,6 +117,12 @@
</span><span class="cx">         m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
</span><span class="cx">         send(Messages::WebPage::Close());
</span><span class="cx">         m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
</span><ins>+
+        // If we were process-swapping on navigation response then there is still a provisional load going on in the previous process
+        // and its layer tree is frozen. Since we didn't end up committing the provisional process, we need to stop the load in the
+        // previous process so that it cancels its navigation and unfreezes its layer tree.
+        if (isProcessSwappingOnNavigationResponse())
+            m_page.send(Messages::WebPage::StopLoading());
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     m_process->removeProvisionalPageProxy(*this);
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessProvisionalPageProxyh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h (286478 => 286479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h     2021-12-03 04:24:41 UTC (rev 286478)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h        2021-12-03 04:37:35 UTC (rev 286479)
</span><span class="lines">@@ -73,7 +73,7 @@
</span><span class="cx"> class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSender {
</span><span class="cx">     WTF_MAKE_FAST_ALLOCATED;
</span><span class="cx"> public:
</span><del>-    ProvisionalPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>, uint64_t navigationID, bool isServerRedirect, const WebCore::ResourceRequest&, ProcessSwapRequestedByClient, bool shouldClosePreviousPageAfterCommit, API::WebsitePolicies*);
</del><ins>+    ProvisionalPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>, uint64_t navigationID, bool isServerRedirect, const WebCore::ResourceRequest&, ProcessSwapRequestedByClient, bool isProcessSwappingOnNavigationResponse, API::WebsitePolicies*);
</ins><span class="cx">     ~ProvisionalPageProxy();
</span><span class="cx"> 
</span><span class="cx">     WebPageProxy& page() const { return m_page; }
</span><span class="lines">@@ -84,7 +84,7 @@
</span><span class="cx">     uint64_t navigationID() const { return m_navigationID; }
</span><span class="cx">     const URL& provisionalURL() const { return m_provisionalLoadURL; }
</span><span class="cx"> 
</span><del>-    bool shouldClosePreviousPageAfterCommit() const { return m_shouldClosePreviousPageAfterCommit; }
</del><ins>+    bool isProcessSwappingOnNavigationResponse() const { return m_isProcessSwappingOnNavigationResponse; }
</ins><span class="cx"> 
</span><span class="cx">     DrawingAreaProxy* drawingArea() const { return m_drawingArea.get(); }
</span><span class="cx">     std::unique_ptr<DrawingAreaProxy> takeDrawingArea();
</span><span class="lines">@@ -171,7 +171,7 @@
</span><span class="cx">     WebCore::ResourceRequest m_request;
</span><span class="cx">     ProcessSwapRequestedByClient m_processSwapRequestedByClient;
</span><span class="cx">     bool m_wasCommitted { false };
</span><del>-    bool m_shouldClosePreviousPageAfterCommit { false };
</del><ins>+    bool m_isProcessSwappingOnNavigationResponse { false };
</ins><span class="cx">     URL m_provisionalLoadURL;
</span><span class="cx"> 
</span><span class="cx"> #if PLATFORM(COCOA)
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessWebPageProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (286478 => 286479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp   2021-12-03 04:24:41 UTC (rev 286478)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp      2021-12-03 04:37:35 UTC (rev 286479)
</span><span class="lines">@@ -3572,7 +3572,7 @@
</span><span class="cx"> 
</span><span class="cx">     removeAllMessageReceivers();
</span><span class="cx">     auto* navigation = navigationState().navigation(m_provisionalPage->navigationID());
</span><del>-    bool didSuspendPreviousPage = navigation && !m_provisionalPage->shouldClosePreviousPageAfterCommit() ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient(), shouldDelayClosingUntilFirstLayerFlush) : false;
</del><ins>+    bool didSuspendPreviousPage = navigation && !m_provisionalPage->isProcessSwappingOnNavigationResponse() ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient(), shouldDelayClosingUntilFirstLayerFlush) : false;
</ins><span class="cx">     m_process->removeWebPage(*this, m_websiteDataStore.ptr() == &m_provisionalPage->process().websiteDataStore() ? WebProcessProxy::EndsUsingDataStore::No : WebProcessProxy::EndsUsingDataStore::Yes);
</span><span class="cx"> 
</span><span class="cx">     // There is no way we'll be able to return to the page in the previous page so close it.
</span><span class="lines">@@ -3602,8 +3602,8 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     bool isServerSideRedirect = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision && navigation.currentRequestIsRedirect();
</span><del>-    bool shouldClosePreviousPageAfterCommit = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted;
-    m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, WTFMove(newProcess), WTFMove(suspendedPage), navigation.navigationID(), isServerSideRedirect, navigation.currentRequest(), processSwapRequestedByClient, shouldClosePreviousPageAfterCommit, websitePolicies.get());
</del><ins>+    bool isProcessSwappingOnNavigationResponse = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted;
+    m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, WTFMove(newProcess), WTFMove(suspendedPage), navigation.navigationID(), isServerSideRedirect, navigation.currentRequest(), processSwapRequestedByClient, isProcessSwappingOnNavigationResponse, websitePolicies.get());
</ins><span class="cx">     auto continuation = [this, protectedThis = Ref { *this }, navigation = Ref { navigation }, shouldTreatAsContinuingLoad, websitePolicies = WTFMove(websitePolicies), existingNetworkResourceLoadIdentifierToResume]() mutable {
</span><span class="cx">         if (auto* item = navigation->targetItem()) {
</span><span class="cx">             LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
</span></span></pre>
</div>
</div>

</body>
</html>