Title: [139964] trunk
- Revision
- 139964
- Author
- morr...@google.com
- Date
- 2013-01-16 21:25:14 -0800 (Wed, 16 Jan 2013)
Log Message
NoEventDispatchAssertion in ContainerNode::removeChildren is too strict
https://bugs.webkit.org/show_bug.cgi?id=106985
Reviewed by Ryosuke Niwa.
Source/WebCore:
This change narrowed the lifetime of NoEventDispatchAssertion in removeChildren().
It is as safe as other mutation method even after this change: childrenChanged() and
ChildNodeRemovalNotifier are used outside the assertion scope.
Test: svg/custom/use-mutation-crash.xhtml
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeChildren):
LayoutTests:
* svg/custom/use-mutation-crash-expected.txt: Added.
* svg/custom/use-mutation-crash.xhtml: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (139963 => 139964)
--- trunk/LayoutTests/ChangeLog 2013-01-17 05:16:27 UTC (rev 139963)
+++ trunk/LayoutTests/ChangeLog 2013-01-17 05:25:14 UTC (rev 139964)
@@ -1,3 +1,13 @@
+2013-01-16 Hajime Morrita <morr...@google.com>
+
+ NoEventDispatchAssertion in ContainerNode::removeChildren is too strict
+ https://bugs.webkit.org/show_bug.cgi?id=106985
+
+ Reviewed by Ryosuke Niwa.
+
+ * svg/custom/use-mutation-crash-expected.txt: Added.
+ * svg/custom/use-mutation-crash.xhtml: Added.
+
2013-01-16 MORITA Hajime <morr...@google.com>
Attr.ownerDocument should change if its parent's owner did
Added: trunk/LayoutTests/svg/custom/use-mutation-crash-expected.txt (0 => 139964)
--- trunk/LayoutTests/svg/custom/use-mutation-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/use-mutation-crash-expected.txt 2013-01-17 05:25:14 UTC (rev 139964)
@@ -0,0 +1 @@
+PASS.
Added: trunk/LayoutTests/svg/custom/use-mutation-crash.xhtml (0 => 139964)
--- trunk/LayoutTests/svg/custom/use-mutation-crash.xhtml (rev 0)
+++ trunk/LayoutTests/svg/custom/use-mutation-crash.xhtml 2013-01-17 05:25:14 UTC (rev 139964)
@@ -0,0 +1,19 @@
+<html>
+<body>
+<pre id="console">PASS.</pre>
+<svg id="tCFSVG" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg">
+
+<g id="target"> </g>
+<use xlink:href=""
+
+<script><![CDATA[
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+target = document.getElementById("target");
+target.textContent = "Hello";
+]]></script>
+
+</svg>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (139963 => 139964)
--- trunk/Source/WebCore/ChangeLog 2013-01-17 05:16:27 UTC (rev 139963)
+++ trunk/Source/WebCore/ChangeLog 2013-01-17 05:25:14 UTC (rev 139964)
@@ -1,3 +1,19 @@
+2013-01-16 Hajime Morrita <morr...@google.com>
+
+ NoEventDispatchAssertion in ContainerNode::removeChildren is too strict
+ https://bugs.webkit.org/show_bug.cgi?id=106985
+
+ Reviewed by Ryosuke Niwa.
+
+ This change narrowed the lifetime of NoEventDispatchAssertion in removeChildren().
+ It is as safe as other mutation method even after this change: childrenChanged() and
+ ChildNodeRemovalNotifier are used outside the assertion scope.
+
+ Test: svg/custom/use-mutation-crash.xhtml
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::removeChildren):
+
2013-01-16 Shinya Kawanaka <shin...@chromium.org>
[Refactoring] HTMLTextFormControlElement should use shadowHost instead of shadowAncestorNode
Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (139963 => 139964)
--- trunk/Source/WebCore/dom/ContainerNode.cpp 2013-01-17 05:16:27 UTC (rev 139963)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2013-01-17 05:25:14 UTC (rev 139964)
@@ -582,43 +582,42 @@
Vector<RefPtr<Node>, 10> removedChildren;
{
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
- NoEventDispatchAssertion assertNoEventDispatch;
- removedChildren.reserveInitialCapacity(childNodeCount());
- while (RefPtr<Node> n = m_firstChild) {
- Node* next = n->nextSibling();
+ {
+ NoEventDispatchAssertion assertNoEventDispatch;
+ removedChildren.reserveInitialCapacity(childNodeCount());
+ 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().
- n->setPreviousSibling(0);
- n->setNextSibling(0);
- n->setParentOrHostNode(0);
- document()->adoptIfNeeded(n.get());
+ // 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().
+ n->setPreviousSibling(0);
+ n->setNextSibling(0);
+ n->setParentOrHostNode(0);
+ document()->adoptIfNeeded(n.get());
- m_firstChild = next;
- if (n == m_lastChild)
- m_lastChild = 0;
- removedChildren.append(n.release());
- }
+ m_firstChild = next;
+ if (n == m_lastChild)
+ m_lastChild = 0;
+ removedChildren.append(n.release());
+ }
- size_t removedChildrenCount = removedChildren.size();
- size_t i;
-
- // 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 (i = 0; i < removedChildrenCount; ++i) {
- Node* removedChild = removedChildren[i].get();
- if (removedChild->attached())
- removedChild->detach();
+ // 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>(removedChildrenCount));
-
- for (i = 0; i < removedChildrenCount; ++i)
+ childrenChanged(false, 0, 0, -static_cast<int>(removedChildren.size()));
+
+ for (size_t i = 0; i < removedChildren.size(); ++i)
ChildNodeRemovalNotifier(this).notify(removedChildren[i].get());
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
