Attachment 357251 Details for Bug 192668 – Patch

[patch] Patch

bug-192668-20181213135759.patch (text/plain), 31.59 KB, created by Chris Dumez on 2018-12-13 13:58:00 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-12-13 13:58:00 PST

Size: 31.59 KB

patch

obsolete

>Subversion Revision: 239170
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c7c333bbc491e236532813334d0f41a531b75075..5f0b0d8931f65f5d93002348edb023c8d9e85403 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-12-13  Chris Dumez  <cdumez@apple.com>
>+
>+        [PSON] We should not need to navigate to 'about:blank' to suspend pages
>+        https://bugs.webkit.org/show_bug.cgi?id=192668
>+        <rdar://problem/46701466>
>+
>+        Reviewed by Alex Christensen.
>+
>+        * history/PageCache.cpp:
>+        (WebCore::PageCache::addIfCacheable):
>+        * history/PageCache.h:
>+        * loader/DocumentLoader.cpp:
>+        (WebCore::DocumentLoader::redirectReceived):
>+        (WebCore::DocumentLoader::willSendRequest):
>+        (WebCore::DocumentLoader::startLoadingMainResource):
>+        * loader/DocumentLoader.h:
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::init):
>+        (WebCore::FrameLoader::stopAllLoaders):
>+        (WebCore::FrameLoader::setDocumentLoader):
>+        (WebCore::FrameLoader::commitProvisionalLoad):
>+        (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
>+        (WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
>+        * loader/FrameLoaderTypes.h:
>+        * loader/PolicyChecker.cpp:
>+        (WebCore::PolicyChecker::checkNavigationPolicy):
>+        * loader/PolicyChecker.h:
>+
> 2018-12-13  Eric Carlson  <eric.carlson@apple.com>
> 
>         [MediaStream] Calculate width or height when constraints contain only the other
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index c9a427397c1ffd3b1433759088f3084c7e825ba3..b538a9d67b06b5d72c06b95a0104006aad5c95b5 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,54 @@
>+2018-12-13  Chris Dumez  <cdumez@apple.com>
>+
>+        [PSON] We should not need to navigate to 'about:blank' to suspend pages
>+        https://bugs.webkit.org/show_bug.cgi?id=192668
>+        <rdar://problem/46701466>
>+
>+        Reviewed by Alex Christensen.
>+
>+        To support PageCache when process-swap on cross-site navigation is enabled,
>+        we've been navigating the previous process to 'about:blank' when swapping.
>+        This would trigger PageCaching of the page in the old process. While
>+        convenient, this design has led to a lot of bugs because we did not really
>+        want a navigation to happen in the old process.
>+
>+        To address the issue, when a WebPage is asked to suspend (for process-swap),
>+        we now attempt to add it to PageCache and save it on the current HistoryItem,
>+        *without* triggering any navigation. Any pending navigation gets cancelled
>+        and we just suspend in place.
>+
>+        Later on, when we want to go back to this HistoryItem, we simply leverage the
>+        existing WebPage::goToBackForwardItem() code path. The only subtlety is that
>+        we're actually asking the WebPage to load a HistoryItem that is the current
>+        one in the History. I had to tweak a some logic / assertions to support this
>+        as this is not something we usually do. However, it actually works with very
>+        little changes and successfully restores the PageCache entry on the current
>+        HistoryItem.
>+
>+        There is no expected overall behavior change and ProcessSwap API tests (which
>+        cover PageCache) still pass. This is merely a simpler design because it avoids
>+        navigating to about:blank.
>+
>+        * UIProcess/SuspendedPageProxy.cpp:
>+        (WebKit::SuspendedPageProxy::didSuspend):
>+        (WebKit::SuspendedPageProxy::didReceiveMessage):
>+        * UIProcess/SuspendedPageProxy.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::didSuspendAfterProcessSwap):
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/WebPageProxy.messages.in:
>+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
>+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
>+        * WebProcess/WebPage/WebDocumentLoader.cpp:
>+        (WebKit::WebDocumentLoader::setNavigationID):
>+        * WebProcess/WebPage/WebFrame.cpp:
>+        (WebKit::WebFrame::didReceivePolicyDecision):
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::suspendForProcessSwap):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/cocoa/WebProcessCocoa.mm:
>+        (WebKit::origin):
>+
> 2018-12-13  Per Arne Vollan  <pvollan@apple.com>
> 
>         [macOS] Remove with-report from 3 services that are currently needed on macOS
>diff --git a/Source/WebCore/history/PageCache.cpp b/Source/WebCore/history/PageCache.cpp
>index 3cf836ced2ea906ec9e447727129c07a44a9ebbf..f13e5a8e8b2cb771070f224acf58beb4b24bda30 100644
>--- a/Source/WebCore/history/PageCache.cpp
>+++ b/Source/WebCore/history/PageCache.cpp
>@@ -425,13 +425,13 @@ static void firePageHideEventRecursively(Frame& frame)
>         firePageHideEventRecursively(*child);
> }
> 
>-void PageCache::addIfCacheable(HistoryItem& item, Page* page)
>+bool PageCache::addIfCacheable(HistoryItem& item, Page* page)
> {
>     if (item.isInPageCache())
>-        return;
>+        return false;
> 
>     if (!page || !canCache(*page))
>-        return;
>+        return false;
> 
>     ASSERT_WITH_MESSAGE(!page->isUtilityPage(), "Utility pages such as SVGImage pages should never go into PageCache");
> 
>@@ -449,7 +449,7 @@ void PageCache::addIfCacheable(HistoryItem& item, Page* page)
>     // could have altered the page in a way that could prevent caching.
>     if (!canCache(*page)) {
>         setPageCacheState(*page, Document::NotInPageCache);
>-        return;
>+        return false;
>     }
> 
>     destroyRenderTree(page->mainFrame());
>@@ -465,6 +465,7 @@ void PageCache::addIfCacheable(HistoryItem& item, Page* page)
>         m_items.add(&item);
>     }
>     prune(PruningReason::ReachedMaxSize);
>+    return true;
> }
> 
> std::unique_ptr<CachedPage> PageCache::take(HistoryItem& item, Page* page)
>diff --git a/Source/WebCore/history/PageCache.h b/Source/WebCore/history/PageCache.h
>index a62df9ac857489cd1d391030a83987803852de6f..e211cc459feefedfbfa94492772e89d032011585 100644
>--- a/Source/WebCore/history/PageCache.h
>+++ b/Source/WebCore/history/PageCache.h
>@@ -51,7 +51,7 @@ public:
>     WEBCORE_EXPORT void setMaxSize(unsigned); // number of pages to cache.
>     unsigned maxSize() const { return m_maxSize; }
> 
>-    void addIfCacheable(HistoryItem&, Page*); // Prunes if maxSize() is exceeded.
>+    WEBCORE_EXPORT bool addIfCacheable(HistoryItem&, Page*); // Prunes if maxSize() is exceeded.
>     WEBCORE_EXPORT void remove(HistoryItem&);
>     CachedPage* get(HistoryItem&, Page*);
>     std::unique_ptr<CachedPage> take(HistoryItem&, Page*);
>diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
>index f6ffb13ff597ee24211f1d49f160bbbb7b8ddd58..1ea7f352a7b152f43cc5239df5396a056f42d82b 100644
>--- a/Source/WebCore/loader/DocumentLoader.cpp
>+++ b/Source/WebCore/loader/DocumentLoader.cpp
>@@ -515,7 +515,7 @@ void DocumentLoader::redirectReceived(CachedResource& resource, ResourceRequest&
>     ASSERT_UNUSED(resource, &resource == m_mainResource);
> #if ENABLE(SERVICE_WORKER)
>     bool isRedirectionFromServiceWorker = redirectResponse.source() == ResourceResponse::Source::ServiceWorker;
>-    willSendRequest(WTFMove(request), redirectResponse, ShouldContinue::Yes, [isRedirectionFromServiceWorker, completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this] (auto&& request) mutable {
>+    willSendRequest(WTFMove(request), redirectResponse, [isRedirectionFromServiceWorker, completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this] (auto&& request) mutable {
>         ASSERT(!m_substituteData.isValid());
>         if (request.isNull() || !m_mainDocumentError.isNull() || !m_frame) {
>             completionHandler({ });
>@@ -548,11 +548,11 @@ void DocumentLoader::redirectReceived(CachedResource& resource, ResourceRequest&
>         });
>     });
> #else
>-    willSendRequest(WTFMove(request), redirectResponse, ShouldContinue::Yes, WTFMove(completionHandler));
>+    willSendRequest(WTFMove(request), redirectResponse, WTFMove(completionHandler));
> #endif
> }
> 
>-void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, ShouldContinue shouldContinue, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
>+void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler)
> {
>     // Note that there are no asserts here as there are for the other callbacks. This is due to the
>     // fact that this "callback" is sent when starting every load, and the state of callback
>@@ -560,8 +560,6 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
>     // callbacks is meant to prevent.
>     ASSERT(!newRequest.isNull());
> 
>-    ASSERT(shouldContinue != ShouldContinue::No);
>-
>     bool didReceiveRedirectResponse = !redirectResponse.isNull();
>     if (!frameLoader()->checkIfFormActionAllowedByCSP(newRequest.url(), didReceiveRedirectResponse)) {
>         cancelMainResourceLoad(frameLoader()->cancelledError(newRequest));
>@@ -628,16 +626,12 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
> 
>     setRequest(newRequest);
> 
>-    if (!didReceiveRedirectResponse && shouldContinue != ShouldContinue::ForSuspension)
>+    if (!didReceiveRedirectResponse)
>         return completionHandler(WTFMove(newRequest));
> 
>     auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) mutable {
>         m_waitingForNavigationPolicy = false;
>         switch (shouldContinue) {
>-        case ShouldContinue::ForSuspension:
>-            // We handle suspension by navigating forward to about:blank, which leaves us setup to navigate back to resume.
>-            request = { WTF::blankURL() };
>-            break;
>         case ShouldContinue::No:
>             stopLoadingForPolicyChange();
>             break;
>@@ -651,11 +645,6 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
>     ASSERT(!m_waitingForNavigationPolicy);
>     m_waitingForNavigationPolicy = true;
> 
>-    if (shouldContinue == ShouldContinue::ForSuspension) {
>-        navigationPolicyCompletionHandler(WTFMove(newRequest), nullptr, shouldContinue);
>-        return;
>-    }
>-
>     frameLoader()->policyChecker().checkNavigationPolicy(WTFMove(newRequest), redirectResponse, WTFMove(navigationPolicyCompletionHandler));
> }
> 
>@@ -1693,10 +1682,8 @@ bool DocumentLoader::maybeLoadEmpty()
>     return true;
> }
> 
>-void DocumentLoader::startLoadingMainResource(ShouldContinue shouldContinue)
>+void DocumentLoader::startLoadingMainResource()
> {
>-    ASSERT(shouldContinue != ShouldContinue::No);
>-
>     m_mainDocumentError = ResourceError();
>     timing().markStartTimeAndFetchStart();
>     ASSERT(!m_mainResource);
>@@ -1722,7 +1709,7 @@ void DocumentLoader::startLoadingMainResource(ShouldContinue shouldContinue)
>     ASSERT(timing().startTime());
>     ASSERT(timing().fetchStart());
> 
>-    willSendRequest(ResourceRequest(m_request), ResourceResponse(), shouldContinue, [this, protectedThis = WTFMove(protectedThis)] (ResourceRequest&& request) mutable {
>+    willSendRequest(ResourceRequest(m_request), ResourceResponse(), [this, protectedThis = WTFMove(protectedThis)] (ResourceRequest&& request) mutable {
>         m_request = request;
> 
>         // willSendRequest() may lead to our Frame being detached or cancelling the load via nulling the ResourceRequest.
>diff --git a/Source/WebCore/loader/DocumentLoader.h b/Source/WebCore/loader/DocumentLoader.h
>index 501a527378f98161166f0a1cbe761425141105e0..feac90020b78a74ebdd77248041d91ab085bcc25 100644
>--- a/Source/WebCore/loader/DocumentLoader.h
>+++ b/Source/WebCore/loader/DocumentLoader.h
>@@ -247,7 +247,7 @@ public:
>     void setDefersLoading(bool);
>     void setMainResourceDataBufferingPolicy(DataBufferingPolicy);
> 
>-    void startLoadingMainResource(ShouldContinue);
>+    void startLoadingMainResource();
>     WEBCORE_EXPORT void cancelMainResourceLoad(const ResourceError&);
>     void willContinueMainResourceLoadAfterRedirect(const ResourceRequest&);
> 
>@@ -367,7 +367,7 @@ private:
>     void clearArchiveResources();
> #endif
> 
>-    void willSendRequest(ResourceRequest&&, const ResourceResponse&, ShouldContinue, CompletionHandler<void(ResourceRequest&&)>&&);
>+    void willSendRequest(ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&);
>     void finishedLoading();
>     void mainReceivedError(const ResourceError&);
>     WEBCORE_EXPORT void redirectReceived(CachedResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) override;
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index 8336522a610628b84e3dd1794609bcf2ec12fb11..55354791147e588a094eb6cbac1b427a17efddae 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -312,7 +312,7 @@ void FrameLoader::init()
>     // This somewhat odd set of steps gives the frame an initial empty document.
>     setPolicyDocumentLoader(m_client.createDocumentLoader(ResourceRequest(URL({ }, emptyString())), SubstituteData()).ptr());
>     setProvisionalDocumentLoader(m_policyDocumentLoader.get());
>-    m_provisionalDocumentLoader->startLoadingMainResource(ShouldContinue::Yes);
>+    m_provisionalDocumentLoader->startLoadingMainResource();
> 
>     Ref<Frame> protect(m_frame);
>     m_frame.document()->cancelParsing();
>@@ -1772,7 +1772,9 @@ void FrameLoader::reload(OptionSet<ReloadOption> options)
> 
> void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy)
> {
>-    ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
>+    if (m_frame.document() && m_frame.document()->pageCacheState() == Document::InPageCache)
>+        return;
>+
>     if (!isStopLoadingAllowed())
>         return;
> 
>@@ -1866,6 +1868,9 @@ void FrameLoader::setDocumentLoader(DocumentLoader* loader)
> {
>     if (!loader && !m_documentLoader)
>         return;
>+
>+    if (loader == m_documentLoader)
>+        return;
>     
>     ASSERT(loader != m_documentLoader);
>     ASSERT(!loader || loader->frameLoader() == this);
>@@ -1953,7 +1958,7 @@ void FrameLoader::commitProvisionalLoad()
> 
>     willTransitionToCommitted();
> 
>-    if (!m_frame.tree().parent() && history().currentItem()) {
>+    if (!m_frame.tree().parent() && history().currentItem() && history().currentItem() != history().provisionalItem()) {
>         // Check to see if we need to cache the page we are navigating away from into the back/forward cache.
>         // We are doing this here because we know for sure that a new page is about to be loaded.
>         PageCache::singleton().addIfCacheable(*history().currentItem(), m_frame.page());
>@@ -3348,11 +3353,11 @@ void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& reque
>         diagnosticLoggingClient.logDiagnosticMessageWithResult(DiagnosticLoggingKeys::pageCacheKey(), DiagnosticLoggingKeys::retrievalKey(), DiagnosticLoggingResultFail, ShouldSample::Yes);
>     }
> 
>-    CompletionHandler<void()> completionHandler = [this, protectedFrame = makeRef(m_frame), shouldContinue] () mutable {
>+    CompletionHandler<void()> completionHandler = [this, protectedFrame = makeRef(m_frame)] () mutable {
>         if (!m_provisionalDocumentLoader)
>             return;
>         
>-        prepareForLoadStart([this, protectedFrame = WTFMove(protectedFrame), shouldContinue] {
>+        prepareForLoadStart([this, protectedFrame = WTFMove(protectedFrame)] {
> 
>             // The load might be cancelled inside of prepareForLoadStart(), nulling out the m_provisionalDocumentLoader,
>             // so we need to null check it again.
>@@ -3369,16 +3374,7 @@ void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& reque
>             
>             m_loadingFromCachedPage = false;
> 
>-            // We handle suspension by navigating forward to about:blank, which leaves us setup to navigate back to resume.
>-            if (shouldContinue == ShouldContinue::ForSuspension) {
>-                // Make sure we do a standard load to about:blank instead of reusing whatever the load type was on process swap.
>-                // For example, using a Back/Forward load to load about blank would cause the current HistoryItem's url to be
>-                // overwritten with 'about:blank', which we do not want.
>-                m_loadType = FrameLoadType::Standard;
>-                m_provisionalDocumentLoader->willContinueMainResourceLoadAfterRedirect({ WTF::blankURL() });
>-            }
>-
>-            m_provisionalDocumentLoader->startLoadingMainResource(shouldContinue);
>+            m_provisionalDocumentLoader->startLoadingMainResource();
>         });
>     };
>     
>@@ -3393,7 +3389,6 @@ void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& reque
> void FrameLoader::continueLoadAfterNewWindowPolicy(const ResourceRequest& request,
>     FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
> {
>-    ASSERT(shouldContinue != ShouldContinue::ForSuspension);
>     if (shouldContinue != ShouldContinue::Yes)
>         return;
> 
>diff --git a/Source/WebCore/loader/FrameLoaderTypes.h b/Source/WebCore/loader/FrameLoaderTypes.h
>index a5a1f7febe79cbac0904dbdc0d75fe26a948fce1..a3d0f242caf0dd748897fdee1c463b1feea1d3b8 100644
>--- a/Source/WebCore/loader/FrameLoaderTypes.h
>+++ b/Source/WebCore/loader/FrameLoaderTypes.h
>@@ -44,7 +44,7 @@ enum class PolicyAction : uint8_t {
>     Use,
>     Download,
>     Ignore,
>-    Suspend,
>+    Suspend, // FIXME: This is only used by WebKit2 so we shouldn't need this in the WebCore enum.
> };
> 
> enum class ReloadOption : uint8_t {
>diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
>index b9f2c53993ee13e9c59301ddd131d73590c44455..3b6d10a3f8d35e7c138d4a3821cfc88aa898e286 100644
>--- a/Source/WebCore/loader/PolicyChecker.cpp
>+++ b/Source/WebCore/loader/PolicyChecker.cpp
>@@ -183,7 +183,7 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, const Resou
>         case PolicyAction::Ignore:
>             return function({ }, nullptr, ShouldContinue::No);
>         case PolicyAction::Suspend:
>-            return function({ WTF::blankURL() }, nullptr, ShouldContinue::ForSuspension);
>+            RELEASE_ASSERT_NOT_REACHED();
>         case PolicyAction::Use:
>             if (!m_frame.loader().client().canHandleRequest(request)) {
>                 handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request));
>diff --git a/Source/WebCore/loader/PolicyChecker.h b/Source/WebCore/loader/PolicyChecker.h
>index 2071ffab1746f05bf420c05a14fefe045538bfef..d7fd7b3fb17a5821cd31457b298b502e26b0d73e 100644
>--- a/Source/WebCore/loader/PolicyChecker.h
>+++ b/Source/WebCore/loader/PolicyChecker.h
>@@ -54,8 +54,7 @@ class ResourceResponse;
> 
> enum class ShouldContinue {
>     Yes,
>-    No,
>-    ForSuspension
>+    No
> };
> 
> enum class PolicyDecisionMode { Synchronous, Asynchronous };
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>index ca5aeef40b6ec6973920713236391b1cf431c46f..7ed22380405003c06f2b637eab0fd1c4f0c23863 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>@@ -130,14 +130,12 @@ void SuspendedPageProxy::unsuspend()
>     m_process->send(Messages::WebPage::SetIsSuspended(false), m_page.pageID());
> }
> 
>-void SuspendedPageProxy::didFinishLoad()
>+void SuspendedPageProxy::didSuspend()
> {
>     LOG(ProcessSwapping, "SuspendedPageProxy %s from process %i finished transition to suspended", loggingString(), m_process->processIdentifier());
> 
>     m_finishedSuspending = true;
> 
>-    m_process->send(Messages::WebProcess::UpdateActivePages(), 0);
>-
> #if PLATFORM(IOS_FAMILY)
>     m_suspensionToken = nullptr;
> #endif
>@@ -159,8 +157,8 @@ void SuspendedPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder& decod
> {
>     ASSERT(decoder.messageReceiverName() == Messages::WebPageProxy::messageReceiverName());
> 
>-    if (decoder.messageName() == Messages::WebPageProxy::DidFinishLoadForFrame::name()) {
>-        didFinishLoad();
>+    if (decoder.messageName() == Messages::WebPageProxy::DidSuspendAfterProcessSwap::name()) {
>+        didSuspend();
>         return;
>     }
> 
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.h b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>index 113bd51e74be3cc7b79930cc2c25fcfa9997f83e..b661f52a86c5236cd7f182adba2576fbf86a0153 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.h
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>@@ -55,7 +55,7 @@ public:
> #endif
> 
> private:
>-    void didFinishLoad();
>+    void didSuspend();
>     void didFailToSuspend();
> 
>     // IPC::MessageReceiver
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 052361f7a2a70d6760c74376cc37a2e819c394a1..358db6fc24e3d7a1638abebddf6d566fede8de45 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -750,6 +750,9 @@ bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, std
>     if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes)
>         return false;
> 
>+    if (isPageOpenedByDOMShowingInitialEmptyDocument())
>+        return false;
>+
>     auto* currentItem = navigation.fromItem();
>     if (!currentItem) {
>         LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " unable to create suspended page for process pid %i - No current back/forward item", pageID(), m_process->processIdentifier());
>@@ -2626,7 +2629,7 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
>         }
> 
>         if (processForNavigation.ptr() != &process()) {
>-            policyAction = PolicyAction::Suspend;
>+            policyAction = isPageOpenedByDOMShowingInitialEmptyDocument() ? PolicyAction::Ignore : 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
>@@ -2744,8 +2747,7 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, s
>         }
>     };
> 
>-    bool isInitialNavigationInNewWindow = openedByDOM() && !hasCommittedAnyProvisionalLoads();
>-    if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isInitialNavigationInNewWindow || !mainFrameIDInPreviousProcess) {
>+    if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isPageOpenedByDOMShowingInitialEmptyDocument() || !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);
>@@ -2759,6 +2761,11 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, s
>     };
> }
> 
>+bool WebPageProxy::isPageOpenedByDOMShowingInitialEmptyDocument() const
>+{
>+    return openedByDOM() && !hasCommittedAnyProvisionalLoads();
>+}
>+
> // MSVC gives a redeclaration error if noreturn is used on the definition and not the declaration, while
> // Cocoa tests segfault if it is moved to the declaration site (even if we move the definition with it!).
> #if !COMPILER(MSVC)
>@@ -2770,6 +2777,15 @@ void WebPageProxy::didFailToSuspendAfterProcessSwap()
>     ASSERT_NOT_REACHED();
> }
> 
>+#if !COMPILER(MSVC)
>+NO_RETURN_DUE_TO_ASSERT
>+#endif
>+void WebPageProxy::didSuspendAfterProcessSwap()
>+{
>+    // Only the SuspendedPageProxy should be getting this call.
>+    ASSERT_NOT_REACHED();
>+}
>+
> void WebPageProxy::setUserAgent(String&& userAgent)
> {
>     if (m_userAgent == userAgent)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index a32e831f59b6ecc1b998024ba357bfe4f6d659d7..3dfd653203c3107fae0c248ec795101e9b0bd271 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1392,6 +1392,8 @@ public:
> 
>     void setDefersLoadingForTesting(bool);
> 
>+    bool isPageOpenedByDOMShowingInitialEmptyDocument() const;
>+
>     WebCore::IntRect syncRootViewToScreen(const WebCore::IntRect& viewRect);
> 
> #if ENABLE(DATALIST_ELEMENT)
>@@ -1567,6 +1569,7 @@ private:
>     void reattachToWebProcess();
>     void swapToWebProcess(Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>&&, ShouldDelayAttachingDrawingArea);
>     void didFailToSuspendAfterProcessSwap();
>+    void didSuspendAfterProcessSwap();
> 
>     void finishAttachingToWebProcess(ShouldDelayAttachingDrawingArea = ShouldDelayAttachingDrawingArea::No);
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>index f7b43adebe5f86881e70da965d1554adc55d5c73..e6e94fe9a38bb04ba3b2e5ca57ed1e66c715ce64 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -505,6 +505,7 @@ messages -> WebPageProxy {
> #endif
> 
>     DidFailToSuspendAfterProcessSwap()
>+    DidSuspendAfterProcessSwap()
> 
>     ImageOrMediaDocumentSizeChanged(WebCore::IntSize newSize)
> 
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 7fdcc2fe80a2799c4deacc346ac2b3137e63f979..c8f805e8984b32352f925707c7ab5d436f1a50ee 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -2197,9 +2197,8 @@ void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API:
>     if (navigation.treatAsSameOriginNavigation())
>         return completionHandler(page.process(), nullptr, "The treatAsSameOriginNavigation flag is set"_s);
> 
>-    bool isInitialLoadInNewWindowOpenedByDOM = page.openedByDOM() && !page.hasCommittedAnyProvisionalLoads();
>     URL sourceURL;
>-    if (isInitialLoadInNewWindowOpenedByDOM && !navigation.requesterOrigin().isEmpty())
>+    if (page.isPageOpenedByDOMShowingInitialEmptyDocument() && !navigation.requesterOrigin().isEmpty())
>         sourceURL = URL { URL(), navigation.requesterOrigin().toString() };
>     else
>         sourceURL = URL { { }, page.pageLoadState().url() };
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>index 3e6f31a59ecd0bcdeab205c32533ccae9c1b25e5..3fdf1e4f5b06d2bccdaab40e992b71e2ddd486e7 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>@@ -735,12 +735,6 @@ void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceRespons
>         return;
>     }
> 
>-    // For suspension loads to about:blank, no need to ask the SuspendedPageProxy.
>-    if (request.url() == WTF::blankURL() && webPage->isSuspended()) {
>-        function(PolicyAction::Use);
>-        return;
>-    }
>-
>     RefPtr<API::Object> userData;
> 
>     // Notify the bundle client.
>diff --git a/Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp b/Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp
>index 40722c862fe0bc5cfa8ca80a3fa83208c73271c0..b2d54a627b94583bda3518c4e7c3364481b605a4 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp
>@@ -50,7 +50,6 @@ void WebDocumentLoader::detachFromFrame()
> void WebDocumentLoader::setNavigationID(uint64_t navigationID)
> {
>     ASSERT(navigationID);
>-    ASSERT(!m_navigationID || m_navigationID == navigationID);
> 
>     m_navigationID = navigationID;
> }
>diff --git a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
>index 9ae378b288c930033c8d9358c1c88ef5de7962e8..395753510567a95a111aa4b07fa10e63138d4108 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
>@@ -275,7 +275,16 @@ void WebFrame::didReceivePolicyDecision(uint64_t listenerID, PolicyAction action
>             documentLoader->setNavigationID(navigationID);
>     }
> 
>+    bool shouldSuspend = false;
>+    if (action == PolicyAction::Suspend) {
>+        shouldSuspend = true;
>+        action = PolicyAction::Ignore;
>+    }
>+
>     function(action);
>+
>+    if (shouldSuspend)
>+        page()->suspendForProcessSwap();
> }
> 
> void WebFrame::startDownload(const WebCore::ResourceRequest& request, const String& suggestedName)
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index cb719d405c43adaee4f2c538afaa821b1abdc61c..b28f040e1b25c3813ae5a0ad6aec5ade85eb37be 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -184,6 +184,7 @@
> #include <WebCore/MouseEvent.h>
> #include <WebCore/NotImplemented.h>
> #include <WebCore/Page.h>
>+#include <WebCore/PageCache.h>
> #include <WebCore/PageConfiguration.h>
> #include <WebCore/PingLoader.h>
> #include <WebCore/PlatformKeyboardEvent.h>
>@@ -1311,6 +1312,27 @@ void WebPage::sendClose()
>     send(Messages::WebPageProxy::ClosePage(false));
> }
> 
>+void WebPage::suspendForProcessSwap()
>+{
>+    auto failedToSuspend = [this, protectedThis = makeRef(*this)] {
>+        close();
>+        send(Messages::WebPageProxy::DidFailToSuspendAfterProcessSwap());
>+    };
>+
>+    auto* currentHistoryItem = m_mainFrame->coreFrame()->loader().history().currentItem();
>+    if (!currentHistoryItem) {
>+        failedToSuspend();
>+        return;
>+    }
>+
>+    if (!PageCache::singleton().addIfCacheable(*currentHistoryItem, corePage())) {
>+        failedToSuspend();
>+        return;
>+    }
>+
>+    send(Messages::WebPageProxy::DidSuspendAfterProcessSwap());
>+}
>+
> void WebPage::loadURLInFrame(URL&& url, uint64_t frameID)
> {
>     WebFrame* frame = WebProcess::singleton().webFrame(frameID);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index e0af2f636866d4beb4a9c5e9e0ba1d0684871004..099b2c5fa09d642ed4ae4c5201fc47a1a1da912f 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -450,6 +450,8 @@ public:
>     void clearMainFrameName();
>     void sendClose();
> 
>+    void suspendForProcessSwap();
>+
>     void sendSetWindowFrame(const WebCore::FloatRect&);
> 
>     double textZoomFactor() const;
>diff --git a/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm b/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm
>index bdae746d5d48e580d33ba9ecda363615f93b9f6c..2f2496bf4ab188ce0f8033a27ae8fbfce279f380 100644
>--- a/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm
>+++ b/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm
>@@ -460,14 +460,6 @@ static NSURL *origin(WebPage& page)
>         return nil;
> 
>     URL mainFrameURL = { URL(), mainFrame->url() };
>-    if (page.isSuspended()) {
>-        // Suspended page are navigated to about:blank upon suspension so we really want to report the previous URL.
>-        if (auto* coreFrame = mainFrame->coreFrame()) {
>-            if (auto* backHistoryItem = coreFrame->loader().history().previousItem())
>-                mainFrameURL = { URL(), backHistoryItem->urlString() };
>-        }
>-    }
>-
>     Ref<SecurityOrigin> mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
>     String mainFrameOriginString;
>     if (!mainFrameOrigin->isUnique())

Actions: View | Formatted Diff | Diff

Attachments on bug 192668: 357238 | 357244 | 357251