<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[170197] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/170197">170197</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2014-06-20 12:14:50 -0700 (Fri, 20 Jun 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Handle scrolling tree modifications which remove intermediate nodes
https://bugs.webkit.org/show_bug.cgi?id=134082
Reviewed by Tim Horton.
When updating the scrolling tree from the state tree, we failed to maintain
the children arrays correctly. Fix by removing all children on scrolling nodes,
and allowing the calls on children to add them back. A temporary hash map
keeps the nodes alive.
The state tree's m_nodesRemovedSinceLastCommit was also made into a HashSet,
to make it easier to handle removal followed by re-insertion.
Source/WebCore:
* WebCore.exp.in:
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::attachNode): If a node is (possibly re-)added,
remove it from m_nodesRemovedSinceLastCommit.remove.
(WebCore::ScrollingStateTree::willRemoveNode):
(WebCore::ScrollingStateTree::setRemovedNodes):
* page/scrolling/ScrollingStateTree.h:
(WebCore::ScrollingStateTree::removedNodes):
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::commitNewTreeState):
(WebCore::ScrollingTree::updateTreeFromStateNode): Clean up to have only one call
to updateBeforeChildren(), and remove all children from the scrolling node
before visiting state children.
(WebCore::ScrollingTree::removeDestroyedNodes): It was very wrong to assume
that all non-root nodes were parented in the root! Now we don't need to
remove from the parent anyway.
* page/scrolling/ScrollingTree.h:
* page/scrolling/ScrollingTreeNode.h:
(WebCore::ScrollingTreeNode::children):
Source/WebKit2:
* Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:
(WebKit::RemoteScrollingCoordinatorTransaction::decode):
(WebKit::RemoteScrollingTreeTextStream::dump):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreWebCoreexpin">trunk/Source/WebCore/WebCore.exp.in</a></li>
<li><a href="#trunkSourceWebCorepagescrollingScrollingStateTreecpp">trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp</a></li>
<li><a href="#trunkSourceWebCorepagescrollingScrollingStateTreeh">trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h</a></li>
<li><a href="#trunkSourceWebCorepagescrollingScrollingTreecpp">trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp</a></li>
<li><a href="#trunkSourceWebCorepagescrollingScrollingTreeh">trunk/Source/WebCore/page/scrolling/ScrollingTree.h</a></li>
<li><a href="#trunkSourceWebCorepagescrollingScrollingTreeNodeh">trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedScrollingRemoteScrollingCoordinatorTransactioncpp">trunk/Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/ChangeLog 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -1,5 +1,40 @@
</span><span class="cx"> 2014-06-19 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><ins>+ Handle scrolling tree modifications which remove intermediate nodes
+ https://bugs.webkit.org/show_bug.cgi?id=134082
+
+ Reviewed by Tim Horton.
+
+ When updating the scrolling tree from the state tree, we failed to maintain
+ the children arrays correctly. Fix by removing all children on scrolling nodes,
+ and allowing the calls on children to add them back. A temporary hash map
+ keeps the nodes alive.
+
+ The state tree's m_nodesRemovedSinceLastCommit was also made into a HashSet,
+ to make it easier to handle removal followed by re-insertion.
+
+ * WebCore.exp.in:
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::attachNode): If a node is (possibly re-)added,
+ remove it from m_nodesRemovedSinceLastCommit.remove.
+ (WebCore::ScrollingStateTree::willRemoveNode):
+ (WebCore::ScrollingStateTree::setRemovedNodes):
+ * page/scrolling/ScrollingStateTree.h:
+ (WebCore::ScrollingStateTree::removedNodes):
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::commitNewTreeState):
+ (WebCore::ScrollingTree::updateTreeFromStateNode): Clean up to have only one call
+ to updateBeforeChildren(), and remove all children from the scrolling node
+ before visiting state children.
+ (WebCore::ScrollingTree::removeDestroyedNodes): It was very wrong to assume
+ that all non-root nodes were parented in the root! Now we don't need to
+ remove from the parent anyway.
+ * page/scrolling/ScrollingTree.h:
+ * page/scrolling/ScrollingTreeNode.h:
+ (WebCore::ScrollingTreeNode::children):
+
+2014-06-19 Simon Fraser <simon.fraser@apple.com>
+
</ins><span class="cx"> Make ScrollingTreeNodes refounted, for easier tree reconfiguration
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=134075
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreWebCoreexpin"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/WebCore.exp.in (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/WebCore.exp.in 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/WebCore.exp.in 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -2835,7 +2835,7 @@
</span><span class="cx"> __ZN7WebCore18ScrollingStateNode8setLayerERKNS_19LayerRepresentationE
</span><span class="cx"> __ZN7WebCore18ScrollingStateTree10attachNodeENS_17ScrollingNodeTypeEyy
</span><span class="cx"> __ZN7WebCore18ScrollingStateTree14stateNodeForIDEy
</span><del>-__ZN7WebCore18ScrollingStateTree15setRemovedNodesEN3WTF6VectorIyLm0ENS1_15CrashOnOverflowEEE
</del><ins>+__ZN7WebCore18ScrollingStateTree15setRemovedNodesEN3WTF7HashSetIyNS1_7IntHashIyEENS1_10HashTraitsIyEEEE
</ins><span class="cx"> __ZN7WebCore18ScrollingStateTree23setHasChangedPropertiesEb
</span><span class="cx"> __ZN7WebCore18ScrollingStateTree6commitENS_19LayerRepresentation4TypeE
</span><span class="cx"> __ZN7WebCore18ScrollingStateTree6createEPNS_25AsyncScrollingCoordinatorE
</span></span></pre></div>
<a id="trunkSourceWebCorepagescrollingScrollingStateTreecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -70,7 +70,6 @@
</span><span class="cx"> ScrollingNodeID ScrollingStateTree::attachNode(ScrollingNodeType nodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(newNodeID);
</span><del>-
</del><span class="cx"> if (ScrollingStateNode* node = stateNodeForID(newNodeID)) {
</span><span class="cx"> if (!parentID)
</span><span class="cx"> return newNodeID;
</span><span class="lines">@@ -128,6 +127,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> m_stateNodeMap.set(newNodeID, newNode);
</span><ins>+ m_nodesRemovedSinceLastCommit.remove(newNodeID);
</ins><span class="cx"> return newNodeID;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -209,12 +209,12 @@
</span><span class="cx">
</span><span class="cx"> void ScrollingStateTree::willRemoveNode(ScrollingStateNode* node)
</span><span class="cx"> {
</span><del>- m_nodesRemovedSinceLastCommit.append(node->scrollingNodeID());
</del><ins>+ m_nodesRemovedSinceLastCommit.add(node->scrollingNodeID());
</ins><span class="cx"> m_stateNodeMap.remove(node->scrollingNodeID());
</span><span class="cx"> setHasChangedProperties();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void ScrollingStateTree::setRemovedNodes(Vector<ScrollingNodeID> nodes)
</del><ins>+void ScrollingStateTree::setRemovedNodes(HashSet<ScrollingNodeID> nodes)
</ins><span class="cx"> {
</span><span class="cx"> m_nodesRemovedSinceLastCommit = std::move(nodes);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorepagescrollingScrollingStateTreeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -56,8 +56,8 @@
</span><span class="cx"> void detachNode(ScrollingNodeID);
</span><span class="cx"> void clear();
</span><span class="cx">
</span><del>- const Vector<ScrollingNodeID>& removedNodes() const { return m_nodesRemovedSinceLastCommit; }
- void setRemovedNodes(Vector<ScrollingNodeID>);
</del><ins>+ const HashSet<ScrollingNodeID>& removedNodes() const { return m_nodesRemovedSinceLastCommit; }
+ void setRemovedNodes(HashSet<ScrollingNodeID>);
</ins><span class="cx">
</span><span class="cx"> // Copies the current tree state and clears the changed properties mask in the original.
</span><span class="cx"> PassOwnPtr<ScrollingStateTree> commit(LayerRepresentation::Type preferredLayerRepresentation);
</span><span class="lines">@@ -89,7 +89,7 @@
</span><span class="cx"> AsyncScrollingCoordinator* m_scrollingCoordinator;
</span><span class="cx"> StateNodeMap m_stateNodeMap;
</span><span class="cx"> RefPtr<ScrollingStateFrameScrollingNode> m_rootStateNode;
</span><del>- Vector<ScrollingNodeID> m_nodesRemovedSinceLastCommit;
</del><ins>+ HashSet<ScrollingNodeID> m_nodesRemovedSinceLastCommit;
</ins><span class="cx"> bool m_hasChangedProperties;
</span><span class="cx"> bool m_hasNewRootStateNode;
</span><span class="cx"> LayerRepresentation::Type m_preferredLayerRepresentation;
</span></span></pre></div>
<a id="trunkSourceWebCorepagescrollingScrollingTreecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -149,11 +149,15 @@
</span><span class="cx"> bool scrollRequestIsProgammatic = rootNode ? rootNode->requestedScrollPositionRepresentsProgrammaticScroll() : false;
</span><span class="cx"> TemporaryChange<bool> changeHandlingProgrammaticScroll(m_isHandlingProgrammaticScroll, scrollRequestIsProgammatic);
</span><span class="cx">
</span><ins>+ {
+ OrphanScrollingNodeMap orphanNodes;
+ updateTreeFromStateNode(rootNode, orphanNodes);
+ }
+
</ins><span class="cx"> removeDestroyedNodes(*scrollingStateTree);
</span><del>- updateTreeFromStateNode(rootNode);
</del><span class="cx"> }
</span><span class="cx">
</span><del>-void ScrollingTree::updateTreeFromStateNode(const ScrollingStateNode* stateNode)
</del><ins>+void ScrollingTree::updateTreeFromStateNode(const ScrollingStateNode* stateNode, OrphanScrollingNodeMap& orphanNodes)
</ins><span class="cx"> {
</span><span class="cx"> if (!stateNode) {
</span><span class="cx"> m_nodeMap.clear();
</span><span class="lines">@@ -161,46 +165,52 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // This fuction recurses through the ScrollingStateTree and updates the corresponding ScrollingTreeNodes.
- // Find the ScrollingTreeNode associated with the current stateNode using the shared ID and our HashMap.
- ScrollingTreeNodeMap::const_iterator it = m_nodeMap.find(stateNode->scrollingNodeID());
</del><ins>+ ScrollingNodeID nodeID = stateNode->scrollingNodeID();
+ ScrollingNodeID parentNodeID = stateNode->parentNodeID();
</ins><span class="cx">
</span><del>- ScrollingTreeNode* node;
- if (it != m_nodeMap.end()) {
</del><ins>+ auto it = m_nodeMap.find(nodeID);
+
+ RefPtr<ScrollingTreeNode> node;
+ if (it != m_nodeMap.end())
</ins><span class="cx"> node = it->value;
</span><del>- node->updateBeforeChildren(*stateNode);
- } else {
- // If the node isn't found, it's either new and needs to be added to the tree, or there is a new ID for our
- // root node.
- ScrollingNodeID nodeID = stateNode->scrollingNodeID();
- if (!stateNode->parent()) {
- // This is the root node. Nuke the node map.
</del><ins>+ else {
+ node = createNode(stateNode->nodeType(), nodeID);
+ if (!parentNodeID) {
+ // This is the root node. Clear the node map.
+ ASSERT(stateNode->nodeType() == FrameScrollingNode);
+ m_rootNode = node;
</ins><span class="cx"> m_nodeMap.clear();
</span><ins>+ }
+ m_nodeMap.set(nodeID, node.get());
+ }
</ins><span class="cx">
</span><del>- m_rootNode = createNode(FrameScrollingNode, nodeID);
- m_nodeMap.set(nodeID, m_rootNode.get());
- m_rootNode->updateBeforeChildren(*stateNode);
- node = m_rootNode.get();
- } else {
- RefPtr<ScrollingTreeNode> newNode = createNode(stateNode->nodeType(), nodeID);
- node = newNode.get();
- m_nodeMap.set(nodeID, node);
- ScrollingTreeNodeMap::const_iterator it = m_nodeMap.find(stateNode->parent()->scrollingNodeID());
- ASSERT(it != m_nodeMap.end());
- if (it != m_nodeMap.end()) {
- ScrollingTreeNode* parent = it->value;
- newNode->setParent(parent);
- parent->appendChild(newNode.release());
- }
- node->updateBeforeChildren(*stateNode);
</del><ins>+ if (parentNodeID) {
+ auto parentIt = m_nodeMap.find(parentNodeID);
+ ASSERT(parentIt != m_nodeMap.end());
+ if (parentIt != m_nodeMap.end()) {
+ ScrollingTreeNode* parent = parentIt->value;
+ node->setParent(parent);
+ parent->appendChild(node);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ node->updateBeforeChildren(*stateNode);
+
+ // Move all children into the orphanNodes map. Live ones will get added back as we recurse over children.
+ if (auto nodeChildren = node->children()) {
+ for (auto& childScrollingNode : *nodeChildren) {
+ childScrollingNode->setParent(nullptr);
+ orphanNodes.add(childScrollingNode->scrollingNodeID(), childScrollingNode);
+ }
+ nodeChildren->clear();
+ }
+
</ins><span class="cx"> // Now update the children if we have any.
</span><span class="cx"> if (auto children = stateNode->children()) {
</span><span class="cx"> for (auto& child : *children)
</span><del>- updateTreeFromStateNode(child.get());
</del><ins>+ updateTreeFromStateNode(child.get(), orphanNodes);
</ins><span class="cx"> }
</span><ins>+
</ins><span class="cx"> node->updateAfterChildren(*stateNode);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -214,10 +224,8 @@
</span><span class="cx"> if (node->scrollingNodeID() == m_latchedNode)
</span><span class="cx"> clearLatchedNode();
</span><span class="cx">
</span><del>- // Never destroy the root node. There will be a new root node in the state tree, and we will
- // associate it with our existing root node in updateTreeFromStateNode().
- if (node->parent())
- m_rootNode->removeChild(node);
</del><ins>+ // We should have unparented this node already via updateTreeFromStateNode().
+ ASSERT(!node->parent());
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorepagescrollingScrollingTreeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -131,7 +131,9 @@
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> void removeDestroyedNodes(const ScrollingStateTree&);
</span><del>- void updateTreeFromStateNode(const ScrollingStateNode*);
</del><ins>+
+ typedef HashMap<ScrollingNodeID, RefPtr<ScrollingTreeNode>> OrphanScrollingNodeMap;
+ void updateTreeFromStateNode(const ScrollingStateNode*, OrphanScrollingNodeMap&);
</ins><span class="cx">
</span><span class="cx"> virtual PassRefPtr<ScrollingTreeNode> createNode(ScrollingNodeType, ScrollingNodeID) = 0;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorepagescrollingScrollingTreeNodeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -60,6 +60,9 @@
</span><span class="cx"> ScrollingTreeNode* parent() const { return m_parent; }
</span><span class="cx"> void setParent(ScrollingTreeNode* parent) { m_parent = parent; }
</span><span class="cx">
</span><ins>+ typedef Vector<RefPtr<ScrollingTreeNode>> ScrollingTreeChildrenVector;
+ ScrollingTreeChildrenVector* children() { return m_children.get(); }
+
</ins><span class="cx"> void appendChild(PassRefPtr<ScrollingTreeNode>);
</span><span class="cx"> void removeChild(ScrollingTreeNode*);
</span><span class="cx">
</span><span class="lines">@@ -67,7 +70,6 @@
</span><span class="cx"> ScrollingTreeNode(ScrollingTree&, ScrollingNodeType, ScrollingNodeID);
</span><span class="cx"> ScrollingTree& scrollingTree() const { return m_scrollingTree; }
</span><span class="cx">
</span><del>- typedef Vector<RefPtr<ScrollingTreeNode>> ScrollingTreeChildrenVector;
</del><span class="cx"> OwnPtr<ScrollingTreeChildrenVector> m_children;
</span><span class="cx">
</span><span class="cx"> private:
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebKit2/ChangeLog 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -1,5 +1,24 @@
</span><span class="cx"> 2014-06-19 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><ins>+ Handle scrolling tree modifications which remove intermediate nodes
+ https://bugs.webkit.org/show_bug.cgi?id=134082
+
+ Reviewed by Tim Horton.
+
+ When updating the scrolling tree from the state tree, we failed to maintain
+ the children arrays correctly. Fix by removing all children on scrolling nodes,
+ and allowing the calls on children to add them back. A temporary hash map
+ keeps the nodes alive.
+
+ The state tree's m_nodesRemovedSinceLastCommit was also made into a HashSet,
+ to make it easier to handle removal followed by re-insertion.
+
+ * Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:
+ (WebKit::RemoteScrollingCoordinatorTransaction::decode):
+ (WebKit::RemoteScrollingTreeTextStream::dump):
+
+2014-06-19 Simon Fraser <simon.fraser@apple.com>
+
</ins><span class="cx"> Make ScrollingTreeNodes refounted, for easier tree reconfiguration
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=134075
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedScrollingRemoteScrollingCoordinatorTransactioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp (170196 => 170197)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp 2014-06-20 19:14:47 UTC (rev 170196)
+++ trunk/Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp 2014-06-20 19:14:50 UTC (rev 170197)
</span><span class="lines">@@ -424,7 +424,7 @@
</span><span class="cx"> m_scrollingStateTree->setHasNewRootStateNode(hasNewRootNode);
</span><span class="cx">
</span><span class="cx"> // Removed nodes
</span><del>- Vector<ScrollingNodeID> removedNodes;
</del><ins>+ HashSet<ScrollingNodeID> removedNodes;
</ins><span class="cx"> if (!decoder.decode(removedNodes))
</span><span class="cx"> return false;
</span><span class="cx">
</span><span class="lines">@@ -683,8 +683,11 @@
</span><span class="cx"> if (stateTree.rootStateNode())
</span><span class="cx"> recursiveDumpNodes(*stateTree.rootStateNode(), changedPropertiesOnly);
</span><span class="cx">
</span><del>- if (!stateTree.removedNodes().isEmpty())
- dumpProperty<Vector<ScrollingNodeID>>(ts, "removed-nodes", stateTree.removedNodes());
</del><ins>+ if (!stateTree.removedNodes().isEmpty()) {
+ Vector<ScrollingNodeID> removedNodes;
+ copyToVector(stateTree.removedNodes(), removedNodes);
+ dumpProperty<Vector<ScrollingNodeID>>(ts, "removed-nodes", removedNodes);
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> WTF::CString RemoteScrollingCoordinatorTransaction::description() const
</span></span></pre>
</div>
</div>
</body>
</html>