Changeset 240443 in webkit

Timestamp:

Jan 24, 2019 10:35:05 AM (5 years ago)

Author:

Antti Koivisto

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.

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/SuspendedPageProxy.cpp (modified) (4 diffs)
• Source/WebKit/UIProcess/SuspendedPageProxy.h (modified) (2 diffs)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.h (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-01-24  Antti Koivisto  <antti@apple.com>
+
+        [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  <cdumez@apple.com>
 

trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp

@@ -27 +27 @@
 #include "SuspendedPageProxy.h"
 
+#include "DrawingAreaProxy.h"
 #include "Logging.h"
 #include "WebPageMessages.h"
@@ -100 +101 @@
     // 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)
@@ -140 +141 @@
 }
 
+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)
 {
@@ -154 +166 @@
 
     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)

trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h

@@ -53 +53 @@
     void waitUntilReadyToUnsuspend(CompletionHandler<void(SuspendedPageProxy*)>&&);
     void unsuspend();
+    void close();
 
 #if !LOG_DISABLED
@@ -70 +71 @@
     uint64_t m_mainFrameID;
     String m_registrableDomain;
+    bool m_isClosed { false };
 
     SuspensionState m_suspensionState { SuspensionState::Suspending };

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -6848 +6848 @@
 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);
 }
 

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -2263 +2263 @@
 }
 
+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)
 {

trunk/Source/WebKit/UIProcess/WebProcessPool.h

@@ -448 +448 @@
     void addSuspendedPage(std::unique_ptr<SuspendedPageProxy>&&);
     void removeAllSuspendedPagesForPage(WebPageProxy&);
+    void closeFailedSuspendedPagesForPage(WebPageProxy&);
     std::unique_ptr<SuspendedPageProxy> takeSuspendedPage(SuspendedPageProxy&);
     void removeSuspendedPage(SuspendedPageProxy&);

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -1341 +1341 @@
 {
     auto failedToSuspend = [this, protectedThis = makeRef(*this)] {
-        close();
         send(Messages::WebPageProxy::DidFailToSuspendAfterProcessSwap());
     };

trunk/Tools/ChangeLog

@@ +1 @@
+2019-01-24  Antti Koivisto  <antti@apple.com>
+
+        [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  <cdumez@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -2414 +2414 @@
     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]);
@@ -2504 +2507 @@
     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]);