Attachment 360980 Details for Bug 194194 – Patch

[patch] Patch

bug-194194-20190202114432.patch (text/plain), 17.30 KB, created by Simon Fraser (smfr) on 2019-02-02 11:44:33 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-02-02 11:44:33 PST

Size: 17.30 KB

patch

obsolete

>Subversion Revision: 240897
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 18d9dfe3af9be41d5dec6ad5a7a2c91ef716b279..6a84c000a228d5a7c79a1832066ade7ae70d1915 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2019-02-02  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Rename "scrollingLayer" in RenderLayerBacking to "scrollContainerLayer" for clarity
>+        https://bugs.webkit.org/show_bug.cgi?id=194194
>+
>+        Rubber-stampted by Antti Koivisto.
>+
>+        Our desired terminology is "scrollContainerLayer" for the outer, clipping layer
>+        for scrolling, and "scrolledContentsLayer" for the contents that move when you scroll.
>+
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::updateDebugIndicators):
>+        (WebCore::RenderLayerBacking::destroyGraphicsLayers):
>+        (WebCore::RenderLayerBacking::updateGeometry):
>+        (WebCore::RenderLayerBacking::setLocationOfScrolledContents):
>+        (WebCore::RenderLayerBacking::updateAfterDescendants):
>+        (WebCore::RenderLayerBacking::updateInternalHierarchy):
>+        (WebCore::RenderLayerBacking::updateDrawsContent):
>+        (WebCore::RenderLayerBacking::updateScrollingLayers):
>+        * rendering/RenderLayerBacking.h:
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
>+        (WebCore::RenderLayerCompositor::updateScrollingNodeForScrollingRole):
>+        (WebCore::LegacyWebKitScrollingLayerCoordinator::registerAllViewportConstrainedLayers):
>+        (WebCore::LegacyWebKitScrollingLayerCoordinator::updateScrollingLayer):
>+        (WebCore::LegacyWebKitScrollingLayerCoordinator::unregisterAllScrollingLayers):
>+        (WebCore::LegacyWebKitScrollingLayerCoordinator::removeScrollingLayer):
>+
> 2019-02-01  Simon Fraser  <simon.fraser@apple.com>
> 
>         Async overflow scroll is jumpy on macOS if the main thread is busy
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index 354249e670ccbd9dff7ff337432e4cb428f55456..eb3bb3b9195a3c50e69cd6ea2326adef3815aadd 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -391,8 +391,8 @@ void RenderLayerBacking::updateDebugIndicators(bool showBorder, bool showRepaint
>     if (m_layerForScrollCorner)
>         m_layerForScrollCorner->setShowDebugBorder(showBorder);
> 
>-    if (m_scrollingLayer)
>-        m_scrollingLayer->setShowDebugBorder(showBorder);
>+    if (m_scrollContainerLayer)
>+        m_scrollContainerLayer->setShowDebugBorder(showBorder);
> 
>     if (m_scrolledContentsLayer) {
>         m_scrolledContentsLayer->setShowDebugBorder(showBorder);
>@@ -482,7 +482,7 @@ void RenderLayerBacking::destroyGraphicsLayers()
>     GraphicsLayer::unparentAndClear(m_backgroundLayer);
>     GraphicsLayer::unparentAndClear(m_childContainmentLayer);
>     GraphicsLayer::unparentAndClear(m_childClippingMaskLayer);
>-    GraphicsLayer::unparentAndClear(m_scrollingLayer);
>+    GraphicsLayer::unparentAndClear(m_scrollContainerLayer);
>     GraphicsLayer::unparentAndClear(m_scrolledContentsLayer);
>     GraphicsLayer::unparentAndClear(m_graphicsLayer);
> }
>@@ -1090,7 +1090,7 @@ void RenderLayerBacking::updateGeometry()
>         clipLayer->setSize(snappedClippingGraphicsLayer.m_snappedRect.size());
>         clipLayer->setOffsetFromRenderer(toLayoutSize(clippingBox.location() - snappedClippingGraphicsLayer.m_snapDelta));
> 
>-        if (m_childClippingMaskLayer && !m_scrollingLayer) {
>+        if (m_childClippingMaskLayer && !m_scrollContainerLayer) {
>             m_childClippingMaskLayer->setSize(clipLayer->size());
>             m_childClippingMaskLayer->setPosition(FloatPoint());
>             m_childClippingMaskLayer->setOffsetFromRenderer(clipLayer->offsetFromRenderer());
>@@ -1181,31 +1181,31 @@ void RenderLayerBacking::updateGeometry()
>         reflectionBacking->graphicsLayer()->setReplicatedLayerPosition(FloatPoint(layerBounds.location() - reflectionLayerBounds.location()));
>     }
> 
>-    if (m_scrollingLayer) {
>+    if (m_scrollContainerLayer) {
>         ASSERT(m_scrolledContentsLayer);
>         auto& renderBox = downcast<RenderBox>(renderer());
>         LayoutRect paddingBoxIncludingScrollbar = renderBox.paddingBoxRectIncludingScrollbar();
>         ScrollOffset scrollOffset = m_owningLayer.scrollOffset();
> 
>         // FIXME: need to do some pixel snapping here.
>-        m_scrollingLayer->setPosition(FloatPoint(paddingBoxIncludingScrollbar.location() - compositedBounds().location()));
>-        m_scrollingLayer->setSize(roundedIntSize(LayoutSize(renderBox.clientWidth(), renderBox.clientHeight())));
>+        m_scrollContainerLayer->setPosition(FloatPoint(paddingBoxIncludingScrollbar.location() - compositedBounds().location()));
>+        m_scrollContainerLayer->setSize(roundedIntSize(LayoutSize(renderBox.clientWidth(), renderBox.clientHeight())));
> 
>         updateScrollOffset(scrollOffset);
> #if PLATFORM(IOS_FAMILY)
>         m_scrolledContentsLayer->setPosition({ }); // FIXME: necessary?
> #endif
> 
>-        FloatSize oldScrollingLayerOffset = m_scrollingLayer->offsetFromRenderer();
>-        m_scrollingLayer->setOffsetFromRenderer(-toFloatSize(paddingBoxIncludingScrollbar.location()));
>+        FloatSize oldScrollingLayerOffset = m_scrollContainerLayer->offsetFromRenderer();
>+        m_scrollContainerLayer->setOffsetFromRenderer(-toFloatSize(paddingBoxIncludingScrollbar.location()));
> 
>         if (m_childClippingMaskLayer) {
>-            m_childClippingMaskLayer->setPosition(m_scrollingLayer->position());
>-            m_childClippingMaskLayer->setSize(m_scrollingLayer->size());
>+            m_childClippingMaskLayer->setPosition(m_scrollContainerLayer->position());
>+            m_childClippingMaskLayer->setSize(m_scrollContainerLayer->size());
>             m_childClippingMaskLayer->setOffsetFromRenderer(-toFloatSize(paddingBoxIncludingScrollbar.location()));
>         }
> 
>-        bool paddingBoxOffsetChanged = oldScrollingLayerOffset != m_scrollingLayer->offsetFromRenderer();
>+        bool paddingBoxOffsetChanged = oldScrollingLayerOffset != m_scrollContainerLayer->offsetFromRenderer();
> 
>         IntSize scrollSize(m_owningLayer.scrollWidth(), m_owningLayer.scrollHeight());
>         if (scrollSize != m_scrolledContentsLayer->size() || paddingBoxOffsetChanged)
>@@ -1238,9 +1238,9 @@ void RenderLayerBacking::setLocationOfScrolledContents(ScrollOffset scrollOffset
> {
> #if PLATFORM(IOS_FAMILY)
>     if (setOrSync == ScrollingLayerPositionAction::Sync)
>-        m_scrollingLayer->syncBoundsOrigin(scrollOffset);
>+        m_scrollContainerLayer->syncBoundsOrigin(scrollOffset);
>     else
>-        m_scrollingLayer->setBoundsOrigin(scrollOffset);
>+        m_scrollContainerLayer->setBoundsOrigin(scrollOffset);
> #else
>     if (setOrSync == ScrollingLayerPositionAction::Sync)
>         m_scrolledContentsLayer->syncPosition(-scrollOffset);
>@@ -1278,9 +1278,9 @@ void RenderLayerBacking::updateAfterDescendants()
>     updateDrawsContent(contentsInfo);
> 
>     m_graphicsLayer->setContentsVisible(m_owningLayer.hasVisibleContent() || hasVisibleNonCompositedDescendants());
>-    if (m_scrollingLayer) {
>-        m_scrollingLayer->setContentsVisible(renderer().style().visibility() == Visibility::Visible);
>-        m_scrollingLayer->setUserInteractionEnabled(renderer().style().pointerEvents() != PointerEvents::None);
>+    if (m_scrollContainerLayer) {
>+        m_scrollContainerLayer->setContentsVisible(renderer().style().visibility() == Visibility::Visible);
>+        m_scrollContainerLayer->setUserInteractionEnabled(renderer().style().pointerEvents() != PointerEvents::None);
>     }
> }
> 
>@@ -1347,9 +1347,9 @@ void RenderLayerBacking::updateInternalHierarchy()
>     if (m_childContainmentLayer)
>         m_graphicsLayer->addChild(*m_childContainmentLayer);
> 
>-    if (m_scrollingLayer) {
>+    if (m_scrollContainerLayer) {
>         auto* superlayer = m_childContainmentLayer ? m_childContainmentLayer.get() : m_graphicsLayer.get();
>-        superlayer->addChild(*m_scrollingLayer);
>+        superlayer->addChild(*m_scrollContainerLayer);
>     }
> 
>     // The clip for child layers does not include space for overflow controls, so they exist as
>@@ -1390,10 +1390,10 @@ void RenderLayerBacking::updateDrawsContent()
> 
> void RenderLayerBacking::updateDrawsContent(PaintedContentsInfo& contentsInfo)
> {
>-    if (m_scrollingLayer) {
>+    if (m_scrollContainerLayer) {
>         // We don't have to consider overflow controls, because we know that the scrollbars are drawn elsewhere.
>         // m_graphicsLayer only needs backing store if the non-scrolling parts (background, outlines, borders, shadows etc) need to paint.
>-        // m_scrollingLayer never has backing store.
>+        // m_scrollContainerLayer never has backing store.
>         // m_scrolledContentsLayer only needs backing store if the scrolled contents need to paint.
>         bool hasNonScrollingPaintedContent = m_owningLayer.hasVisibleContent() && m_owningLayer.hasVisibleBoxDecorationsOrBackground();
>         m_graphicsLayer->setDrawsContent(hasNonScrollingPaintedContent);
>@@ -1738,14 +1738,14 @@ void RenderLayerBacking::updateChildClippingStrategy(bool needsDescendantsClippi
> 
> bool RenderLayerBacking::updateScrollingLayers(bool needsScrollingLayers)
> {
>-    if (needsScrollingLayers == !!m_scrollingLayer)
>+    if (needsScrollingLayers == !!m_scrollContainerLayer)
>         return false;
> 
>-    if (!m_scrollingLayer) {
>+    if (!m_scrollContainerLayer) {
>         // Outer layer which corresponds with the scroll view.
>-        m_scrollingLayer = createGraphicsLayer("scrolling container", GraphicsLayer::Type::Scrolling);
>-        m_scrollingLayer->setDrawsContent(false);
>-        m_scrollingLayer->setMasksToBounds(true);
>+        m_scrollContainerLayer = createGraphicsLayer("scrolling container", GraphicsLayer::Type::Scrolling);
>+        m_scrollContainerLayer->setDrawsContent(false);
>+        m_scrollContainerLayer->setMasksToBounds(true);
> 
>         // Inner layer which renders the content that scrolls.
>         m_scrolledContentsLayer = createGraphicsLayer("scrolled contents");
>@@ -1756,21 +1756,21 @@ bool RenderLayerBacking::updateScrollingLayers(bool needsScrollingLayers)
>         if (!m_foregroundLayer)
>             paintPhase |= GraphicsLayerPaintForeground;
>         m_scrolledContentsLayer->setPaintingPhase(paintPhase);
>-        m_scrollingLayer->addChild(*m_scrolledContentsLayer);
>+        m_scrollContainerLayer->addChild(*m_scrolledContentsLayer);
>     } else {
>         compositor().willRemoveScrollingLayerWithBacking(m_owningLayer, *this);
> 
>-        willDestroyLayer(m_scrollingLayer.get());
>+        willDestroyLayer(m_scrollContainerLayer.get());
>         willDestroyLayer(m_scrolledContentsLayer.get());
>         
>-        GraphicsLayer::unparentAndClear(m_scrollingLayer);
>+        GraphicsLayer::unparentAndClear(m_scrollContainerLayer);
>         GraphicsLayer::unparentAndClear(m_scrolledContentsLayer);
>     }
> 
>     m_graphicsLayer->setPaintingPhase(paintingPhaseForPrimaryLayer());
>     m_graphicsLayer->setNeedsDisplay(); // Because painting phases changed.
> 
>-    if (m_scrollingLayer)
>+    if (m_scrollContainerLayer)
>         compositor().didAddScrollingLayer(m_owningLayer);
>     
>     return true;
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.h b/Source/WebCore/rendering/RenderLayerBacking.h
>index 7bb13f9f743ec5b0f3583938b1cbfc92dadb8d5f..10d1d772575e5ae39a7857af7aabbc23a6d35ee4 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.h
>+++ b/Source/WebCore/rendering/RenderLayerBacking.h
>@@ -104,9 +104,9 @@ public:
>     bool requiresBackgroundLayer() const { return m_requiresBackgroundLayer; }
>     void setRequiresBackgroundLayer(bool);
> 
>-    bool hasScrollingLayer() const { return m_scrollingLayer != nullptr; }
>-    GraphicsLayer* scrollingLayer() const { return m_scrollingLayer.get(); }
>-    GraphicsLayer* scrollingContentsLayer() const { return m_scrolledContentsLayer.get(); }
>+    bool hasScrollingLayer() const { return m_scrollContainerLayer != nullptr; }
>+    GraphicsLayer* scrollContainerLayer() const { return m_scrollContainerLayer.get(); }
>+    GraphicsLayer* scrolledContentsLayer() const { return m_scrolledContentsLayer.get(); }
> 
>     OptionSet<ScrollCoordinationRole> coordinatedScrollingRoles() const;
> 
>@@ -385,7 +385,7 @@ private:
>     RefPtr<GraphicsLayer> m_layerForVerticalScrollbar;
>     RefPtr<GraphicsLayer> m_layerForScrollCorner;
> 
>-    RefPtr<GraphicsLayer> m_scrollingLayer; // Only used if the layer is using composited scrolling.
>+    RefPtr<GraphicsLayer> m_scrollContainerLayer; // Only used if the layer is using composited scrolling.
>     RefPtr<GraphicsLayer> m_scrolledContentsLayer; // Only used if the layer is using composited scrolling.
> 
>     LayoutRect m_compositedBounds;
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 91f405d93cf2d38d80451c7a6d366c05e58065d3..f2eac310afa239dc971878423db86c2e0192ea09 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -553,7 +553,7 @@ void RenderLayerCompositor::didChangePlatformLayerForLayer(RenderLayer& layer, c
>             // FIXME: Reorganize the layers and pass the scrollContainerLayer.
>             nodeLayers = { nullptr, nullptr, m_scrollLayer.get(), fixedRootBackgroundLayer(), clipLayer(), m_rootContentsLayer.get() };
>         } else
>-            nodeLayers = { layer.backing()->graphicsLayer(), backing->scrollingLayer(), backing->scrollingContentsLayer() };
>+            nodeLayers = { layer.backing()->graphicsLayer(), backing->scrollContainerLayer(), backing->scrolledContentsLayer() };
> 
>         scrollingCoordinator->setNodeLayers(nodeID, nodeLayers);
>     }
>@@ -4027,7 +4027,7 @@ ScrollingNodeID RenderLayerCompositor::updateScrollingNodeForScrollingRole(Rende
>         }
>         
>         if (changes & ScrollingNodeChangeFlags::Layer)
>-            scrollingCoordinator->setNodeLayers(newNodeID, { layer.backing()->graphicsLayer(), layer.backing()->scrollingLayer(), layer.backing()->scrollingContentsLayer() });
>+            scrollingCoordinator->setNodeLayers(newNodeID, { layer.backing()->graphicsLayer(), layer.backing()->scrollContainerLayer(), layer.backing()->scrolledContentsLayer() });
> 
>         if (changes & ScrollingNodeChangeFlags::LayerGeometry && treeState.parentNodeID) {
>             RenderLayer* scrollingAncestorLayer = m_scrollingNodeToLayerMap.get(treeState.parentNodeID.value());
>@@ -4235,7 +4235,8 @@ void LegacyWebKitScrollingLayerCoordinator::registerAllViewportConstrainedLayers
>             const RenderLayer* enclosingTouchScrollableLayer = nullptr;
>             if (compositor.isAsyncScrollableStickyLayer(*layer, &enclosingTouchScrollableLayer) && enclosingTouchScrollableLayer) {
>                 ASSERT(enclosingTouchScrollableLayer->isComposited());
>-                stickyContainerMap.add(layer->backing()->graphicsLayer()->platformLayer(), enclosingTouchScrollableLayer->backing()->scrollingLayer()->platformLayer());
>+                // what
>+                stickyContainerMap.add(layer->backing()->graphicsLayer()->platformLayer(), enclosingTouchScrollableLayer->backing()->scrollContainerLayer()->platformLayer());
>             }
>         } else if (layer->renderer().isFixedPositioned())
>             constraints = std::make_unique<FixedPositionViewportConstraints>(compositor.computeFixedViewportConstraints(*layer));
>@@ -4273,7 +4274,7 @@ void LegacyWebKitScrollingLayerCoordinator::updateScrollingLayer(RenderLayer& la
> 
>     bool allowHorizontalScrollbar = !scrollbarHasDisplayNone(layer.horizontalScrollbar());
>     bool allowVerticalScrollbar = !scrollbarHasDisplayNone(layer.verticalScrollbar());
>-    m_chromeClient.addOrUpdateScrollingLayer(layer.renderer().element(), backing->scrollingLayer()->platformLayer(), backing->scrollingContentsLayer()->platformLayer(),
>+    m_chromeClient.addOrUpdateScrollingLayer(layer.renderer().element(), backing->scrollContainerLayer()->platformLayer(), backing->scrolledContentsLayer()->platformLayer(),
>         layer.scrollableContentsSize(), allowHorizontalScrollbar, allowVerticalScrollbar);
> }
> 
>@@ -4296,7 +4297,7 @@ void LegacyWebKitScrollingLayerCoordinator::unregisterAllScrollingLayers()
>     for (auto* layer : m_scrollingLayers) {
>         auto* backing = layer->backing();
>         ASSERT(backing);
>-        m_chromeClient.removeScrollingLayer(layer->renderer().element(), backing->scrollingLayer()->platformLayer(), backing->scrollingContentsLayer()->platformLayer());
>+        m_chromeClient.removeScrollingLayer(layer->renderer().element(), backing->scrollContainerLayer()->platformLayer(), backing->scrolledContentsLayer()->platformLayer());
>     }
> }
> 
>@@ -4310,9 +4311,9 @@ void LegacyWebKitScrollingLayerCoordinator::removeScrollingLayer(RenderLayer& la
> {
>     m_scrollingLayersNeedingUpdate.remove(&layer);
>     if (m_scrollingLayers.remove(&layer)) {
>-        auto* scrollingLayer = backing.scrollingLayer()->platformLayer();
>-        auto* contentsLayer = backing.scrollingContentsLayer()->platformLayer();
>-        m_chromeClient.removeScrollingLayer(layer.renderer().element(), scrollingLayer, contentsLayer);
>+        auto* scrollContainerLayer = backing.scrollContainerLayer()->platformLayer();
>+        auto* scrolledContentsLayer = backing.scrolledContentsLayer()->platformLayer();
>+        m_chromeClient.removeScrollingLayer(layer.renderer().element(), scrollContainerLayer, scrolledContentsLayer);
>     }
> }
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194194: 360980