Attachment 356814 Details for Bug 192482 – Patch

[patch] Patch

bug-192482-20181207084912.patch (text/plain), 50.12 KB, created by Chris Dumez on 2018-12-07 08:49:13 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-12-07 08:49:13 PST

Size: 50.12 KB

patch

obsolete

>Subversion Revision: 238911
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index bc6fa2fce937a4b68e11d965aa7b486ca78a36ae..6131f6b7c5e2407aec5aa05e6447c5276a210f49 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,106 @@
>+2018-12-06  Chris Dumez  <cdumez@apple.com>
>+
>+        PSON logic gets confused by concurrent decidePolicyForNavigationAction requests
>+        https://bugs.webkit.org/show_bug.cgi?id=192482
>+        <rdar://problem/46470145>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        It is possible to get 2 parallel decidePolicyForNavigationAction() requests from the
>+        WebProcess when a new load is started before responding to the existing policy
>+        decision. This would lead to several issues with regards to PSON:
>+        - We would decide to swap for the first policy decision and tell the WebProcess to
>+          suspend. However, because the WebProcess issued a new decidePolicyForNavigationAction
>+          since for the same frame, the previous one is invalidated and the WebProcess would
>+          ignore our request to suspend.
>+        - We would hit assertions later on because the navigation has been destroyed and yet
>+          we're getting a didStartProvisionalLoad for it.
>+        - swapToWebProcess() was asynchronous so that it would wait for the destination
>+          SuspendedPage to finish suspending before asking it to unsuspend. This led to various
>+          problems because anything can happen in the UIProcess while we're waiting for the
>+          suspension (e.g. another load). Also, we may create the SuspendedPageProxy for
>+          the current page too late and start getting IPC from the previous process about
>+          the suspension load to about:blank.
>+
>+        To address these issues, the following design is now implemented:
>+        - swapToWebProcess() is no longer asynchronous
>+        - instead, WebProcessPool::processForNavigation() is now asynchronous. This is better
>+          because at this point we have not yet told the WebProcess about the policy decision.
>+          We already properly deal with the policy decision being made asynchronously. This
>+          also allows us to choose another process if the SuspendedPage we'd like to use
>+          failed to suspend.
>+        - If the WebProcess receives a PolicyAction::Suspend but ignores it, have it send an
>+          IPC to the UIProcess so that the SuspendedPageProxy knows about it. We then destroy
>+          the SuspendedPageProxy and make sure it is not used.
>+        - After the asynchronous process selection, if the navigation has been destroy, abort
>+          the process-swap to avoid hitting assertions later on due to doing a load for a
>+          destroyed navigation.
>+
>+        * UIProcess/SuspendedPageProxy.cpp:
>+        (WebKit::SuspendedPageProxy::~SuspendedPageProxy):
>+        Make sure m_whenReadyToConsume completion handler gets called if necessary if the
>+        SuspendedPageProxy gets destroyed. We pass null so that the SuspendedPage is not
>+        used.
>+
>+        (WebKit::SuspendedPageProxy::whenReadyToConsume):
>+        Add whenReadyToConsume() utility method to get a completion handler called when
>+        the SuspendedPageProxy is ready to be used. This basically means we have to wait
>+        for the page to finish its about:blank suspension load. If the suspension fails
>+        then we call the completion handler with null to indicate that the suspended
>+        page is not usable.
>+
>+        (WebKit::SuspendedPageProxy::unsuspendAndConsume):
>+        Rename unsuspend() to unsuspendAndConsume() and make it synchronous. This only gets
>+        called after whenReadyToConsume()'s completion handler has been called and if we
>+        do decide to use the SuspendedPageProxy. It tells the WebProcess to unsuspend and
>+        removes the SuspendedPageProxy from the WebProcessPool.
>+
>+        (WebKit::SuspendedPageProxy::didFinishLoad):
>+        rename m_finishedSuspendingHandler to m_whenReadyToConsume.
>+
>+        (WebKit::SuspendedPageProxy::didFailToSuspend):
>+        Add new didFailToSuspend() that gets called when the WebProcess sends us an IPC telling
>+        us it ignored our request to suspend. We then call m_whenReadyToConsume completion
>+        handler with null and destroy the SuspendedPageProxy.
>+
>+        (WebKit::SuspendedPageProxy::didReceiveMessage):
>+        * UIProcess/SuspendedPageProxy.h:
>+
>+        * UIProcess/WebNavigationState.h:
>+        (WebKit::WebNavigationState::hasNavigation const):
>+        Add utility function to query if a navigation is still valid.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::swapToWebProcess):
>+        Update method so that it is no longer asynchronous. Some of its code was also moved to
>+        continueNavigationInNewProcess() for clarity.
>+
>+        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
>+        Deal with WebProcessPool::processForNavigation() now being asynchronous.
>+
>+        (WebKit::WebPageProxy::continueNavigationInNewProcess):
>+        Update code now that swapToWebProcess() is no longer asynchronous. Some of the swapToWebProcess()
>+        code was also moved here for clarity.
>+
>+        (WebKit::WebPageProxy::didFailToSuspendAfterProcessSwap):
>+
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/WebPageProxy.messages.in:
>+        Add new DidFailToSuspendAfterProcessSwap IPC message.
>+
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::processForNavigation):
>+        (WebKit::WebProcessPool::processForNavigationInternal):
>+        * UIProcess/WebProcessPool.h:
>+        Make asynchronous. If we decide to use a SuspendedPageProxy's process, then call
>+        whenReadyToConsume() on it to wait for it to suspend. If the SuspendedPageProxy
>+        fails to suspend use a new process instead.
>+
>+        * WebProcess/WebPage/WebFrame.cpp:
>+        (WebKit::WebFrame::didReceivePolicyDecision):
>+        If we ignore the policy decision and the decision was "Suspend", send the DidFailToSuspendAfterProcessSwap
>+        IPC to the UIProcess so that the SuspendedPageProxy knows about it.
>+
> 2018-12-05  Chris Dumez  <cdumez@apple.com>
> 
>         Simplify logic inside WebPageProxy::continueNavigationInNewProcess()
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>index 0fea9a2b920b170d7ca76ca1f25ad210aa4690fe..ca5aeef40b6ec6973920713236391b1cf431c46f 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>@@ -91,6 +91,9 @@ SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&
> 
> SuspendedPageProxy::~SuspendedPageProxy()
> {
>+    if (m_readyToUnsuspendHandler)
>+        m_readyToUnsuspendHandler(nullptr);
>+
>     if (!m_isSuspended)
>         return;
> 
>@@ -106,22 +109,25 @@ SuspendedPageProxy::~SuspendedPageProxy()
>     });
> }
> 
>-void SuspendedPageProxy::unsuspend(CompletionHandler<void()>&& completionHandler)
>+void SuspendedPageProxy::waitUntilReadyToUnsuspend(CompletionHandler<void(SuspendedPageProxy*)>&& completionHandler)
>+{
>+    if (m_readyToUnsuspendHandler)
>+        m_readyToUnsuspendHandler(nullptr);
>+
>+    if (!m_finishedSuspending)
>+        m_readyToUnsuspendHandler = WTFMove(completionHandler);
>+    else
>+        completionHandler(this);
>+}
>+
>+void SuspendedPageProxy::unsuspend()
> {
>     ASSERT(m_isSuspended);
>+    ASSERT(m_finishedSuspending);
> 
>-    auto doUnsuspend = [this, completionHandler = WTFMove(completionHandler)]() mutable {
>-        m_isSuspended = false;
>-        m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
>-        m_process->send(Messages::WebPage::SetIsSuspended(false), m_page.pageID());
>-        completionHandler();
>-    };
>-
>-    if (!m_finishedSuspending) {
>-        ASSERT(!m_finishedSuspendingHandler);
>-        m_finishedSuspendingHandler = WTFMove(doUnsuspend);
>-    } else
>-        doUnsuspend();
>+    m_isSuspended = false;
>+    m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
>+    m_process->send(Messages::WebPage::SetIsSuspended(false), m_page.pageID());
> }
> 
> void SuspendedPageProxy::didFinishLoad()
>@@ -136,8 +142,17 @@ void SuspendedPageProxy::didFinishLoad()
>     m_suspensionToken = nullptr;
> #endif
> 
>-    if (auto finishedSuspendingHandler = WTFMove(m_finishedSuspendingHandler))
>-        finishedSuspendingHandler();
>+    if (m_readyToUnsuspendHandler)
>+        m_readyToUnsuspendHandler(this);
>+}
>+
>+void SuspendedPageProxy::didFailToSuspend()
>+{
>+    // We are unusable due to failure to suspend so remove ourselves from the WebProcessPool.
>+    auto protectedThis = m_process->processPool().takeSuspendedPage(*this);
>+
>+    if (m_readyToUnsuspendHandler)
>+        m_readyToUnsuspendHandler(nullptr);
> }
> 
> void SuspendedPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder& decoder)
>@@ -148,6 +163,12 @@ void SuspendedPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder& decod
>         didFinishLoad();
>         return;
>     }
>+
>+    if (decoder.messageName() == Messages::WebPageProxy::DidFailToSuspendAfterProcessSwap::name()) {
>+        didFailToSuspend();
>+        return;
>+    }
>+
> #if !LOG_DISABLED
>     if (!messageNamesToIgnoreWhileSuspended().contains(decoder.messageName()))
>         LOG(ProcessSwapping, "SuspendedPageProxy received unexpected WebPageProxy message '%s'", decoder.messageName().toString().data());
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.h b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>index f31d8ba2a5ed7775d51a568f534881e7941a3a33..113bd51e74be3cc7b79930cc2c25fcfa9997f83e 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.h
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>@@ -47,7 +47,8 @@ public:
>     uint64_t mainFrameID() const { return m_mainFrameID; }
>     const String& registrableDomain() const { return m_registrableDomain; }
> 
>-    void unsuspend(CompletionHandler<void()>&&);
>+    void waitUntilReadyToUnsuspend(CompletionHandler<void(SuspendedPageProxy*)>&&);
>+    void unsuspend();
> 
> #if !LOG_DISABLED
>     const char* loggingString() const;
>@@ -55,6 +56,7 @@ public:
> 
> private:
>     void didFinishLoad();
>+    void didFailToSuspend();
> 
>     // IPC::MessageReceiver
>     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
>@@ -68,7 +70,7 @@ private:
>     bool m_isSuspended { true };
> 
>     bool m_finishedSuspending { false };
>-    CompletionHandler<void()> m_finishedSuspendingHandler;
>+    CompletionHandler<void(SuspendedPageProxy*)> m_readyToUnsuspendHandler;
> #if PLATFORM(IOS_FAMILY)
>     ProcessThrottler::BackgroundActivityToken m_suspensionToken;
> #endif
>diff --git a/Source/WebKit/UIProcess/WebNavigationState.h b/Source/WebKit/UIProcess/WebNavigationState.h
>index d958eb4bdff552105f49b9a103fb87c1778b0cf2..9eb9d28addd5b5b006d50b08a3d74fb69d162392 100644
>--- a/Source/WebKit/UIProcess/WebNavigationState.h
>+++ b/Source/WebKit/UIProcess/WebNavigationState.h
>@@ -54,6 +54,7 @@ public:
>     Ref<API::Navigation> createReloadNavigation();
>     Ref<API::Navigation> createLoadDataNavigation(std::unique_ptr<API::SubstituteData>&&);
> 
>+    bool hasNavigation(uint64_t navigationID) const { return m_navigations.contains(navigationID); }
>     API::Navigation* navigation(uint64_t navigationID);
>     RefPtr<API::Navigation> takeNavigation(uint64_t navigationID);
>     void didDestroyNavigation(uint64_t navigationID);
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 8e5946faa92e271f0c8aca55f91c06532528bc4c..ccfa86947fe01e1ef0c3ddc0ad986ee0b84a4fb9 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -763,52 +763,30 @@ bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, std
>     return true;
> }
> 
>-void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, API::Navigation& navigation, std::optional<uint64_t> mainFrameIDInPreviousProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, CompletionHandler<void(bool)>&& completionHandler)
>+void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, std::unique_ptr<SuspendedPageProxy>&& destinationSuspendedPage, ShouldDelayAttachingDrawingArea shouldDelayAttachingDrawingArea)
> {
>     ASSERT(!m_isClosed);
>     RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::swapToWebProcess\n", this);
> 
>-    std::unique_ptr<SuspendedPageProxy> destinationSuspendedPage;
>-    if (auto* backForwardListItem = navigation.targetItem()) {
>-        if (backForwardListItem->suspendedPage() && &backForwardListItem->suspendedPage()->process() == process.ptr()) {
>-            destinationSuspendedPage = this->process().processPool().takeSuspendedPage(*backForwardListItem->suspendedPage());
>-            ASSERT(destinationSuspendedPage);
>-        }
>-    }
>-
>-    auto* destinationSuspendedPagePtr = destinationSuspendedPage.get();
>-    auto doSwap = [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), process = WTFMove(process), mainFrameIDInPreviousProcess, processSwapRequestedByClient, destinationSuspendedPage = WTFMove(destinationSuspendedPage), completionHandler = WTFMove(completionHandler)]() mutable {
>-        processDidTerminate(ProcessTerminationReason::NavigationSwap);
>-
>-        m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
>-        bool didSuspendPreviousPage = suspendCurrentPageIfPossible(navigation, mainFrameIDInPreviousProcess, processSwapRequestedByClient);
>-        m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::No);
>-
>-        m_process = WTFMove(process);
>-        m_isValid = true;
>-
>-        // If we are reattaching to a SuspendedPage, then the WebProcess' WebPage already exists and
>-        // WebPageProxy::didCreateMainFrame() will not be called to initialize m_mainFrame. In such
>-        // case, we need to initialize m_mainFrame to reflect the fact the the WebProcess' WebPage
>-        // already exists and already has a main frame.
>-        if (destinationSuspendedPage) {
>-            ASSERT(!m_mainFrame);
>-            ASSERT(&destinationSuspendedPage->process() == m_process.ptr());
>-            m_mainFrame = WebFrameProxy::create(*this, destinationSuspendedPage->mainFrameID());
>-            m_process->frameCreated(destinationSuspendedPage->mainFrameID(), *m_mainFrame);
>-        }
>+    m_process = WTFMove(process);
>+    m_isValid = true;
> 
>-        m_process->addExistingWebPage(*this, m_pageID, WebProcessProxy::BeginsUsingDataStore::No);
>-        m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
>+    // If we are reattaching to a SuspendedPage, then the WebProcess' WebPage already exists and
>+    // WebPageProxy::didCreateMainFrame() will not be called to initialize m_mainFrame. In such
>+    // case, we need to initialize m_mainFrame to reflect the fact the the WebProcess' WebPage
>+    // already exists and already has a main frame.
>+    if (destinationSuspendedPage) {
>+        ASSERT(!m_mainFrame);
>+        ASSERT(&destinationSuspendedPage->process() == m_process.ptr());
>+        destinationSuspendedPage->unsuspend();
>+        m_mainFrame = WebFrameProxy::create(*this, destinationSuspendedPage->mainFrameID());
>+        m_process->frameCreated(destinationSuspendedPage->mainFrameID(), *m_mainFrame);
>+    }
> 
>-        finishAttachingToWebProcess(didSuspendPreviousPage ? ShouldDelayAttachingDrawingArea::Yes : ShouldDelayAttachingDrawingArea::No);
>-        completionHandler(didSuspendPreviousPage);
>-    };
>+    m_process->addExistingWebPage(*this, m_pageID, WebProcessProxy::BeginsUsingDataStore::No);
>+    m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
> 
>-    if (destinationSuspendedPagePtr)
>-        destinationSuspendedPagePtr->unsuspend(WTFMove(doSwap));
>-    else
>-        doSwap();
>+    finishAttachingToWebProcess(shouldDelayAttachingDrawingArea);
> }
> 
> void WebPageProxy::finishAttachingToWebProcess(ShouldDelayAttachingDrawingArea shouldDelayAttachingDrawingArea)
>@@ -2618,23 +2596,30 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
>             changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
>     }
> 
>-    Ref<WebProcessProxy> processForNavigation = process();
>-    if (policyAction == PolicyAction::Use && frame.isMainFrame()) {
>-        String reason;
>-        processForNavigation = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, reason);
>-        ASSERT(!reason.isNull());
>+    if (policyAction != PolicyAction::Use || !frame.isMainFrame() || !navigation) {
>+        receivedPolicyDecision(policyAction, navigation, WTFMove(data), WTFMove(sender));
>+        return;
>+    }
>+
>+    process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, [this, protectedThis = makeRef(*this), policyAction, navigation = makeRef(*navigation), data = WTFMove(data), sender = WTFMove(sender), processSwapRequestedByClient](Ref<WebProcessProxy>&& processForNavigation, SuspendedPageProxy* destinationSuspendedPage, const String& reason) mutable {
>+        // If the navigation has been destroyed, then no need to proceed.
>+        if (isClosed() || !navigationState().hasNavigation(navigation->navigationID())) {
>+            receivedPolicyDecision(policyAction, navigation.ptr(), WTFMove(data), WTFMove(sender));
>+            return;
>+        }
>+
>         if (processForNavigation.ptr() != &process()) {
>             policyAction = PolicyAction::Suspend;
>             RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), processForNavigation->processIdentifier(), reason.utf8().data());
>             LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), processForNavigation->processIdentifier(), navigation->navigationID(), navigation->loggingString());
>         } else
>             RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, keep using process %i for navigation, reason: %{public}s", this, processIdentifier(), reason.utf8().data());
>-    }
> 
>-    receivedPolicyDecision(policyAction, navigation, WTFMove(data), WTFMove(sender));
>+        receivedPolicyDecision(policyAction, navigation.ptr(), WTFMove(data), WTFMove(sender));
> 
>-    if (processForNavigation.ptr() != &process())
>-        continueNavigationInNewProcess(*navigation, WTFMove(processForNavigation), processSwapRequestedByClient);
>+        if (processForNavigation.ptr() != &process())
>+            continueNavigationInNewProcess(navigation, destinationSuspendedPage ? process().processPool().takeSuspendedPage(*destinationSuspendedPage) : nullptr, WTFMove(processForNavigation), processSwapRequestedByClient);
>+    });
> }
> 
> void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies, Ref<PolicyDecisionSender>&& sender)
>@@ -2669,7 +2654,7 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation*
>     sender->send(action, navigation ? navigation->navigationID() : 0, downloadID, WTFMove(websitePolicies));
> }
> 
>-void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, Ref<WebProcessProxy>&& process, ProcessSwapRequestedByClient processSwapRequestedByClient)
>+void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPageProxy, Ref<WebProcessProxy>&& process, ProcessSwapRequestedByClient processSwapRequestedByClient)
> {
>     LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString());
> 
>@@ -2679,73 +2664,88 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
> 
>     ASSERT(m_process.ptr() != process.ptr());
> 
>-    swapToWebProcess(WTFMove(process), navigation, mainFrameIDInPreviousProcess, processSwapRequestedByClient, [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), previousProcess = WTFMove(previousProcess), mainFrameIDInPreviousProcess, mainFrameURL = WTFMove(mainFrameURL)](bool didSuspendPreviousPage) mutable {
>-        if (auto* item = navigation->targetItem()) {
>-            LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
>+    processDidTerminate(ProcessTerminationReason::NavigationSwap);
> 
>-            auto transaction = m_pageLoadState.transaction();
>-            m_pageLoadState.setPendingAPIRequestURL(transaction, item->url());
>+    m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
>+    bool didSuspendPreviousPage = suspendCurrentPageIfPossible(navigation, mainFrameIDInPreviousProcess, processSwapRequestedByClient);
>+    m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::No);
> 
>-            auto itemStates = m_backForwardList->filteredItemStates([this, targetItem = item](WebBackForwardListItem& item) {
>-                if (auto* page = item.suspendedPage()) {
>-                    if (&page->process() == m_process.ptr())
>-                        return false;
>-                }
>-                return &item != targetItem;
>-            });
>-            m_process->send(Messages::WebPage::UpdateBackForwardListForReattach(WTFMove(itemStates)), m_pageID);
>-            m_process->send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), item->itemID(), *navigation->backForwardFrameLoadType(), ShouldTreatAsContinuingLoad::Yes), m_pageID);
>-            m_process->responsivenessTimer().start();
>+    swapToWebProcess(WTFMove(process), WTFMove(suspendedPageProxy), didSuspendPreviousPage ? ShouldDelayAttachingDrawingArea::Yes : ShouldDelayAttachingDrawingArea::No);
> 
>-            return;
>-        }
>+    if (auto* item = navigation.targetItem()) {
>+        LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
> 
>-        if (m_backForwardList->currentItem() && (navigation->lockBackForwardList() == LockBackForwardList::Yes || navigation->lockHistory() == LockHistory::Yes)) {
>-            // If WebCore is supposed to lock the history for this load, then the new process needs to know about the current history item so it can update
>-            // it instead of creating a new one.
>-            m_process->send(Messages::WebPage::SetCurrentHistoryItemForReattach(m_backForwardList->currentItem()->itemState()), m_pageID);
>-        }
>+        auto transaction = m_pageLoadState.transaction();
>+        m_pageLoadState.setPendingAPIRequestURL(transaction, item->url());
> 
>-        // FIXME: Work out timing of responding with the last policy delegate, etc
>-        ASSERT(!navigation->currentRequest().isEmpty());
>-        if (auto& substituteData = navigation->substituteData())
>-            loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get());
>-        else
>-            loadRequestWithNavigation(navigation, ResourceRequest { navigation->currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes);
>+        auto itemStates = m_backForwardList->filteredItemStates([this, targetItem = item](WebBackForwardListItem& item) {
>+            if (auto* page = item.suspendedPage()) {
>+                if (&page->process() == m_process.ptr())
>+                    return false;
>+            }
>+            return &item != targetItem;
>+        });
>+        m_process->send(Messages::WebPage::UpdateBackForwardListForReattach(WTFMove(itemStates)), m_pageID);
>+        m_process->send(Messages::WebPage::GoToBackForwardItem(navigation.navigationID(), item->itemID(), *navigation.backForwardFrameLoadType(), ShouldTreatAsContinuingLoad::Yes), m_pageID);
>+        m_process->responsivenessTimer().start();
> 
>-        ASSERT(!m_mainFrame);
>-        m_mainFrameCreationHandler = [this, protectedThis = WTFMove(protectedThis), navigationID = navigation->navigationID(), request =  navigation->currentRequest(), mainFrameURL, isServerRedirect = navigation->currentRequestIsRedirect()]() mutable {
>-            ASSERT(m_mainFrame);
>-            // This navigation was destroyed so no need to notify of redirect.
>-            if (!navigationState().navigation(navigationID))
>-                return;
>+        return;
>+    }
> 
>-            // Restore the main frame's committed URL as some clients may rely on it until the next load is committed.
>-            m_mainFrame->frameLoadState().setURL(mainFrameURL);
>+    if (m_backForwardList->currentItem() && (navigation.lockBackForwardList() == LockBackForwardList::Yes || navigation.lockHistory() == LockHistory::Yes)) {
>+        // If WebCore is supposed to lock the history for this load, then the new process needs to know about the current history item so it can update
>+        // it instead of creating a new one.
>+        m_process->send(Messages::WebPage::SetCurrentHistoryItemForReattach(m_backForwardList->currentItem()->itemState()), m_pageID);
>+    }
> 
>-            // Normally, notification of a server redirect comes from the WebContent process.
>-            // If we are process swapping in response to a server redirect then that notification will not come from the new WebContent process.
>-            // In this case we have the UIProcess synthesize the redirect notification at the appropriate time.
>-            if (isServerRedirect) {
>-                m_mainFrame->frameLoadState().didStartProvisionalLoad(request.url());
>-                didReceiveServerRedirectForProvisionalLoadForFrame(m_mainFrame->frameID(), navigationID, WTFMove(request), { });
>-            }
>-        };
>+    // FIXME: Work out timing of responding with the last policy delegate, etc
>+    ASSERT(!navigation.currentRequest().isEmpty());
>+    if (auto& substituteData = navigation.substituteData())
>+        loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get());
>+    else
>+        loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes);
>+
>+    ASSERT(!m_mainFrame);
>+    m_mainFrameCreationHandler = [this, weakThis = makeWeakPtr(*this), navigationID = navigation.navigationID(), request =  navigation.currentRequest(), mainFrameURL, isServerRedirect = navigation.currentRequestIsRedirect()]() mutable {
>+        if (!weakThis)
>+            return;
> 
>-        bool isInitialNavigationInNewWindow = openedByDOM() && !hasCommittedAnyProvisionalLoads();
>-        if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isInitialNavigationInNewWindow || !mainFrameIDInPreviousProcess) {
>-            // There is no way we'll be able to return to the page in the previous page so close it.
>-            if (!didSuspendPreviousPage)
>-                previousProcess->send(Messages::WebPage::Close(), m_pageID);
>+        ASSERT(m_mainFrame);
>+        // This navigation was destroyed so no need to notify of redirect.
>+        if (!navigationState().navigation(navigationID))
>             return;
>+
>+        // Restore the main frame's committed URL as some clients may rely on it until the next load is committed.
>+        m_mainFrame->frameLoadState().setURL(mainFrameURL);
>+
>+        // Normally, notification of a server redirect comes from the WebContent process.
>+        // If we are process swapping in response to a server redirect then that notification will not come from the new WebContent process.
>+        // In this case we have the UIProcess synthesize the redirect notification at the appropriate time.
>+        if (isServerRedirect) {
>+            m_mainFrame->frameLoadState().didStartProvisionalLoad(request.url());
>+            didReceiveServerRedirectForProvisionalLoadForFrame(m_mainFrame->frameID(), navigationID, WTFMove(request), { });
>         }
>+    };
> 
>-        m_mainFrameWindowCreationHandler = [this, previousProcess = WTFMove(previousProcess), mainFrameIDInPreviousProcess = *mainFrameIDInPreviousProcess](const GlobalWindowIdentifier& windowIdentifier) {
>-            ASSERT(m_mainFrame);
>-            GlobalFrameIdentifier navigatedFrameIdentifierInNewProcess { pageID(), m_mainFrame->frameID() };
>-            previousProcess->send(Messages::WebPage::FrameBecameRemote(mainFrameIDInPreviousProcess, navigatedFrameIdentifierInNewProcess, windowIdentifier), pageID());
>-        };
>-    });
>+    bool isInitialNavigationInNewWindow = openedByDOM() && !hasCommittedAnyProvisionalLoads();
>+    if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isInitialNavigationInNewWindow || !mainFrameIDInPreviousProcess) {
>+        // There is no way we'll be able to return to the page in the previous page so close it.
>+        if (!didSuspendPreviousPage)
>+            previousProcess->send(Messages::WebPage::Close(), m_pageID);
>+        return;
>+    }
>+
>+    m_mainFrameWindowCreationHandler = [this, previousProcess = WTFMove(previousProcess), mainFrameIDInPreviousProcess = *mainFrameIDInPreviousProcess](const GlobalWindowIdentifier& windowIdentifier) {
>+        ASSERT(m_mainFrame);
>+        GlobalFrameIdentifier navigatedFrameIdentifierInNewProcess { pageID(), m_mainFrame->frameID() };
>+        previousProcess->send(Messages::WebPage::FrameBecameRemote(mainFrameIDInPreviousProcess, navigatedFrameIdentifierInNewProcess, windowIdentifier), pageID());
>+    };
>+}
>+
>+NO_RETURN_DUE_TO_ASSERT void WebPageProxy::didFailToSuspendAfterProcessSwap()
>+{
>+    // Only the SuspendedPageProxy should be getting this call.
>+    ASSERT_NOT_REACHED();
> }
> 
> void WebPageProxy::setUserAgent(String&& userAgent)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 734d211719e94bff95a961f3f0d6309ccc3efa15..5ed6070b9b32f88c2f49bce4f3c139fc556e4562 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1559,7 +1559,8 @@ private:
>     void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
> 
>     void reattachToWebProcess();
>-    void swapToWebProcess(Ref<WebProcessProxy>&&, API::Navigation&, std::optional<uint64_t> mainFrameIDInPreviousProcess, ProcessSwapRequestedByClient, CompletionHandler<void(bool)>&&);
>+    void swapToWebProcess(Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>&&, ShouldDelayAttachingDrawingArea);
>+    void didFailToSuspendAfterProcessSwap();
> 
>     void finishAttachingToWebProcess(ShouldDelayAttachingDrawingArea = ShouldDelayAttachingDrawingArea::No);
> 
>@@ -1878,7 +1879,7 @@ private:
> 
>     void reportPageLoadResult(const WebCore::ResourceError& = { });
> 
>-    void continueNavigationInNewProcess(API::Navigation&, Ref<WebProcessProxy>&&, ProcessSwapRequestedByClient);
>+    void continueNavigationInNewProcess(API::Navigation&, std::unique_ptr<SuspendedPageProxy>&&, Ref<WebProcessProxy>&&, ProcessSwapRequestedByClient);
> 
>     void setNeedsFontAttributes(bool);
>     void updateFontAttributesAfterEditorStateChange();
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>index 5e7056b6011ce6fa3e347161a9cb78ebd5bf4564..2a18a70257bc4e7369a592034b7cc92a8cf0ed66 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -504,6 +504,8 @@ messages -> WebPageProxy {
>     RequestPointerUnlock()
> #endif
> 
>+    DidFailToSuspendAfterProcessSwap()
>+
>     ImageOrMediaDocumentSizeChanged(WebCore::IntSize newSize)
> 
>     UseFixedLayoutDidChange(bool useFixedLayout)
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 2c371a3bfca1d93f7c7fe305ffb8c299dc455b07..dd9070ec02056d5de98915b7458f17129d80c461 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -2112,32 +2112,32 @@ void WebProcessPool::removeProcessFromOriginCacheSet(WebProcessProxy& process)
>         m_swappedProcessesPerRegistrableDomain.remove(registrableDomain);
> }
> 
>-Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, String& reason)
>-{
>-    auto process = processForNavigationInternal(page, navigation, processSwapRequestedByClient, reason);
>-
>-    // We are process-swapping so automatic process prewarming would be beneficial if the client has not explicitly enabled / disabled it.
>-    bool doingAnAutomaticProcessSwap = processSwapRequestedByClient == ProcessSwapRequestedByClient::No && process.ptr() != &page.process();
>-    if (doingAnAutomaticProcessSwap && !configuration().wasAutomaticProcessWarmingSetByClient() && !configuration().clientWouldBenefitFromAutomaticProcessPrewarming()) {
>-        RELEASE_LOG(PerformanceLogging, "Automatically turning on process prewarming because the client would benefit from it");
>-        configuration().setClientWouldBenefitFromAutomaticProcessPrewarming(true);
>-    }
>+void WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&& completionHandler)
>+{
>+    processForNavigationInternal(page, navigation, processSwapRequestedByClient, [this, page = makeRefPtr(page), navigation = makeRef(navigation), processSwapRequestedByClient, completionHandler = WTFMove(completionHandler)](Ref<WebProcessProxy>&& process, SuspendedPageProxy* suspendedPage, const String& reason) mutable {
>+        // We are process-swapping so automatic process prewarming would be beneficial if the client has not explicitly enabled / disabled it.
>+        bool doingAnAutomaticProcessSwap = processSwapRequestedByClient == ProcessSwapRequestedByClient::No && process.ptr() != &page->process();
>+        if (doingAnAutomaticProcessSwap && !configuration().wasAutomaticProcessWarmingSetByClient() && !configuration().clientWouldBenefitFromAutomaticProcessPrewarming()) {
>+            RELEASE_LOG(PerformanceLogging, "Automatically turning on process prewarming because the client would benefit from it");
>+            configuration().setClientWouldBenefitFromAutomaticProcessPrewarming(true);
>+        }
> 
>-    if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page.process()) {
>-        static std::once_flag onceFlag;
>-        std::call_once(onceFlag, [] {
>-            WTFLogAlways("WARNING: The option to always keep swapped web processes alive is active. This is meant for debugging and testing only.");
>-        });
>+        if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page->process()) {
>+            static std::once_flag onceFlag;
>+            std::call_once(onceFlag, [] {
>+                WTFLogAlways("WARNING: The option to always keep swapped web processes alive is active. This is meant for debugging and testing only.");
>+            });
> 
>-        addProcessToOriginCacheSet(page);
>+            addProcessToOriginCacheSet(*page);
> 
>-        LOG(ProcessSwapping, "(ProcessSwapping) Navigating from %s to %s, keeping around old process. Now holding on to old processes for %u origins.", page.currentURL().utf8().data(), navigation.currentRequest().url().string().utf8().data(), m_swappedProcessesPerRegistrableDomain.size());
>-    }
>+            LOG(ProcessSwapping, "(ProcessSwapping) Navigating from %s to %s, keeping around old process. Now holding on to old processes for %u origins.", page->currentURL().utf8().data(), navigation->currentRequest().url().string().utf8().data(), m_swappedProcessesPerRegistrableDomain.size());
>+        }
> 
>-    return process;
>+        completionHandler(WTFMove(process), suspendedPage, reason);
>+    });
> }
> 
>-Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, String& reason)
>+void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&& completionHandler)
> {
>     auto& targetURL = navigation.currentRequest().url();
> 
>@@ -2150,44 +2150,37 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
>         return createNewWebProcess(page.websiteDataStore());
>     };
> 
>-    if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes) {
>-        reason = "Process swap was requested by the client"_s;
>-        return createNewProcess();
>-    }
>+    if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes)
>+        return completionHandler(createNewProcess(), nullptr, "Process swap was requested by the client"_s);
> 
>-    if (!m_configuration->processSwapsOnNavigation()) {
>-        reason = "Feature is disabled"_s;
>-        return page.process();
>-    }
>+    if (!m_configuration->processSwapsOnNavigation())
>+        return completionHandler(page.process(), nullptr, "Feature is disabled"_s);
> 
>-    if (m_automationSession) {
>-        reason = "An automation session is active"_s;
>-        return page.process();
>-    }
>+    if (m_automationSession)
>+        return completionHandler(page.process(), nullptr, "An automation session is active"_s);
> 
>     if (!page.process().hasCommittedAnyProvisionalLoads()) {
>-        reason = "Process has not yet committed any provisional loads"_s;
>         tryPrewarmWithDomainInformation(page.process(), targetURL);
>-        return page.process();
>+        return completionHandler(page.process(), nullptr, "Process has not yet committed any provisional loads"_s);
>     }
> 
>     // FIXME: We should support process swap when a window has been opened via window.open() without 'noopener'.
>     // The issue is that the opener has a handle to the WindowProxy.
>-    if (navigation.openedByDOMWithOpener() && !m_configuration->processSwapsOnWindowOpenWithOpener()) {
>-        reason = "Browsing context been opened by DOM without 'noopener'"_s;
>-        return page.process();
>-    }
>+    if (navigation.openedByDOMWithOpener() && !m_configuration->processSwapsOnWindowOpenWithOpener())
>+        return completionHandler(page.process(), nullptr, "Browsing context been opened by DOM without 'noopener'"_s);
> 
>     // FIXME: We should support process swap when a window has opened other windows via window.open.
>-    if (navigation.hasOpenedFrames()) {
>-        reason = "Browsing context has opened other windows"_s;
>-        return page.process();
>-    }
>+    if (navigation.hasOpenedFrames())
>+        return completionHandler(page.process(), nullptr, "Browsing context has opened other windows"_s);
> 
>     if (auto* backForwardListItem = navigation.targetItem()) {
>         if (auto* suspendedPage = backForwardListItem->suspendedPage()) {
>-            reason = "Using target back/forward item's process"_s;
>-            return suspendedPage->process();
>+            return suspendedPage->waitUntilReadyToUnsuspend([createNewProcess = WTFMove(createNewProcess), completionHandler = WTFMove(completionHandler)](SuspendedPageProxy* suspendedPage) mutable {
>+                if (!suspendedPage)
>+                    return completionHandler(createNewProcess(), nullptr, "Using new process because target back/forward item's process failed to suspend"_s);
>+                Ref<WebProcessProxy> process = suspendedPage->process();
>+                completionHandler(WTFMove(process), suspendedPage, "Using target back/forward item's process"_s);
>+            });
>         }
> 
>         // If the target back/forward item and the current back/forward item originated
>@@ -2196,17 +2189,13 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
>             auto uiProcessIdentifier = Process::identifier();
>             // In case of session restore, the item's process identifier is the UIProcess' identifier, in which case we do not want to do this check
>             // or we'd never swap after a session restore.
>-            if (fromItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier && backForwardListItem->itemID().processIdentifier != uiProcessIdentifier) {
>-                reason = "Source and target back/forward item originated in the same process"_s;
>-                return page.process();
>-            }
>+            if (fromItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier && backForwardListItem->itemID().processIdentifier != uiProcessIdentifier)
>+                return completionHandler(page.process(), nullptr, "Source and target back/forward item originated in the same process"_s);
>         }
>     }
> 
>-    if (navigation.treatAsSameOriginNavigation()) {
>-        reason = "The treatAsSameOriginNavigation flag is set"_s;
>-        return page.process();
>-    }
>+    if (navigation.treatAsSameOriginNavigation())
>+        return completionHandler(page.process(), nullptr, "The treatAsSameOriginNavigation flag is set"_s);
> 
>     bool isInitialLoadInNewWindowOpenedByDOM = page.openedByDOM() && !page.hasCommittedAnyProvisionalLoads();
>     URL sourceURL;
>@@ -2220,11 +2209,10 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
>         RELEASE_LOG(ProcessSwapping, "Using related page %p's URL as source URL for process swap decision", page.configuration().relatedPage());
>     }
> 
>-    if (!sourceURL.isValid() || !targetURL.isValid() || sourceURL.isEmpty() || sourceURL.protocolIsAbout() || registrableDomainsAreEqual(sourceURL, targetURL)) {
>-        reason = "Navigation is same-site"_s;
>-        return page.process();
>-    }
>-    reason = "Navigation is cross-site"_s;
>+    if (!sourceURL.isValid() || !targetURL.isValid() || sourceURL.isEmpty() || sourceURL.protocolIsAbout() || registrableDomainsAreEqual(sourceURL, targetURL))
>+        return completionHandler(page.process(), nullptr, "Navigation is same-site"_s);
>+
>+    String reason = "Navigation is cross-site"_s;
>     
>     auto registrableDomain = toRegistrableDomain(targetURL);
>     if (m_configuration->alwaysKeepAndReuseSwappedProcesses()) {
>@@ -2242,7 +2230,7 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
>                     return &suspendedPage->page() == &page && &suspendedPage->process() == process;
>                 });
> 
>-                return makeRef(*process);
>+                return completionHandler(makeRef(*process), nullptr, reason);
>             }
>         }
>     }
>@@ -2262,10 +2250,10 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
>         if (&process->websiteDataStore() != &page.websiteDataStore())
>             process->send(Messages::WebProcess::AddWebsiteDataStore(page.websiteDataStore().parameters()), 0);
> 
>-        return process;
>+        return completionHandler(WTFMove(process), nullptr, reason);
>     }
> 
>-    return createNewProcess();
>+    return completionHandler(createNewProcess(), nullptr, reason);
> }
> 
> void WebProcessPool::addSuspendedPage(std::unique_ptr<SuspendedPageProxy>&& suspendedPage)
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
>index 03da3890ff2eafd41d76f2cef95b2e90d07be0ba..4fd79241748995f53060000b6416e3fc4140abf2 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.h
>+++ b/Source/WebKit/UIProcess/WebProcessPool.h
>@@ -445,7 +445,7 @@ public:
>     BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
> #endif
> 
>-    Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, String& reason);
>+    void processForNavigation(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&&);
> 
>     // SuspendedPageProxy management.
>     void addSuspendedPage(std::unique_ptr<SuspendedPageProxy>&&);
>@@ -479,7 +479,7 @@ private:
>     void platformInitializeWebProcess(WebProcessCreationParameters&);
>     void platformInvalidateContext();
> 
>-    Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, String& reason);
>+    void processForNavigationInternal(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&&);
> 
>     RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
>index 7b15c65712179640af497f2f19c71996b7134528..9ae378b288c930033c8d9358c1c88ef5de7962e8 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
>@@ -255,17 +255,11 @@ void WebFrame::invalidatePolicyListener()
> 
> void WebFrame::didReceivePolicyDecision(uint64_t listenerID, PolicyAction action, uint64_t navigationID, DownloadID downloadID, std::optional<WebsitePoliciesData>&& websitePolicies)
> {
>-    if (!m_coreFrame)
>-        return;
>-
>-    if (!m_policyListenerID)
>-        return;
>-
>-    if (listenerID != m_policyListenerID)
>-        return;
>-
>-    if (!m_policyFunction)
>+    if (!m_coreFrame || !m_policyListenerID || listenerID != m_policyListenerID || !m_policyFunction) {
>+        if (action == PolicyAction::Suspend)
>+            page()->send(Messages::WebPageProxy::DidFailToSuspendAfterProcessSwap());
>         return;
>+    }
> 
>     FramePolicyFunction function = WTFMove(m_policyFunction);
>     bool forNavigationAction = m_policyFunctionForNavigationAction == ForNavigationAction::Yes;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 01cd4c9386721765d60ddcb620b25615564866c8..129bc043dbb9f99f147e20d1fbe24b4757f4b05d 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-12-06  Chris Dumez  <cdumez@apple.com>
>+
>+        PSON logic gets confused by concurrent decidePolicyForNavigationAction requests
>+        https://bugs.webkit.org/show_bug.cgi?id=192482
>+        <rdar://problem/46470145>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2018-12-05  Jonathan Bedard  <jbedard@apple.com>
> 
>         webkitpy: Clean-up apple_additions
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index a2b6af23bf8e1b9b661eb2f7c8d12633d49f049c..db0909ee672a4a2c83759292f335ae3a7e570c23 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -2473,6 +2473,76 @@ TEST(ProcessSwap, ProcessReuseeTLDPlus2)
>     EXPECT_EQ(pid1, pid3);
> }
> 
>+TEST(ProcessSwap, ConcurrentHistoryNavigations)
>+{
>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
>+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
>+
>+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [webViewConfiguration setProcessPool:processPool.get()];
>+    auto handler = adoptNS([[PSONScheme alloc] init]);
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+
>+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]]];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto webkitPID = [webView _webProcessIdentifier];
>+
>+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]]];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto applePID = [webView _webProcessIdentifier];
>+
>+    EXPECT_NE(webkitPID, applePID);
>+
>+    [webView goBack];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [[webView URL] absoluteString]);
>+
>+    auto* backForwardList = [webView backForwardList];
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.currentItem.URL absoluteString]);
>+    EXPECT_TRUE(!backForwardList.backItem);
>+    EXPECT_EQ(1U, backForwardList.forwardList.count);
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [backForwardList.forwardItem.URL absoluteString]);
>+
>+    // Concurrent requests to go forward, which process swaps.
>+    [webView goForward];
>+    [webView goForward];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [backForwardList.currentItem.URL absoluteString]);
>+    EXPECT_TRUE(!backForwardList.forwardItem);
>+    EXPECT_EQ(1U, backForwardList.backList.count);
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.backItem.URL absoluteString]);
>+
>+    [webView goBack];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [[webView URL] absoluteString]);
>+
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.currentItem.URL absoluteString]);
>+    EXPECT_TRUE(!backForwardList.backItem);
>+    EXPECT_EQ(1U, backForwardList.forwardList.count);
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [backForwardList.forwardItem.URL absoluteString]);
>+}
>+
> TEST(ProcessSwap, NavigateToInvalidURL)
> {
>     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);

Actions: View | Formatted Diff | Diff

Attachments on bug 192482: 356767 | 356814