Title: [142505] trunk/Source/WebCore
- Revision
- 142505
- Author
- simon.fra...@apple.com
- Date
- 2013-02-11 13:28:18 -0800 (Mon, 11 Feb 2013)
Log Message
ScrollingTree node maps keep getting larger
https://bugs.webkit.org/show_bug.cgi?id=109348
Reviewed by Sam Weinig.
When navigating between pages, nodes would get left in the ScrollingTree's
node map, and the ScrollingStateTree's node map, so these would get larger
and larger as you browse.
Simplify map maintenance by clearing the map when setting a new root node
(which happens on the first commit of a new page). Also, don't keep root nodes
around, but create them afresh for each page, which simplifies their ID
management.
This is closer to the original behavior; keeping the root nodes around was
a fix for bug 99668, but we avoid regressing that fix by bailing early
from frameViewLayoutUpdated() if there is no root state node (we'll get
called again anyway).
This now allows state nodeIDs to be purely read-only.
* page/scrolling/ScrollingStateNode.h:
* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::ScrollingStateTree):
(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::clear):
(WebCore::ScrollingStateTree::removeNode):
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::updateTreeFromStateNode):
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::frameViewLayoutUpdated):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (142504 => 142505)
--- trunk/Source/WebCore/ChangeLog 2013-02-11 21:28:12 UTC (rev 142504)
+++ trunk/Source/WebCore/ChangeLog 2013-02-11 21:28:18 UTC (rev 142505)
@@ -1,5 +1,39 @@
2013-02-11 Simon Fraser <simon.fra...@apple.com>
+ ScrollingTree node maps keep getting larger
+ https://bugs.webkit.org/show_bug.cgi?id=109348
+
+ Reviewed by Sam Weinig.
+
+ When navigating between pages, nodes would get left in the ScrollingTree's
+ node map, and the ScrollingStateTree's node map, so these would get larger
+ and larger as you browse.
+
+ Simplify map maintenance by clearing the map when setting a new root node
+ (which happens on the first commit of a new page). Also, don't keep root nodes
+ around, but create them afresh for each page, which simplifies their ID
+ management.
+
+ This is closer to the original behavior; keeping the root nodes around was
+ a fix for bug 99668, but we avoid regressing that fix by bailing early
+ from frameViewLayoutUpdated() if there is no root state node (we'll get
+ called again anyway).
+
+ This now allows state nodeIDs to be purely read-only.
+
+ * page/scrolling/ScrollingStateNode.h:
+ * page/scrolling/ScrollingStateTree.cpp:
+ (WebCore::ScrollingStateTree::ScrollingStateTree):
+ (WebCore::ScrollingStateTree::attachNode):
+ (WebCore::ScrollingStateTree::clear):
+ (WebCore::ScrollingStateTree::removeNode):
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::updateTreeFromStateNode):
+ * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+ (WebCore::ScrollingCoordinatorMac::frameViewLayoutUpdated):
+
+2013-02-11 Simon Fraser <simon.fra...@apple.com>
+
Move m_stateNodeMap from ScrollingCoordinatorMac to ScrollingStateTree
https://bugs.webkit.org/show_bug.cgi?id=109361
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h (142504 => 142505)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2013-02-11 21:28:12 UTC (rev 142504)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateNode.h 2013-02-11 21:28:18 UTC (rev 142505)
@@ -79,7 +79,6 @@
void setScrollingStateTree(ScrollingStateTree* tree) { m_scrollingStateTree = tree; }
ScrollingNodeID scrollingNodeID() const { return m_nodeID; }
- void setScrollingNodeID(ScrollingNodeID nodeID) { m_nodeID = nodeID; }
ScrollingStateNode* parent() const { return m_parent; }
void setParent(ScrollingStateNode* parent) { m_parent = parent; }
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (142504 => 142505)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2013-02-11 21:28:12 UTC (rev 142504)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp 2013-02-11 21:28:18 UTC (rev 142505)
@@ -41,8 +41,7 @@
}
ScrollingStateTree::ScrollingStateTree()
- : m_rootStateNode(ScrollingStateScrollingNode::create(this, 0))
- , m_hasChangedProperties(false)
+ : m_hasChangedProperties(false)
{
}
@@ -70,7 +69,7 @@
// If we're resetting the root node, we should clear the HashMap and destroy the current children.
clear();
- rootStateNode()->setScrollingNodeID(newNodeID);
+ setRootStateNode(ScrollingStateScrollingNode::create(this, newNodeID));
newNode = rootStateNode();
} else {
ScrollingStateNode* parent = stateNodeForID(parentID);
@@ -120,6 +119,7 @@
void ScrollingStateTree::clear()
{
removeNode(rootStateNode());
+ m_stateNodeMap.clear();
}
PassOwnPtr<ScrollingStateTree> ScrollingStateTree::commit()
@@ -140,6 +140,15 @@
void ScrollingStateTree::removeNode(ScrollingStateNode* node)
{
+ if (!node)
+ return;
+
+ if (node == m_rootStateNode) {
+ didRemoveNode(node->scrollingNodeID());
+ m_rootStateNode = nullptr;
+ return;
+ }
+
ASSERT(m_rootStateNode);
m_rootStateNode->removeChild(node);
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (142504 => 142505)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2013-02-11 21:28:12 UTC (rev 142504)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2013-02-11 21:28:18 UTC (rev 142505)
@@ -159,10 +159,10 @@
// root node.
ScrollingNodeID nodeID = stateNode->scrollingNodeID();
if (!stateNode->parent()) {
- // This is the root node.
- if (!m_rootNode)
- m_rootNode = ScrollingTreeScrollingNode::create(this, nodeID);
+ // This is the root node. Nuke the node map.
+ m_nodeMap.clear();
+ m_rootNode = ScrollingTreeScrollingNode::create(this, nodeID);
m_nodeMap.set(nodeID, m_rootNode.get());
m_rootNode->update(stateNode);
} else {
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (142504 => 142505)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2013-02-11 21:28:12 UTC (rev 142504)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2013-02-11 21:28:18 UTC (rev 142505)
@@ -107,6 +107,10 @@
ASSERT(isMainThread());
ASSERT(m_page);
+ // If there isn't a root node yet, don't do anything. We'll be called again after creating one.
+ if (!m_scrollingStateTree->rootStateNode())
+ return;
+
// Compute the region of the page that we can't do fast scrolling for. This currently includes
// all scrollable areas, such as subframes, overflow divs and list boxes. We need to do this even if the
// frame view whose layout was updated is not the main frame.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
