Attachment 358024 Details for Bug 193010 – Patch

[patch] Patch

bug-193010-20181222113817.patch (text/plain), 16.84 KB, created by Simon Fraser (smfr) on 2018-12-22 11:38:18 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-12-22 11:38:18 PST

Size: 16.84 KB

patch

obsolete

>Subversion Revision: 239533
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7ac8f56edcdc1fa3c885d2008da971164792d2b4..9337b6545bd0c75ffc299b0b5f1ecaa110c3a023 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-12-22  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Rename LayerScrollCoordinationRole to ScrollCoordinationRole and make an enum class
>+        https://bugs.webkit.org/show_bug.cgi?id=193010
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Move the enum LayerScrollCoordinationRole from RenderLayer.h to RenderLayerCompositor.h,
>+        and make it an enum class.
>+
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::scrollLayerID const):
>+        * rendering/RenderLayer.h:
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::~RenderLayerBacking):
>+        (WebCore::RenderLayerBacking::detachFromScrollingCoordinator):
>+        (WebCore::operator<<):
>+        * rendering/RenderLayerBacking.h:
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::updateScrollCoordinatedStatus):
>+        (WebCore::RenderLayerCompositor::removeFromScrollCoordinatedLayers):
>+        (WebCore::scrollCoordinationRoleForNodeType):
>+        (WebCore::RenderLayerCompositor::attachScrollingNode):
>+        (WebCore::RenderLayerCompositor::detachScrollCoordinatedLayer):
>+        (WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer):
>+        (WebCore::RenderLayerCompositor::willRemoveScrollingLayerWithBacking):
>+        * rendering/RenderLayerCompositor.h:
>+
> 2018-12-22  Simon Fraser  <simon.fraser@apple.com>
> 
>         Change ScrollingNodeType to an enum class
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index ff60973f96423a803ecc82398b81dd5e6faa03f1..df0e105a51e1a84197df4004371f81a631fa3b3e 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -905,7 +905,7 @@ uint64_t FrameView::scrollLayerID() const
>     if (!backing)
>         return 0;
> 
>-    return backing->scrollingNodeIDForRole(Scrolling);
>+    return backing->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
> }
> 
> ScrollableArea* FrameView::scrollableAreaForScrollLayerID(uint64_t nodeID) const
>diff --git a/Source/WebCore/rendering/RenderLayer.h b/Source/WebCore/rendering/RenderLayer.h
>index 24d87313b3aeb5a0adf2c26c775b58045587f5da..78ff638c1984aec767d01f751f5d15be07960bf4 100644
>--- a/Source/WebCore/rendering/RenderLayer.h
>+++ b/Source/WebCore/rendering/RenderLayer.h
>@@ -107,11 +107,6 @@ enum ShouldApplyRootOffsetToFragments {
>     IgnoreRootOffsetForFragments
> };
> 
>-enum LayerScrollCoordinationRole {
>-    ViewportConstrained = 1 << 0,
>-    Scrolling           = 1 << 1
>-};
>-
> enum class RequestState {
>     Unknown,
>     DontCare,
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index a177659fe368ccfb6f930bfc77aca143679242f0..be8bba0a7c86eb19e424d75b1b04051e1ae9158c 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -244,7 +244,7 @@ RenderLayerBacking::~RenderLayerBacking()
>     updateBackgroundLayer(false);
>     updateMaskingLayer(false, false);
>     updateScrollingLayers(false);
>-    detachFromScrollingCoordinator({ Scrolling, ViewportConstrained });
>+    detachFromScrollingCoordinator({ ScrollCoordinationRole::Scrolling, ScrollCoordinationRole::ViewportConstrained });
>     destroyGraphicsLayers();
> }
> 
>@@ -1772,7 +1772,7 @@ bool RenderLayerBacking::updateScrollingLayers(bool needsScrollingLayers)
>     return true;
> }
> 
>-void RenderLayerBacking::detachFromScrollingCoordinator(OptionSet<LayerScrollCoordinationRole> roles)
>+void RenderLayerBacking::detachFromScrollingCoordinator(OptionSet<ScrollCoordinationRole> roles)
> {
>     if (!m_scrollingNodeID && !m_viewportConstrainedNodeID)
>         return;
>@@ -1781,13 +1781,13 @@ void RenderLayerBacking::detachFromScrollingCoordinator(OptionSet<LayerScrollCoo
>     if (!scrollingCoordinator)
>         return;
> 
>-    if ((roles & Scrolling) && m_scrollingNodeID) {
>+    if ((roles.contains(ScrollCoordinationRole::Scrolling)) && m_scrollingNodeID) {
>         LOG(Compositing, "Detaching Scrolling node %" PRIu64, m_scrollingNodeID);
>         scrollingCoordinator->detachFromStateTree(m_scrollingNodeID);
>         m_scrollingNodeID = 0;
>     }
>     
>-    if ((roles & ViewportConstrained) && m_viewportConstrainedNodeID) {
>+    if ((roles.contains(ScrollCoordinationRole::ViewportConstrained)) && m_viewportConstrainedNodeID) {
>         LOG(Compositing, "Detaching ViewportConstrained node %" PRIu64, m_viewportConstrainedNodeID);
>         scrollingCoordinator->detachFromStateTree(m_viewportConstrainedNodeID);
>         m_viewportConstrainedNodeID = 0;
>@@ -3082,9 +3082,9 @@ TextStream& operator<<(TextStream& ts, const RenderLayerBacking& backing)
>         ts << " paintsIntoCompositedAncestor";
> 
>     ts << " primary layer ID " << backing.graphicsLayer()->primaryLayerID();
>-    if (auto nodeID = backing.scrollingNodeIDForRole(ViewportConstrained))
>+    if (auto nodeID = backing.scrollingNodeIDForRole(ScrollCoordinationRole::ViewportConstrained))
>         ts << " viewport constrained scrolling node " << nodeID;
>-    if (auto nodeID = backing.scrollingNodeIDForRole(Scrolling))
>+    if (auto nodeID = backing.scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling))
>         ts << " scrolling node " << nodeID;
>     return ts;
> }
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.h b/Source/WebCore/rendering/RenderLayerBacking.h
>index dd235040c9dd9c26684476279856e76a9a61b76a..8d5f43c4815d9d97ada4ed7c719b49297af86bcc 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.h
>+++ b/Source/WebCore/rendering/RenderLayerBacking.h
>@@ -30,6 +30,7 @@
> #include "GraphicsLayer.h"
> #include "GraphicsLayerClient.h"
> #include "RenderLayer.h"
>+#include "RenderLayerCompositor.h"
> #include "ScrollingCoordinator.h"
> 
> namespace WebCore {
>@@ -104,26 +105,26 @@ public:
>     GraphicsLayer* scrollingLayer() const { return m_scrollingLayer.get(); }
>     GraphicsLayer* scrollingContentsLayer() const { return m_scrollingContentsLayer.get(); }
> 
>-    void detachFromScrollingCoordinator(OptionSet<LayerScrollCoordinationRole>);
>+    void detachFromScrollingCoordinator(OptionSet<ScrollCoordinationRole>);
>     
>-    ScrollingNodeID scrollingNodeIDForRole(LayerScrollCoordinationRole role) const
>+    ScrollingNodeID scrollingNodeIDForRole(ScrollCoordinationRole role) const
>     {
>         switch (role) {
>-        case Scrolling:
>+        case ScrollCoordinationRole::Scrolling:
>             return m_scrollingNodeID;
>-        case ViewportConstrained:
>+        case ScrollCoordinationRole::ViewportConstrained:
>             return m_viewportConstrainedNodeID;
>         }
>         return 0;
>     }
>     
>-    void setScrollingNodeIDForRole(ScrollingNodeID nodeID, LayerScrollCoordinationRole role)
>+    void setScrollingNodeIDForRole(ScrollingNodeID nodeID, ScrollCoordinationRole role)
>     {
>         switch (role) {
>-        case Scrolling:
>+        case ScrollCoordinationRole::Scrolling:
>             m_scrollingNodeID = nodeID;
>             break;
>-        case ViewportConstrained:
>+        case ScrollCoordinationRole::ViewportConstrained:
>             m_viewportConstrainedNodeID = nodeID;
>             setIsScrollCoordinatedWithViewportConstrainedRole(nodeID);
>             break;
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 3cebd74edb4e53913a54105a941564ef5c9c86e0..d34f659794e10ff5afec58a7c95b7e17de8d0fd2 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -3662,15 +3662,15 @@ static bool canCoordinateScrollingForLayer(const RenderLayer& layer)
> 
> void RenderLayerCompositor::updateScrollCoordinatedStatus(RenderLayer& layer, OptionSet<ScrollingNodeChangeFlags> changes)
> {
>-    OptionSet<LayerScrollCoordinationRole> coordinationRoles;
>+    OptionSet<ScrollCoordinationRole> coordinationRoles;
>     if (isViewportConstrainedFixedOrStickyLayer(layer))
>-        coordinationRoles.add(ViewportConstrained);
>+        coordinationRoles.add(ScrollCoordinationRole::ViewportConstrained);
> 
>     if (useCoordinatedScrollingForLayer(layer))
>-        coordinationRoles.add(Scrolling);
>+        coordinationRoles.add(ScrollCoordinationRole::Scrolling);
> 
>     if (layer.isComposited())
>-        layer.backing()->setIsScrollCoordinatedWithViewportConstrainedRole(coordinationRoles.contains(ViewportConstrained));
>+        layer.backing()->setIsScrollCoordinatedWithViewportConstrainedRole(coordinationRoles.contains(ScrollCoordinationRole::ViewportConstrained));
> 
>     if (coordinationRoles && canCoordinateScrollingForLayer(layer)) {
>         if (m_scrollCoordinatedLayers.add(&layer).isNewEntry)
>@@ -3691,7 +3691,7 @@ void RenderLayerCompositor::removeFromScrollCoordinatedLayers(RenderLayer& layer
>     m_scrollCoordinatedLayers.remove(&layer);
>     m_scrollCoordinatedLayersNeedingUpdate.remove(&layer);
> 
>-    detachScrollCoordinatedLayer(layer, { Scrolling, ViewportConstrained });
>+    detachScrollCoordinatedLayer(layer, { ScrollCoordinationRole::Scrolling, ScrollCoordinationRole::ViewportConstrained });
> }
> 
> FixedPositionViewportConstraints RenderLayerCompositor::computeFixedViewportConstraints(RenderLayer& layer) const
>@@ -3810,19 +3810,19 @@ void RenderLayerCompositor::reattachSubframeScrollLayers()
>     }
> }
> 
>-static inline LayerScrollCoordinationRole scrollCoordinationRoleForNodeType(ScrollingNodeType nodeType)
>+static inline ScrollCoordinationRole scrollCoordinationRoleForNodeType(ScrollingNodeType nodeType)
> {
>     switch (nodeType) {
>     case ScrollingNodeType::MainFrame:
>     case ScrollingNodeType::Subframe:
>     case ScrollingNodeType::Overflow:
>-        return Scrolling;
>+        return ScrollCoordinationRole::Scrolling;
>     case ScrollingNodeType::Fixed:
>     case ScrollingNodeType::Sticky:
>-        return ViewportConstrained;
>+        return ScrollCoordinationRole::ViewportConstrained;
>     }
>     ASSERT_NOT_REACHED();
>-    return Scrolling;
>+    return ScrollCoordinationRole::Scrolling;
> }
> 
> ScrollingNodeID RenderLayerCompositor::attachScrollingNode(RenderLayer& layer, ScrollingNodeType nodeType, ScrollingNodeID parentNodeID)
>@@ -3834,7 +3834,7 @@ ScrollingNodeID RenderLayerCompositor::attachScrollingNode(RenderLayer& layer, S
>     if (!backing)
>         return 0;
> 
>-    LayerScrollCoordinationRole role = scrollCoordinationRoleForNodeType(nodeType);
>+    ScrollCoordinationRole role = scrollCoordinationRoleForNodeType(nodeType);
>     ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(role);
>     if (!nodeID)
>         nodeID = scrollingCoordinator->uniqueScrollingNodeID();
>@@ -3849,19 +3849,19 @@ ScrollingNodeID RenderLayerCompositor::attachScrollingNode(RenderLayer& layer, S
>     return nodeID;
> }
> 
>-void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer, OptionSet<LayerScrollCoordinationRole> roles)
>+void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer, OptionSet<ScrollCoordinationRole> roles)
> {
>     auto* backing = layer.backing();
>     if (!backing)
>         return;
> 
>-    if (roles & Scrolling) {
>-        if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(Scrolling))
>+    if (roles.contains(ScrollCoordinationRole::Scrolling)) {
>+        if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling))
>             m_scrollingNodeToLayerMap.remove(nodeID);
>     }
> 
>-    if (roles & ViewportConstrained) {
>-        if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(ViewportConstrained))
>+    if (roles.contains(ScrollCoordinationRole::ViewportConstrained)) {
>+        if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(ScrollCoordinationRole::ViewportConstrained))
>             m_scrollingNodeToLayerMap.remove(nodeID);
>     }
> 
>@@ -3888,7 +3888,7 @@ void RenderLayerCompositor::updateScrollCoordinationForThisFrame(ScrollingNodeID
>     scrollingCoordinator->updateFrameScrollingNode(nodeID, m_scrollLayer.get(), m_rootContentLayer.get(), fixedRootBackgroundLayer(), clipLayer(), scrollingGeometry);
> }
> 
>-void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, OptionSet<LayerScrollCoordinationRole> reasons, OptionSet<ScrollingNodeChangeFlags> changes)
>+void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, OptionSet<ScrollCoordinationRole> roles, OptionSet<ScrollingNodeChangeFlags> changes)
> {
>     bool isRenderViewLayer = layer.isRenderViewLayer();
> 
>@@ -3905,7 +3905,7 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Opt
>             return;
> 
>         updateScrollCoordinationForThisFrame(parentDocumentHostingNodeID);
>-        if (!(reasons & ViewportConstrained) && isRenderViewLayer)
>+        if (!(roles.contains(ScrollCoordinationRole::ViewportConstrained)) && isRenderViewLayer)
>             return;
>     }
> 
>@@ -3917,7 +3917,7 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Opt
> 
>     // Always call this even if the backing is already attached because the parent may have changed.
>     // If a node plays both roles, fixed/sticky is always the ancestor node of scrolling.
>-    if (reasons & ViewportConstrained) {
>+    if (roles.contains(ScrollCoordinationRole::ViewportConstrained)) {
>         ScrollingNodeType nodeType = ScrollingNodeType::MainFrame;
>         if (layer.renderer().isFixedPositioned())
>             nodeType = ScrollingNodeType::Fixed;
>@@ -3952,9 +3952,9 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Opt
>         
>         parentNodeID = nodeID;
>     } else
>-        detachScrollCoordinatedLayer(layer, ViewportConstrained);
>+        detachScrollCoordinatedLayer(layer, ScrollCoordinationRole::ViewportConstrained);
>         
>-    if (reasons & Scrolling) {
>+    if (roles.contains(ScrollCoordinationRole::Scrolling)) {
>         if (isRenderViewLayer)
>             updateScrollCoordinationForThisFrame(parentNodeID);
>         else {
>@@ -3988,7 +3988,7 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Opt
>             scrollingCoordinator->updateOverflowScrollingNode(nodeID, backing->scrollingLayer(), backing->scrollingContentsLayer(), scrollingGeometry);
>         }
>     } else
>-        detachScrollCoordinatedLayer(layer, Scrolling);
>+        detachScrollCoordinatedLayer(layer, ScrollCoordinationRole::Scrolling);
> }
> 
> ScrollableArea* RenderLayerCompositor::scrollableAreaForScrollLayerID(ScrollingNodeID nodeID) const
>@@ -4070,7 +4070,7 @@ void RenderLayerCompositor::unregisterAllScrollingLayers()
> void RenderLayerCompositor::willRemoveScrollingLayerWithBacking(RenderLayer& layer, RenderLayerBacking& backing)
> {
>     if (auto* scrollingCoordinator = this->scrollingCoordinator()) {
>-        backing.detachFromScrollingCoordinator(Scrolling);
>+        backing.detachFromScrollingCoordinator(ScrollCoordinationRole::Scrolling);
> 
>         // For Coordinated Graphics.
>         scrollingCoordinator->scrollableAreaScrollLayerDidChange(layer);
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.h b/Source/WebCore/rendering/RenderLayerCompositor.h
>index 70dde532e9ea63f5f6c3035b9d867938ff55377b..0e0e6d409bd013c34e8a77bcacda3e2cfef24591 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.h
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.h
>@@ -83,6 +83,11 @@ enum class CompositingReason {
>     EmbeddedView                           = 1 << 26,
> };
> 
>+enum class ScrollCoordinationRole {
>+    ViewportConstrained = 1 << 0,
>+    Scrolling           = 1 << 1
>+};
>+
> // RenderLayerCompositor manages the hierarchy of
> // composited RenderLayers. It determines which RenderLayers
> // become compositing, and creates and maintains a hierarchy of
>@@ -446,8 +451,8 @@ private:
> 
>     void updateScrollCoordinationForThisFrame(ScrollingNodeID);
>     ScrollingNodeID attachScrollingNode(RenderLayer&, ScrollingNodeType, ScrollingNodeID parentNodeID);
>-    void updateScrollCoordinatedLayer(RenderLayer&, OptionSet<LayerScrollCoordinationRole>, OptionSet<ScrollingNodeChangeFlags>);
>-    void detachScrollCoordinatedLayer(RenderLayer&, OptionSet<LayerScrollCoordinationRole>);
>+    void updateScrollCoordinatedLayer(RenderLayer&, OptionSet<ScrollCoordinationRole>, OptionSet<ScrollingNodeChangeFlags>);
>+    void detachScrollCoordinatedLayer(RenderLayer&, OptionSet<ScrollCoordinationRole>);
>     void reattachSubframeScrollLayers();
>     
>     FixedPositionViewportConstraints computeFixedViewportConstraints(RenderLayer&) const;
>@@ -466,6 +471,7 @@ private:
>     bool hasCoordinatedScrolling() const;
>     bool useCoordinatedScrollingForLayer(const RenderLayer&) const;
> 
>+    // FIXME: make the coordinated/async terminology consistent.
>     bool isAsyncScrollableStickyLayer(const RenderLayer&, const RenderLayer** enclosingAcceleratedOverflowLayer = nullptr) const;
>     bool isViewportConstrainedFixedOrStickyLayer(const RenderLayer&) const;
>

Flags:

zalan: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193010: 358024