Attachment 373190 Details for Bug 199348 – patch

[patch] patch

acting-stationary-split.patch (text/plain), 11.18 KB, created by Antti Koivisto on 2019-06-30 00:31:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-06-30 00:31:16 PDT

Size: 11.18 KB

patch

obsolete

>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,8 +125,10 @@ 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();
> 
>@@ -135,16 +137,10 @@ void RemoteScrollingCoordinatorProxy::es
>     
>     for (auto nodeID : m_scrollingTree->nodesWithRelatedOverflow()) {
>         auto* node = m_scrollingTree->nodeForID(nodeID);
>-        if (!node) {
>-            ASSERT_NOT_REACHED();
>-            continue;
>-        }
> 
>-        Vector<GraphicsLayer::PlatformLayerID> scrollContainerLayerIDs;
>-        RemoteLayerTreeNode* layerNode = nullptr;
>-        ScrollPositioningBehavior behavior = ScrollPositioningBehavior::None;
>+        if (is<ScrollingTreePositionedNode>(node)) {
>+            Vector<GraphicsLayer::PlatformLayerID> stationaryScrollContainerIDs;
> 
>-        if (is<ScrollingTreePositionedNode>(*node)) {
>             auto& positionedNode = downcast<ScrollingTreePositionedNode>(*node);
>             for (auto overflowNodeID : positionedNode.relatedOverflowScrollingNodes()) {
>                 auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(overflowNodeID));
>@@ -152,28 +148,29 @@ void RemoteScrollingCoordinatorProxy::es
>                     ASSERT_NOT_REACHED();
>                     continue;
>                 }
>-                scrollContainerLayerIDs.append(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
>+                stationaryScrollContainerIDs.append(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
>             }
>-            layerNode = RemoteLayerTreeNode::forCALayer(positionedNode.layer());
>-            behavior = ScrollPositioningBehavior::Stationary;
>-        } else if (is<ScrollingTreeOverflowScrollProxyNode>(*node)) {
>+
>+            if (auto* layerNode = RemoteLayerTreeNode::forCALayer(positionedNode.layer())) {
>+                layerNode->setStationaryScrollContainerIDs(WTFMove(stationaryScrollContainerIDs));
>+                m_layersWithScrollingRelations.add(layerNode->layerID());
>+            }
>+            continue;
>+        }
>+
>+        if (is<ScrollingTreeOverflowScrollProxyNode>(node)) {
>             auto& scrollProxyNode = downcast<ScrollingTreeOverflowScrollProxyNode>(*node);
>             auto* overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(m_scrollingTree->nodeForID(scrollProxyNode.overflowScrollingNodeID()));
>             if (!overflowNode)
>                 continue;
>-
>-            scrollContainerLayerIDs.append(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
>-            layerNode = RemoteLayerTreeNode::forCALayer(scrollProxyNode.layer());
>-            behavior = ScrollPositioningBehavior::Moves;
>-        }
>-
>-        if (!layerNode) {
>-            ASSERT_NOT_REACHED();
>+            if (auto* layerNode = RemoteLayerTreeNode::forCALayer(scrollProxyNode.layer())) {
>+                layerNode->setActingScrollContainerID(RemoteLayerTreeNode::layerID(overflowNode->scrollContainerLayer()));
>+                m_layersWithScrollingRelations.add(layerNode->layerID());
>+            }
>             continue;
>         }
>-        layerNode->setRelatedScrollContainerBehaviorAndIDs(behavior, WTFMove(scrollContainerLayerIDs));
> 
>-        m_layersWithScrollingRelations.add(layerNode->layerID());
>+        ASSERT_NOT_REACHED();
>     }
> }
>

Actions: View | Formatted Diff | Diff

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