- Revision
- 240443
- Author
- [email protected]
- Date
- 2019-01-24 10:35:05 -0800 (Thu, 24 Jan 2019)
Log Message
[PSON] Flash on back navigation on Mac
https://bugs.webkit.org/show_bug.cgi?id=193716
<rdar://problem/47148458>
Reviewed by Chris Dumez.
Source/WebKit:
We close the page immediately if we fail to suspend. Layers disappear and we get a flash.
* UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::~SuspendedPageProxy):
(WebKit::SuspendedPageProxy::close):
Track closed state so we don't send the message twice, causing unhandled message errors in web process.
(WebKit::SuspendedPageProxy::didProcessRequestToSuspend):
Close the suspended page if the suspension fails.
Skip this if we are using web process side compositing on Mac.
* UIProcess/SuspendedPageProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::enterAcceleratedCompositingMode):
On Mac, close the failed SuspendedPageProxy when entering compositing mode. At this point we don't need it to keep layers alive.
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::closeFailedSuspendedPagesForPage):
* UIProcess/WebProcessPool.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::suspendForProcessSwap):
Don't close the page on suspension failure. This is now managed by the UI process.
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Closing of the previous page is delayed so waiting for didFinishNavigation is
not sufficient to guarantee we have received all the messages. Wait for them.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (240442 => 240443)
--- trunk/Source/WebKit/ChangeLog 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/ChangeLog 2019-01-24 18:35:05 UTC (rev 240443)
@@ -1,3 +1,38 @@
+2019-01-24 Antti Koivisto <[email protected]>
+
+ [PSON] Flash on back navigation on Mac
+ https://bugs.webkit.org/show_bug.cgi?id=193716
+ <rdar://problem/47148458>
+
+ Reviewed by Chris Dumez.
+
+ We close the page immediately if we fail to suspend. Layers disappear and we get a flash.
+
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::~SuspendedPageProxy):
+ (WebKit::SuspendedPageProxy::close):
+
+ Track closed state so we don't send the message twice, causing unhandled message errors in web process.
+
+ (WebKit::SuspendedPageProxy::didProcessRequestToSuspend):
+
+ Close the suspended page if the suspension fails.
+ Skip this if we are using web process side compositing on Mac.
+
+ * UIProcess/SuspendedPageProxy.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::enterAcceleratedCompositingMode):
+
+ On Mac, close the failed SuspendedPageProxy when entering compositing mode. At this point we don't need it to keep layers alive.
+
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::closeFailedSuspendedPagesForPage):
+ * UIProcess/WebProcessPool.h:
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::suspendForProcessSwap):
+
+ Don't close the page on suspension failure. This is now managed by the UI process.
+
2019-01-24 Chris Dumez <[email protected]>
Regression(PSON) Back/Forward list items' URL sometimes gets replaced with the URL of a subframe
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (240442 => 240443)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-01-24 18:35:05 UTC (rev 240443)
@@ -26,6 +26,7 @@
#include "config.h"
#include "SuspendedPageProxy.h"
+#include "DrawingAreaProxy.h"
#include "Logging.h"
#include "WebPageMessages.h"
#include "WebPageProxy.h"
@@ -99,7 +100,7 @@
// If the suspended page was not consumed before getting destroyed, then close the corresponding page
// on the WebProcess side.
- m_process->send(Messages::WebPage::Close(), m_page.pageID());
+ close();
if (m_suspensionState == SuspensionState::Suspending)
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
@@ -139,6 +140,17 @@
m_process->send(Messages::WebPage::SetIsSuspended(false), m_page.pageID());
}
+void SuspendedPageProxy::close()
+{
+ ASSERT(m_suspensionState != SuspensionState::Resumed);
+
+ if (m_isClosed)
+ return;
+
+ m_isClosed = true;
+ m_process->send(Messages::WebPage::Close(), m_page.pageID());
+}
+
void SuspendedPageProxy::didProcessRequestToSuspend(SuspensionState newSuspensionState)
{
LOG(ProcessSwapping, "SuspendedPageProxy %s from process %i finished transition to suspended", loggingString(), m_process->processIdentifier());
@@ -154,6 +166,15 @@
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
+ bool shouldDelayClosingOnFailure = false;
+#if PLATFORM(MAC)
+ // With web process side tiles, we need to keep the suspended page around on failure to avoid flashing.
+ // It is removed by WebPageProxy::enterAcceleratedCompositingMode when the target page is ready.
+ shouldDelayClosingOnFailure = m_page.drawingArea() && m_page.drawingArea()->type() == DrawingAreaTypeTiledCoreAnimation;
+#endif
+ if (m_suspensionState == SuspensionState::FailedToSuspend && !shouldDelayClosingOnFailure)
+ close();
+
if (m_readyToUnsuspendHandler)
m_readyToUnsuspendHandler(this);
}
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (240442 => 240443)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2019-01-24 18:35:05 UTC (rev 240443)
@@ -52,6 +52,7 @@
void waitUntilReadyToUnsuspend(CompletionHandler<void(SuspendedPageProxy*)>&&);
void unsuspend();
+ void close();
#if !LOG_DISABLED
const char* loggingString() const;
@@ -69,6 +70,7 @@
Ref<WebProcessProxy> m_process;
uint64_t m_mainFrameID;
String m_registrableDomain;
+ bool m_isClosed { false };
SuspensionState m_suspensionState { SuspensionState::Suspending };
CompletionHandler<void(SuspendedPageProxy*)> m_readyToUnsuspendHandler;
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (240442 => 240443)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-01-24 18:35:05 UTC (rev 240443)
@@ -6847,7 +6847,12 @@
void WebPageProxy::enterAcceleratedCompositingMode(const LayerTreeContext& layerTreeContext)
{
+#if PLATFORM(MAC)
+ ASSERT(m_drawingArea->type() == DrawingAreaTypeTiledCoreAnimation);
+#endif
pageClient().enterAcceleratedCompositingMode(layerTreeContext);
+ // We needed the failed suspended page to stay alive to avoid flashing. Now we can get rid of it.
+ m_process->processPool().closeFailedSuspendedPagesForPage(*this);
}
void WebPageProxy::exitAcceleratedCompositingMode()
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (240442 => 240443)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-01-24 18:35:05 UTC (rev 240443)
@@ -2262,6 +2262,14 @@
});
}
+void WebProcessPool::closeFailedSuspendedPagesForPage(WebPageProxy& page)
+{
+ for (auto& suspendedPage : m_suspendedPages) {
+ if (&suspendedPage->page() == &page && suspendedPage->failedToSuspend())
+ suspendedPage->close();
+ }
+}
+
std::unique_ptr<SuspendedPageProxy> WebProcessPool::takeSuspendedPage(SuspendedPageProxy& suspendedPage)
{
return m_suspendedPages.takeFirst([&suspendedPage](auto& item) {
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (240442 => 240443)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2019-01-24 18:35:05 UTC (rev 240443)
@@ -447,6 +447,7 @@
// SuspendedPageProxy management.
void addSuspendedPage(std::unique_ptr<SuspendedPageProxy>&&);
void removeAllSuspendedPagesForPage(WebPageProxy&);
+ void closeFailedSuspendedPagesForPage(WebPageProxy&);
std::unique_ptr<SuspendedPageProxy> takeSuspendedPage(SuspendedPageProxy&);
void removeSuspendedPage(SuspendedPageProxy&);
bool hasSuspendedPageFor(WebProcessProxy&) const;
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (240442 => 240443)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-01-24 18:35:05 UTC (rev 240443)
@@ -1340,7 +1340,6 @@
void WebPage::suspendForProcessSwap()
{
auto failedToSuspend = [this, protectedThis = makeRef(*this)] {
- close();
send(Messages::WebPageProxy::DidFailToSuspendAfterProcessSwap());
};
Modified: trunk/Tools/ChangeLog (240442 => 240443)
--- trunk/Tools/ChangeLog 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Tools/ChangeLog 2019-01-24 18:35:05 UTC (rev 240443)
@@ -1,3 +1,16 @@
+2019-01-24 Antti Koivisto <[email protected]>
+
+ [PSON] Flash on back navigation on Mac
+ https://bugs.webkit.org/show_bug.cgi?id=193716
+ <rdar://problem/47148458>
+
+ Reviewed by Chris Dumez.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+ Closing of the previous page is delayed so waiting for didFinishNavigation is
+ not sufficient to guarantee we have received all the messages. Wait for them.
+
2019-01-24 Chris Dumez <[email protected]>
Regression(PSON) Back/Forward list items' URL sometimes gets replaced with the URL of a subframe
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (240442 => 240443)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-01-24 18:30:35 UTC (rev 240442)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-01-24 18:35:05 UTC (rev 240443)
@@ -2413,6 +2413,9 @@
TestWebKitAPI::Util::run(&done);
done = false;
+ while ([receivedMessages count] < 7)
+ TestWebKitAPI::Util::sleep(0.1);
+
EXPECT_EQ(7u, [receivedMessages count]);
EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html - pageshow NOT persisted", receivedMessages.get()[0]);
if ([receivedMessages.get()[1] hasPrefix:@"pson://www.webkit.org/main.html"]) {
@@ -2503,6 +2506,9 @@
TestWebKitAPI::Util::run(&done);
done = false;
+ while ([receivedMessages count] < 7)
+ TestWebKitAPI::Util::sleep(0.1);
+
EXPECT_EQ(7u, [receivedMessages count]);
EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html - load", receivedMessages.get()[0]);
if ([receivedMessages.get()[1] hasPrefix:@"pson://www.webkit.org/main.html"]) {
