Diff
Modified: trunk/Source/WebCore/ChangeLog (240550 => 240551)
--- trunk/Source/WebCore/ChangeLog 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/ChangeLog 2019-01-27 02:10:22 UTC (rev 240551)
@@ -1,3 +1,92 @@
+2019-01-26 Simon Fraser <[email protected]>
+
+ Allow scrolling tree nodes to exist in a detached state
+ https://bugs.webkit.org/show_bug.cgi?id=193754
+
+ Reviewed by Zalan Bujtas.
+
+ 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 Devin Rousso <[email protected]>
Web Inspector: provide a way to edit the user agent of a remote target
Modified: trunk/Source/WebCore/dom/Document.cpp (240550 => 240551)
--- trunk/Source/WebCore/dom/Document.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/dom/Document.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -5200,7 +5200,7 @@
if (page && m_frame->isMainFrame()) {
v->resetScrollbarsAndClearContentsSize();
if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
- scrollingCoordinator->clearStateTree();
+ scrollingCoordinator->clearAllNodes();
}
}
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -486,22 +486,47 @@
}
}
-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 @@
// 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)
Modified: trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h 2019-01-27 02:10:22 UTC (rev 240551)
@@ -97,10 +97,14 @@
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;
Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2019-01-27 02:10:22 UTC (rev 240551)
@@ -168,12 +168,23 @@
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; }
+
+ // 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 detachFromStateTree(ScrollingNodeID) { }
- virtual void clearStateTree() { }
-
virtual void setNodeLayers(ScrollingNodeID, GraphicsLayer* /*layer*/, GraphicsLayer* /*scrolledContentsLayer*/ = nullptr, GraphicsLayer* /*counterScrollingLayer*/ = nullptr, GraphicsLayer* /*insetClipLayer*/ = nullptr) { }
struct ScrollingGeometry {
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -111,6 +111,21 @@
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());
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2019-01-27 02:10:22 UTC (rev 240551)
@@ -240,7 +240,10 @@
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;
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -29,6 +29,7 @@
#if ENABLE(ASYNC_SCROLLING) || USE(COORDINATED_GRAPHICS)
#include "AsyncScrollingCoordinator.h"
+#include "Logging.h"
#include "ScrollingStateFixedNode.h"
#include "ScrollingStateFrameHostingNode.h"
#include "ScrollingStateFrameScrollingNode.h"
@@ -35,18 +36,12 @@
#include "ScrollingStateOverflowScrollingNode.h"
#include "ScrollingStateStickyNode.h"
#include <wtf/text/CString.h>
+#include <wtf/text/TextStream.h>
-#ifndef NDEBUG
-#include <stdio.h>
-#endif
-
namespace WebCore {
ScrollingStateTree::ScrollingStateTree(AsyncScrollingCoordinator* scrollingCoordinator)
: m_scrollingCoordinator(scrollingCoordinator)
- , m_hasChangedProperties(false)
- , m_hasNewRootStateNode(false)
- , m_preferredLayerRepresentation(LayerRepresentation::GraphicsLayerRepresentation)
{
}
@@ -93,8 +88,35 @@
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 @@
#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 @@
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 @@
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 @@
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,11 +291,11 @@
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;
@@ -243,7 +302,7 @@
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::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 @@
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)
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (240550 => 240551)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2019-01-27 02:10:22 UTC (rev 240551)
@@ -51,8 +51,11 @@
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,9 +69,9 @@
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 @@
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
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (240550 => 240551)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -1778,19 +1778,19 @@
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;
}
}
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (240550 => 240551)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -3788,7 +3788,7 @@
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 @@
if (!nodeID)
nodeID = scrollingCoordinator->uniqueScrollingNodeID();
- nodeID = scrollingCoordinator->attachToStateTree(nodeType, nodeID, parentNodeID);
+ nodeID = scrollingCoordinator->insertNode(nodeType, nodeID, parentNodeID);
if (!nodeID)
return 0;
Modified: trunk/Source/WebKit/ChangeLog (240550 => 240551)
--- trunk/Source/WebKit/ChangeLog 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebKit/ChangeLog 2019-01-27 02:10:22 UTC (rev 240551)
@@ -1,3 +1,13 @@
+2019-01-26 Simon Fraser <[email protected]>
+
+ Allow scrolling tree nodes to exist in a detached state
+ https://bugs.webkit.org/show_bug.cgi?id=193754
+
+ Reviewed by Zalan Bujtas.
+
+ * Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
+ (WebKit::RemoteScrollingCoordinatorTransaction::decode):
+
2019-01-25 Tim Horton <[email protected]>
REGRESSION (r238818): Snapshot is removed too late after swiping back on Twitter
Modified: trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp (240550 => 240551)
--- trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp 2019-01-26 23:54:29 UTC (rev 240550)
+++ trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp 2019-01-27 02:10:22 UTC (rev 240551)
@@ -454,7 +454,7 @@
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());
