Changeset 240663 in webkit

Timestamp:

Jan 29, 2019 10:18:52 AM (5 years ago)

Author:

Antti Koivisto

Message:

REGRESSION (PSON): Flash on link navigation on Mac
​https://bugs.webkit.org/show_bug.cgi?id=193961
<rdar://problem/47482507>

Reviewed by Chris Dumez.

The target page sends EnterAcceleratedCompositingMode message too early, before we have a valid layer tree.

• WebProcess/WebPage/DrawingArea.h:

(WebKit::DrawingArea::attach): Deleted.

Not needed anymore.

• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::reinitializeWebPage):
(WebKit::WebPage::didCompletePageTransition):
(WebKit::m_shouldAttachDrawingAreaOnPageTransition): Deleted.

Move message sending logic fully to TiledCoreAnimationDrawingArea.
Unfreezing the layer tree is sufficient to trigger the message.

• WebProcess/WebPage/WebPage.h:
• WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
• WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:

(WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea):

There is no need to treat process swap case differently.

(WebKit::TiledCoreAnimationDrawingArea::sendEnterAcceleratedCompositingModeIfNeeded):

Send this after the first successful layer flush with the root layer set.

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

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/WebPage/DrawingArea.h (modified) (1 diff)
• WebProcess/WebPage/WebPage.cpp (modified) (3 diffs)
• WebProcess/WebPage/WebPage.h (modified) (1 diff)
• WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h (modified) (3 diffs)
• WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-01-29  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (PSON): Flash on link navigation on Mac
+        https://bugs.webkit.org/show_bug.cgi?id=193961
+        <rdar://problem/47482507>
+
+        Reviewed by Chris Dumez.
+
+        The target page sends EnterAcceleratedCompositingMode message too early, before we have a valid layer tree.
+
+        * WebProcess/WebPage/DrawingArea.h:
+        (WebKit::DrawingArea::attach): Deleted.
+
+        Not needed anymore.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::reinitializeWebPage):
+        (WebKit::WebPage::didCompletePageTransition):
+        (WebKit::m_shouldAttachDrawingAreaOnPageTransition): Deleted.
+
+        Move message sending logic fully to TiledCoreAnimationDrawingArea.
+        Unfreezing the layer tree is sufficient to trigger the message.
+
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea):
+
+        There is no need to treat process swap case differently.
+
+        (WebKit::TiledCoreAnimationDrawingArea::sendEnterAcceleratedCompositingModeIfNeeded):
+
+        Send this after the first successful layer flush with the root layer set.
+
+        (WebKit::TiledCoreAnimationDrawingArea::flushLayers):
+        (WebKit::TiledCoreAnimationDrawingArea::attach): Deleted.
+
 2019-01-29  Timothy Hatcher  <timothy@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/DrawingArea.h

@@ -150 +150 @@
 #endif
 
-    virtual void attach() { };
-
 protected:
     DrawingArea(DrawingAreaType, WebPage&);

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

@@ -402 +402 @@
     , m_overrideContentSecurityPolicy { parameters.overrideContentSecurityPolicy }
     , m_cpuLimit(parameters.cpuLimit)
-#if PLATFORM(MAC)
-    , m_shouldAttachDrawingAreaOnPageTransition(parameters.isProcessSwap)
-#endif
 {
     ASSERT(m_pageID);
@@ -701 +698 @@
         m_drawingArea->updatePreferences(parameters.store);
         m_drawingArea->setPaintingEnabled(true);
-#if PLATFORM(MAC)
-        m_shouldAttachDrawingAreaOnPageTransition = parameters.isProcessSwap;
-#endif
         unfreezeLayerTree(LayerTreeFreezeReason::PageSuspended);
     }
@@ -3100 +3094 @@
     if (!isInitialEmptyDocument)
         unfreezeLayerTree(LayerTreeFreezeReason::ProcessSwap);
-
-#if PLATFORM(MAC)
-    if (m_shouldAttachDrawingAreaOnPageTransition && !isInitialEmptyDocument) {
-        m_shouldAttachDrawingAreaOnPageTransition = false;
-        // Unfreezing the layer tree above schedules a layer flush so we delay attaching the drawing area
-        // after the next event loop iteration.
-        RunLoop::main().dispatch([this, weakThis = makeWeakPtr(*this)] {
-            if (weakThis && m_drawingArea)
-                m_drawingArea->attach();
-        });
-    }
-#endif
 }
 

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -1806 +1806 @@
     bool m_isSuspended { false };
     bool m_needsFontAttributes { false };
-#if PLATFORM(MAC)
-    bool m_shouldAttachDrawingAreaOnPageTransition { false };
-#endif
 };
 

trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h

@@ -50 +50 @@
 class LayerHostingContext;
 
-class TiledCoreAnimationDrawingArea : public DrawingArea {
+class TiledCoreAnimationDrawingArea : public DrawingArea, public CanMakeWeakPtr<TiledCoreAnimationDrawingArea> {
 public:
     TiledCoreAnimationDrawingArea(WebPage&, const WebPageCreationParameters&);
@@ -103 +103 @@
     void setShouldScaleViewToFitDocument(bool) override;
 
-    void attach() override;
+    void sendEnterAcceleratedCompositingModeIfNeeded();
 
     void adjustTransientZoom(double scale, WebCore::FloatPoint origin) override;
@@ -178 +178 @@
     bool m_isThrottlingLayerFlushes { false };
     bool m_isLayerFlushThrottlingTemporarilyDisabledForInteraction { false };
+    bool m_needsSendEnterAcceleratedCompositingMode { true };
 
     WebCore::Timer m_layerFlushThrottlingTimer;

trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

@@ -97 +97 @@
     updateLayerHostingContext();
     setColorSpace(parameters.colorSpace);
-
-    if (!parameters.isProcessSwap)
-        attach();
 }
 
@@ -107 +104 @@
 }
 
-void TiledCoreAnimationDrawingArea::attach()
-{
-    LayerTreeContext layerTreeContext;
-    layerTreeContext.contextID = m_layerHostingContext->contextID();
-    m_webPage.send(Messages::DrawingAreaProxy::EnterAcceleratedCompositingMode(0, layerTreeContext));
+void TiledCoreAnimationDrawingArea::sendEnterAcceleratedCompositingModeIfNeeded()
+{
+    if (!m_rootLayer)
+        return;
+
+    if (!m_needsSendEnterAcceleratedCompositingMode)
+        return;
+    m_needsSendEnterAcceleratedCompositingMode = false;
+
+    // Let the first commit complete before sending.
+    RunLoop::main().dispatch([this, weakThis = makeWeakPtr(*this)] {
+        if (!weakThis)
+            return;
+        LayerTreeContext layerTreeContext;
+        layerTreeContext.contextID = m_layerHostingContext->contextID();
+        m_webPage.send(Messages::DrawingAreaProxy::EnterAcceleratedCompositingMode(0, layerTreeContext));
+    });
 }
 
@@ -503 +512 @@
         }
 
-        if (didFlushAllFrames)
+        if (didFlushAllFrames) {
+            sendEnterAcceleratedCompositingModeIfNeeded();
             invalidateLayerFlushRunLoopObserver();
+        }
 
         if (m_isThrottlingLayerFlushes)