Changeset 238947 in webkit


Ignore:
Timestamp:
Dec 6, 2018 5:47:04 PM (5 years ago)
Author:
commit-queue@webkit.org
Message:

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

Patch by Frederic Wang <fwang@igalia.com> on 2018-12-06
Reviewed by Simon Fraser.

Based on an earlier patch by Simon Fraser.

Source/WebCore:

Previously ScrollingCoordinator just allowed nodes to be "attached" with a given parent,
but with no control over sibling order. To allow for correct hit-testing overflow and
frame scrolling nodes, we have to build the scrolling tree in z-order.

This patch adds a 'childIndex' parameter to attachNode() which gives control over
sibling order. For now, RenderLayerCompositor always uses the default 'notFound' value
for childIndex so the current behavior (appending new nodes at the end of child list) is
preserved.

No new tests, behavior unchanged and already covered by existing tests.

  • 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): Make explicit that we want to append
the new node at the end of child list.

Location:
trunk/Source
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r238946 r238947  
     12018-12-06  Frederic Wang  <fwang@igalia.com>
     2
     3        Allow control over child order when adding nodes to the scrolling tree
     4        https://bugs.webkit.org/show_bug.cgi?id=176914
     5
     6        Reviewed by Simon Fraser.
     7
     8        Based on an earlier patch by Simon Fraser.
     9
     10        Previously ScrollingCoordinator just allowed nodes to be "attached" with a given parent,
     11        but with no control over sibling order. To allow for correct hit-testing overflow and
     12        frame scrolling nodes, we have to build the scrolling tree in z-order.
     13
     14        This patch adds a 'childIndex' parameter to attachNode() which gives control over
     15        sibling order. For now, RenderLayerCompositor always uses the default 'notFound' value
     16        for childIndex so the current behavior (appending new nodes at the end of child list) is
     17        preserved.
     18
     19        No new tests, behavior unchanged and already covered by existing tests.
     20
     21        * page/scrolling/AsyncScrollingCoordinator.cpp:
     22        (WebCore::AsyncScrollingCoordinator::attachToStateTree):
     23        (WebCore::AsyncScrollingCoordinator::ensureRootStateNodeForFrameView):
     24        * page/scrolling/AsyncScrollingCoordinator.h:
     25        * page/scrolling/ScrollingCoordinator.h:
     26        (WebCore::ScrollingCoordinator::attachToStateTree):
     27        * page/scrolling/ScrollingStateNode.cpp:
     28        (WebCore::ScrollingStateNode::insertChild):
     29        (WebCore::ScrollingStateNode::indexOfChild const):
     30        * page/scrolling/ScrollingStateNode.h:
     31        * page/scrolling/ScrollingStateTree.cpp:
     32        (WebCore::ScrollingStateTree::nodeTypeAndParentMatch const):
     33        (WebCore::ScrollingStateTree::attachNode):
     34        * page/scrolling/ScrollingStateTree.h:
     35
    1362018-12-06  Yongjun Zhang  <yongjun_zhang@apple.com>
    237
  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

    r238885 r238947  
    475475}
    476476
    477 ScrollingNodeID AsyncScrollingCoordinator::attachToStateTree(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
    478 {
    479     return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID);
     477ScrollingNodeID AsyncScrollingCoordinator::attachToStateTree(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex)
     478{
     479    return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID, childIndex);
    480480}
    481481
     
    510510    // RenderLayerCompositor::updateBacking where the node has already been created.
    511511    ASSERT(frameView.frame().isMainFrame());
    512     attachToStateTree(MainFrameScrollingNode, frameView.scrollLayerID(), 0);
     512    attachToStateTree(MainFrameScrollingNode, frameView.scrollLayerID(), 0, 0);
    513513}
    514514
  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h

    r238885 r238947  
    9898    WEBCORE_EXPORT bool requestScrollPositionUpdate(FrameView&, const IntPoint&) override;
    9999
    100     WEBCORE_EXPORT ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID) override;
     100    WEBCORE_EXPORT ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) override;
    101101    WEBCORE_EXPORT void detachFromStateTree(ScrollingNodeID) override;
    102102    WEBCORE_EXPORT void clearStateTree() override;
  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

    r238898 r238947  
    165165    virtual bool requestScrollPositionUpdate(FrameView&, const IntPoint&) { return false; }
    166166    virtual bool handleWheelEvent(FrameView&, const PlatformWheelEvent&) { return true; }
    167     virtual ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID /*parentID*/) { return newNodeID; }
     167    virtual ScrollingNodeID attachToStateTree(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID /*parentID*/, size_t /*childIndex*/ = notFound) { return newNodeID; }
     168
    168169    virtual void detachFromStateTree(ScrollingNodeID) { }
    169170    virtual void clearStateTree() { }
  • trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp

    r238898 r238947  
    9898}
    9999
     100void ScrollingStateNode::insertChild(Ref<ScrollingStateNode>&& childNode, size_t index)
     101{
     102    childNode->setParent(this);
     103
     104    if (!m_children) {
     105        ASSERT(!index);
     106        m_children = std::make_unique<Vector<RefPtr<ScrollingStateNode>>>();
     107    }
     108
     109    m_children->insert(index, WTFMove(childNode));
     110}
     111
     112size_t ScrollingStateNode::indexOfChild(ScrollingStateNode& childNode) const
     113{
     114    if (!m_children)
     115        return notFound;
     116
     117    return m_children->find(&childNode);
     118}
     119
    100120void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action)
    101121{
  • trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h

    r238898 r238947  
    237237
    238238    void appendChild(Ref<ScrollingStateNode>&&);
     239    void insertChild(Ref<ScrollingStateNode>&&, size_t index);
     240
     241    size_t indexOfChild(ScrollingStateNode&) const;
    239242
    240243    String scrollingStateTreeAsText(ScrollingStateTreeAsTextBehavior = ScrollingStateTreeAsTextBehaviorNormal) const;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp

    r238898 r238947  
    8383}
    8484
    85 bool ScrollingStateTree::nodeTypeAndParentMatch(ScrollingStateNode& node, ScrollingNodeType nodeType, ScrollingNodeID parentID) const
     85bool ScrollingStateTree::nodeTypeAndParentMatch(ScrollingStateNode& node, ScrollingNodeType nodeType, ScrollingStateNode* parentNode) const
    8686{
    8787    if (node.nodeType() != nodeType)
    8888        return false;
    8989
    90     auto* parent = stateNodeForID(parentID);
    91     if (!parent)
    92         return true;
    93 
    94     return node.parent() == parent;
    95 }
    96 
    97 ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
     90    return node.parent() == parentNode;
     91}
     92
     93ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex)
    9894{
    9995    ASSERT(newNodeID);
    10096
    10197    if (auto* node = stateNodeForID(newNodeID)) {
    102         if (nodeTypeAndParentMatch(*node, nodeType, parentID))
     98        auto* parent = stateNodeForID(parentID);
     99        if (nodeTypeAndParentMatch(*node, nodeType, parent)) {
     100            if (!parentID)
     101                return newNodeID;
     102
     103            size_t currentIndex = parent->indexOfChild(*node);
     104            if (currentIndex == childIndex)
     105                return newNodeID;
     106
     107            ASSERT(currentIndex != notFound);
     108            Ref<ScrollingStateNode> protectedNode(*node);
     109            parent->children()->remove(currentIndex);
     110
     111            if (childIndex == notFound)
     112                parent->appendChild(WTFMove(protectedNode));
     113            else
     114                parent->insertChild(WTFMove(protectedNode), childIndex);
     115
    103116            return newNodeID;
     117        }
    104118
    105119#if ENABLE(ASYNC_SCROLLING)
     
    115129    ScrollingStateNode* newNode = nullptr;
    116130    if (!parentID) {
     131        ASSERT(!childIndex || childIndex == notFound);
    117132        // If we're resetting the root node, we should clear the HashMap and destroy the current children.
    118133        clear();
     
    123138    } else {
    124139        auto* parent = stateNodeForID(parentID);
    125         if (!parent)
     140        if (!parent) {
     141            ASSERT_NOT_REACHED();
    126142            return 0;
     143        }
    127144
    128145        if (nodeType == SubframeScrollingNode && parentID) {
    129146            if (auto orphanedNode = m_orphanedSubframeNodes.take(newNodeID)) {
    130147                newNode = orphanedNode.get();
    131                 parent->appendChild(orphanedNode.releaseNonNull());
     148                if (childIndex == notFound)
     149                    parent->appendChild(orphanedNode.releaseNonNull());
     150                else
     151                    parent->insertChild(orphanedNode.releaseNonNull(), childIndex);
    132152            }
    133153        }
     
    136156            auto stateNode = createNode(nodeType, newNodeID);
    137157            newNode = stateNode.ptr();
    138             parent->appendChild(WTFMove(stateNode));
    139         }
    140     }
    141 
    142     m_stateNodeMap.set(newNodeID, newNode);
     158            if (childIndex == notFound)
     159                parent->appendChild(WTFMove(stateNode));
     160            else
     161                parent->insertChild(WTFMove(stateNode), childIndex);
     162        }
     163    }
     164
     165    addNode(*newNode);
    143166    m_nodesRemovedSinceLastCommit.remove(newNodeID);
    144167    return newNodeID;
  • trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h

    r238898 r238947  
    5252    WEBCORE_EXPORT ScrollingStateNode* stateNodeForID(ScrollingNodeID) const;
    5353
    54     WEBCORE_EXPORT ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID);
     54    WEBCORE_EXPORT ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID, size_t childIndex);
    5555    void detachNode(ScrollingNodeID);
    5656    void clear();
     
    8282    Ref<ScrollingStateNode> createNode(ScrollingNodeType, ScrollingNodeID);
    8383
    84     bool nodeTypeAndParentMatch(ScrollingStateNode&, ScrollingNodeType, ScrollingNodeID parentID) const;
     84    bool nodeTypeAndParentMatch(ScrollingStateNode&, ScrollingNodeType, ScrollingStateNode* parentNode) const;
    8585
    8686    enum class SubframeNodeRemoval { Delete, Orphan };
  • trunk/Source/WebKit/ChangeLog

    r238946 r238947  
     12018-12-06  Frederic Wang  <fwang@igalia.com>
     2
     3        Allow control over child order when adding nodes to the scrolling tree
     4        https://bugs.webkit.org/show_bug.cgi?id=176914
     5
     6        Reviewed by Simon Fraser.
     7
     8        Based on an earlier patch by Simon Fraser.
     9
     10        * Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
     11        (WebKit::RemoteScrollingCoordinatorTransaction::decode): Make explicit that we want to append
     12        the new node at the end of child list.
     13
    1142018-12-06  Yongjun Zhang  <yongjun_zhang@apple.com>
    215
  • trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp

    r228264 r238947  
    414414            return false;
    415415
    416         m_scrollingStateTree->attachNode(nodeType, nodeID, parentNodeID);
     416        m_scrollingStateTree->attachNode(nodeType, nodeID, parentNodeID, notFound); // Append new node.
    417417        ScrollingStateNode* newNode = m_scrollingStateTree->stateNodeForID(nodeID);
    418418        ASSERT(newNode);
Note: See TracChangeset for help on using the changeset viewer.