- Revision
- 286509
- Author
- [email protected]
- Date
- 2021-12-03 12:29:20 -0800 (Fri, 03 Dec 2021)
Log Message
Cherry-pick r286479. rdar://problem/85918531
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):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286479 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebKit/ChangeLog (286508 => 286509)
--- branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-03 20:23:46 UTC (rev 286508)
+++ branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-03 20:29:20 UTC (rev 286509)
@@ -1,3 +1,75 @@
+2021-12-03 Russell Epstein <[email protected]>
+
+ Cherry-pick r286479. rdar://problem/85918531
+
+ 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):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286479 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-12-02 Chris Dumez <[email protected]>
+
+ 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):
+
2021-12-01 Alan Coon <[email protected]>
Cherry-pick r285865. rdar://problem/75139294
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (286508 => 286509)
--- branches/safari-612-branch/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2021-12-03 20:23:46 UTC (rev 286508)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2021-12-03 20:29:20 UTC (rev 286509)
@@ -58,7 +58,7 @@
#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__)
#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__)
-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)
+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)
: m_page(page)
, m_webPageID(suspendedPage ? suspendedPage->webPageID() : PageIdentifier::generate())
, m_process(WTFMove(process))
@@ -66,7 +66,7 @@
, m_isServerRedirect(isServerRedirect)
, m_request(request)
, m_processSwapRequestedByClient(processSwapRequestedByClient)
- , m_shouldClosePreviousPageAfterCommit(shouldClosePreviousPageAfterCommit)
+ , m_isProcessSwappingOnNavigationResponse(isProcessSwappingOnNavigationResponse)
#if PLATFORM(IOS_FAMILY)
, m_provisionalLoadActivity(m_process->throttler().foregroundActivity("Provisional Load"_s))
#endif
@@ -112,6 +112,12 @@
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
send(Messages::WebPage::Close());
m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
+
+ // 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());
}
m_process->removeProvisionalPageProxy(*this);
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/ProvisionalPageProxy.h (286508 => 286509)
--- branches/safari-612-branch/Source/WebKit/UIProcess/ProvisionalPageProxy.h 2021-12-03 20:23:46 UTC (rev 286508)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/ProvisionalPageProxy.h 2021-12-03 20:29:20 UTC (rev 286509)
@@ -73,7 +73,7 @@
class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSender {
WTF_MAKE_FAST_ALLOCATED;
public:
- ProvisionalPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>, uint64_t navigationID, bool isServerRedirect, const WebCore::ResourceRequest&, ProcessSwapRequestedByClient, bool shouldClosePreviousPageAfterCommit, API::WebsitePolicies*);
+ ProvisionalPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>, uint64_t navigationID, bool isServerRedirect, const WebCore::ResourceRequest&, ProcessSwapRequestedByClient, bool isProcessSwappingOnNavigationResponse, API::WebsitePolicies*);
~ProvisionalPageProxy();
WebPageProxy& page() const { return m_page; }
@@ -84,7 +84,7 @@
uint64_t navigationID() const { return m_navigationID; }
const URL& provisionalURL() const { return m_provisionalLoadURL; }
- bool shouldClosePreviousPageAfterCommit() const { return m_shouldClosePreviousPageAfterCommit; }
+ bool isProcessSwappingOnNavigationResponse() const { return m_isProcessSwappingOnNavigationResponse; }
DrawingAreaProxy* drawingArea() const { return m_drawingArea.get(); }
std::unique_ptr<DrawingAreaProxy> takeDrawingArea();
@@ -170,7 +170,7 @@
WebCore::ResourceRequest m_request;
ProcessSwapRequestedByClient m_processSwapRequestedByClient;
bool m_wasCommitted { false };
- bool m_shouldClosePreviousPageAfterCommit { false };
+ bool m_isProcessSwappingOnNavigationResponse { false };
URL m_provisionalLoadURL;
#if PLATFORM(COCOA)
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (286508 => 286509)
--- branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-12-03 20:23:46 UTC (rev 286508)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-12-03 20:29:20 UTC (rev 286509)
@@ -3608,7 +3608,7 @@
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
auto* navigation = navigationState().navigation(m_provisionalPage->navigationID());
- bool didSuspendPreviousPage = navigation && !m_provisionalPage->shouldClosePreviousPageAfterCommit() ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient(), shouldDelayClosingUntilFirstLayerFlush) : false;
+ bool didSuspendPreviousPage = navigation && !m_provisionalPage->isProcessSwappingOnNavigationResponse() ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient(), shouldDelayClosingUntilFirstLayerFlush) : false;
m_process->removeWebPage(*this, m_websiteDataStore.ptr() == &m_provisionalPage->process().websiteDataStore() ? WebProcessProxy::EndsUsingDataStore::No : WebProcessProxy::EndsUsingDataStore::Yes);
// There is no way we'll be able to return to the page in the previous page so close it.
@@ -3638,8 +3638,8 @@
}
bool isServerSideRedirect = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision && navigation.currentRequestIsRedirect();
- bool shouldClosePreviousPageAfterCommit = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted;
- m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, WTFMove(newProcess), WTFMove(suspendedPage), navigation.navigationID(), isServerSideRedirect, navigation.currentRequest(), processSwapRequestedByClient, shouldClosePreviousPageAfterCommit, websitePolicies.get());
+ bool isProcessSwappingOnNavigationResponse = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted;
+ m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, WTFMove(newProcess), WTFMove(suspendedPage), navigation.navigationID(), isServerSideRedirect, navigation.currentRequest(), processSwapRequestedByClient, isProcessSwappingOnNavigationResponse, websitePolicies.get());
auto continuation = [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), shouldTreatAsContinuingLoad, websitePolicies = WTFMove(websitePolicies), existingNetworkResourceLoadIdentifierToResume]() mutable {
if (auto* item = navigation->targetItem()) {
LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
