REGRESSION (PSON): White or Black flash occurs when process swapping on navigation...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Sep 2018 15:41:31 +0000 (15:41 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Sep 2018 15:41:31 +0000 (15:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189663
<rdar://problem/44184955>

Reviewed by Geoff Garen.

We need to keep the layer tree of the previous page alive and visible until we have something
to render on the new page. With PSON on Mac this means that we should keep displaying the
layer tree from the previus process.

This patch moves the management of 'attaching' the drawing area (Mac only concept) from web process
to UI process. This is when we parent the layer tree to the view root layer. It also ensures that
the layer tree is not deleted too early on process swap and that it still eventually gets deleted.

* UIProcess/DrawingAreaProxy.h:
(WebKit::DrawingAreaProxy::attachInWebProcess):
* UIProcess/SuspendedPageProxy.cpp:
(WebKit::messageNamesToIgnoreWhileSuspended):
(WebKit::SuspendedPageProxy::tearDownDrawingAreaInWebProcess):

We no longer tear down drawing area (layer tree) for suspended pages automatically. Send an explicit
message for it.

* UIProcess/SuspendedPageProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::reattachToWebProcess):

Only call didRelaunchProcess when process actually relaunched (not navigation process launch) to
match not calling processDidExit in resetStateAfterProcessExited.

(WebKit::WebPageProxy::didCompletePageTransition):

Attach the drawing area if appropriate.

(WebKit::WebPageProxy::decidePolicyForNavigationAction):

Send suspend message to WebPage immediately instead waiting for the runloop callback. This is needed so we
can avoid flashing the initial empty document load when the new Page object is created.

(WebKit::WebPageProxy::resetStateAfterProcessExited):

Don't call processDidExit when suspending, not exiting the process (this function needs a new name or rafactoring).
This avoids clearing the drawing area and flashing to black.

(WebKit::WebPageProxy::enterAcceleratedCompositingMode):

This is called when we have switched to the new layer tree.
Tear down the drawing area in the previus process.

* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h:
* UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:
(WebKit::TiledCoreAnimationDrawingAreaProxy::attachInWebProcess):

Send a message to the web process to attach the drawing area.

* WebProcess/WebPage/DrawingArea.h:
(WebKit::DrawingArea::attach):
(WebKit::DrawingArea::attachDrawingArea): Deleted.

Rename to be less redundant.

* WebProcess/WebPage/DrawingArea.messages.in:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::reinitializeWebPage):

Don't attach drawing area automatically. It will be done by a message from UI process.

(WebKit::WebPage::setLayerTreeStateIsFrozen):

Layer tree is always frozen in a suspended process (if it exists).

(WebKit::WebPage::didStartPageTransition):
(WebKit::WebPage::didCompletePageTransition):

Notify UI process of transition completion.

(WebKit::WebPage::setIsSuspended):
(WebKit::WebPage::tearDownDrawingAreaForSuspend):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea):

Don't attach drawing area automatically. It will be done by a message from UI process.

(WebKit::TiledCoreAnimationDrawingArea::attach):
(WebKit::TiledCoreAnimationDrawingArea::attachDrawingArea): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236138 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/DrawingAreaProxy.h
Source/WebKit/UIProcess/SuspendedPageProxy.cpp
Source/WebKit/UIProcess/SuspendedPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.messages.in
Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h
Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm
Source/WebKit/WebProcess/WebPage/DrawingArea.h
Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

index 1a3de03..eb3d4c3 100644 (file)
@@ -1,3 +1,96 @@
+2018-09-18  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (PSON): White or Black flash occurs when process swapping on navigation on Mac
+        https://bugs.webkit.org/show_bug.cgi?id=189663
+        <rdar://problem/44184955>
+
+        Reviewed by Geoff Garen.
+
+        We need to keep the layer tree of the previous page alive and visible until we have something
+        to render on the new page. With PSON on Mac this means that we should keep displaying the
+        layer tree from the previus process.
+
+        This patch moves the management of 'attaching' the drawing area (Mac only concept) from web process
+        to UI process. This is when we parent the layer tree to the view root layer. It also ensures that
+        the layer tree is not deleted too early on process swap and that it still eventually gets deleted.
+
+        * UIProcess/DrawingAreaProxy.h:
+        (WebKit::DrawingAreaProxy::attachInWebProcess):
+        * UIProcess/SuspendedPageProxy.cpp:
+        (WebKit::messageNamesToIgnoreWhileSuspended):
+        (WebKit::SuspendedPageProxy::tearDownDrawingAreaInWebProcess):
+
+        We no longer tear down drawing area (layer tree) for suspended pages automatically. Send an explicit
+        message for it.
+
+        * UIProcess/SuspendedPageProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::reattachToWebProcess):
+
+        Only call didRelaunchProcess when process actually relaunched (not navigation process launch) to
+        match not calling processDidExit in resetStateAfterProcessExited.
+
+        (WebKit::WebPageProxy::didCompletePageTransition):
+
+        Attach the drawing area if appropriate.
+
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+
+        Send suspend message to WebPage immediately instead waiting for the runloop callback. This is needed so we
+        can avoid flashing the initial empty document load when the new Page object is created.
+
+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
+
+        Don't call processDidExit when suspending, not exiting the process (this function needs a new name or rafactoring).
+        This avoids clearing the drawing area and flashing to black.
+
+        (WebKit::WebPageProxy::enterAcceleratedCompositingMode):
+
+        This is called when we have switched to the new layer tree.
+        Tear down the drawing area in the previus process.
+
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h:
+        * UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:
+        (WebKit::TiledCoreAnimationDrawingAreaProxy::attachInWebProcess):
+
+        Send a message to the web process to attach the drawing area.
+
+        * WebProcess/WebPage/DrawingArea.h:
+        (WebKit::DrawingArea::attach):
+        (WebKit::DrawingArea::attachDrawingArea): Deleted.
+
+        Rename to be less redundant.
+
+        * WebProcess/WebPage/DrawingArea.messages.in:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::reinitializeWebPage):
+
+        Don't attach drawing area automatically. It will be done by a message from UI process.
+
+        (WebKit::WebPage::setLayerTreeStateIsFrozen):
+
+        Layer tree is always frozen in a suspended process (if it exists).
+
+        (WebKit::WebPage::didStartPageTransition):
+        (WebKit::WebPage::didCompletePageTransition):
+
+        Notify UI process of transition completion.
+
+        (WebKit::WebPage::setIsSuspended):
+        (WebKit::WebPage::tearDownDrawingAreaForSuspend):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea):
+
+        Don't attach drawing area automatically. It will be done by a message from UI process.
+
+        (WebKit::TiledCoreAnimationDrawingArea::attach):
+        (WebKit::TiledCoreAnimationDrawingArea::attachDrawingArea): Deleted.
+
 2018-09-18  Claudio Saavedra  <csaavedra@igalia.com>
 
         [WPE] Implement mouse event modifiers
index 21470bf..8dda378 100644 (file)
@@ -82,6 +82,8 @@ public:
     void viewExposedRectChangedTimerFired();
 #endif
 
+    virtual void attachInWebProcess() { }
+
     virtual void updateDebugIndicator() { }
 
     virtual void waitForDidUpdateActivityState(ActivityStateChangeID) { }
index 5d1fc5c..d562677 100644 (file)
@@ -54,6 +54,7 @@ static const HashSet<IPC::StringReference>& messageNamesToIgnoreWhileSuspended()
         messageNames.get().add("DidDestroyNavigation");
         messageNames.get().add("DidFinishDocumentLoadForFrame");
         messageNames.get().add("DidFinishProgress");
+        messageNames.get().add("DidCompletePageTransition");
         messageNames.get().add("DidFirstLayoutForFrame");
         messageNames.get().add("DidFirstVisuallyNonEmptyLayoutForFrame");
         messageNames.get().add("DidNavigateWithNavigationData");
@@ -109,6 +110,11 @@ void SuspendedPageProxy::destroyWebPageInWebProcess()
     m_page.suspendedPageClosed(*this);
 }
 
+void SuspendedPageProxy::tearDownDrawingAreaInWebProcess()
+{
+    m_process->send(Messages::WebPage::TearDownDrawingAreaForSuspend(), m_page.pageID());
+}
+
 void SuspendedPageProxy::didFinishLoad()
 {
     ASSERT(m_process);
index 4921109..7be0856 100644 (file)
@@ -49,6 +49,7 @@ public:
 
     void webProcessDidClose(WebProcessProxy&);
     void destroyWebPageInWebProcess();
+    void tearDownDrawingAreaInWebProcess();
 
 #if !LOG_DISABLED
     const char* loggingString() const;
index 85190cf..90d798f 100644 (file)
@@ -56,8 +56,8 @@
 #include "AuthenticationDecisionListener.h"
 #include "DataReference.h"
 #include "DownloadProxy.h"
+#include "DrawingAreaMessages.h"
 #include "DrawingAreaProxy.h"
-#include "DrawingAreaProxyMessages.h"
 #include "EventDispatcherMessages.h"
 #include "FrameInfoData.h"
 #include "LoadParameters.h"
@@ -798,7 +798,9 @@ void WebPageProxy::reattachToWebProcess(Ref<WebProcessProxy>&& process, API::Nav
 
     initializeWebPage();
 
-    pageClient().didRelaunchProcess();
+    if (!navigation)
+        pageClient().didRelaunchProcess();
+
     m_drawingArea->waitForBackingStoreUpdateOnNextPaint();
 }
 
@@ -2449,6 +2451,8 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
         if (proposedProcess.ptr() != &process()) {
             RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), proposedProcess->processIdentifier(), reason.utf8().data());
             LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), proposedProcess->processIdentifier(), navigation->navigationID(), navigation->loggingString());
+
+            process().send(Messages::WebPage::SetIsSuspended(true), pageID());
             
             RunLoop::main().dispatch([this, protectedThis = makeRef(*this), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
                 continueNavigationInNewProcess(navigation, WTFMove(proposedProcess));
@@ -3445,6 +3449,22 @@ void WebPageProxy::didFinishProgress()
     m_pageLoadState.commitChanges();
 }
 
+void WebPageProxy::didCompletePageTransition(bool isInitialEmptyDocument)
+{
+#if PLATFORM(MAC)
+    if (!m_drawingArea)
+        return;
+
+    // Attach drawing area for the initial empty document only if this is not a process swap.
+    if (isInitialEmptyDocument && m_suspendedPage)
+        return;
+
+    m_drawingArea->attachInWebProcess();
+#else
+    UNUSED_PARAM(isInitialEmptyDocument);
+#endif
+}
+
 void WebPageProxy::setNetworkRequestsInProgress(bool networkRequestsInProgress)
 {
     auto transaction = m_pageLoadState.transaction();
@@ -6167,7 +6187,9 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
 
     m_editorState = EditorState();
 
-    pageClient().processDidExit();
+    if (terminationReason != ProcessTerminationReason::NavigationSwap)
+        pageClient().processDidExit();
+
     pageClient().clearAllEditCommands();
 
     auto resetStateReason = terminationReason == ProcessTerminationReason::NavigationSwap ? ResetStateReason::NavigationSwap : ResetStateReason::WebProcessExited;
@@ -6308,6 +6330,10 @@ WebPageCreationParameters WebPageProxy::creationParameters()
 void WebPageProxy::enterAcceleratedCompositingMode(const LayerTreeContext& layerTreeContext)
 {
     pageClient().enterAcceleratedCompositingMode(layerTreeContext);
+
+    // We have completed the page transition and can tear down the layers in the suspended process.
+    if (m_suspendedPage)
+        m_suspendedPage->tearDownDrawingAreaInWebProcess();
 }
 
 void WebPageProxy::exitAcceleratedCompositingMode()
index 27cb6a2..f6ace11 100644 (file)
@@ -1423,6 +1423,7 @@ private:
     void didStartProgress();
     void didChangeProgress(double);
     void didFinishProgress();
+    void didCompletePageTransition(bool isInitialEmptyDocument);
     void setNetworkRequestsInProgress(bool);
 
     void hasInsecureContent(WebCore::HasInsecureContent&);
index 5c48092..4cfc7f6 100644 (file)
@@ -117,6 +117,8 @@ messages -> WebPageProxy {
     DidFinishProgress()
     DidStartProgress()
 
+    DidCompletePageTransition(bool isInitialEmptyDocument)
+
     SetNetworkRequestsInProgress(bool networkRequestsInProgress)
 
     # Frame lifetime messages
index e4a4447..08367ca 100644 (file)
@@ -38,6 +38,8 @@ public:
 
 private:
     // DrawingAreaProxy
+    void attachInWebProcess() override;
+
     void deviceScaleFactorDidChange() override;
     void sizeDidChange() override;
     void colorSpaceDidChange() override;
index eb6bdbf..84ce267 100644 (file)
@@ -53,6 +53,11 @@ TiledCoreAnimationDrawingAreaProxy::~TiledCoreAnimationDrawingAreaProxy()
     m_callbacks.invalidate(CallbackBase::Error::OwnerWasInvalidated);
 }
 
+void TiledCoreAnimationDrawingAreaProxy::attachInWebProcess()
+{
+    m_webPageProxy.process().send(Messages::DrawingArea::Attach(), m_webPageProxy.pageID());
+}
+
 void TiledCoreAnimationDrawingAreaProxy::deviceScaleFactorDidChange()
 {
     m_webPageProxy.process().send(Messages::DrawingArea::SetDeviceScaleFactor(m_webPageProxy.deviceScaleFactor()), m_webPageProxy.pageID());
index e090b2e..4677739 100644 (file)
@@ -151,7 +151,7 @@ public:
     void displayWasRefreshed();
 #endif
 
-    virtual void attachDrawingArea() { };
+    virtual void attach() { };
 
 protected:
     DrawingArea(DrawingAreaType, WebPage&);
index dbfb294..e88cb98 100644 (file)
@@ -45,7 +45,11 @@ messages -> DrawingArea {
     DestroyNativeSurfaceHandleForCompositing() -> (bool handled)
 #endif
 
-#if PLATFORM(MAC) && ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
+#if PLATFORM(MAC)
+    Attach()
+
+#if ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     DisplayWasRefreshed()
 #endif
+#endif
 }
index 14769b8..2cf02d6 100644 (file)
@@ -657,7 +657,6 @@ void WebPage::reinitializeWebPage(WebPageCreationParameters&& parameters)
         m_drawingArea->updatePreferences(parameters.store);
         m_drawingArea->setPaintingEnabled(true);
     }
-    m_drawingArea->attachDrawingArea();
 
     if (m_activityState != parameters.activityState)
         setActivityState(parameters.activityState, ActivityStateChangeAsynchronous, Vector<CallbackID>());
@@ -2215,7 +2214,7 @@ void WebPage::setLayerTreeStateIsFrozen(bool frozen)
     if (!drawingArea)
         return;
 
-    drawingArea->setLayerTreeStateIsFrozen(frozen);
+    drawingArea->setLayerTreeStateIsFrozen(frozen || m_isSuspended);
 }
 
 void WebPage::callVolatilityCompletionHandlers(bool succeeded)
@@ -2846,7 +2845,7 @@ void WebPage::continueWillSubmitForm(uint64_t frameID, uint64_t listenerID)
 
 void WebPage::didStartPageTransition()
 {
-    m_drawingArea->setLayerTreeStateIsFrozen(true);
+    setLayerTreeStateIsFrozen(true);
 
 #if PLATFORM(MAC)
     bool hasPreviouslyFocusedDueToUserInteraction = m_hasEverFocusedElementDueToUserInteractionSincePageTransition;
@@ -2870,8 +2869,11 @@ void WebPage::didStartPageTransition()
 
 void WebPage::didCompletePageTransition()
 {
-    if (m_drawingArea)
-        m_drawingArea->setLayerTreeStateIsFrozen(false);
+    // FIXME: Layer tree freezing should be managed entirely in the UI process side.
+    setLayerTreeStateIsFrozen(false);
+
+    bool isInitialEmptyDocument = !m_mainFrame;
+    send(Messages::WebPageProxy::DidCompletePageTransition(isInitialEmptyDocument));
 }
 
 void WebPage::show()
@@ -6000,8 +6002,20 @@ void WebPage::setIsSuspended(bool suspended)
         return;
 
     m_isSuspended = suspended;
-    if (m_isSuspended)
-        m_drawingArea = nullptr;
+
+#if PLATFORM(MAC)
+    // Drawing area is cleared by a message from the UI process.
+    setLayerTreeStateIsFrozen(true);
+#else
+    tearDownDrawingAreaForSuspend();
+#endif
+}
+
+void WebPage::tearDownDrawingAreaForSuspend()
+{
+    if (!m_isSuspended)
+        return;
+    m_drawingArea = nullptr;
 }
 
 void WebPage::frameBecameRemote(uint64_t frameID, GlobalFrameIdentifier&& remoteFrameIdentifier, GlobalWindowIdentifier&& remoteWindowIdentifier)
index 7af9993..2d2e8f5 100644 (file)
@@ -1414,6 +1414,7 @@ private:
     void urlSchemeTaskDidComplete(uint64_t handlerIdentifier, uint64_t taskIdentifier, const WebCore::ResourceError&);
 
     void setIsSuspended(bool);
+    void tearDownDrawingAreaForSuspend();
 
     RefPtr<WebImage> snapshotAtSize(const WebCore::IntRect&, const WebCore::IntSize& bitmapSize, SnapshotOptions);
     RefPtr<WebImage> snapshotNode(WebCore::Node&, SnapshotOptions, unsigned maximumPixelCount = std::numeric_limits<unsigned>::max());
index 6b278f4..43d4dbd 100644 (file)
@@ -506,6 +506,7 @@ messages -> WebPage LegacyReceiver {
     URLSchemeTaskDidComplete(uint64_t handlerIdentifier, uint64_t taskIdentifier, WebCore::ResourceError error)
 
     SetIsSuspended(bool suspended)
+    TearDownDrawingAreaForSuspend();
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
     StorageAccessResponse(bool wasGranted, uint64_t contextId)
index 0329b45..a7ec567 100644 (file)
@@ -103,7 +103,7 @@ private:
     void addTransactionCallbackID(CallbackID) override;
     void setShouldScaleViewToFitDocument(bool) override;
 
-    void attachDrawingArea() override;
+    void attach() override;
 
     void adjustTransientZoom(double scale, WebCore::FloatPoint origin) override;
     void commitTransientZoom(double scale, WebCore::FloatPoint origin) override;
index b89395a..341796f 100644 (file)
@@ -91,8 +91,6 @@ TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea(WebPage& webPage, c
 
     updateLayerHostingContext();
     setColorSpace(parameters.colorSpace);
-
-    attachDrawingArea();
 }
 
 TiledCoreAnimationDrawingArea::~TiledCoreAnimationDrawingArea()
@@ -101,7 +99,7 @@ TiledCoreAnimationDrawingArea::~TiledCoreAnimationDrawingArea()
 }
 
 
-void TiledCoreAnimationDrawingArea::attachDrawingArea()
+void TiledCoreAnimationDrawingArea::attach()
 {
     LayerTreeContext layerTreeContext;
     layerTreeContext.contextID = m_layerHostingContext->contextID();