Attachment 369603 Details for Bug 197799 – Patch

[patch] Patch

bug-197799-20190510150419.patch (text/plain), 13.00 KB, created by Chris Dumez on 2019-05-10 15:04:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-10 15:04:20 PDT

Size: 13.00 KB

patch

obsolete

>Subversion Revision: 245188
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 1497a3ecf88d4a2a1d7cab6e7934837e14d29976..eb7c3787f822732a3156c43298c08c5ce1b81898 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-05-10  Chris Dumez  <cdumez@apple.com>
>+
>+        [PSON] Prevent flashing when the process-swap is forced by the client
>+        https://bugs.webkit.org/show_bug.cgi?id=197799
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When the process-swap is forced by the client, we would not construct a SuspendedPageProxy for
>+        the previous page, which would cause a white/black flash upon navigation on macOS. The reason
>+        we did not construct a SuspendedPageProxy is that it would be unsafe to keep the page around
>+        in this case because other windows might have an opener link to the page when the swap is forced
>+        and we need those opener / openee links to get severed.
>+
>+        The new approach to maintain the Web facing behavior without flashing is to create a suspended
>+        page proxy for the previous page when the process swap is forced by the client. We then close
>+        the page as soon as we can do so without flashing (when pageEnteredAcceleratedCompositingMode()
>+        has been called).
>+
>+        * UIProcess/SuspendedPageProxy.cpp:
>+        (WebKit::SuspendedPageProxy::SuspendedPageProxy):
>+        (WebKit::SuspendedPageProxy::close):
>+        (WebKit::SuspendedPageProxy::pageEnteredAcceleratedCompositingMode):
>+        (WebKit::SuspendedPageProxy::closeWithoutFlashing):
>+        (WebKit::SuspendedPageProxy::didProcessRequestToSuspend):
>+        * UIProcess/SuspendedPageProxy.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
>+        (WebKit::WebPageProxy::commitProvisionalPage):
>+        * UIProcess/WebPageProxy.h:
>+
> 2019-05-10  Chris Dumez  <cdumez@apple.com>
> 
>         The active tab sometimes app naps even though it should not
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>index 9dbfa3c4e4dff2d2b9f4b90161a7da46ce5f6938..4d6c85dc893811e2007ff8f54c8e719ff546e65a 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>@@ -78,13 +78,11 @@ static const HashSet<IPC::StringReference>& messageNamesToIgnoreWhileSuspended()
> }
> #endif
> 
>-SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, uint64_t mainFrameID)
>+SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, uint64_t mainFrameID, ShouldDelayClosingUntilEnteringAcceleratedCompositingMode shouldDelayClosingUntilEnteringAcceleratedCompositingMode)
>     : m_page(page)
>     , m_process(WTFMove(process))
>     , m_mainFrameID(mainFrameID)
>-#if PLATFORM(MAC)
>-    , m_shouldDelayClosingOnFailure(m_page.drawingArea() && m_page.drawingArea()->type() == DrawingAreaTypeTiledCoreAnimation)
>-#endif
>+    , m_shouldDelayClosingUntilEnteringAcceleratedCompositingMode(shouldDelayClosingUntilEnteringAcceleratedCompositingMode)
>     , m_suspensionTimeoutTimer(RunLoop::main(), this, &SuspendedPageProxy::suspensionTimedOut)
> #if PLATFORM(IOS_FAMILY)
>     , m_suspensionToken(m_process->throttler().backgroundActivityToken())
>@@ -159,20 +157,31 @@ void SuspendedPageProxy::close()
>     if (m_isClosed)
>         return;
> 
>+    RELEASE_LOG(ProcessSwapping, "%p - SuspendedPageProxy::close()", this);
>     m_isClosed = true;
>     m_process->send(Messages::WebPage::Close(), m_page.pageID());
> }
> 
> void SuspendedPageProxy::pageEnteredAcceleratedCompositingMode()
> {
>-    m_shouldDelayClosingOnFailure = false;
>+    m_shouldDelayClosingUntilEnteringAcceleratedCompositingMode = ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::No;
> 
>-    if (m_suspensionState == SuspensionState::FailedToSuspend) {
>-        // We needed the failed suspended page to stay alive to avoid flashing. Now we can get rid of it.
>+    if (m_suspensionState == SuspensionState::FailedToSuspend || m_shouldCloseWhenEnteringAcceleratedCompositingMode) {
>+        // We needed the suspended page to stay alive to avoid flashing. Now we can get rid of it.
>         close();
>     }
> }
> 
>+void SuspendedPageProxy::closeWithoutFlashing()
>+{
>+    RELEASE_LOG(ProcessSwapping, "%p - SuspendedPageProxy::closeWithoutFlashing() shouldDelayClosingUntilEnteringAcceleratedCompositingMode? %d", this, m_shouldDelayClosingUntilEnteringAcceleratedCompositingMode == ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::Yes);
>+    if (m_shouldDelayClosingUntilEnteringAcceleratedCompositingMode == ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::Yes) {
>+        m_shouldCloseWhenEnteringAcceleratedCompositingMode = true;
>+        return;
>+    }
>+    close();
>+}
>+
> void SuspendedPageProxy::didProcessRequestToSuspend(SuspensionState newSuspensionState)
> {
>     LOG(ProcessSwapping, "SuspendedPageProxy %s from process %i finished transition to suspended", loggingString(), m_process->processIdentifier());
>@@ -191,7 +200,7 @@ void SuspendedPageProxy::didProcessRequestToSuspend(SuspensionState newSuspensio
> 
>     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
> 
>-    if (m_suspensionState == SuspensionState::FailedToSuspend && !m_shouldDelayClosingOnFailure)
>+    if (m_suspensionState == SuspensionState::FailedToSuspend && m_shouldDelayClosingUntilEnteringAcceleratedCompositingMode == ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::No)
>         close();
> 
>     if (m_readyToUnsuspendHandler)
>diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.h b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>index 8da2fa216d198ac036616ff26573a69f2f7c0598..8fe1c12a708396785b33db836f807741ad79d50e 100644
>--- a/Source/WebKit/UIProcess/SuspendedPageProxy.h
>+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.h
>@@ -37,10 +37,12 @@ namespace WebKit {
> class WebPageProxy;
> class WebProcessProxy;
> 
>+enum class ShouldDelayClosingUntilEnteringAcceleratedCompositingMode : bool { No, Yes };
>+
> class SuspendedPageProxy final: public IPC::MessageReceiver, public CanMakeWeakPtr<SuspendedPageProxy> {
>     WTF_MAKE_FAST_ALLOCATED;
> public:
>-    SuspendedPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, uint64_t mainFrameID);
>+    SuspendedPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, uint64_t mainFrameID, ShouldDelayClosingUntilEnteringAcceleratedCompositingMode);
>     ~SuspendedPageProxy();
> 
>     WebPageProxy& page() const { return m_page; }
>@@ -51,9 +53,9 @@ public:
> 
>     void waitUntilReadyToUnsuspend(CompletionHandler<void(SuspendedPageProxy*)>&&);
>     void unsuspend();
>-    void close();
> 
>     void pageEnteredAcceleratedCompositingMode();
>+    void closeWithoutFlashing();
> 
> #if !LOG_DISABLED
>     const char* loggingString() const;
>@@ -64,6 +66,8 @@ private:
>     void didProcessRequestToSuspend(SuspensionState);
>     void suspensionTimedOut();
> 
>+    void close();
>+
>     // IPC::MessageReceiver
>     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
>     void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) final;
>@@ -72,7 +76,8 @@ private:
>     Ref<WebProcessProxy> m_process;
>     uint64_t m_mainFrameID;
>     bool m_isClosed { false };
>-    bool m_shouldDelayClosingOnFailure { false };
>+    ShouldDelayClosingUntilEnteringAcceleratedCompositingMode m_shouldDelayClosingUntilEnteringAcceleratedCompositingMode { ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::No };
>+    bool m_shouldCloseWhenEnteringAcceleratedCompositingMode { false };
> 
>     SuspensionState m_suspensionState { SuspensionState::Suspending };
>     CompletionHandler<void(SuspendedPageProxy*)> m_readyToUnsuspendHandler;
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index e59657217abbe33de029ba51c8d35852978bb60e..1b6b5168609d675d74a8ee1ae991ba5428fa24d1 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -742,19 +742,13 @@ void WebPageProxy::launchProcess(const RegistrableDomain& registrableDomain)
>     finishAttachingToWebProcess(IsProcessSwap::No);
> }
> 
>-bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, Optional<uint64_t> mainFrameID, ProcessSwapRequestedByClient processSwapRequestedByClient)
>+bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, Optional<uint64_t> mainFrameID, ProcessSwapRequestedByClient processSwapRequestedByClient, ShouldDelayClosingUntilEnteringAcceleratedCompositingMode shouldDelayClosingUntilEnteringAcceleratedCompositingMode)
> {
>     m_lastSuspendedPage = nullptr;
> 
>     if (!mainFrameID)
>         return false;
> 
>-    // If the client forced a swap then it may not be Web-compatible to suspend the previous page because other windows may have an opener link to the page.
>-    if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes) {
>-        RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because the swap was requested by the client", m_process->processIdentifier());
>-        return false;
>-    }
>-
>     if (!hasCommittedAnyProvisionalLoads()) {
>         RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because has not committed any load yet", m_process->processIdentifier());
>         return false;
>@@ -781,10 +775,15 @@ bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, Opt
>     }
> 
>     RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Suspending current page for process pid %i", m_process->processIdentifier());
>-    auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *mainFrameID);
>+    auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *mainFrameID, shouldDelayClosingUntilEnteringAcceleratedCompositingMode);
> 
>     LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), suspendedPage->loggingString(), m_process->processIdentifier(), fromItem ? fromItem->itemID().logString() : 0);
> 
>+    // If the client forced a swap then it may not be web-compatible to keep the previous page because other windows may have an opener link to it. We thus close it as soon as we
>+    // can do so without flashing.
>+    if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes)
>+        suspendedPage->closeWithoutFlashing();
>+
>     if (fromItem && m_preferences->usesPageCache())
>         fromItem->setSuspendedPage(suspendedPage.get());
> 
>@@ -2884,11 +2883,19 @@ void WebPageProxy::commitProvisionalPage(uint64_t frameID, uint64_t navigationID
> 
>     ASSERT(m_process.ptr() != &m_provisionalPage->process());
> 
>+    auto shouldDelayClosingUntilEnteringAcceleratedCompositingMode = ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::No;
>+#if PLATFORM(MAC)
>+    // On macOS, when not using UI-side compositing, we need to make sure we do not close the page in the previous process until we've
>+    // entered accelerated compositing for the new page or we will flash on navigation.
>+    if (drawingArea()->type() == DrawingAreaTypeTiledCoreAnimation)
>+        shouldDelayClosingUntilEnteringAcceleratedCompositingMode = ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::Yes;
>+#endif
>+
>     processDidTerminate(ProcessTerminationReason::NavigationSwap);
> 
>     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
>     auto* navigation = navigationState().navigation(m_provisionalPage->navigationID());
>-    bool didSuspendPreviousPage = navigation ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient()) : false;
>+    bool didSuspendPreviousPage = navigation ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient(), shouldDelayClosingUntilEnteringAcceleratedCompositingMode) : 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.
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index b239a9c73a5fd18a00ffa821c81a5fc03469e01f..8d8fe812238d356497d48789eabf1e2f4735a48b 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1554,7 +1554,7 @@ private:
>     void updateThrottleState();
>     void updateHiddenPageThrottlingAutoIncreases();
> 
>-    bool suspendCurrentPageIfPossible(API::Navigation&, Optional<uint64_t> mainFrameID, ProcessSwapRequestedByClient);
>+    bool suspendCurrentPageIfPossible(API::Navigation&, Optional<uint64_t> mainFrameID, ProcessSwapRequestedByClient, ShouldDelayClosingUntilEnteringAcceleratedCompositingMode);
> 
>     enum class ResetStateReason {
>         PageInvalidated,

Actions: View | Formatted Diff | Diff

Attachments on bug 197799: 369603