[PSON] Prevent flashing when the process-swap is forced by the client
https://bugs.webkit.org/show_bug.cgi?id=197799
Reviewed by Geoffrey Garen.
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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@245198 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index a40a2d4..cdb9040 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 Geoffrey Garen.
+
+ 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 Brent Fulgham <bfulgham@apple.com>
Streamline test-and-clear operation for ContextMenu
diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
index 9dbfa3c..4d6c85d 100644
--- a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
@@ -78,13 +78,11 @@
}
#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 @@
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 @@
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 8da2fa2..8fe1c12 100644
--- a/Source/WebKit/UIProcess/SuspendedPageProxy.h
+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.h
@@ -37,10 +37,12 @@
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 @@
void waitUntilReadyToUnsuspend(CompletionHandler<void(SuspendedPageProxy*)>&&);
void unsuspend();
- void close();
void pageEnteredAcceleratedCompositingMode();
+ void closeWithoutFlashing();
#if !LOG_DISABLED
const char* loggingString() const;
@@ -64,6 +66,8 @@
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 @@
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 e596572..1b6b516 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -742,19 +742,13 @@
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 @@
}
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 @@
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 b239a9c..8d8fe81 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.h
+++ b/Source/WebKit/UIProcess/WebPageProxy.h
@@ -1554,7 +1554,7 @@
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,
