Title: [239042] trunk
Revision
239042
Author
[email protected]
Date
2018-12-10 12:43:40 -0800 (Mon, 10 Dec 2018)

Log Message

Allow control over child order when adding nodes to the scrolling tree
https://bugs.webkit.org/show_bug.cgi?id=176914
<rdar://problem/46542237>

Re-land r239010 after over-zealous rollout.

Source/WebCore:

* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::attachToStateTree):
(WebCore::AsyncScrollingCoordinator::ensureRootStateNodeForFrameView):
* page/scrolling/AsyncScrollingCoordinator.h:
* page/scrolling/ScrollingCoordinator.h:
(WebCore::ScrollingCoordinator::attachToStateTree):
* page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::insertChild):
(WebCore::ScrollingStateNode::indexOfChild const):
* page/scrolling/ScrollingStateNode.h:
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::nodeTypeAndParentMatch const):
(WebCore::ScrollingStateTree::attachNode):
* page/scrolling/ScrollingStateTree.h:

Source/WebKit:

* Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
(WebKit::RemoteScrollingCoordinatorTransaction::decode):

LayoutTests:

* platform/mac-wk2/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (239041 => 239042)


--- trunk/LayoutTests/ChangeLog	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/LayoutTests/ChangeLog	2018-12-10 20:43:40 UTC (rev 239042)
@@ -10,22 +10,16 @@
         * http/tests/xmlhttprequest/supported-xml-content-types-expected.txt:
         * http/tests/xmlhttprequest/supported-xml-content-types.html:
 
-2018-12-09  Commit Queue  <[email protected]>
+2018-12-10  Simon Fraser  <[email protected]>
 
-        Unreviewed, rolling out r239010.
-        https://bugs.webkit.org/show_bug.cgi?id=192537
+        Allow control over child order when adding nodes to the scrolling tree
+        https://bugs.webkit.org/show_bug.cgi?id=176914
+        <rdar://problem/46542237>
+        
+        Re-land r239010 after over-zealous rollout.
 
-        Breaks fast/visual-viewport/tiled-drawing/zoomed-fixed-
-        scrolling-layers-state.html again (Requested by ap on
-        #webkit).
+        * platform/mac-wk2/TestExpectations:
 
-        Reverted changeset:
-
-        "Allow control over child order when adding nodes to the
-        scrolling tree"
-        https://bugs.webkit.org/show_bug.cgi?id=176914
-        https://trac.webkit.org/changeset/239010
-
 2018-12-08  Eric Carlson  <[email protected]>
 
         [MediaStream] Scaled video frames should be resized in letterbox mode

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (239041 => 239042)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2018-12-10 20:43:40 UTC (rev 239042)
@@ -321,6 +321,8 @@
 
 webkit.org/b/148408 tiled-drawing/scrolling/root-overflow-with-mousewheel.html [ Pass Failure Timeout ]
 
+webkit.org/b/192529 fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html [ Pass Failure ]
+
 webkit.org/b/139820 fast/frames/lots-of-objects.html [ Pass Timeout ]
 webkit.org/b/139820 fast/frames/lots-of-iframes.html [ Pass Timeout ]
 

Modified: trunk/Source/WebCore/ChangeLog (239041 => 239042)


--- trunk/Source/WebCore/ChangeLog	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/ChangeLog	2018-12-10 20:43:40 UTC (rev 239042)
@@ -1,3 +1,26 @@
+2018-12-10  Simon Fraser  <[email protected]>
+
+        Allow control over child order when adding nodes to the scrolling tree
+        https://bugs.webkit.org/show_bug.cgi?id=176914
+        <rdar://problem/46542237>
+        
+        Re-land r239010 after over-zealous rollout.
+
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::attachToStateTree):
+        (WebCore::AsyncScrollingCoordinator::ensureRootStateNodeForFrameView):
+        * page/scrolling/AsyncScrollingCoordinator.h:
+        * page/scrolling/ScrollingCoordinator.h:
+        (WebCore::ScrollingCoordinator::attachToStateTree):
+        * page/scrolling/ScrollingStateNode.cpp:
+        (WebCore::ScrollingStateNode::insertChild):
+        (WebCore::ScrollingStateNode::indexOfChild const):
+        * page/scrolling/ScrollingStateNode.h:
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::nodeTypeAndParentMatch const):
+        (WebCore::ScrollingStateTree::attachNode):
+        * page/scrolling/ScrollingStateTree.h:
+
 2018-12-10  Antti Koivisto  <[email protected]>
 
         Document should throttle style recalc even when m_pendingStyleRecalcShouldForce is true.

Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp	2018-12-10 20:43:40 UTC (rev 239042)
@@ -474,9 +474,10 @@
         scrollableArea.horizontalScrollbarLayerDidChange();
 }
 
-ScrollingNodeID AsyncScrollingCoordinator::attachToStateTree(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
+ScrollingNodeID AsyncScrollingCoordinator::attachToStateTree(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex)
 {
-    return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID);
+    LOG_WITH_STREAM(Scrolling, stream << "AsyncScrollingCoordinator::attachToStateTree " << nodeType << " node " << newNodeID << " parent " << parentID << " index " << childIndex);
+    return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID, childIndex);
 }
 
 void AsyncScrollingCoordinator::detachFromStateTree(ScrollingNodeID nodeID)
@@ -509,7 +510,7 @@
     // For non-main frames, it is only possible to arrive in this function from
     // RenderLayerCompositor::updateBacking where the node has already been created.
     ASSERT(frameView.frame().isMainFrame());
-    attachToStateTree(MainFrameScrollingNode, frameView.scrollLayerID(), 0);
+    attachToStateTree(MainFrameScrollingNode, frameView.scrollLayerID(), 0, 0);
 }
 
 void AsyncScrollingCoordinator::updateFrameScrollingNode(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer, const ScrollingGeometry& scrollingGeometry)

Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h	2018-12-10 20:43:40 UTC (rev 239042)
@@ -97,7 +97,7 @@
 
     WEBCORE_EXPORT bool requestScrollPositionUpdate(FrameView&, const IntPoint&) override;
 
-    WEBCORE_EXPORT ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID) override;
+    WEBCORE_EXPORT ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) override;
     WEBCORE_EXPORT void detachFromStateTree(ScrollingNodeID) override;
     WEBCORE_EXPORT void clearStateTree() override;
     

Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h	2018-12-10 20:43:40 UTC (rev 239042)
@@ -164,7 +164,8 @@
     virtual void commitTreeStateIfNeeded() { }
     virtual bool requestScrollPositionUpdate(FrameView&, const IntPoint&) { return false; }
     virtual bool handleWheelEvent(FrameView&, const PlatformWheelEvent&) { return true; }
-    virtual ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID /*parentID*/) { return newNodeID; }
+    virtual ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID /*parentID*/, size_t /*childIndex*/ = notFound) { return newNodeID; }
+
     virtual void detachFromStateTree(ScrollingNodeID) { }
     virtual void clearStateTree() { }
 

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp	2018-12-10 20:43:40 UTC (rev 239042)
@@ -97,6 +97,26 @@
     m_children->append(WTFMove(childNode));
 }
 
+void ScrollingStateNode::insertChild(Ref<ScrollingStateNode>&& childNode, size_t index)
+{
+    childNode->setParent(this);
+
+    if (!m_children) {
+        ASSERT(!index);
+        m_children = std::make_unique<Vector<RefPtr<ScrollingStateNode>>>();
+    }
+
+    m_children->insert(index, WTFMove(childNode));
+}
+
+size_t ScrollingStateNode::indexOfChild(ScrollingStateNode& childNode) const
+{
+    if (!m_children)
+        return notFound;
+
+    return m_children->find(&childNode);
+}
+
 void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action)
 {
     if (!m_children)

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h	2018-12-10 20:43:40 UTC (rev 239042)
@@ -236,7 +236,10 @@
     Vector<RefPtr<ScrollingStateNode>>* children() const { return m_children.get(); }
 
     void appendChild(Ref<ScrollingStateNode>&&);
+    void insertChild(Ref<ScrollingStateNode>&&, size_t index);
 
+    size_t indexOfChild(ScrollingStateNode&) const;
+
     String scrollingStateTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const;
 
 protected:

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2018-12-10 20:43:40 UTC (rev 239042)
@@ -82,25 +82,39 @@
     return ScrollingStateFixedNode::create(*this, nodeID);
 }
 
-bool ScrollingStateTree::nodeTypeAndParentMatch(ScrollingStateNode& node, ScrollingNodeType nodeType, ScrollingNodeID parentID) const
+bool ScrollingStateTree::nodeTypeAndParentMatch(ScrollingStateNode& node, ScrollingNodeType nodeType, ScrollingStateNode* parentNode) const
 {
     if (node.nodeType() != nodeType)
         return false;
 
-    auto* parent = stateNodeForID(parentID);
-    if (!parent)
-        return true;
-
-    return node.parent() == parent;
+    return node.parent() == parentNode;
 }
 
-ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
+ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex)
 {
     ASSERT(newNodeID);
 
     if (auto* node = stateNodeForID(newNodeID)) {
-        if (nodeTypeAndParentMatch(*node, nodeType, parentID))
+        auto* parent = stateNodeForID(parentID);
+        if (nodeTypeAndParentMatch(*node, nodeType, parent)) {
+            if (!parentID)
+                return newNodeID;
+
+            size_t currentIndex = parent->indexOfChild(*node);
+            if (currentIndex == childIndex)
+                return newNodeID;
+
+            ASSERT(currentIndex != notFound);
+            Ref<ScrollingStateNode> protectedNode(*node);
+            parent->children()->remove(currentIndex);
+
+            if (childIndex == notFound)
+                parent->appendChild(WTFMove(protectedNode));
+            else
+                parent->insertChild(WTFMove(protectedNode), childIndex);
+
             return newNodeID;
+        }
 
 #if ENABLE(ASYNC_SCROLLING)
         // If the type has changed, we need to destroy and recreate the node with a new ID.
@@ -114,6 +128,7 @@
 
     ScrollingStateNode* newNode = nullptr;
     if (!parentID) {
+        ASSERT(!childIndex || childIndex == notFound);
         // If we're resetting the root node, we should clear the HashMap and destroy the current children.
         clear();
 
@@ -122,13 +137,18 @@
         m_hasNewRootStateNode = true;
     } else {
         auto* parent = stateNodeForID(parentID);
-        if (!parent)
+        if (!parent) {
+            ASSERT_NOT_REACHED();
             return 0;
+        }
 
         if (nodeType == SubframeScrollingNode && parentID) {
             if (auto orphanedNode = m_orphanedSubframeNodes.take(newNodeID)) {
                 newNode = orphanedNode.get();
-                parent->appendChild(orphanedNode.releaseNonNull());
+                if (childIndex == notFound)
+                    parent->appendChild(orphanedNode.releaseNonNull());
+                else
+                    parent->insertChild(orphanedNode.releaseNonNull(), childIndex);
             }
         }
 
@@ -135,11 +155,14 @@
         if (!newNode) {
             auto stateNode = createNode(nodeType, newNodeID);
             newNode = stateNode.ptr();
-            parent->appendChild(WTFMove(stateNode));
+            if (childIndex == notFound)
+                parent->appendChild(WTFMove(stateNode));
+            else
+                parent->insertChild(WTFMove(stateNode), childIndex);
         }
     }
 
-    m_stateNodeMap.set(newNodeID, newNode);
+    addNode(*newNode);
     m_nodesRemovedSinceLastCommit.remove(newNodeID);
     return newNodeID;
 }

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (239041 => 239042)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2018-12-10 20:43:40 UTC (rev 239042)
@@ -51,7 +51,7 @@
     ScrollingStateFrameScrollingNode* rootStateNode() const { return m_rootStateNode.get(); }
     WEBCORE_EXPORT ScrollingStateNode* stateNodeForID(ScrollingNodeID) const;
 
-    WEBCORE_EXPORT ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID);
+    WEBCORE_EXPORT ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID, size_t childIndex);
     void detachNode(ScrollingNodeID);
     void clear();
     
@@ -81,7 +81,7 @@
 
     Ref<ScrollingStateNode> createNode(ScrollingNodeType, ScrollingNodeID);
 
-    bool nodeTypeAndParentMatch(ScrollingStateNode&, ScrollingNodeType, ScrollingNodeID parentID) const;
+    bool nodeTypeAndParentMatch(ScrollingStateNode&, ScrollingNodeType, ScrollingStateNode* parentNode) const;
 
     enum class SubframeNodeRemoval { Delete, Orphan };
     void removeNodeAndAllDescendants(ScrollingStateNode*, SubframeNodeRemoval = SubframeNodeRemoval::Delete);

Modified: trunk/Source/WebKit/ChangeLog (239041 => 239042)


--- trunk/Source/WebKit/ChangeLog	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebKit/ChangeLog	2018-12-10 20:43:40 UTC (rev 239042)
@@ -1,3 +1,14 @@
+2018-12-10  Simon Fraser  <[email protected]>
+
+        Allow control over child order when adding nodes to the scrolling tree
+        https://bugs.webkit.org/show_bug.cgi?id=176914
+        <rdar://problem/46542237>
+        
+        Re-land r239010 after over-zealous rollout.
+
+        * Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
+        (WebKit::RemoteScrollingCoordinatorTransaction::decode):
+
 2018-12-10  Wenson Hsieh  <[email protected]>
 
         [iOS] Caret is obscured by finger when dragging over an editable element

Modified: trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp (239041 => 239042)


--- trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp	2018-12-10 20:42:53 UTC (rev 239041)
+++ trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp	2018-12-10 20:43:40 UTC (rev 239042)
@@ -412,7 +412,7 @@
         if (!decoder.decode(parentNodeID))
             return false;
 
-        m_scrollingStateTree->attachNode(nodeType, nodeID, parentNodeID);
+        m_scrollingStateTree->attachNode(nodeType, nodeID, parentNodeID, notFound); // Append new node.
         ScrollingStateNode* newNode = m_scrollingStateTree->stateNodeForID(nodeID);
         ASSERT(newNode);
         ASSERT(!parentNodeID || newNode->parent());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to