PageOverlayController's layers should be created lazily
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Feb 2019 21:16:22 +0000 (21:16 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Feb 2019 21:16:22 +0000 (21:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194199
Source/WebCore:

Reviewed by Tim Horton.

Expose PageOverlayController::hasDocumentOverlays() and hasViewOverlays()
and use them to only parent the overlay-hosting layers when necessary.

For document overlays, RenderLayerCompositor::appendDocumentOverlayLayers() can
simply do nothing if there are none. Updates are triggered via Page::installedPageOverlaysChanged(),
which calls FrameView::setNeedsCompositingConfigurationUpdate() to trigger the root layer
compositing updates that parents the layerWithDocumentOverlays().

View overlays are added to the layer tree via the DrawingArea. When we go between having
none and some view overlays, Page::installedPageOverlaysChanged() calls attachViewOverlayGraphicsLayer()
on the ChromeClient, and the DrawingArea responds by calling updateRootLayers() and scheduling a
compositing flush (this has to be done manually because view overlay layers are outside the
subtree managed by RenderLayerCompositor).

Now that GraphicsLayers are ref-counted, we can let the DrawingArea simply retain its m_viewOverlayRootLayer;
there is no need for RenderLayerCompositor::attachRootLayer()/detachRootLayer() to do anything with view
overlay layers. This implies that a page can navigate (new FrameView) and view overlays will persist, without
having to be manually removed and re-added. We can also remove the Frame argument to attachViewOverlayGraphicsLayer().

* loader/EmptyClients.h:
* page/ChromeClient.h:
* page/FrameView.cpp:
(WebCore::FrameView::setNeedsCompositingConfigurationUpdate): These functions need to schedule a compositing flush
because there may be nothing else that does.
(WebCore::FrameView::setNeedsCompositingGeometryUpdate):
* page/Page.cpp:
(WebCore::Page::installedPageOverlaysChanged):
* page/Page.h:
* page/PageOverlayController.cpp:
(WebCore::PageOverlayController::hasDocumentOverlays const):
(WebCore::PageOverlayController::hasViewOverlays const):
(WebCore::PageOverlayController::attachViewOverlayLayers): PageOverlayController has the Page so it
might as well be the one to call through the ChromeClient.
(WebCore::PageOverlayController::detachViewOverlayLayers):
(WebCore::PageOverlayController::installPageOverlay):
(WebCore::PageOverlayController::uninstallPageOverlay):
* page/PageOverlayController.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateCompositingLayers): isFullUpdate is always true; remove it.
(WebCore::RenderLayerCompositor::appendDocumentOverlayLayers):
(WebCore::RenderLayerCompositor::attachRootLayer):
(WebCore::RenderLayerCompositor::detachRootLayer):

Source/WebKit:

rdar://problem/46571593

Reviewed by Tim Horton.

Expose PageOverlayController::hasDocumentOverlays() and hasViewOverlays()
and use them to only parent the overlay-hosting layers when necessary.

For document overlays, RenderLayerCompositor::appendDocumentOverlayLayers() can
simply do nothing if there are none. Updates are triggered via Page::installedPageOverlaysChanged(),
which calls FrameView::setNeedsCompositingConfigurationUpdate() to trigger the root layer
compositing updates that parents the layerWithDocumentOverlays().

View overlays are added to the layer tree via the DrawingArea. When we go between having
none and some view overlays, Page::installedPageOverlaysChanged() calls attachViewOverlayGraphicsLayer()
on the ChromeClient, and the DrawingArea responds by calling updateRootLayers() and scheduling a
compositing flush (this has to be done manually because view overlay layers are outside the
subtree managed by RenderLayerCompositor).

Now that GraphicsLayers are ref-counted, we can let the DrawingArea simply retain its m_viewOverlayRootLayer;
there is no need for RenderLayerCompositor::attachRootLayer()/detachRootLayer() to do anything with view
overlay layers. This implies that a page can navigate (new FrameView) and view overlays will persist, without
having to be manually removed and re-added. We can also remove the Frame argument to attachViewOverlayGraphicsLayer().

* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::attachViewOverlayGraphicsLayer):
* WebProcess/WebCoreSupport/WebChromeClient.h:
* WebProcess/WebPage/AcceleratedDrawingArea.cpp:
(WebKit::AcceleratedDrawingArea::attachViewOverlayGraphicsLayer):
* WebProcess/WebPage/AcceleratedDrawingArea.h:
* WebProcess/WebPage/DrawingArea.h:
(WebKit::DrawingArea::attachViewOverlayGraphicsLayer):
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::attachViewOverlayGraphicsLayer):
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::attachViewOverlayGraphicsLayer):
(WebKit::TiledCoreAnimationDrawingArea::mainFrameContentSizeChanged):

Source/WebKitLegacy/mac:

rdar://problem/46571593

Reviewed by Tim Horton.

* WebCoreSupport/WebChromeClient.h:
* WebCoreSupport/WebChromeClient.mm:
(WebChromeClient::attachViewOverlayGraphicsLayer):

Source/WebKitLegacy/win:

rdar://problem/46571593

Reviewed by Tim Horton.

Expose PageOverlayController::hasDocumentOverlays() and hasViewOverlays()
and use them to only parent the overlay-hosting layers when necessary.

For document overlays, RenderLayerCompositor::appendDocumentOverlayLayers() can
simply do nothing if there are none. Updates are triggered via Page::installedPageOverlaysChanged(),
which calls FrameView::setNeedsCompositingConfigurationUpdate() to trigger the root layer
compositing updates that parents the layerWithDocumentOverlays().

View overlays are added to the layer tree via the DrawingArea. When we go between having
none and some view overlays, Page::installedPageOverlaysChanged() calls attachViewOverlayGraphicsLayer()
on the ChromeClient, and the DrawingArea responds by calling updateRootLayers() and scheduling a
compositing flush (this has to be done manually because view overlay layers are outside the
subtree managed by RenderLayerCompositor).

Now that GraphicsLayers are ref-counted, we can let the DrawingArea simply retain its m_viewOverlayRootLayer;
there is no need for RenderLayerCompositor::attachRootLayer()/detachRootLayer() to do anything with view
overlay layers. This implies that a page can navigate (new FrameView) and view overlays will persist, without
having to be manually removed and re-added. We can also remove the Frame argument to attachViewOverlayGraphicsLayer().

* WebCoreSupport/WebChromeClient.cpp:
(WebChromeClient::attachViewOverlayGraphicsLayer):
* WebCoreSupport/WebChromeClient.h:

LayoutTests:

rdar://problem/46571593

Reviewed by Tim Horton.

* pageoverlay/overlay-remove-reinsert-view-expected.txt: We no longer unparent the overlays
on view removal, so new results.
* platform/ios-wk2/TestExpectations: Unskip some iOS tests.
* platform/ios-wk2/pageoverlay/overlay-installation-expected.txt: Added.
* platform/ios-wk2/pageoverlay/overlay-large-document-expected.txt: Added.
* platform/ios-wk2/pageoverlay/overlay-large-document-scrolled-expected.txt: Added.
* platform/ios/TestExpectations: Unskip some iOS tests.

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

33 files changed:
LayoutTests/ChangeLog
LayoutTests/pageoverlay/overlay-remove-reinsert-view-expected.txt
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/ios-wk2/pageoverlay/overlay-installation-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/pageoverlay/overlay-large-document-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/pageoverlay/overlay-large-document-scrolled-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/EmptyClients.h
Source/WebCore/page/ChromeClient.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/Page.cpp
Source/WebCore/page/Page.h
Source/WebCore/page/PageOverlayController.cpp
Source/WebCore/page/PageOverlayController.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp
Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.h
Source/WebKit/WebProcess/WebPage/DrawingArea.h
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h
Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.cpp
Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.h

index 444bccb..fe81fce 100644 (file)
@@ -1,3 +1,19 @@
+2019-02-04  Simon Fraser  <simon.fraser@apple.com>
+
+        PageOverlayController's layers should be created lazily
+        https://bugs.webkit.org/show_bug.cgi?id=194199
+        rdar://problem/46571593
+
+        Reviewed by Tim Horton.
+
+        * pageoverlay/overlay-remove-reinsert-view-expected.txt: We no longer unparent the overlays
+        on view removal, so new results.
+        * platform/ios-wk2/TestExpectations: Unskip some iOS tests.
+        * platform/ios-wk2/pageoverlay/overlay-installation-expected.txt: Added.
+        * platform/ios-wk2/pageoverlay/overlay-large-document-expected.txt: Added.
+        * platform/ios-wk2/pageoverlay/overlay-large-document-scrolled-expected.txt: Added.
+        * platform/ios/TestExpectations: Unskip some iOS tests.
+
 2019-02-04  Eric Liang  <ericliang@apple.com>
 
         Check if slider value changed after calling AX Increment or Decrement on disabled sliders.
index e46a847..0b989a0 100644 (file)
@@ -3,7 +3,6 @@ CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (512, 512, 512, 512)
 CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 512, 512, 512)
 CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (512, 0, 512, 512)
 CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 0, 512, 512)
-CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 0, 785, 585)
 Initial layers
 
 View-relative:
@@ -38,10 +37,34 @@ Document-relative:
 Layers after removal
 
 View-relative:
-(no view-relative overlay root)
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 800.00 600.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
 
 Document-relative:
-(no document-relative overlay root)
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 5008.00 5016.00)
+      (usingTiledLayer 1)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+      (tile cache coverage 0, 0 1024 x 1024)
+      (tile size 512 x 512)
+      (top left tile 0, 0 tiles grid 2 x 2)
+      (in window 0)
+    )
+  )
+)
 Layers after re-insertion
 
 View-relative:
@@ -49,7 +72,7 @@ View-relative:
   (children 1
     (GraphicsLayer
       (anchor 0.00 0.00)
-      (bounds 785.00 585.00)
+      (bounds 800.00 600.00)
       (drawsContent 1)
       (backgroundColor #00000000)
     )
index 0849d6b..8dc1fb3 100644 (file)
@@ -91,10 +91,6 @@ js/dom/dom-static-property-for-in-iteration.html [ Failure ]
 # JavaScript tests that time out:
 js/dom/dfg-inline-resolve.html
 
-# Page overlay tests that fail:
-pageoverlay/overlay-large-document-scrolled.html [ Failure ]
-pageoverlay/overlay-large-document.html [ Failure ]
-
 # Scrollbar test that fail:
 scrollbars/corner-resizer-window-inactive.html [ ImageOnlyFailure ]
 scrollbars/scrolling-by-page-on-keyboard-spacebar.html [ Failure ]
diff --git a/LayoutTests/platform/ios-wk2/pageoverlay/overlay-installation-expected.txt b/LayoutTests/platform/ios-wk2/pageoverlay/overlay-installation-expected.txt
new file mode 100644 (file)
index 0000000..9b8e308
--- /dev/null
@@ -0,0 +1,26 @@
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 0, 800, 600)
+View-relative:
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 800.00 600.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
+
+Document-relative:
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 800.00 600.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/ios-wk2/pageoverlay/overlay-large-document-expected.txt b/LayoutTests/platform/ios-wk2/pageoverlay/overlay-large-document-expected.txt
new file mode 100644 (file)
index 0000000..66f89fd
--- /dev/null
@@ -0,0 +1,29 @@
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 0, 512, 512)
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (512, 0, 512, 512)
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 512, 512, 512)
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (512, 512, 512, 512)
+View-relative:
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 800.00 600.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
+
+Document-relative:
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 5008.00 5016.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/ios-wk2/pageoverlay/overlay-large-document-scrolled-expected.txt b/LayoutTests/platform/ios-wk2/pageoverlay/overlay-large-document-scrolled-expected.txt
new file mode 100644 (file)
index 0000000..66f89fd
--- /dev/null
@@ -0,0 +1,29 @@
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 0, 512, 512)
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (512, 0, 512, 512)
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (0, 512, 512, 512)
+CONSOLE MESSAGE: MockPageOverlayClient::drawRect dirtyRect (512, 512, 512, 512)
+View-relative:
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 800.00 600.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
+
+Document-relative:
+(GraphicsLayer
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 5008.00 5016.00)
+      (drawsContent 1)
+      (backgroundColor #00000000)
+    )
+  )
+)
+
index dd9781f..3ca2d7a 100644 (file)
@@ -1384,11 +1384,6 @@ scrollbars/scrolling-by-page-on-keyboard-spacebar.html [ Failure ]
 # JavaScript tests that fail:
 js/dom/dom-static-property-for-in-iteration.html [ Failure ]
 
-# Page overlay tests that fail:
-pageoverlay/overlay-large-document-scrolled.html [ Failure ]
-pageoverlay/overlay-large-document.html [ Failure ]
-webkit.org/b/153337 pageoverlay/overlay-installation.html [ Pass Failure ]
-
 # These tests require an OpenType MATH font on the Apple bots.
 webkit.org/b/160161 mathml/opentype/vertical.html [ Skip ]
 webkit.org/b/160161 mathml/opentype/large-operators.html [ Skip ]
index 4240d8e..8498090 100644 (file)
@@ -1,3 +1,53 @@
+2019-02-04  Simon Fraser  <simon.fraser@apple.com>
+
+        PageOverlayController's layers should be created lazily
+        https://bugs.webkit.org/show_bug.cgi?id=194199
+
+        Reviewed by Tim Horton.
+
+        Expose PageOverlayController::hasDocumentOverlays() and hasViewOverlays()
+        and use them to only parent the overlay-hosting layers when necessary.
+
+        For document overlays, RenderLayerCompositor::appendDocumentOverlayLayers() can
+        simply do nothing if there are none. Updates are triggered via Page::installedPageOverlaysChanged(),
+        which calls FrameView::setNeedsCompositingConfigurationUpdate() to trigger the root layer
+        compositing updates that parents the layerWithDocumentOverlays().
+
+        View overlays are added to the layer tree via the DrawingArea. When we go between having
+        none and some view overlays, Page::installedPageOverlaysChanged() calls attachViewOverlayGraphicsLayer()
+        on the ChromeClient, and the DrawingArea responds by calling updateRootLayers() and scheduling a
+        compositing flush (this has to be done manually because view overlay layers are outside the
+        subtree managed by RenderLayerCompositor).
+        
+        Now that GraphicsLayers are ref-counted, we can let the DrawingArea simply retain its m_viewOverlayRootLayer;
+        there is no need for RenderLayerCompositor::attachRootLayer()/detachRootLayer() to do anything with view
+        overlay layers. This implies that a page can navigate (new FrameView) and view overlays will persist, without
+        having to be manually removed and re-added. We can also remove the Frame argument to attachViewOverlayGraphicsLayer().
+
+        * loader/EmptyClients.h:
+        * page/ChromeClient.h:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setNeedsCompositingConfigurationUpdate): These functions need to schedule a compositing flush
+        because there may be nothing else that does.
+        (WebCore::FrameView::setNeedsCompositingGeometryUpdate):
+        * page/Page.cpp:
+        (WebCore::Page::installedPageOverlaysChanged):
+        * page/Page.h:
+        * page/PageOverlayController.cpp:
+        (WebCore::PageOverlayController::hasDocumentOverlays const):
+        (WebCore::PageOverlayController::hasViewOverlays const):
+        (WebCore::PageOverlayController::attachViewOverlayLayers): PageOverlayController has the Page so it
+        might as well be the one to call through the ChromeClient.
+        (WebCore::PageOverlayController::detachViewOverlayLayers):
+        (WebCore::PageOverlayController::installPageOverlay):
+        (WebCore::PageOverlayController::uninstallPageOverlay):
+        * page/PageOverlayController.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateCompositingLayers): isFullUpdate is always true; remove it.
+        (WebCore::RenderLayerCompositor::appendDocumentOverlayLayers):
+        (WebCore::RenderLayerCompositor::attachRootLayer):
+        (WebCore::RenderLayerCompositor::detachRootLayer):
+
 2019-02-04  Eric Liang  <ericliang@apple.com>
 
         When performing Increment or Decrement on sliders, check to see if the slider is disabled.
index ae588f7..f054763 100644 (file)
@@ -151,7 +151,7 @@ class EmptyChromeClient : public ChromeClient {
     void scrollRectIntoView(const IntRect&) const final { }
 
     void attachRootGraphicsLayer(Frame&, GraphicsLayer*) final { }
-    void attachViewOverlayGraphicsLayer(Frame&, GraphicsLayer*) final { }
+    void attachViewOverlayGraphicsLayer(GraphicsLayer*) final { }
     void setNeedsOneShotDrawingSynchronization() final { }
     void scheduleCompositingLayerFlush() final { }
 
index a75d97c..ec11807 100644 (file)
@@ -308,7 +308,7 @@ public:
 
     // Pass nullptr as the GraphicsLayer to detatch the root layer.
     virtual void attachRootGraphicsLayer(Frame&, GraphicsLayer*) = 0;
-    virtual void attachViewOverlayGraphicsLayer(Frame&, GraphicsLayer*) = 0;
+    virtual void attachViewOverlayGraphicsLayer(GraphicsLayer*) = 0;
     // Sets a flag to specify that the next time content is drawn to the window,
     // the changes appear on the screen in synchrony with updates to GraphicsLayers.
     virtual void setNeedsOneShotDrawingSynchronization() = 0;
index 97016cc..5baa0f2 100644 (file)
@@ -2925,6 +2925,7 @@ void FrameView::setNeedsCompositingConfigurationUpdate()
     if (renderView->usesCompositing()) {
         if (auto* rootLayer = renderView->layer())
             rootLayer->setNeedsCompositingConfigurationUpdate();
+        renderView->compositor().scheduleCompositingLayerUpdate();
     }
 }
 
@@ -2934,6 +2935,7 @@ void FrameView::setNeedsCompositingGeometryUpdate()
     if (renderView->usesCompositing()) {
         if (auto* rootLayer = renderView->layer())
             rootLayer->setNeedsCompositingGeometryUpdate();
+        renderView->compositor().scheduleCompositingLayerUpdate();
     }
 }
 
index 893fc58..78be183 100644 (file)
@@ -2637,6 +2637,19 @@ void Page::appearanceDidChange()
     }
 }
 
+void Page::installedPageOverlaysChanged()
+{
+    if (isInWindow()) {
+        if (pageOverlayController().hasViewOverlays())
+            chrome().client().attachViewOverlayGraphicsLayer(&pageOverlayController().layerWithViewOverlays());
+        else
+            chrome().client().attachViewOverlayGraphicsLayer(nullptr);
+    }
+
+    if (auto* frameView = mainFrame().view())
+        frameView->setNeedsCompositingConfigurationUpdate();
+}
+
 void Page::setUnobscuredSafeAreaInsets(const FloatBoxExtent& insets)
 {
     if (m_unobscuredSafeAreaInsets == insets)
index f314ceb..9464a8d 100644 (file)
@@ -413,11 +413,14 @@ public:
 
     WheelEventDeltaFilter* wheelEventDeltaFilter() { return m_recentWheelEventDeltaFilter.get(); }
     PageOverlayController& pageOverlayController() { return *m_pageOverlayController; }
+    
+    void installedPageOverlaysChanged();
 
 #if PLATFORM(MAC)
 #if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
     ServicesOverlayController& servicesOverlayController() { return *m_servicesOverlayController; }
 #endif // ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
+
     ScrollLatchingState* latchingState();
     void pushNewLatchingState();
     void popLatchingState();
index b7eb949..86a6852 100644 (file)
@@ -65,6 +65,36 @@ void PageOverlayController::createRootLayersIfNeeded()
     m_viewOverlayRootLayer->setName("View overlay container");
 }
 
+bool PageOverlayController::hasDocumentOverlays() const
+{
+    for (const auto& overlay : m_pageOverlays) {
+        if (overlay->overlayType() == PageOverlay::OverlayType::Document)
+            return true;
+    }
+    return false;
+}
+
+bool PageOverlayController::hasViewOverlays() const
+{
+    for (const auto& overlay : m_pageOverlays) {
+        if (overlay->overlayType() == PageOverlay::OverlayType::View)
+            return true;
+    }
+    return false;
+}
+
+void PageOverlayController::attachViewOverlayLayers()
+{
+    if (hasViewOverlays())
+        m_page.chrome().client().attachViewOverlayGraphicsLayer(&layerWithViewOverlays());
+}
+
+void PageOverlayController::detachViewOverlayLayers()
+{
+    m_page.chrome().client().attachViewOverlayGraphicsLayer(nullptr);
+    willDetachRootLayer();
+}
+
 GraphicsLayer* PageOverlayController::documentOverlayRootLayer() const
 {
     return m_documentOverlayRootLayer.get();
@@ -144,7 +174,7 @@ void PageOverlayController::installPageOverlay(PageOverlay& overlay, PageOverlay
     m_pageOverlays.append(&overlay);
 
     auto layer = GraphicsLayer::create(m_page.chrome().client().graphicsLayerFactory(), *this);
-    layer->setAnchorPoint(FloatPoint3D());
+    layer->setAnchorPoint({ });
     layer->setBackgroundColor(overlay.backgroundColor());
     layer->setName("Overlay content");
 
@@ -173,6 +203,8 @@ void PageOverlayController::installPageOverlay(PageOverlay& overlay, PageOverlay
 
     if (fadeMode == PageOverlay::FadeMode::Fade)
         overlay.startFadeInAnimation();
+
+    m_page.installedPageOverlaysChanged();
 }
 
 void PageOverlayController::uninstallPageOverlay(PageOverlay& overlay, PageOverlay::FadeMode fadeMode)
@@ -191,6 +223,7 @@ void PageOverlayController::uninstallPageOverlay(PageOverlay& overlay, PageOverl
     ASSERT_UNUSED(removed, removed);
 
     updateForceSynchronousScrollLayerPositionUpdates();
+    m_page.installedPageOverlaysChanged();
 }
 
 void PageOverlayController::updateForceSynchronousScrollLayerPositionUpdates()
index 82f7368..e61482b 100644 (file)
@@ -39,16 +39,20 @@ class PlatformMouseEvent;
 
 class PageOverlayController final : public GraphicsLayerClient {
     WTF_MAKE_FAST_ALLOCATED;
+    friend class MockPageOverlayClient;
 public:
     PageOverlayController(Page&);
     virtual ~PageOverlayController();
 
+    bool hasDocumentOverlays() const;
+    bool hasViewOverlays() const;
+
+    void attachViewOverlayLayers();
+    void detachViewOverlayLayers();
+
     GraphicsLayer& layerWithDocumentOverlays();
     GraphicsLayer& layerWithViewOverlays();
 
-    WEBCORE_EXPORT GraphicsLayer* documentOverlayRootLayer() const;
-    WEBCORE_EXPORT GraphicsLayer* viewOverlayRootLayer() const;
-
     const Vector<RefPtr<PageOverlay>>& pageOverlays() const { return m_pageOverlays; }
 
     WEBCORE_EXPORT void installPageOverlay(PageOverlay&, PageOverlay::FadeMode);
@@ -59,8 +63,6 @@ public:
     void clearPageOverlay(PageOverlay&);
     GraphicsLayer& layerForOverlay(PageOverlay&) const;
 
-    void willDetachRootLayer();
-
     void didChangeViewSize();
     void didChangeDocumentSize();
     void didChangeSettings();
@@ -82,6 +84,11 @@ public:
 private:
     void createRootLayersIfNeeded();
 
+    WEBCORE_EXPORT GraphicsLayer* documentOverlayRootLayer() const;
+    WEBCORE_EXPORT GraphicsLayer* viewOverlayRootLayer() const;
+
+    void willDetachRootLayer();
+
     void updateSettingsForLayer(GraphicsLayer&);
     void updateForceSynchronousScrollLayerPositionUpdates();
 
index 82f9e58..19dab8a 100644 (file)
@@ -673,8 +673,10 @@ bool RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
 
     // Avoid updating the layers with old values. Compositing layers will be updated after the layout is finished.
     // This happens when m_updateCompositingLayersTimer fires before layout is updated.
-    if (m_renderView.needsLayout())
+    if (m_renderView.needsLayout()) {
+        LOG_WITH_STREAM(Compositing, stream << "RenderLayerCompositor " << this << " updateCompositingLayers " << updateType << " - m_renderView.needsLayout, bailing ");
         return false;
+    }
 
     if (!m_compositing && (m_forceCompositingMode || (isMainFrameCompositor() && page().pageOverlayController().overlayCount())))
         enableCompositingMode(true);
@@ -707,7 +709,6 @@ bool RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
         return true;
     }
 
-    bool isFullUpdate = true;
     ++m_compositingUpdateCount;
 
     AnimationUpdateBlock animationUpdateBlock(&m_renderView.frameView().frame().animation());
@@ -757,15 +758,13 @@ bool RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
         updateBackingAndHierarchy(*updateRoot, childList, scrollingTreeState);
 
         // Host the document layer in the RenderView's root layer.
-        if (isFullUpdate) {
-            appendDocumentOverlayLayers(childList);
-            // Even when childList is empty, don't drop out of compositing mode if there are
-            // composited layers that we didn't hit in our traversal (e.g. because of visibility:hidden).
-            if (childList.isEmpty() && !needsCompositingForContentOrOverlays())
-                destroyRootLayer();
-            else if (m_rootContentsLayer)
-                m_rootContentsLayer->setChildren(WTFMove(childList));
-        }
+        appendDocumentOverlayLayers(childList);
+        // Even when childList is empty, don't drop out of compositing mode if there are
+        // composited layers that we didn't hit in our traversal (e.g. because of visibility:hidden).
+        if (childList.isEmpty() && !needsCompositingForContentOrOverlays())
+            destroyRootLayer();
+        else if (m_rootContentsLayer)
+            m_rootContentsLayer->setChildren(WTFMove(childList));
     }
 
 #if !LOG_DISABLED
@@ -1271,6 +1270,9 @@ void RenderLayerCompositor::appendDocumentOverlayLayers(Vector<Ref<GraphicsLayer
     if (!isMainFrameCompositor() || !m_compositing)
         return;
 
+    if (!page().pageOverlayController().hasDocumentOverlays())
+        return;
+
     Ref<GraphicsLayer> overlayHost = page().pageOverlayController().layerWithDocumentOverlays();
     childList.append(WTFMove(overlayHost));
 }
@@ -3550,8 +3552,6 @@ void RenderLayerCompositor::attachRootLayer(RootLayerAttachment attachment)
         case RootLayerAttachedViaChromeClient: {
             auto& frame = m_renderView.frameView().frame();
             page().chrome().client().attachRootGraphicsLayer(frame, rootGraphicsLayer());
-            if (frame.isMainFrame())
-                page().chrome().client().attachViewOverlayGraphicsLayer(frame, &page().pageOverlayController().layerWithViewOverlays());
             break;
         }
         case RootLayerAttachedViaEnclosingFrame: {
@@ -3598,10 +3598,6 @@ void RenderLayerCompositor::detachRootLayer()
     case RootLayerAttachedViaChromeClient: {
         auto& frame = m_renderView.frameView().frame();
         page().chrome().client().attachRootGraphicsLayer(frame, nullptr);
-        if (frame.isMainFrame()) {
-            page().chrome().client().attachViewOverlayGraphicsLayer(frame, nullptr);
-            page().pageOverlayController().willDetachRootLayer();
-        }
     }
     break;
     case RootLayerUnattached:
index 12ebcda..12d218a 100644 (file)
@@ -1,3 +1,46 @@
+2019-02-04  Simon Fraser  <simon.fraser@apple.com>
+
+        PageOverlayController's layers should be created lazily
+        https://bugs.webkit.org/show_bug.cgi?id=194199
+        rdar://problem/46571593
+
+        Reviewed by Tim Horton.
+        
+        Expose PageOverlayController::hasDocumentOverlays() and hasViewOverlays()
+        and use them to only parent the overlay-hosting layers when necessary.
+
+        For document overlays, RenderLayerCompositor::appendDocumentOverlayLayers() can
+        simply do nothing if there are none. Updates are triggered via Page::installedPageOverlaysChanged(),
+        which calls FrameView::setNeedsCompositingConfigurationUpdate() to trigger the root layer
+        compositing updates that parents the layerWithDocumentOverlays().
+
+        View overlays are added to the layer tree via the DrawingArea. When we go between having
+        none and some view overlays, Page::installedPageOverlaysChanged() calls attachViewOverlayGraphicsLayer()
+        on the ChromeClient, and the DrawingArea responds by calling updateRootLayers() and scheduling a
+        compositing flush (this has to be done manually because view overlay layers are outside the
+        subtree managed by RenderLayerCompositor).
+        
+        Now that GraphicsLayers are ref-counted, we can let the DrawingArea simply retain its m_viewOverlayRootLayer;
+        there is no need for RenderLayerCompositor::attachRootLayer()/detachRootLayer() to do anything with view
+        overlay layers. This implies that a page can navigate (new FrameView) and view overlays will persist, without
+        having to be manually removed and re-added. We can also remove the Frame argument to attachViewOverlayGraphicsLayer().
+
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::attachViewOverlayGraphicsLayer):
+        * WebProcess/WebCoreSupport/WebChromeClient.h:
+        * WebProcess/WebPage/AcceleratedDrawingArea.cpp:
+        (WebKit::AcceleratedDrawingArea::attachViewOverlayGraphicsLayer):
+        * WebProcess/WebPage/AcceleratedDrawingArea.h:
+        * WebProcess/WebPage/DrawingArea.h:
+        (WebKit::DrawingArea::attachViewOverlayGraphicsLayer):
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
+        (WebKit::RemoteLayerTreeDrawingArea::attachViewOverlayGraphicsLayer):
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::attachViewOverlayGraphicsLayer):
+        (WebKit::TiledCoreAnimationDrawingArea::mainFrameContentSizeChanged):
+
 2019-02-04  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         Unreviewed, improve preprocessor guard
index 9a68415..cc605ff 100644 (file)
@@ -892,10 +892,10 @@ void WebChromeClient::attachRootGraphicsLayer(Frame&, GraphicsLayer* layer)
         m_page.exitAcceleratedCompositingMode();
 }
 
-void WebChromeClient::attachViewOverlayGraphicsLayer(Frame& frame, GraphicsLayer* graphicsLayer)
+void WebChromeClient::attachViewOverlayGraphicsLayer(GraphicsLayer* graphicsLayer)
 {
     if (auto drawingArea = m_page.drawingArea())
-        drawingArea->attachViewOverlayGraphicsLayer(&frame, graphicsLayer);
+        drawingArea->attachViewOverlayGraphicsLayer(graphicsLayer);
 }
 
 void WebChromeClient::setNeedsOneShotDrawingSynchronization()
index 38048be..9d07505 100644 (file)
@@ -212,7 +212,7 @@ private:
 
     WebCore::GraphicsLayerFactory* graphicsLayerFactory() const final;
     void attachRootGraphicsLayer(WebCore::Frame&, WebCore::GraphicsLayer*) final;
-    void attachViewOverlayGraphicsLayer(WebCore::Frame&, WebCore::GraphicsLayer*) final;
+    void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) final;
     void setNeedsOneShotDrawingSynchronization() final;
     void scheduleCompositingLayerFlush() final;
     bool adjustLayerFlushThrottling(WebCore::LayerFlushThrottleState::Flags) final;
index 9920465..edfe46b 100644 (file)
@@ -470,11 +470,8 @@ void AcceleratedDrawingArea::activityStateDidChange(OptionSet<ActivityState::Fla
     }
 }
 
-void AcceleratedDrawingArea::attachViewOverlayGraphicsLayer(Frame* frame, GraphicsLayer* viewOverlayRootLayer)
+void AcceleratedDrawingArea::attachViewOverlayGraphicsLayer(GraphicsLayer* viewOverlayRootLayer)
 {
-    if (!frame->isMainFrame())
-        return;
-
     if (m_layerTreeHost)
         m_layerTreeHost->setViewOverlayRootLayer(viewOverlayRootLayer);
     else if (m_previousLayerTreeHost)
index 70abe43..0927d44 100644 (file)
@@ -70,7 +70,7 @@ protected:
 #endif
 
     void activityStateDidChange(OptionSet<WebCore::ActivityState::Flag>, ActivityStateChangeID, const Vector<CallbackID>& /* callbackIDs */) override;
-    void attachViewOverlayGraphicsLayer(WebCore::Frame*, WebCore::GraphicsLayer*) override;
+    void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) override;
 
     void layerHostDidFlushLayers() override;
 
index c69033e..6d8459d 100644 (file)
@@ -128,7 +128,7 @@ public:
 
     virtual bool adjustLayerFlushThrottling(WebCore::LayerFlushThrottleState::Flags) { return false; }
 
-    virtual void attachViewOverlayGraphicsLayer(WebCore::Frame*, WebCore::GraphicsLayer*) { }
+    virtual void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) { }
 
     virtual void setShouldScaleViewToFitDocument(bool) { }
 
index ccfaab8..711e258 100644 (file)
@@ -66,7 +66,7 @@ private:
     void scheduleInitialDeferredPaint() override;
     void scheduleCompositingLayerFlush() override;
     void scheduleCompositingLayerFlushImmediately() override;
-    void attachViewOverlayGraphicsLayer(WebCore::Frame*, WebCore::GraphicsLayer*) override;
+    void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) override;
 
     void addTransactionCallbackID(CallbackID) override;
 
@@ -172,8 +172,8 @@ private:
 
     OptionSet<WebCore::LayoutMilestone> m_pendingNewlyReachedLayoutMilestones;
 
-    WebCore::GraphicsLayer* m_contentLayer { nullptr };
-    WebCore::GraphicsLayer* m_viewOverlayRootLayer { nullptr };
+    RefPtr<WebCore::GraphicsLayer> m_contentLayer;
+    RefPtr<WebCore::GraphicsLayer> m_viewOverlayRootLayer;
 };
 
 } // namespace WebKit
index 1086c6f..ce2d5cb 100644 (file)
@@ -125,11 +125,8 @@ void RemoteLayerTreeDrawingArea::updateRootLayers()
     m_rootLayer->setChildren(WTFMove(children));
 }
 
-void RemoteLayerTreeDrawingArea::attachViewOverlayGraphicsLayer(Frame* frame, GraphicsLayer* viewOverlayRootLayer)
+void RemoteLayerTreeDrawingArea::attachViewOverlayGraphicsLayer(GraphicsLayer* viewOverlayRootLayer)
 {
-    if (!frame->isMainFrame())
-        return;
-
     m_viewOverlayRootLayer = viewOverlayRootLayer;
     updateRootLayers();
 }
index 5acd449..2a74ee0 100644 (file)
@@ -1220,7 +1220,7 @@ void WebPage::enterAcceleratedCompositingMode(GraphicsLayer* layer)
 
 void WebPage::exitAcceleratedCompositingMode()
 {
-    m_drawingArea->setRootCompositingLayer(0);
+    m_drawingArea->setRootCompositingLayer(nullptr);
 }
 
 void WebPage::close()
index aaa512d..9411ed2 100644 (file)
@@ -84,7 +84,7 @@ private:
     void activityStateDidChange(OptionSet<WebCore::ActivityState::Flag> changed, ActivityStateChangeID, const Vector<CallbackID>&) override;
     void didUpdateActivityStateTimerFired();
 
-    void attachViewOverlayGraphicsLayer(WebCore::Frame*, WebCore::GraphicsLayer*) override;
+    void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) override;
 
     bool dispatchDidReachLayoutMilestone(OptionSet<WebCore::LayoutMilestone>) override;
 
@@ -163,7 +163,7 @@ private:
     Vector<CallbackID> m_nextActivityStateChangeCallbackIDs;
     ActivityStateChangeID m_activityStateChangeID { ActivityStateChangeAsynchronous };
 
-    WebCore::GraphicsLayer* m_viewOverlayRootLayer;
+    RefPtr<WebCore::GraphicsLayer> m_viewOverlayRootLayer;
 
     bool m_shouldScaleViewToFitDocument { false };
     bool m_isScalingViewToFitDocument { false };
index 99708c1..b591607 100644 (file)
@@ -270,18 +270,15 @@ void TiledCoreAnimationDrawingArea::updateRootLayers()
         [m_hostingLayer addSublayer:m_debugInfoLayer.get()];
 }
 
-void TiledCoreAnimationDrawingArea::attachViewOverlayGraphicsLayer(Frame* frame, GraphicsLayer* viewOverlayRootLayer)
+void TiledCoreAnimationDrawingArea::attachViewOverlayGraphicsLayer(GraphicsLayer* viewOverlayRootLayer)
 {
-    if (!frame->isMainFrame())
-        return;
-
     m_viewOverlayRootLayer = viewOverlayRootLayer;
     updateRootLayers();
+    scheduleCompositingLayerFlush();
 }
 
 void TiledCoreAnimationDrawingArea::mainFrameContentSizeChanged(const IntSize& size)
 {
-
 }
 
 void TiledCoreAnimationDrawingArea::updateIntrinsicContentSizeIfNeeded()
index 33d630c..baa3226 100644 (file)
@@ -1,3 +1,15 @@
+2019-02-04  Simon Fraser  <simon.fraser@apple.com>
+
+        PageOverlayController's layers should be created lazily
+        https://bugs.webkit.org/show_bug.cgi?id=194199
+        rdar://problem/46571593
+
+        Reviewed by Tim Horton.
+
+        * WebCoreSupport/WebChromeClient.h:
+        * WebCoreSupport/WebChromeClient.mm:
+        (WebChromeClient::attachViewOverlayGraphicsLayer):
+
 2019-02-03  Simon Fraser  <simon.fraser@apple.com>
 
         Make setNeedsLayout on the root more explicitly about triggering its side-effects
index 37a32f8..febb01b 100644 (file)
@@ -168,7 +168,7 @@ private:
     bool shouldPaintEntireContents() const final;
 
     void attachRootGraphicsLayer(WebCore::Frame&, WebCore::GraphicsLayer*) override;
-    void attachViewOverlayGraphicsLayer(WebCore::Frame&, WebCore::GraphicsLayer*) final;
+    void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) final;
     void setNeedsOneShotDrawingSynchronization() final;
     void scheduleCompositingLayerFlush() final;
 
index 3e0a5c2..cb030d2 100644 (file)
@@ -948,7 +948,7 @@ void WebChromeClient::attachRootGraphicsLayer(Frame& frame, GraphicsLayer* graph
 #endif
 }
 
-void WebChromeClient::attachViewOverlayGraphicsLayer(Frame&, GraphicsLayer*)
+void WebChromeClient::attachViewOverlayGraphicsLayer(GraphicsLayer*)
 {
     // FIXME: If we want view-relative page overlays in Legacy WebKit, this would be the place to hook them up.
 }
index ab77a87..d1f7fe4 100644 (file)
@@ -1,3 +1,34 @@
+2019-02-04  Simon Fraser  <simon.fraser@apple.com>
+
+        PageOverlayController's layers should be created lazily
+        https://bugs.webkit.org/show_bug.cgi?id=194199
+        rdar://problem/46571593
+
+        Reviewed by Tim Horton.
+
+        Expose PageOverlayController::hasDocumentOverlays() and hasViewOverlays()
+        and use them to only parent the overlay-hosting layers when necessary.
+
+        For document overlays, RenderLayerCompositor::appendDocumentOverlayLayers() can
+        simply do nothing if there are none. Updates are triggered via Page::installedPageOverlaysChanged(),
+        which calls FrameView::setNeedsCompositingConfigurationUpdate() to trigger the root layer
+        compositing updates that parents the layerWithDocumentOverlays().
+
+        View overlays are added to the layer tree via the DrawingArea. When we go between having
+        none and some view overlays, Page::installedPageOverlaysChanged() calls attachViewOverlayGraphicsLayer()
+        on the ChromeClient, and the DrawingArea responds by calling updateRootLayers() and scheduling a
+        compositing flush (this has to be done manually because view overlay layers are outside the
+        subtree managed by RenderLayerCompositor).
+        
+        Now that GraphicsLayers are ref-counted, we can let the DrawingArea simply retain its m_viewOverlayRootLayer;
+        there is no need for RenderLayerCompositor::attachRootLayer()/detachRootLayer() to do anything with view
+        overlay layers. This implies that a page can navigate (new FrameView) and view overlays will persist, without
+        having to be manually removed and re-added. We can also remove the Frame argument to attachViewOverlayGraphicsLayer().
+
+        * WebCoreSupport/WebChromeClient.cpp:
+        (WebChromeClient::attachViewOverlayGraphicsLayer):
+        * WebCoreSupport/WebChromeClient.h:
+
 2019-02-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         Validate navigation policy decisions to avoid crashes in continueLoadAfterNavigationPolicy
index cec75fa..0fcb912 100644 (file)
@@ -727,7 +727,7 @@ void WebChromeClient::attachRootGraphicsLayer(Frame&, GraphicsLayer* graphicsLay
     m_webView->setRootChildLayer(graphicsLayer);
 }
 
-void WebChromeClient::attachViewOverlayGraphicsLayer(Frame&, GraphicsLayer*)
+void WebChromeClient::attachViewOverlayGraphicsLayer(GraphicsLayer*)
 {
     // FIXME: If we want view-relative page overlays in Legacy WebKit on Windows, this would be the place to hook them up.
 }
index be1cd2c..cc41d42 100644 (file)
@@ -123,7 +123,7 @@ public:
 
     // Pass 0 as the GraphicsLayer to detatch the root layer.
     void attachRootGraphicsLayer(WebCore::Frame&, WebCore::GraphicsLayer*) final;
-    void attachViewOverlayGraphicsLayer(WebCore::Frame&, WebCore::GraphicsLayer*) final;
+    void attachViewOverlayGraphicsLayer(WebCore::GraphicsLayer*) final;
     // Sets a flag to specify that the next time content is drawn to the window,
     // the changes appear on the screen in synchrony with updates to GraphicsLayers.
     void setNeedsOneShotDrawingSynchronization() final { }