Diff
Modified: trunk/Source/WebKit/ChangeLog (245197 => 245198)
--- trunk/Source/WebKit/ChangeLog 2019-05-10 21:36:45 UTC (rev 245197)
+++ trunk/Source/WebKit/ChangeLog 2019-05-10 23:25:28 UTC (rev 245198)
@@ -1,3 +1,33 @@
+2019-05-10 Chris Dumez <[email protected]>
+
+ [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 <[email protected]>
Streamline test-and-clear operation for ContextMenu
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (245197 => 245198)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-05-10 21:36:45 UTC (rev 245197)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-05-10 23:25:28 UTC (rev 245198)
@@ -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,6 +157,7 @@
if (m_isClosed)
return;
+ RELEASE_LOG(ProcessSwapping, "%p - SuspendedPageProxy::close()", this);
m_isClosed = true;
m_process->send(Messages::WebPage::Close(), m_page.pageID());
}
@@ -165,14 +164,24 @@
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)
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (245197 => 245198)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-05-10 21:36:45 UTC (rev 245197)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-05-10 23:25:28 UTC (rev 245198)
@@ -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;
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (245197 => 245198)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-05-10 21:36:45 UTC (rev 245197)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-05-10 23:25:28 UTC (rev 245198)
@@ -742,7 +742,7 @@
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;
@@ -749,12 +749,6 @@
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.
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (245197 => 245198)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-05-10 21:36:45 UTC (rev 245197)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-05-10 23:25:28 UTC (rev 245198)
@@ -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,
