WebKit Bugzilla
Attachment 360248 Details for
Bug 193754
: Allow scrolling tree nodes to exist in a detached state
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193754-20190126130807.patch (text/plain), 32.24 KB, created by
Simon Fraser (smfr)
on 2019-01-26 13:08:08 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Simon Fraser (smfr)
Created:
2019-01-26 13:08:08 PST
Size:
32.24 KB
patch
obsolete
>Subversion Revision: 240548 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index f8099584ab8f69a4bf454e85977e532fb6556cfc..9a697b645b0dd7a5bed8e22cfb8a465fc9f628b3 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,92 @@ >+2019-01-26 Simon Fraser <simon.fraser@apple.com> >+ >+ Allow scrolling tree nodes to exist in a detached state >+ https://bugs.webkit.org/show_bug.cgi?id=193754 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ One of the (questionable?) design decisions of the scrolling tree is that the tree implementation >+ is hidden behind the ScrollingCoordinator interface. That interface only allowed nodes to exist >+ in a connected state; attachToStateTree() required a non-zero parent for any node that was not >+ the root. >+ >+ This makes it impossible to coordinate the hookup of the scrolling tree across frame boundaries; >+ the scrolling tree has to have been fully constructed in ancestor frames before subframe nodes >+ can be attached. This is a significant difference from compositing, where a subframe can create >+ GraphicsLayers which don't have to be parented right away, and actually get parented later via >+ a compositing update in the parent frame. >+ >+ We want to be able to hook up the scrolling tree via the same code paths as GraphicsLayer >+ connection (anything else is too confusing). So we need to be able to instantiate scrolling >+ tree nodes in a disconnected state, and attach them later. >+ >+ To achieve this, add the notion of "unparented" nodes to ScrollingCoordinator and the ScrollingStateTree. >+ Allow clients to create unparented nodes, which can be attached later. ScrollingCoordinator stores >+ the roots of unparented subtrees in an owning HashMap. Nodes in unparented trees are still referenced >+ by m_stateNodeMap, so it's possible to find them and set state on them. >+ >+ Clean up the ScrollingCoordinator interface to remove "state tree" terminology; the state vs. scrolling tree >+ is really an implementation detail. >+ >+ This also removes the special-casing of ScrollingNodeType::Subframe nodes which ScrollingStateTree stored >+ in m_orphanedSubframeNodes; now the unparenting is controlled by the client. >+ >+ Currently no code creates unparented nodes so there is no behavior change. >+ >+ * dom/Document.cpp: >+ (WebCore::Document::setPageCacheState): >+ * page/scrolling/AsyncScrollingCoordinator.cpp: >+ (WebCore::AsyncScrollingCoordinator::createNode): >+ (WebCore::AsyncScrollingCoordinator::insertNode): >+ (WebCore::AsyncScrollingCoordinator::unparentNode): >+ (WebCore::AsyncScrollingCoordinator::unparentChildrenAndDestroyNode): >+ (WebCore::AsyncScrollingCoordinator::detachAndDestroySubtree): >+ (WebCore::AsyncScrollingCoordinator::clearAllNodes): >+ (WebCore::AsyncScrollingCoordinator::parentOfNode const): >+ (WebCore::AsyncScrollingCoordinator::ensureRootStateNodeForFrameView): >+ (WebCore::AsyncScrollingCoordinator::attachToStateTree): Deleted. >+ (WebCore::AsyncScrollingCoordinator::detachFromStateTree): Deleted. >+ (WebCore::AsyncScrollingCoordinator::clearStateTree): Deleted. >+ * page/scrolling/AsyncScrollingCoordinator.h: >+ * page/scrolling/ScrollingCoordinator.h: >+ (WebCore::ScrollingCoordinator::handleWheelEvent): >+ (WebCore::ScrollingCoordinator::createNode): >+ (WebCore::ScrollingCoordinator::insertNode): >+ (WebCore::ScrollingCoordinator::unparentNode): >+ (WebCore::ScrollingCoordinator::unparentChildrenAndDestroyNode): >+ (WebCore::ScrollingCoordinator::detachAndDestroySubtree): >+ (WebCore::ScrollingCoordinator::clearAllNodes): >+ (WebCore::ScrollingCoordinator::parentOfNode const): >+ (WebCore::ScrollingCoordinator::childrenOfNode const): >+ (WebCore::ScrollingCoordinator::attachToStateTree): Deleted. >+ (WebCore::ScrollingCoordinator::detachFromStateTree): Deleted. >+ (WebCore::ScrollingCoordinator::clearStateTree): Deleted. >+ * page/scrolling/ScrollingStateNode.cpp: >+ (WebCore::ScrollingStateNode::removeFromParent): >+ (WebCore::ScrollingStateNode::removeChild): >+ * page/scrolling/ScrollingStateNode.h: >+ * page/scrolling/ScrollingStateTree.cpp: >+ (WebCore::ScrollingStateTree::ScrollingStateTree): >+ (WebCore::ScrollingStateTree::createUnparentedNode): >+ (WebCore::ScrollingStateTree::insertNode): >+ (WebCore::ScrollingStateTree::unparentNode): >+ (WebCore::ScrollingStateTree::unparentChildrenAndDestroyNode): >+ (WebCore::ScrollingStateTree::detachAndDestroySubtree): >+ (WebCore::ScrollingStateTree::clear): >+ (WebCore::ScrollingStateTree::commit): >+ (WebCore::ScrollingStateTree::removeNodeAndAllDescendants): >+ (WebCore::ScrollingStateTree::recursiveNodeWillBeRemoved): >+ (showScrollingStateTree): >+ (WebCore::ScrollingStateTree::attachNode): Deleted. >+ (WebCore::ScrollingStateTree::detachNode): Deleted. >+ * page/scrolling/ScrollingStateTree.h: >+ (WebCore::ScrollingStateTree::nodeCount const): >+ * rendering/RenderLayerBacking.cpp: >+ (WebCore::RenderLayerBacking::detachFromScrollingCoordinator): >+ * rendering/RenderLayerCompositor.cpp: >+ (WebCore::RenderLayerCompositor::reattachSubframeScrollLayers): >+ (WebCore::RenderLayerCompositor::attachScrollingNode): >+ > 2019-01-26 Zalan Bujtas <zalan@apple.com> > > [LFC] The initial values for top/bottom in contentHeightForFormattingContextRoot should not be 0. >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 6aae160ba07f1b6ecb14a86ac29d0a3111717493..6ba5aa90ebae2611f4b37419dfaf018af4f60907 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,13 @@ >+2019-01-26 Simon Fraser <simon.fraser@apple.com> >+ >+ Allow scrolling tree nodes to exist in a detached state >+ https://bugs.webkit.org/show_bug.cgi?id=193754 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp: >+ (WebKit::RemoteScrollingCoordinatorTransaction::decode): >+ > 2019-01-25 Tim Horton <timothy_horton@apple.com> > > REGRESSION (r238818): Snapshot is removed too late after swiping back on Twitter >diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp >index ad33faf9d125d7679d7142c285a4993f435ce06b..f86b36f90680d4278fcd45f147fb378cba8857d5 100644 >--- a/Source/WebCore/dom/Document.cpp >+++ b/Source/WebCore/dom/Document.cpp >@@ -5200,7 +5200,7 @@ void Document::setPageCacheState(PageCacheState state) > if (page && m_frame->isMainFrame()) { > v->resetScrollbarsAndClearContentsSize(); > if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) >- scrollingCoordinator->clearStateTree(); >+ scrollingCoordinator->clearAllNodes(); > } > } > >diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp >index 0493ec1160c3102c3b1ae4d715f24e1300dbdca8..8f12a31da23926b06b1bd8c164e2966be8b63ae8 100644 >--- a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp >+++ b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp >@@ -486,22 +486,47 @@ void AsyncScrollingCoordinator::scrollableAreaScrollbarLayerDidChange(Scrollable > } > } > >-ScrollingNodeID AsyncScrollingCoordinator::attachToStateTree(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) >+ScrollingNodeID AsyncScrollingCoordinator::createNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID) > { >- LOG_WITH_STREAM(Scrolling, stream << "AsyncScrollingCoordinator::attachToStateTree " << nodeType << " node " << newNodeID << " parent " << parentID << " index " << childIndex); >- return m_scrollingStateTree->attachNode(nodeType, newNodeID, parentID, childIndex); >+ LOG_WITH_STREAM(Scrolling, stream << "AsyncScrollingCoordinator::createNode " << nodeType << " node " << newNodeID); >+ return m_scrollingStateTree->createUnparentedNode(nodeType, newNodeID); > } > >-void AsyncScrollingCoordinator::detachFromStateTree(ScrollingNodeID nodeID) >+ScrollingNodeID AsyncScrollingCoordinator::insertNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) > { >- m_scrollingStateTree->detachNode(nodeID); >+ LOG_WITH_STREAM(Scrolling, stream << "AsyncScrollingCoordinator::insertNode " << nodeType << " node " << newNodeID << " parent " << parentID << " index " << childIndex); >+ return m_scrollingStateTree->insertNode(nodeType, newNodeID, parentID, childIndex); > } > >-void AsyncScrollingCoordinator::clearStateTree() >+void AsyncScrollingCoordinator::unparentNode(ScrollingNodeID nodeID) >+{ >+ m_scrollingStateTree->unparentNode(nodeID); >+} >+ >+void AsyncScrollingCoordinator::unparentChildrenAndDestroyNode(ScrollingNodeID nodeID) >+{ >+ m_scrollingStateTree->unparentChildrenAndDestroyNode(nodeID); >+} >+ >+void AsyncScrollingCoordinator::detachAndDestroySubtree(ScrollingNodeID nodeID) >+{ >+ m_scrollingStateTree->detachAndDestroySubtree(nodeID); >+} >+ >+void AsyncScrollingCoordinator::clearAllNodes() > { > m_scrollingStateTree->clear(); > } > >+ScrollingNodeID AsyncScrollingCoordinator::parentOfNode(ScrollingNodeID nodeID) const >+{ >+ auto* scrollingNode = m_scrollingStateTree->stateNodeForID(nodeID); >+ if (!scrollingNode) >+ return 0; >+ >+ return scrollingNode->parentNodeID(); >+} >+ > Vector<ScrollingNodeID> AsyncScrollingCoordinator::childrenOfNode(ScrollingNodeID nodeID) const > { > auto* scrollingNode = m_scrollingStateTree->stateNodeForID(nodeID); >@@ -540,7 +565,7 @@ void AsyncScrollingCoordinator::ensureRootStateNodeForFrameView(FrameView& frame > // 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(ScrollingNodeType::MainFrame, frameView.scrollLayerID(), 0, 0); >+ insertNode(ScrollingNodeType::MainFrame, frameView.scrollLayerID(), 0, 0); > } > > void AsyncScrollingCoordinator::setNodeLayers(ScrollingNodeID nodeID, GraphicsLayer* layer, GraphicsLayer* scrolledContentsLayer, GraphicsLayer* counterScrollingLayer, GraphicsLayer* insetClipLayer) >diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h >index 57177edf691b1a1b9c53ce2b599806af728ff045..cc23da8f0be8926a1443ae15394042cf27abed34 100644 >--- a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h >+++ b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h >@@ -97,10 +97,14 @@ private: > > WEBCORE_EXPORT bool requestScrollPositionUpdate(FrameView&, const IntPoint&) 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; >- >+ WEBCORE_EXPORT ScrollingNodeID createNode(ScrollingNodeType, ScrollingNodeID newNodeID) override; >+ WEBCORE_EXPORT ScrollingNodeID insertNode(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) override; >+ WEBCORE_EXPORT void unparentNode(ScrollingNodeID) override; >+ WEBCORE_EXPORT void unparentChildrenAndDestroyNode(ScrollingNodeID) override; >+ WEBCORE_EXPORT void detachAndDestroySubtree(ScrollingNodeID) override; >+ WEBCORE_EXPORT void clearAllNodes() override; >+ >+ WEBCORE_EXPORT ScrollingNodeID parentOfNode(ScrollingNodeID) const override; > WEBCORE_EXPORT Vector<ScrollingNodeID> childrenOfNode(ScrollingNodeID) const override; > > WEBCORE_EXPORT void setNodeLayers(ScrollingNodeID, GraphicsLayer* /*layer*/, GraphicsLayer* /*scrolledContentsLayer*/ = nullptr, GraphicsLayer* /*counterScrollingLayer*/ = nullptr, GraphicsLayer* /*insetClipLayer*/ = nullptr) override; >diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.h b/Source/WebCore/page/scrolling/ScrollingCoordinator.h >index ac976e3bca226ab5b750eb600b07758c60961b1e..6547daab9a903d0b4ee89a28e30664694b26fa35 100644 >--- a/Source/WebCore/page/scrolling/ScrollingCoordinator.h >+++ b/Source/WebCore/page/scrolling/ScrollingCoordinator.h >@@ -168,11 +168,22 @@ public: > 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*/, size_t /*childIndex*/ = notFound) { return newNodeID; } >- virtual Vector<ScrollingNodeID> childrenOfNode(ScrollingNodeID) const { return { }; } > >- virtual void detachFromStateTree(ScrollingNodeID) { } >- virtual void clearStateTree() { } >+ // Create an unparented node. >+ virtual ScrollingNodeID createNode(ScrollingNodeType, ScrollingNodeID newNodeID) { return newNodeID; } >+ // Parent a node in the scrolling tree. This may return a new nodeID if the node type changed. parentID = 0 sets the root node. >+ virtual ScrollingNodeID insertNode(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID /*parentID*/, size_t /*childIndex*/ = notFound) { return newNodeID; } >+ // Node will be unparented, but not destroyed. It's the client's responsibility to either re-parent or destroy this node. >+ virtual void unparentNode(ScrollingNodeID) { } >+ // Node will be destroyed, and its children left unparented. >+ virtual void unparentChildrenAndDestroyNode(ScrollingNodeID) { } >+ // Node will be unparented, and it and its children destroyed. >+ virtual void detachAndDestroySubtree(ScrollingNodeID) { } >+ // Destroy the tree, including both parented and unparented nodes. >+ virtual void clearAllNodes() { } >+ >+ virtual ScrollingNodeID parentOfNode(ScrollingNodeID) const { return 0; } >+ virtual Vector<ScrollingNodeID> childrenOfNode(ScrollingNodeID) const { return { }; } > > virtual void setNodeLayers(ScrollingNodeID, GraphicsLayer* /*layer*/, GraphicsLayer* /*scrolledContentsLayer*/ = nullptr, GraphicsLayer* /*counterScrollingLayer*/ = nullptr, GraphicsLayer* /*insetClipLayer*/ = nullptr) { } > >diff --git a/Source/WebCore/page/scrolling/ScrollingStateNode.cpp b/Source/WebCore/page/scrolling/ScrollingStateNode.cpp >index e10b21a333b8dd50649def53a26b5ba7445470eb..0fa8299511cb8911cabd5029c7911ab00dfadf44 100644 >--- a/Source/WebCore/page/scrolling/ScrollingStateNode.cpp >+++ b/Source/WebCore/page/scrolling/ScrollingStateNode.cpp >@@ -111,6 +111,21 @@ void ScrollingStateNode::insertChild(Ref<ScrollingStateNode>&& childNode, size_t > setPropertyChanged(ChildNodes); > } > >+void ScrollingStateNode::removeFromParent() >+{ >+ if (!m_parent) >+ return; >+ >+ m_parent->removeChild(*this); >+} >+ >+void ScrollingStateNode::removeChild(ScrollingStateNode& childNode) >+{ >+ auto childIndex = indexOfChild(childNode); >+ if (childIndex != notFound) >+ removeChildAtIndex(childIndex); >+} >+ > void ScrollingStateNode::removeChildAtIndex(size_t index) > { > ASSERT(m_children && index < m_children->size()); >diff --git a/Source/WebCore/page/scrolling/ScrollingStateNode.h b/Source/WebCore/page/scrolling/ScrollingStateNode.h >index c28872f1fc2e060b707b1509a9b45de2bd4d6a6f..a0747ec65e645bf54fa4170b8c37c2ab32c19eea 100644 >--- a/Source/WebCore/page/scrolling/ScrollingStateNode.h >+++ b/Source/WebCore/page/scrolling/ScrollingStateNode.h >@@ -240,7 +240,10 @@ public: > void appendChild(Ref<ScrollingStateNode>&&); > void insertChild(Ref<ScrollingStateNode>&&, size_t index); > >+ // Note that node ownership is via the parent, so these functions can trigger node deletion. >+ void removeFromParent(); > void removeChildAtIndex(size_t index); >+ void removeChild(ScrollingStateNode&); > > size_t indexOfChild(ScrollingStateNode&) const; > >diff --git a/Source/WebCore/page/scrolling/ScrollingStateTree.cpp b/Source/WebCore/page/scrolling/ScrollingStateTree.cpp >index b1a26b9ce90e383ad21c316f004a97fdab17fb17..b6076c00a8de10f8417ba26b2736f8c58dca3792 100644 >--- a/Source/WebCore/page/scrolling/ScrollingStateTree.cpp >+++ b/Source/WebCore/page/scrolling/ScrollingStateTree.cpp >@@ -29,24 +29,19 @@ > #if ENABLE(ASYNC_SCROLLING) || USE(COORDINATED_GRAPHICS) > > #include "AsyncScrollingCoordinator.h" >+#include "Logging.h" > #include "ScrollingStateFixedNode.h" > #include "ScrollingStateFrameHostingNode.h" > #include "ScrollingStateFrameScrollingNode.h" > #include "ScrollingStateOverflowScrollingNode.h" > #include "ScrollingStateStickyNode.h" > #include <wtf/text/CString.h> >- >-#ifndef NDEBUG >-#include <stdio.h> >-#endif >+#include <wtf/text/TextStream.h> > > namespace WebCore { > > ScrollingStateTree::ScrollingStateTree(AsyncScrollingCoordinator* scrollingCoordinator) > : m_scrollingCoordinator(scrollingCoordinator) >- , m_hasChangedProperties(false) >- , m_hasNewRootStateNode(false) >- , m_preferredLayerRepresentation(LayerRepresentation::GraphicsLayerRepresentation) > { > } > >@@ -93,8 +88,35 @@ bool ScrollingStateTree::nodeTypeAndParentMatch(ScrollingStateNode& node, Scroll > return node.parent() == parentNode; > } > >-ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) >+ScrollingNodeID ScrollingStateTree::createUnparentedNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID) > { >+ LOG_WITH_STREAM(Scrolling, stream << "ScrollingStateTree " << this << " createUnparentedNode " << newNodeID); >+ >+ if (auto* node = stateNodeForID(newNodeID)) { >+ if (node->nodeType() == nodeType) { >+ // If the node exists and is parented, unparent it. It may already be in m_unparentedNodes. >+ unparentNode(newNodeID); >+ return newNodeID; >+ } >+ >+#if ENABLE(ASYNC_SCROLLING) >+ // If the type has changed, we need to destroy and recreate the node with a new ID. >+ if (nodeType != node->nodeType()) { >+ unparentChildrenAndDestroyNode(newNodeID); >+ newNodeID = m_scrollingCoordinator->uniqueScrollingNodeID(); >+ } >+#endif >+ } >+ >+ auto stateNode = createNode(nodeType, newNodeID); >+ addNode(stateNode.get()); >+ m_unparentedNodes.add(newNodeID, WTFMove(stateNode)); >+ return newNodeID; >+} >+ >+ScrollingNodeID ScrollingStateTree::insertNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) >+{ >+ LOG_WITH_STREAM(Scrolling, stream << "ScrollingStateTree " << this << " insertNode " << newNodeID << " in parent " << parentID << " at " << childIndex); > ASSERT(newNodeID); > > if (auto* node = stateNodeForID(newNodeID)) { >@@ -126,7 +148,7 @@ ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, Scrol > #endif > > // The node is being re-parented. To do that, we'll remove it, and then create a new node. >- removeNodeAndAllDescendants(node, SubframeNodeRemoval::Orphan); >+ unparentNode(newNodeID); > } > > ScrollingStateNode* newNode = nullptr; >@@ -145,13 +167,13 @@ ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, Scrol > return 0; > } > >- if (nodeType == ScrollingNodeType::Subframe && parentID) { >- if (auto orphanedNode = m_orphanedSubframeNodes.take(newNodeID)) { >- newNode = orphanedNode.get(); >+ if (parentID) { >+ if (auto unparentedNode = m_unparentedNodes.take(newNodeID)) { >+ newNode = unparentedNode.get(); > if (childIndex == notFound) >- parent->appendChild(orphanedNode.releaseNonNull()); >+ parent->appendChild(unparentedNode.releaseNonNull()); > else >- parent->insertChild(orphanedNode.releaseNonNull(), childIndex); >+ parent->insertChild(unparentedNode.releaseNonNull(), childIndex); > } > } > >@@ -170,17 +192,57 @@ ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, Scrol > return newNodeID; > } > >-void ScrollingStateTree::detachNode(ScrollingNodeID nodeID) >+void ScrollingStateTree::unparentNode(ScrollingNodeID nodeID) > { > if (!nodeID) > return; > >- // The node may not be found if clearStateTree() was recently called. >+ LOG_WITH_STREAM(Scrolling, stream << "ScrollingStateTree " << this << " unparentNode " << nodeID); >+ >+ // The node may not be found if clear() was recently called. >+ RefPtr<ScrollingStateNode> protectedNode = m_stateNodeMap.get(nodeID); >+ if (!protectedNode) >+ return; >+ >+ protectedNode->removeFromParent(); >+ m_unparentedNodes.add(nodeID, WTFMove(protectedNode)); >+} >+ >+void ScrollingStateTree::unparentChildrenAndDestroyNode(ScrollingNodeID nodeID) >+{ >+ if (!nodeID) >+ return; >+ >+ // The node may not be found if clear() was recently called. >+ RefPtr<ScrollingStateNode> protectedNode = m_stateNodeMap.take(nodeID); >+ if (!protectedNode) >+ return; >+ >+ if (auto* children = protectedNode->children()) { >+ for (auto child : *children) { >+ child->removeFromParent(); >+ m_unparentedNodes.add(child->scrollingNodeID(), WTFMove(child)); >+ } >+ children->clear(); >+ } >+ >+ protectedNode->removeFromParent(); >+ willRemoveNode(protectedNode.get()); >+} >+ >+void ScrollingStateTree::detachAndDestroySubtree(ScrollingNodeID nodeID) >+{ >+ if (!nodeID) >+ return; >+ >+ // The node may not be found if clear() was recently called. > auto* node = m_stateNodeMap.take(nodeID); > if (!node) > return; > >- removeNodeAndAllDescendants(node, SubframeNodeRemoval::Orphan); >+ // If the node was unparented, remove it from m_unparentedNodes (keeping it alive until this function returns). >+ auto unparentedNode = m_unparentedNodes.take(nodeID); >+ removeNodeAndAllDescendants(node); > } > > void ScrollingStateTree::clear() >@@ -189,17 +251,14 @@ void ScrollingStateTree::clear() > removeNodeAndAllDescendants(rootStateNode()); > > m_stateNodeMap.clear(); >- m_orphanedSubframeNodes.clear(); >+ m_unparentedNodes.clear(); > } > > std::unique_ptr<ScrollingStateTree> ScrollingStateTree::commit(LayerRepresentation::Type preferredLayerRepresentation) > { >- if (!m_orphanedSubframeNodes.isEmpty()) { >- // If we still have orphaned subtrees, remove them from m_stateNodeMap since they will be deleted >- // when clearing m_orphanedSubframeNodes. >- for (auto& orphanNode : m_orphanedSubframeNodes.values()) >- recursiveNodeWillBeRemoved(orphanNode.get(), SubframeNodeRemoval::Delete); >- m_orphanedSubframeNodes.clear(); >+ if (!m_unparentedNodes.isEmpty()) { >+ // We expect temporarily to have unparented nodes when committing when connecting across iframe boundaries, but unparented nodes should not stick around for a long time. >+ LOG(Scrolling, "Committing with %u unparented nodes", m_unparentedNodes.size()); > } > > // This function clones and resets the current state tree, but leaves the tree structure intact. >@@ -232,18 +291,18 @@ void ScrollingStateTree::addNode(ScrollingStateNode& node) > m_stateNodeMap.add(node.scrollingNodeID(), &node); > } > >-void ScrollingStateTree::removeNodeAndAllDescendants(ScrollingStateNode* node, SubframeNodeRemoval subframeNodeRemoval) >+void ScrollingStateTree::removeNodeAndAllDescendants(ScrollingStateNode* node) > { > auto* parent = node->parent(); > >- recursiveNodeWillBeRemoved(node, subframeNodeRemoval); >+ recursiveNodeWillBeRemoved(node); > > if (node == m_rootStateNode) > m_rootStateNode = nullptr; > else if (parent) { > ASSERT(parent->children()); > ASSERT(parent->children()->find(node) != notFound); >- if (auto children = parent->children()) { >+ if (auto* children = parent->children()) { > size_t index = children->find(node); > if (index != notFound) > children->remove(index); >@@ -251,19 +310,14 @@ void ScrollingStateTree::removeNodeAndAllDescendants(ScrollingStateNode* node, S > } > } > >-void ScrollingStateTree::recursiveNodeWillBeRemoved(ScrollingStateNode* currNode, SubframeNodeRemoval subframeNodeRemoval) >+void ScrollingStateTree::recursiveNodeWillBeRemoved(ScrollingStateNode* currNode) > { > currNode->setParent(nullptr); >- if (subframeNodeRemoval == SubframeNodeRemoval::Orphan && currNode != m_rootStateNode && currNode->isFrameScrollingNode()) { >- m_orphanedSubframeNodes.add(currNode->scrollingNodeID(), currNode); >- return; >- } >- > willRemoveNode(currNode); > >- if (auto children = currNode->children()) { >+ if (auto* children = currNode->children()) { > for (auto& child : *children) >- recursiveNodeWillBeRemoved(child.get(), subframeNodeRemoval); >+ recursiveNodeWillBeRemoved(child.get()); > } > } > >@@ -302,12 +356,12 @@ void showScrollingStateTree(const WebCore::ScrollingStateTree* tree) > > auto rootNode = tree->rootStateNode(); > if (!rootNode) { >- fprintf(stderr, "Scrolling state tree %p with no root node\n", tree); >+ WTFLogAlways("Scrolling state tree %p with no root node\n", tree); > return; > } > > String output = rootNode->scrollingStateTreeAsText(WebCore::ScrollingStateTreeAsTextBehaviorDebug); >- fprintf(stderr, "%s\n", output.utf8().data()); >+ WTFLogAlways("%s\n", output.utf8().data()); > } > > void showScrollingStateTree(const WebCore::ScrollingStateNode* node) >diff --git a/Source/WebCore/page/scrolling/ScrollingStateTree.h b/Source/WebCore/page/scrolling/ScrollingStateTree.h >index 5875586edcef90ac016ba00e2306bec6c14d5b2f..a7e5f258ca2054a3a671a18b68293518670be9e8 100644 >--- a/Source/WebCore/page/scrolling/ScrollingStateTree.h >+++ b/Source/WebCore/page/scrolling/ScrollingStateTree.h >@@ -51,8 +51,11 @@ public: > ScrollingStateFrameScrollingNode* rootStateNode() const { return m_rootStateNode.get(); } > WEBCORE_EXPORT ScrollingStateNode* stateNodeForID(ScrollingNodeID) const; > >- WEBCORE_EXPORT ScrollingNodeID attachNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID, size_t childIndex); >- void detachNode(ScrollingNodeID); >+ ScrollingNodeID createUnparentedNode(ScrollingNodeType, ScrollingNodeID); >+ WEBCORE_EXPORT ScrollingNodeID insertNode(ScrollingNodeType, ScrollingNodeID, ScrollingNodeID parentID, size_t childIndex); >+ void unparentNode(ScrollingNodeID); >+ void unparentChildrenAndDestroyNode(ScrollingNodeID); >+ void detachAndDestroySubtree(ScrollingNodeID); > void clear(); > > const HashSet<ScrollingNodeID>& removedNodes() const { return m_nodesRemovedSinceLastCommit; } >@@ -66,8 +69,8 @@ public: > > bool hasNewRootStateNode() const { return m_hasNewRootStateNode; } > void setHasNewRootStateNode(bool hasNewRoot) { m_hasNewRootStateNode = hasNewRoot; } >- >- int nodeCount() const { return m_stateNodeMap.size(); } >+ >+ unsigned nodeCount() const { return m_stateNodeMap.size(); } > > typedef HashMap<ScrollingNodeID, ScrollingStateNode*> StateNodeMap; > const StateNodeMap& nodeMap() const { return m_stateNodeMap; } >@@ -83,20 +86,22 @@ private: > > bool nodeTypeAndParentMatch(ScrollingStateNode&, ScrollingNodeType, ScrollingStateNode* parentNode) const; > >- enum class SubframeNodeRemoval { Delete, Orphan }; >- void removeNodeAndAllDescendants(ScrollingStateNode*, SubframeNodeRemoval = SubframeNodeRemoval::Delete); >+ void removeNodeAndAllDescendants(ScrollingStateNode*); > >- void recursiveNodeWillBeRemoved(ScrollingStateNode* currNode, SubframeNodeRemoval); >+ void recursiveNodeWillBeRemoved(ScrollingStateNode* currNode); > void willRemoveNode(ScrollingStateNode*); > > AsyncScrollingCoordinator* m_scrollingCoordinator; >+ // Contains all the nodes we know about (those in the m_rootStateNode tree, and in m_unparentedNodes subtrees). > StateNodeMap m_stateNodeMap; >+ // Owns roots of unparented subtrees. >+ HashMap<ScrollingNodeID, RefPtr<ScrollingStateNode>> m_unparentedNodes; >+ > RefPtr<ScrollingStateFrameScrollingNode> m_rootStateNode; > HashSet<ScrollingNodeID> m_nodesRemovedSinceLastCommit; >- HashMap<ScrollingNodeID, RefPtr<ScrollingStateNode>> m_orphanedSubframeNodes; >- bool m_hasChangedProperties; >- bool m_hasNewRootStateNode; >- LayerRepresentation::Type m_preferredLayerRepresentation; >+ bool m_hasChangedProperties { false }; >+ bool m_hasNewRootStateNode { false }; >+ LayerRepresentation::Type m_preferredLayerRepresentation { LayerRepresentation::GraphicsLayerRepresentation }; > }; > > } // namespace WebCore >diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp >index dacaa3ddda006c9d4fba14c41fef57a23136d11a..818e7ba74dc31c5cf433cd5c58658e50d92a073f 100644 >--- a/Source/WebCore/rendering/RenderLayerBacking.cpp >+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp >@@ -1778,19 +1778,19 @@ void RenderLayerBacking::detachFromScrollingCoordinator(OptionSet<ScrollCoordina > > if (roles.contains(ScrollCoordinationRole::Scrolling) && m_scrollingNodeID) { > LOG(Compositing, "Detaching Scrolling node %" PRIu64, m_scrollingNodeID); >- scrollingCoordinator->detachFromStateTree(m_scrollingNodeID); >+ scrollingCoordinator->unparentChildrenAndDestroyNode(m_scrollingNodeID); > m_scrollingNodeID = 0; > } > > if (roles.contains(ScrollCoordinationRole::Scrolling) && m_frameHostingNodeID) { > LOG(Compositing, "Detaching FrameHosting node %" PRIu64, m_frameHostingNodeID); >- scrollingCoordinator->detachFromStateTree(m_frameHostingNodeID); >+ scrollingCoordinator->unparentChildrenAndDestroyNode(m_frameHostingNodeID); > m_frameHostingNodeID = 0; > } > > if (roles.contains(ScrollCoordinationRole::ViewportConstrained) && m_viewportConstrainedNodeID) { > LOG(Compositing, "Detaching ViewportConstrained node %" PRIu64, m_viewportConstrainedNodeID); >- scrollingCoordinator->detachFromStateTree(m_viewportConstrainedNodeID); >+ scrollingCoordinator->unparentChildrenAndDestroyNode(m_viewportConstrainedNodeID); > m_viewportConstrainedNodeID = 0; > } > } >diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp >index 2e50790faaf5d9c65aa0f9a0eeb142644605a5e6..28c82381964c0e5ee92fe5bf1ccd99be89ec691a 100644 >--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp >+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp >@@ -3788,7 +3788,7 @@ void RenderLayerCompositor::reattachSubframeScrollLayers() > if (!parentNodeID) > continue; > >- scrollingCoordinator->attachToStateTree(child->isMainFrame() ? ScrollingNodeType::MainFrame : ScrollingNodeType::Subframe, frameScrollingNodeID, parentNodeID); >+ scrollingCoordinator->insertNode(child->isMainFrame() ? ScrollingNodeType::MainFrame : ScrollingNodeType::Subframe, frameScrollingNodeID, parentNodeID); > } > } > >@@ -3823,7 +3823,7 @@ ScrollingNodeID RenderLayerCompositor::attachScrollingNode(RenderLayer& layer, S > if (!nodeID) > nodeID = scrollingCoordinator->uniqueScrollingNodeID(); > >- nodeID = scrollingCoordinator->attachToStateTree(nodeType, nodeID, parentNodeID); >+ nodeID = scrollingCoordinator->insertNode(nodeType, nodeID, parentNodeID); > if (!nodeID) > return 0; > >diff --git a/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp b/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp >index e95ca5ba5337d0392841a1113b86e064a19bd983..30221e478631a6359545baed5b7d7266bc5ef9df 100644 >--- a/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp >+++ b/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp >@@ -454,7 +454,7 @@ bool RemoteScrollingCoordinatorTransaction::decode(IPC::Decoder& decoder) > if (!decoder.decode(parentNodeID)) > return false; > >- m_scrollingStateTree->attachNode(nodeType, nodeID, parentNodeID, notFound); // Append new node. >+ m_scrollingStateTree->insertNode(nodeType, nodeID, parentNodeID, notFound); > ScrollingStateNode* newNode = m_scrollingStateTree->stateNodeForID(nodeID); > ASSERT(newNode); > ASSERT(!parentNodeID || newNode->parent());
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
zalan
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193754
: 360248