Attachment 373195 Details for Bug 199348 – patch

[patch] patch

acting-stationary-split-2.patch (text/plain), 17.76 KB, created by Antti Koivisto on 2019-06-30 12:12:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-06-30 12:12:48 PDT

Size: 17.76 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 246954)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2019-06-30  Antti Koivisto  <antti@apple.com>
>+
>+        Use separate variables for moving and stationary scrolling relationships in RemoteLayerTreeNode
>+        https://bugs.webkit.org/show_bug.cgi?id=199348
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * 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 RefPtrs 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  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Implement Layout::printLayoutTreeForLiveDocuments
>Index: Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp	(revision 246949)
>+++ Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp	(working copy)
>@@ -31,6 +31,7 @@
> #include "GraphicsLayer.h"
> #include "Logging.h"
> #include "ScrollingStateTree.h"
>+#include "ScrollingTree.h"
> #include <wtf/text/TextStream.h>
> 
> namespace WebCore {
>Index: Source/WebCore/page/scrolling/ScrollingTree.cpp
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTree.cpp	(revision 246949)
>+++ Source/WebCore/page/scrolling/ScrollingTree.cpp	(working copy)
>@@ -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 @@ void ScrollingTree::commitTreeState(std:
>         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;
>Index: Source/WebCore/page/scrolling/ScrollingTree.h
>===================================================================
>--- Source/WebCore/page/scrolling/ScrollingTree.h	(revision 246949)
>+++ Source/WebCore/page/scrolling/ScrollingTree.h	(working copy)
>@@ -44,6 +44,8 @@ class ScrollingStateTree;
> class ScrollingStateNode;
> class ScrollingTreeFrameScrollingNode;
> class ScrollingTreeNode;
>+class ScrollingTreeOverflowScrollProxyNode;
>+class ScrollingTreePositionedNode;
> class ScrollingTreeScrollingNode;
> 
> class ScrollingTree : public ThreadSafeRefCounted<ScrollingTree> {
>@@ -154,7 +156,8 @@ public:
>     using RelatedNodesMap = HashMap<ScrollingNodeID, Vector<ScrollingNodeID>>;
>     RelatedNodesMap& overflowRelatedNodes() { return m_overflowRelatedNodesMap; }
> 
>-    HashSet<ScrollingNodeID>& nodesWithRelatedOverflow() { return m_nodesWithRelatedOverflow; }
>+    HashSet<RefPtr<ScrollingTreeOverflowScrollProxyNode>>& activeOverflowScrollProxyNodes() { return m_activeOverflowScrollProxyNodes; }
>+    HashSet<RefPtr<ScrollingTreePositionedNode>>& activePositionedNodes() { return m_activePositionedNodes; }
> 
>     WEBCORE_EXPORT String scrollingTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal);
> 
>@@ -179,7 +182,9 @@ private:
>     ScrollingTreeNodeMap m_nodeMap;
> 
>     RelatedNodesMap m_overflowRelatedNodesMap;
>-    HashSet<ScrollingNodeID> m_nodesWithRelatedOverflow;
>+
>+    HashSet<RefPtr<ScrollingTreeOverflowScrollProxyNode>> m_activeOverflowScrollProxyNodes;
>+    HashSet<RefPtr<ScrollingTreePositionedNode>> m_activePositionedNodes;
> 
>     struct TreeState {
>         ScrollingNodeID latchedNodeID { 0 };
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm	(revision 246949)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreeOverflowScrollProxyNode.mm	(working copy)
>@@ -62,9 +62,9 @@ void ScrollingTreeOverflowScrollProxyNod
>         relatedNodes.ensure(m_overflowScrollingNodeID, [] {
>             return Vector<ScrollingNodeID>();
>         }).iterator->value.append(scrollingNodeID());
>-    }
> 
>-    scrollingTree().nodesWithRelatedOverflow().add(scrollingNodeID());
>+        scrollingTree().activeOverflowScrollProxyNodes().add(this);
>+    }
> }
> 
> FloatSize ScrollingTreeOverflowScrollProxyNode::scrollDeltaSinceLastCommit() const
>Index: Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm
>===================================================================
>--- Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	(revision 246949)
>+++ Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm	(working copy)
>@@ -64,7 +64,7 @@ void ScrollingTreePositionedNode::commit
>         m_constraints = positionedStateNode.layoutConstraints();
> 
>     if (!m_relatedOverflowScrollingNodes.isEmpty())
>-        scrollingTree().nodesWithRelatedOverflow().add(scrollingNodeID());
>+        scrollingTree().activePositionedNodes().add(this);
> }
> 
> FloatSize ScrollingTreePositionedNode::scrollDeltaSinceLastCommit() const
>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 246953)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,30 @@
>+2019-06-30  Antti Koivisto  <antti@apple.com>
>+
>+        Use separate variables for moving and stationary scrolling relationships in RemoteLayerTreeNode
>+        https://bugs.webkit.org/show_bug.cgi?id=199348
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-28  Brent Fulgham  <bfulgham@apple.com>
> 
>         Revise WebContent sandbox based on seed feedback
>Index: Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h
>===================================================================
>--- Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h	(revision 246949)
>+++ Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.h	(working copy)
>@@ -59,10 +59,13 @@ public:
>     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();
> 
>@@ -83,8 +86,8 @@ private:
> 
>     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;
> };
> 
> }
>Index: Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm
>===================================================================
>--- Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm	(revision 246949)
>+++ Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeNode.mm	(working copy)
>@@ -92,12 +92,6 @@ void RemoteLayerTreeNode::setEventRegion
>     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];
>Index: Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm
>===================================================================
>--- Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm	(revision 246949)
>+++ Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm	(working copy)
>@@ -79,15 +79,11 @@ static bool isScrolledBy(WKChildScrollVi
>             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,11 @@ UIScrollView *findActingScrollParent(UIS
>             // 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()))
>+                return (WKChildScrollView *)actingParent->uiView();
>+
>+            scrollersToSkip.add(node->stationaryScrollContainerIDs().begin(), node->stationaryScrollContainerIDs().end());
>         }
>     }
>     return nil;
>Index: Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm
>===================================================================
>--- Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm	(revision 246949)
>+++ Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm	(working copy)
>@@ -125,55 +125,42 @@ void RemoteScrollingCoordinatorProxy::sc
> 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();
> 
>     // 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()));
>-            if (!overflowNode)
>+        for (auto overflowNodeID : positionedNode->relatedOverflowScrollingNodes()) {
>+            auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(overflowNodeID));
>+            if (!overflowNode) {
>+                ASSERT_NOT_REACHED();
>                 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());
>     }
> }
>

Flags:

darin: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 199348: 373190 | 373195 | 373210 | 373211