Changeset 84701 in webkit

Timestamp:

Apr 22, 2011 4:11:45 PM (13 years ago)

Author:

msaboff@apple.com

Message:

2011-04-22 Michael Saboff <​msaboff@apple.com>

Reviewed by Maciej Stachowiak.

Creating copy of ContainerNode's when inserting or removing is inefficient
​https://bugs.webkit.org/show_bug.cgi?id=58695

Eliminated node copying in willRemove() and insertedIntoDocument().

No new tests as this is a more efficient implementation of
existing code that is covered by existing tests.

• dom/ContainerNode.cpp: (WebCore::ContainerNode::willRemove): Changed method to use RefPtr<> to protect against modification during removal. (WebCore::ContainerNode::insertedIntoDocument): Changed method to use RefPtr<> and two other deletion checks to protect against modification during insertion.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ContainerNode.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-04-22  Michael Saboff  <msaboff@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Creating copy of ContainerNode's when inserting or removing is inefficient
+        https://bugs.webkit.org/show_bug.cgi?id=58695
+
+        Eliminated node copying in willRemove() and insertedIntoDocument().
+
+        No new tests as this is a more efficient implementation of
+        existing code that is covered by existing tests.
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::willRemove): Changed method to use
+        RefPtr<> to protect against modification during removal.
+        (WebCore::ContainerNode::insertedIntoDocument): Changed method to use
+        RefPtr<> and two other deletion checks to protect against 
+        modification during insertion.
+
 2011-04-22  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/WebCore/dom/ContainerNode.cpp

@@ -367 +367 @@
 void ContainerNode::willRemove()
 {
-    Vector<RefPtr<Node>, 10> nodes;
-    nodes.reserveInitialCapacity(childNodeCount());
-    for (Node* n = m_lastChild; n; n = n->previousSibling())
-        nodes.append(n);
-    for (; nodes.size(); nodes.removeLast())
-        nodes.last().get()->willRemove();
+    RefPtr<Node> protect(this);
+
+    for (RefPtr<Node> child = firstChild(); child; child = child->nextSibling()) {
+        if (child->parentNode() != this) // Check for child being removed from subtree while removing.
+            break;
+        child->willRemove();
+    }
     Node::willRemove();
 }
@@ -746 +747 @@
 void ContainerNode::insertedIntoDocument()
 {
+    RefPtr<Node> protect(this);
+
     Node::insertedIntoDocument();
     insertedIntoTree(false);
 
-    // Determine set of children before operating on any of them.
-    NodeVector children;
-    collectNodes(this, children);
-
-    NodeVector::iterator it;
-    for (it = children.begin(); it != children.end() && inDocument(); ++it) {
-        Node* child = it->get();
+    for (RefPtr<Node> child = m_firstChild; child; child = child->nextSibling()) {
+        // Guard against mutation during re-parenting.
+        if (!inDocument()) // Check for self being removed from document while reparenting.
+            break;
+        if (child->parentNode() != this) // Check for child being removed from subtree while reparenting.
+            break;
         child->insertedIntoDocument();
     }