Diff
Modified: trunk/Source/WebCore/ChangeLog (238946 => 238947)
--- trunk/Source/WebCore/ChangeLog 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/ChangeLog 2018-12-07 01:47:04 UTC (rev 238947)
@@ -1,3 +1,38 @@
+2018-12-06 Frederic Wang <[email protected]>
+
+ Allow control over child order when adding nodes to the scrolling tree
+ https://bugs.webkit.org/show_bug.cgi?id=176914
+
+ Reviewed by Simon Fraser.
+
+ Based on an earlier patch by Simon Fraser.
+
+ 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:
+
2018-12-06 Yongjun Zhang <[email protected]>
We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag.
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2018-12-07 01:47:04 UTC (rev 238947)
@@ -474,9 +474,9 @@
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);
+ return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID, childIndex);
}
void AsyncScrollingCoordinator::detachFromStateTree(ScrollingNodeID nodeID)
@@ -509,7 +509,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 (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2018-12-07 01:47:04 UTC (rev 238947)
@@ -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 (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2018-12-07 01:47:04 UTC (rev 238947)
@@ -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 (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2018-12-07 01:47:04 UTC (rev 238947)
@@ -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 (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2018-12-07 01:47:04 UTC (rev 238947)
@@ -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 (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2018-12-07 01:47:04 UTC (rev 238947)
@@ -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 (238946 => 238947)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2018-12-07 01:47:04 UTC (rev 238947)
@@ -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 (238946 => 238947)
--- trunk/Source/WebKit/ChangeLog 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebKit/ChangeLog 2018-12-07 01:47:04 UTC (rev 238947)
@@ -1,3 +1,16 @@
+2018-12-06 Frederic Wang <[email protected]>
+
+ Allow control over child order when adding nodes to the scrolling tree
+ https://bugs.webkit.org/show_bug.cgi?id=176914
+
+ Reviewed by Simon Fraser.
+
+ Based on an earlier patch by Simon Fraser.
+
+ * Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
+ (WebKit::RemoteScrollingCoordinatorTransaction::decode): Make explicit that we want to append
+ the new node at the end of child list.
+
2018-12-06 Yongjun Zhang <[email protected]>
We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag.
Modified: trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp (238946 => 238947)
--- trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp 2018-12-07 01:35:25 UTC (rev 238946)
+++ trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp 2018-12-07 01:47:04 UTC (rev 238947)
@@ -413,7 +413,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());
