Title: [148754] trunk/Source/WebCore
- Revision
- 148754
- Author
- [email protected]
- Date
- 2013-04-19 10:25:54 -0700 (Fri, 19 Apr 2013)
Log Message
ContainerNode::removeChildren should first detach the children then remove them
https://bugs.webkit.org/show_bug.cgi?id=113433
Reviewed by Ryosuke Niwa.
Currently, ContainerNode::removeChildren initially removes the nodes from the DOM tree and then
calls detach() on each of them. This is anti-intuitive and can lead to subtle bugs because the
detached renderers are not backed by a valid DOM tree any more. With the patch, the nodes are first
detached and then removed from the DOM.
The patch also lets the tree in a consistent state after each node removal by clearing the previous
sibling pointer of the node following the one removed.
I haven't found any proof the performance will get worse if the detachment happens when the children
are still in the DOM tree.
Tests: No changed visible functionality.
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeChildren):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (148753 => 148754)
--- trunk/Source/WebCore/ChangeLog 2013-04-19 17:17:42 UTC (rev 148753)
+++ trunk/Source/WebCore/ChangeLog 2013-04-19 17:25:54 UTC (rev 148754)
@@ -1,3 +1,24 @@
+2013-04-19 Andrei Bucur <[email protected]>
+
+ ContainerNode::removeChildren should first detach the children then remove them
+ https://bugs.webkit.org/show_bug.cgi?id=113433
+
+ Reviewed by Ryosuke Niwa.
+
+ Currently, ContainerNode::removeChildren initially removes the nodes from the DOM tree and then
+ calls detach() on each of them. This is anti-intuitive and can lead to subtle bugs because the
+ detached renderers are not backed by a valid DOM tree any more. With the patch, the nodes are first
+ detached and then removed from the DOM.
+ The patch also lets the tree in a consistent state after each node removal by clearing the previous
+ sibling pointer of the node following the one removed.
+ I haven't found any proof the performance will get worse if the detachment happens when the children
+ are still in the DOM tree.
+
+ Tests: No changed visible functionality.
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::removeChildren):
+
2013-04-19 Dirk Schulze <[email protected]>
Refactor transform code in StyleResolver
Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (148753 => 148754)
--- trunk/Source/WebCore/dom/ContainerNode.cpp 2013-04-19 17:17:42 UTC (rev 148753)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2013-04-19 17:25:54 UTC (rev 148754)
@@ -616,9 +616,9 @@
while (RefPtr<Node> n = m_firstChild) {
Node* next = n->nextSibling();
- // Remove the node from the tree before calling detach or removedFromDocument (4427024, 4129744).
- // removeChild() does this after calling detach(). There is no explanation for
- // this discrepancy between removeChild() and its optimized version removeChildren().
+ if (n->attached())
+ n->detach();
+
n->setPreviousSibling(0);
n->setNextSibling(0);
n->setParentOrShadowHostNode(0);
@@ -627,20 +627,11 @@
m_firstChild = next;
if (n == m_lastChild)
m_lastChild = 0;
+ else
+ m_firstChild->setPreviousSibling(0);
+
removedChildren.append(n.release());
}
-
- // Detach the nodes only after properly removed from the tree because
- // a. detaching requires a proper DOM tree (for counters and quotes for
- // example) and during the previous loop the next sibling still points to
- // the node being removed while the node being removed does not point back
- // and does not point to the same parent as its next sibling.
- // b. destroying Renderers of standalone nodes is sometimes faster.
- for (size_t i = 0; i < removedChildren.size(); ++i) {
- Node* removedChild = removedChildren[i].get();
- if (removedChild->attached())
- removedChild->detach();
- }
}
childrenChanged(false, 0, 0, -static_cast<int>(removedChildren.size()));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
