Attachment 350013 Details for Bug 189663 – patch

[patch] patch

pson-no-flash-9.patch (text/plain), 18.28 KB, created by Antti Koivisto on 2018-09-18 04:17:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2018-09-18 04:17:13 PDT

Size: 18.28 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 236113)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -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-17  Zan Dobersek  <zdobersek@igalia.com>
> 
>         Unreviewed build fix after r236101.
>Index: Source/WebKit/UIProcess/DrawingAreaProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/DrawingAreaProxy.h	(revision 236113)
>+++ Source/WebKit/UIProcess/DrawingAreaProxy.h	(working copy)
>@@ -82,6 +82,8 @@ public:
>     void viewExposedRectChangedTimerFired();
> #endif
> 
>+    virtual void attachInWebProcess() { }
>+
>     virtual void updateDebugIndicator() { }
> 
>     virtual void waitForDidUpdateActivityState(ActivityStateChangeID) { }
>Index: Source/WebKit/UIProcess/SuspendedPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/SuspendedPageProxy.cpp	(revision 236113)
>+++ Source/WebKit/UIProcess/SuspendedPageProxy.cpp	(working copy)
>@@ -54,6 +54,7 @@ static const HashSet<IPC::StringReferenc
>         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::destroyWebPageI
>     m_page.suspendedPageClosed(*this);
> }
> 
>+void SuspendedPageProxy::tearDownDrawingAreaInWebProcess()
>+{
>+    m_process->send(Messages::WebPage::TearDownDrawingAreaForSuspend(), m_page.pageID());
>+}
>+
> void SuspendedPageProxy::didFinishLoad()
> {
>     ASSERT(m_process);
>Index: Source/WebKit/UIProcess/SuspendedPageProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/SuspendedPageProxy.h	(revision 236113)
>+++ Source/WebKit/UIProcess/SuspendedPageProxy.h	(working copy)
>@@ -49,6 +49,7 @@ public:
> 
>     void webProcessDidClose(WebProcessProxy&);
>     void destroyWebPageInWebProcess();
>+    void tearDownDrawingAreaInWebProcess();
> 
> #if !LOG_DISABLED
>     const char* loggingString() const;
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 236113)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -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(
> 
>     initializeWebPage();
> 
>-    pageClient().didRelaunchProcess();
>+    if (!navigation)
>+        pageClient().didRelaunchProcess();
>+
>     m_drawingArea->waitForBackingStoreUpdateOnNextPaint();
> }
> 
>@@ -2449,6 +2451,8 @@ void WebPageProxy::receivedNavigationPol
>         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::resetStateAfterProces
> 
>     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::
> 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: Source/WebKit/UIProcess/WebPageProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.h	(revision 236113)
>+++ Source/WebKit/UIProcess/WebPageProxy.h	(working copy)
>@@ -1423,6 +1423,7 @@ private:
>     void didStartProgress();
>     void didChangeProgress(double);
>     void didFinishProgress();
>+    void didCompletePageTransition(bool isInitialEmptyDocument);
>     void setNetworkRequestsInProgress(bool);
> 
>     void hasInsecureContent(WebCore::HasInsecureContent&);
>Index: Source/WebKit/UIProcess/WebPageProxy.messages.in
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.messages.in	(revision 236113)
>+++ Source/WebKit/UIProcess/WebPageProxy.messages.in	(working copy)
>@@ -117,6 +117,8 @@ messages -> WebPageProxy {
>     DidFinishProgress()
>     DidStartProgress()
> 
>+    DidCompletePageTransition(bool isInitialEmptyDocument)
>+
>     SetNetworkRequestsInProgress(bool networkRequestsInProgress)
> 
>     # Frame lifetime messages
>Index: Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h	(revision 236113)
>+++ Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h	(working copy)
>@@ -38,6 +38,8 @@ public:
> 
> private:
>     // DrawingAreaProxy
>+    void attachInWebProcess() override;
>+
>     void deviceScaleFactorDidChange() override;
>     void sizeDidChange() override;
>     void colorSpaceDidChange() override;
>Index: Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm
>===================================================================
>--- Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm	(revision 236113)
>+++ Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm	(working copy)
>@@ -53,6 +53,11 @@ TiledCoreAnimationDrawingAreaProxy::~Til
>     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: Source/WebKit/WebProcess/WebPage/DrawingArea.h
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/DrawingArea.h	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/DrawingArea.h	(working copy)
>@@ -151,7 +151,7 @@ public:
>     void displayWasRefreshed();
> #endif
> 
>-    virtual void attachDrawingArea() { };
>+    virtual void attach() { };
> 
> protected:
>     DrawingArea(DrawingAreaType, WebPage&);
>Index: Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in	(working copy)
>@@ -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: Source/WebKit/WebProcess/WebPage/WebPage.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.cpp	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.cpp	(working copy)
>@@ -657,7 +657,6 @@ void WebPage::reinitializeWebPage(WebPag
>         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(
>     if (!drawingArea)
>         return;
> 
>-    drawingArea->setLayerTreeStateIsFrozen(frozen);
>+    drawingArea->setLayerTreeStateIsFrozen(frozen || m_isSuspended);
> }
> 
> void WebPage::callVolatilityCompletionHandlers(bool succeeded)
>@@ -2846,7 +2845,7 @@ void WebPage::continueWillSubmitForm(uin
> 
> 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 suspen
>         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: Source/WebKit/WebProcess/WebPage/WebPage.h
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.h	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.h	(working copy)
>@@ -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: Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.messages.in	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.messages.in	(working copy)
>@@ -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: Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h	(working copy)
>@@ -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: Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm	(revision 236113)
>+++ Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm	(working copy)
>@@ -91,8 +91,6 @@ TiledCoreAnimationDrawingArea::TiledCore
> 
>     updateLayerHostingContext();
>     setColorSpace(parameters.colorSpace);
>-
>-    attachDrawingArea();
> }
> 
> TiledCoreAnimationDrawingArea::~TiledCoreAnimationDrawingArea()
>@@ -101,7 +99,7 @@ TiledCoreAnimationDrawingArea::~TiledCor
> }
> 
> 
>-void TiledCoreAnimationDrawingArea::attachDrawingArea()
>+void TiledCoreAnimationDrawingArea::attach()
> {
>     LayerTreeContext layerTreeContext;
>     layerTreeContext.contextID = m_layerHostingContext->contextID();

Flags:

commit-queue: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189663: 349886 | 350013 | 350021 | 350112 | 350116 | 350186