[webkit-changes] [246962] trunk/Source

Sun, 30 Jun 2019 22:53:38 -0700

Title: [246962] trunk/Source
Revision
246962
Author
[email protected]
Date
2019-06-30 22:52:30 -0700 (Sun, 30 Jun 2019)

Log Message

Use separate variables for moving and stationary scrolling relationships in RemoteLayerTreeNode
https://bugs.webkit.org/show_bug.cgi?id=199348

Reviewed by Darin Adler.

Source/WebCore:

* page/scrolling/ScrollingStateStickyNode.cpp:
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::commitTreeState):
* page/scrolling/ScrollingTree.h:
(WebCore::ScrollingTree::activeOverflowScrollProxyNodes):
(WebCore::ScrollingTree::activePositionedNodes):
(WebCore::ScrollingTree::nodesWithRelatedOverflow): Deleted.

Use separate sets for overflow proxies and positioned nodes.
Use Refs to nodes instead of ids to simplify client code. This doesn't affect lifetimes, these sets are cleared
at the beginning of each commit.

* page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm:
(WebCore::ScrollingTreeOverflowScrollProxyNode::commitStateBeforeChildren):
* page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
(WebCore::ScrollingTreePositionedNode::commitStateBeforeChildren):

Source/WebKit:

A layer can have only one acting scroll parent. Not using a vector for that case makes the code clearer.

* UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h:
(WebKit::RemoteLayerTreeNode::actingScrollContainerID const):
(WebKit::RemoteLayerTreeNode::stationaryScrollContainerIDs const):

Separate fields for the acting container and stationary containers.

(WebKit::RemoteLayerTreeNode::setActingScrollContainerID):
(WebKit::RemoteLayerTreeNode::setStationaryScrollContainerIDs):
(WebKit::RemoteLayerTreeNode::relatedScrollContainerIDs const): Deleted.
(WebKit::RemoteLayerTreeNode::relatedScrollContainerPositioningBehavior const): Deleted.
* UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm:
(WebKit::RemoteLayerTreeNode::setRelatedScrollContainerBehaviorAndIDs): Deleted.
* UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
(WebKit::isScrolledBy):
(WebKit::findActingScrollParent):
* UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:
(WebKit::RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (246961 => 246962)


--- trunk/Source/WebCore/ChangeLog	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebCore/ChangeLog	2019-07-01 05:52:30 UTC (rev 246962)
@@ -1,3 +1,27 @@
+2019-06-30  Antti Koivisto  <[email protected]>
+
+        Use separate variables for moving and stationary scrolling relationships in RemoteLayerTreeNode
+        https://bugs.webkit.org/show_bug.cgi?id=199348
+
+        Reviewed by Darin Adler.
+
+        * page/scrolling/ScrollingStateStickyNode.cpp:
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::commitTreeState):
+        * page/scrolling/ScrollingTree.h:
+        (WebCore::ScrollingTree::activeOverflowScrollProxyNodes):
+        (WebCore::ScrollingTree::activePositionedNodes):
+        (WebCore::ScrollingTree::nodesWithRelatedOverflow): Deleted.
+
+        Use separate sets for overflow proxies and positioned nodes.
+        Use Refs to nodes instead of ids to simplify client code. This doesn't affect lifetimes, these sets are cleared
+        at the beginning of each commit.
+
+        * page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm:
+        (WebCore::ScrollingTreeOverflowScrollProxyNode::commitStateBeforeChildren):
+        * page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
+        (WebCore::ScrollingTreePositionedNode::commitStateBeforeChildren):
+
 2019-06-30  Andres Gonzalez  <[email protected]>
 
         Enhance support of aria-haspopup per ARIA 1.1 specification.

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp (246961 => 246962)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp	2019-07-01 05:52:30 UTC (rev 246962)
@@ -31,6 +31,7 @@
 #include "GraphicsLayer.h"
 #include "Logging.h"
 #include "ScrollingStateTree.h"
+#include "ScrollingTree.h"
 #include <wtf/text/TextStream.h>
 
 namespace WebCore {

Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (246961 => 246962)


--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp	2019-07-01 05:52:30 UTC (rev 246962)
@@ -35,7 +35,9 @@
 #include "ScrollingStateTree.h"
 #include "ScrollingTreeFrameScrollingNode.h"
 #include "ScrollingTreeNode.h"
+#include "ScrollingTreeOverflowScrollProxyNode.h"
 #include "ScrollingTreeOverflowScrollingNode.h"
+#include "ScrollingTreePositionedNode.h"
 #include "ScrollingTreeScrollingNode.h"
 #include <wtf/SetForScope.h>
 #include <wtf/text/TextStream.h>
@@ -169,7 +171,8 @@
         unvisitedNodes.add(nodeID);
 
     m_overflowRelatedNodesMap.clear();
-    m_nodesWithRelatedOverflow.clear();
+    m_activeOverflowScrollProxyNodes.clear();
+    m_activePositionedNodes.clear();
 
     // orphanNodes keeps child nodes alive while we rebuild child lists.
     OrphanScrollingNodeMap orphanNodes;

Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (246961 => 246962)


--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h	2019-07-01 05:52:30 UTC (rev 246962)
@@ -44,6 +44,8 @@
 class ScrollingStateNode;
 class ScrollingTreeFrameScrollingNode;
 class ScrollingTreeNode;
+class ScrollingTreeOverflowScrollProxyNode;
+class ScrollingTreePositionedNode;
 class ScrollingTreeScrollingNode;
 
 class ScrollingTree : public ThreadSafeRefCounted<ScrollingTree> {
@@ -154,7 +156,8 @@
     using RelatedNodesMap = HashMap<ScrollingNodeID, Vector<ScrollingNodeID>>;
     RelatedNodesMap& overflowRelatedNodes() { return m_overflowRelatedNodesMap; }
 
-    HashSet<ScrollingNodeID>& nodesWithRelatedOverflow() { return m_nodesWithRelatedOverflow; }
+    HashSet<Ref<ScrollingTreeOverflowScrollProxyNode>>& activeOverflowScrollProxyNodes() { return m_activeOverflowScrollProxyNodes; }
+    HashSet<Ref<ScrollingTreePositionedNode>>& activePositionedNodes() { return m_activePositionedNodes; }
 
     WEBCORE_EXPORT String scrollingTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal);
 
@@ -179,8 +182,10 @@
     ScrollingTreeNodeMap m_nodeMap;
 
     RelatedNodesMap m_overflowRelatedNodesMap;
-    HashSet<ScrollingNodeID> m_nodesWithRelatedOverflow;
 
+    HashSet<Ref<ScrollingTreeOverflowScrollProxyNode>> m_activeOverflowScrollProxyNodes;
+    HashSet<Ref<ScrollingTreePositionedNode>> m_activePositionedNodes;
+
     struct TreeState {
         ScrollingNodeID latchedNodeID { 0 };
         EventTrackingRegions eventTrackingRegions;

Modified: trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm (246961 => 246962)


--- trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm	2019-07-01 05:52:30 UTC (rev 246962)
@@ -62,9 +62,9 @@
         relatedNodes.ensure(m_overflowScrollingNodeID, [] {
             return Vector<ScrollingNodeID>();
         }).iterator->value.append(scrollingNodeID());
+
+        scrollingTree().activeOverflowScrollProxyNodes().add(*this);
     }
-
-    scrollingTree().nodesWithRelatedOverflow().add(scrollingNodeID());
 }
 
 FloatSize ScrollingTreeOverflowScrollProxyNode::scrollDeltaSinceLastCommit() const

Modified: trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm (246961 => 246962)


--- trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	2019-07-01 05:52:30 UTC (rev 246962)
@@ -64,7 +64,7 @@
         m_constraints = positionedStateNode.layoutConstraints();
 
     if (!m_relatedOverflowScrollingNodes.isEmpty())
-        scrollingTree().nodesWithRelatedOverflow().add(scrollingNodeID());
+        scrollingTree().activePositionedNodes().add(*this);
 }
 
 FloatSize ScrollingTreePositionedNode::scrollDeltaSinceLastCommit() const

Modified: trunk/Source/WebKit/ChangeLog (246961 => 246962)


--- trunk/Source/WebKit/ChangeLog	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebKit/ChangeLog	2019-07-01 05:52:30 UTC (rev 246962)
@@ -1,3 +1,30 @@
+2019-06-30  Antti Koivisto  <[email protected]>
+
+        Use separate variables for moving and stationary scrolling relationships in RemoteLayerTreeNode
+        https://bugs.webkit.org/show_bug.cgi?id=199348
+
+        Reviewed by Darin Adler.
+
+        A layer can have only one acting scroll parent. Not using a vector for that case makes the code clearer.
+
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h:
+        (WebKit::RemoteLayerTreeNode::actingScrollContainerID const):
+        (WebKit::RemoteLayerTreeNode::stationaryScrollContainerIDs const):
+
+        Separate fields for the acting container and stationary containers.
+
+        (WebKit::RemoteLayerTreeNode::setActingScrollContainerID):
+        (WebKit::RemoteLayerTreeNode::setStationaryScrollContainerIDs):
+        (WebKit::RemoteLayerTreeNode::relatedScrollContainerIDs const): Deleted.
+        (WebKit::RemoteLayerTreeNode::relatedScrollContainerPositioningBehavior const): Deleted.
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm:
+        (WebKit::RemoteLayerTreeNode::setRelatedScrollContainerBehaviorAndIDs): Deleted.
+        * UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
+        (WebKit::isScrolledBy):
+        (WebKit::findActingScrollParent):
+        * UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:
+        (WebKit::RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations):
+
 2019-06-30  Fujii Hironori  <[email protected]>
 
         [Win] Multiline mode of tooltip control does word-wrapping very slowly

Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h (246961 => 246962)


--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h	2019-07-01 05:52:30 UTC (rev 246962)
@@ -59,11 +59,14 @@
     const WebCore::EventRegion& eventRegion() const { return m_eventRegion; }
     void setEventRegion(const WebCore::EventRegion&);
 
-    // If empty the layer is scrolled normally by an ancestor scroller.
-    const auto& relatedScrollContainerIDs() const { return m_relatedScrollContainerIDs; }
-    WebCore::ScrollPositioningBehavior relatedScrollContainerPositioningBehavior() const { return m_relatedScrollContainerPositioningBehavior; }
-    void setRelatedScrollContainerBehaviorAndIDs(WebCore::ScrollPositioningBehavior, Vector<WebCore::GraphicsLayer::PlatformLayerID>&&);
+    // Non-ancestor scroller that controls positioning of the layer.
+    WebCore::GraphicsLayer::PlatformLayerID actingScrollContainerID() const { return m_actingScrollContainerID; }
+    // Ancestor scrollers that don't affect positioning of the layer.
+    const Vector<WebCore::GraphicsLayer::PlatformLayerID>& stationaryScrollContainerIDs() const { return m_stationaryScrollContainerIDs; }
 
+    void setActingScrollContainerID(WebCore::GraphicsLayer::PlatformLayerID value) { m_actingScrollContainerID = value; }
+    void setStationaryScrollContainerIDs(Vector<WebCore::GraphicsLayer::PlatformLayerID>&& value) { m_stationaryScrollContainerIDs = WTFMove(value); }
+
     void detachFromParent();
 
     static WebCore::GraphicsLayer::PlatformLayerID layerID(CALayer *);
@@ -83,8 +86,8 @@
 
     WebCore::EventRegion m_eventRegion;
 
-    Vector<WebCore::GraphicsLayer::PlatformLayerID> m_relatedScrollContainerIDs;
-    WebCore::ScrollPositioningBehavior m_relatedScrollContainerPositioningBehavior { WebCore::ScrollPositioningBehavior::None };
+    WebCore::GraphicsLayer::PlatformLayerID m_actingScrollContainerID { 0 };
+    Vector<WebCore::GraphicsLayer::PlatformLayerID> m_stationaryScrollContainerIDs;
 };
 
 }

Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm (246961 => 246962)


--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm	2019-07-01 05:52:30 UTC (rev 246962)
@@ -92,12 +92,6 @@
     m_eventRegion = eventRegion;
 }
 
-void RemoteLayerTreeNode::setRelatedScrollContainerBehaviorAndIDs(WebCore::ScrollPositioningBehavior behavior, Vector<WebCore::GraphicsLayer::PlatformLayerID>&& scrollContainerIDs)
-{
-    m_relatedScrollContainerPositioningBehavior = behavior;
-    m_relatedScrollContainerIDs = WTFMove(scrollContainerIDs);
-}
-
 void RemoteLayerTreeNode::initializeLayer()
 {
     [layer() setValue:[NSValue valueWithPointer:this] forKey:WKRemoteLayerTreeNodePropertyKey];

Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm (246961 => 246962)


--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm	2019-07-01 05:52:30 UTC (rev 246962)
@@ -79,15 +79,11 @@
             return true;
 
         auto* node = RemoteLayerTreeNode::forCALayer(view.layer);
-        if (node && scrollLayerID && node->relatedScrollContainerIDs().contains(scrollLayerID)) {
-            switch (node->relatedScrollContainerPositioningBehavior()) {
-            case WebCore::ScrollPositioningBehavior::Moves:
+        if (node && scrollLayerID) {
+            if (node->actingScrollContainerID() == scrollLayerID)
                 return true;
-            case WebCore::ScrollPositioningBehavior::Stationary:
+            if (node->stationaryScrollContainerIDs().contains(scrollLayerID))
                 return false;
-            case WebCore::ScrollPositioningBehavior::None:
-                ASSERT_NOT_REACHED();
-            }
         }
     }
 
@@ -133,21 +129,13 @@
             // FIXME: Ideally we would return the scroller we want in all cases but the current UIKit SPI only allows returning a non-ancestor.
             return nil;
         }
-
         if (auto* node = RemoteLayerTreeNode::forCALayer(view.layer)) {
-            switch (node->relatedScrollContainerPositioningBehavior()) {
-            case WebCore::ScrollPositioningBehavior::Moves:
-                if (!node->relatedScrollContainerIDs().isEmpty()) {
-                    if (auto* nonAncestorScrollingNode = host.nodeForID(node->relatedScrollContainerIDs()[0]))
-                        return (WKChildScrollView *)nonAncestorScrollingNode->uiView();
-                }
-                break;
-            case WebCore::ScrollPositioningBehavior::Stationary:
-                scrollersToSkip.add(node->relatedScrollContainerIDs().begin(), node->relatedScrollContainerIDs().end());
-                break;
-            case WebCore::ScrollPositioningBehavior::None:
-                break;
+            if (auto* actingParent = host.nodeForID(node->actingScrollContainerID())) {
+                if ([actingParent->uiView() isKindOfClass:[UIScrollView class]])
+                    return (UIScrollView *)actingParent->uiView();
             }
+
+            scrollersToSkip.add(node->stationaryScrollContainerIDs().begin(), node->stationaryScrollContainerIDs().end());
         }
     }
     return nil;

Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm (246961 => 246962)


--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm	2019-07-01 04:03:42 UTC (rev 246961)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm	2019-07-01 05:52:30 UTC (rev 246962)
@@ -125,8 +125,10 @@
 void RemoteScrollingCoordinatorProxy::establishLayerTreeScrollingRelations(const RemoteLayerTreeHost& remoteLayerTreeHost)
 {
     for (auto layerID : m_layersWithScrollingRelations) {
-        if (auto* layerNode = remoteLayerTreeHost.nodeForID(layerID))
-            layerNode->setRelatedScrollContainerBehaviorAndIDs({ }, { });
+        if (auto* layerNode = remoteLayerTreeHost.nodeForID(layerID)) {
+            layerNode->setActingScrollContainerID(0);
+            layerNode->setStationaryScrollContainerIDs({ });
+        }
     }
     m_layersWithScrollingRelations.clear();
 
@@ -133,47 +135,30 @@
     // Usually a scroll view scrolls its descendant layers. In some positioning cases it also controls non-descendants, or doesn't control a descendant.
     // To do overlap hit testing correctly we tell layers about such relations.
     
-    for (auto nodeID : m_scrollingTree->nodesWithRelatedOverflow()) {
-        auto* node = m_scrollingTree->nodeForID(nodeID);
-        if (!node) {
-            ASSERT_NOT_REACHED();
-            continue;
-        }
+    for (auto& positionedNode : m_scrollingTree->activePositionedNodes()) {
+        Vector<GraphicsLayer::PlatformLayerID> stationaryScrollContainerIDs;
 
-        Vector<GraphicsLayer::PlatformLayerID> scrollContainerLayerIDs;
-        RemoteLayerTreeNode* layerNode = nullptr;
-        ScrollPositioningBehavior behavior = ScrollPositioningBehavior::None;
-
-        if (is<ScrollingTreePositionedNode>(*node)) {
-            auto& positionedNode = downcast<ScrollingTreePositionedNode>(*node);
-            for (auto overflowNodeID : positionedNode.relatedOverflowScrollingNodes()) {
-                auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(overflowNodeID));
-                if (!overflowNode) {
-                    ASSERT_NOT_REACHED();
-                    continue;
-                }
-                scrollContainerLayerIDs.append(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
-            }
-            layerNode = RemoteLayerTreeNode::forCALayer(positionedNode.layer());
-            behavior = ScrollPositioningBehavior::Stationary;
-        } else if (is<ScrollingTreeOverflowScrollProxyNode>(*node)) {
-            auto& scrollProxyNode = downcast<ScrollingTreeOverflowScrollProxyNode>(*node);
-            auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(scrollProxyNode.overflowScrollingNodeID()));
+        for (auto overflowNodeID : positionedNode->relatedOverflowScrollingNodes()) {
+            auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(overflowNodeID));
             if (!overflowNode)
                 continue;
+            stationaryScrollContainerIDs.append(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
+        }
 
-            scrollContainerLayerIDs.append(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
-            layerNode = RemoteLayerTreeNode::forCALayer(scrollProxyNode.layer());
-            behavior = ScrollPositioningBehavior::Moves;
+        if (auto* layerNode = RemoteLayerTreeNode::forCALayer(positionedNode->layer())) {
+            layerNode->setStationaryScrollContainerIDs(WTFMove(stationaryScrollContainerIDs));
+            m_layersWithScrollingRelations.add(layerNode->layerID());
         }
+    }
 
-        if (!layerNode) {
-            ASSERT_NOT_REACHED();
+    for (auto& scrollProxyNode : m_scrollingTree->activeOverflowScrollProxyNodes()) {
+        auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(scrollProxyNode->overflowScrollingNodeID()));
+        if (!overflowNode)
             continue;
+        if (auto* layerNode = RemoteLayerTreeNode::forCALayer(scrollProxyNode->layer())) {
+            layerNode->setActingScrollContainerID(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
+            m_layersWithScrollingRelations.add(layerNode->layerID());
         }
-        layerNode->setRelatedScrollContainerBehaviorAndIDs(behavior, WTFMove(scrollContainerLayerIDs));
-
-        m_layersWithScrollingRelations.add(layerNode->layerID());
     }
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to