Title: [131238] trunk/Source/WebCore
- Revision
- 131238
- Author
- bda...@apple.com
- Date
- 2012-10-12 16:51:46 -0700 (Fri, 12 Oct 2012)
Log Message
https://bugs.webkit.org/show_bug.cgi?id=99211
When ScrollingStateNodes are destroyed, they should be removed
ScrollingCoordinator's HashMap
Reviewed by Sam Weinig.
This patch adds a new member variable to ScrollingStateTree. It's a
Vector of ScrollingNodeIDs. It will contain the IDs of nodes that
have been removed from the tree since the last time the tree was
committed.
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::ScrollingStateTree):
When we do commit, copy the Vector over into the cloned tree, and
then clear our own Vector.
(WebCore::ScrollingStateTree::commit):
Call didRemoveNode().
(WebCore::ScrollingStateTree::removeNode):
Append the removed node's id to the vector.
(WebCore::ScrollingStateTree::didRemoveNode):
(WebCore):
* page/scrolling/ScrollingStateTree.h:
(ScrollingStateTree):
Call didRemoveNode().
* page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::removeChild):
Fix the FIXME!
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::detachFromStateTree):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (131237 => 131238)
--- trunk/Source/WebCore/ChangeLog 2012-10-12 23:49:56 UTC (rev 131237)
+++ trunk/Source/WebCore/ChangeLog 2012-10-12 23:51:46 UTC (rev 131238)
@@ -1,3 +1,39 @@
+2012-10-12 Beth Dakin <bda...@apple.com>
+
+ https://bugs.webkit.org/show_bug.cgi?id=99211
+ When ScrollingStateNodes are destroyed, they should be removed
+ ScrollingCoordinator's HashMap
+
+ Reviewed by Sam Weinig.
+
+ This patch adds a new member variable to ScrollingStateTree. It's a
+ Vector of ScrollingNodeIDs. It will contain the IDs of nodes that
+ have been removed from the tree since the last time the tree was
+ committed.
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::ScrollingStateTree):
+
+ When we do commit, copy the Vector over into the cloned tree, and
+ then clear our own Vector.
+ (WebCore::ScrollingStateTree::commit):
+
+ Call didRemoveNode().
+ (WebCore::ScrollingStateTree::removeNode):
+
+ Append the removed node's id to the vector.
+ (WebCore::ScrollingStateTree::didRemoveNode):
+ (WebCore):
+ * page/scrolling/ScrollingStateTree.h:
+ (ScrollingStateTree):
+
+ Call didRemoveNode().
+ * page/scrolling/ScrollingStateNode.cpp:
+ (WebCore::ScrollingStateNode::removeChild):
+
+ Fix the FIXME!
+ * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+ (WebCore::ScrollingCoordinatorMac::detachFromStateTree):
+
2012-10-12 Brady Eidson <beid...@apple.com>
Setup basic NetworkProcess messaging and initialization.
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp (131237 => 131238)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2012-10-12 23:49:56 UTC (rev 131237)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.cpp 2012-10-12 23:51:46 UTC (rev 131238)
@@ -82,6 +82,7 @@
return;
if (size_t index = m_children->find(node)) {
+ m_scrollingStateTree->didRemoveNode(node->scrollingNodeID());
m_children->remove(index);
return;
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (131237 => 131238)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2012-10-12 23:49:56 UTC (rev 131237)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2012-10-12 23:51:46 UTC (rev 131238)
@@ -46,14 +46,18 @@
PassOwnPtr<ScrollingStateTree> ScrollingStateTree::commit()
{
- OwnPtr<ScrollingStateTree> treeState = ScrollingStateTree::create();
- treeState->setRootStateNode(static_pointer_cast<ScrollingStateScrollingNode>(m_rootStateNode->cloneAndResetNode()));
+ // This function clones and resets the current state tree, but leaves the tree structure intact.
+ OwnPtr<ScrollingStateTree> treeStateClone = ScrollingStateTree::create();
+ treeStateClone->setRootStateNode(static_pointer_cast<ScrollingStateScrollingNode>(m_rootStateNode->cloneAndResetNode()));
+ // Copy the IDs of the nodes that have been removed since the last commit into the clone.
+ treeStateClone->m_nodesRemovedSinceLastCommit.swap(m_nodesRemovedSinceLastCommit);
+
// Now the clone tree has changed properties, and the original tree does not.
- treeState->setHasChangedProperties(true);
+ treeStateClone->m_hasChangedProperties = true;
m_hasChangedProperties = false;
- return treeState.release();
+ return treeStateClone.release();
}
void ScrollingStateTree::removeNode(ScrollingStateNode* node)
@@ -61,6 +65,7 @@
ASSERT(m_rootStateNode);
if (node == m_rootStateNode) {
+ didRemoveNode(m_rootStateNode->scrollingNodeID());
m_rootStateNode = 0;
return;
}
@@ -68,6 +73,11 @@
m_rootStateNode->removeChild(node);
}
+void ScrollingStateTree::didRemoveNode(ScrollingNodeID nodeID)
+{
+ m_nodesRemovedSinceLastCommit.append(nodeID);
+}
+
void ScrollingStateTree::rootLayerDidChange()
{
// If the root layer has changed, then destroyed and re-created the root state node. That means that the
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (131237 => 131238)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2012-10-12 23:49:56 UTC (rev 131237)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h 2012-10-12 23:51:46 UTC (rev 131238)
@@ -61,6 +61,8 @@
PassOwnPtr<ScrollingStateTree> commit();
void removeNode(ScrollingStateNode*);
+ void didRemoveNode(ScrollingNodeID);
+ const Vector<ScrollingNodeID>& removedNodes() const { return m_nodesRemovedSinceLastCommit; }
void rootLayerDidChange();
@@ -70,7 +72,10 @@
private:
ScrollingStateTree();
+ PassOwnPtr<ScrollingStateTree> clone();
+
OwnPtr<ScrollingStateScrollingNode> m_rootStateNode;
+ Vector<ScrollingNodeID> m_nodesRemovedSinceLastCommit;
bool m_hasChangedProperties;
};
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (131237 => 131238)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2012-10-12 23:49:56 UTC (rev 131237)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2012-10-12 23:51:46 UTC (rev 131238)
@@ -258,10 +258,14 @@
return;
ScrollingStateNode* node = m_stateNodeMap.take(scrollLayerID);
+ m_scrollingStateTree->removeNode(node);
- // FIXME: removeNode() will destroy children, and those children might still be in the HashMap.
- // This will be a big problem once there are actually children in the tree.
- m_scrollingStateTree->removeNode(node);
+ // ScrollingStateTree::removeNode() will destroy children, so we have to make sure we remove those children
+ // from the HashMap.
+ const Vector<ScrollingNodeID>& removedNodes = m_scrollingStateTree->removedNodes();
+ size_t size = removedNodes.size();
+ for (size_t i = 0; i < size; ++i)
+ m_stateNodeMap.remove(removedNodes[i]);
}
void ScrollingCoordinatorMac::clearStateTree()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
