Title: [119050] trunk
Revision
119050
Author
[email protected]
Date
2012-05-30 21:25:01 -0700 (Wed, 30 May 2012)

Log Message

Crash in ContainerNode::parserAddChild.
https://bugs.webkit.org/show_bug.cgi?id=87903

Reviewed by Ryosuke Niwa.

Source/WebCore:

Call the ChildNodeInsertionNotifier.notify call at the end since
it can destroy |this| and some of the local pointers like |last|.
This also matches the order of calls - childrenChanged precedes
ChildNodeInsertionNotifier.notify in updateTreeAfterInsertion and
ContainerNode::parserInsertBefore.

Also remove a FIXME since we use ChildNodeInsertionNotifier.notify
instead of ChildNodeInsertionNotifier.notifyInsertedIntoDocument
(as recommended in the FIXME).

Test: fast/dom/child-insertion-notify-crash.html

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::parserAddChild):

LayoutTests:

* fast/dom/child-insertion-notify-crash-expected.txt: Added.
* fast/dom/child-insertion-notify-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (119049 => 119050)


--- trunk/LayoutTests/ChangeLog	2012-05-31 04:23:37 UTC (rev 119049)
+++ trunk/LayoutTests/ChangeLog	2012-05-31 04:25:01 UTC (rev 119050)
@@ -1,3 +1,13 @@
+2012-05-30  Abhishek Arya  <[email protected]>
+
+        Crash in ContainerNode::parserAddChild.
+        https://bugs.webkit.org/show_bug.cgi?id=87903
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/dom/child-insertion-notify-crash-expected.txt: Added.
+        * fast/dom/child-insertion-notify-crash.html: Added.
+
 2012-05-30  Rick Byers  <[email protected]>
 
         [chromium] Implement pointer and hover CSS media features

Added: trunk/LayoutTests/fast/dom/child-insertion-notify-crash-expected.txt (0 => 119050)


--- trunk/LayoutTests/fast/dom/child-insertion-notify-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/child-insertion-notify-crash-expected.txt	2012-05-31 04:25:01 UTC (rev 119050)
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS. WebKit didn't crash.

Added: trunk/LayoutTests/fast/dom/child-insertion-notify-crash.html (0 => 119050)


--- trunk/LayoutTests/fast/dom/child-insertion-notify-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/child-insertion-notify-crash.html	2012-05-31 04:25:01 UTC (rev 119050)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<script src=""
+<script>
+function crash()
+{
+    document.getElementById("test").innerHTML = "PASS. WebKit didn't crash.";
+    gc();    
+}
+</script>
+<div dir="auto" id="test">A<iframe _onload_="crash()"></iframe></div>
+<script src=""
+</html>
+
Property changes on: trunk/LayoutTests/fast/dom/child-insertion-notify-crash.html
___________________________________________________________________

Added: svn:executable

Modified: trunk/Source/WebCore/ChangeLog (119049 => 119050)


--- trunk/Source/WebCore/ChangeLog	2012-05-31 04:23:37 UTC (rev 119049)
+++ trunk/Source/WebCore/ChangeLog	2012-05-31 04:25:01 UTC (rev 119050)
@@ -1,3 +1,25 @@
+2012-05-30  Abhishek Arya  <[email protected]>
+
+        Crash in ContainerNode::parserAddChild.
+        https://bugs.webkit.org/show_bug.cgi?id=87903
+
+        Reviewed by Ryosuke Niwa.
+
+        Call the ChildNodeInsertionNotifier.notify call at the end since
+        it can destroy |this| and some of the local pointers like |last|.
+        This also matches the order of calls - childrenChanged precedes
+        ChildNodeInsertionNotifier.notify in updateTreeAfterInsertion and
+        ContainerNode::parserInsertBefore.
+
+        Also remove a FIXME since we use ChildNodeInsertionNotifier.notify
+        instead of ChildNodeInsertionNotifier.notifyInsertedIntoDocument
+        (as recommended in the FIXME).
+
+        Test: fast/dom/child-insertion-notify-crash.html
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::parserAddChild):
+
 2012-05-30  Rick Byers  <[email protected]>
 
         [chromium] Implement pointer and hover CSS media features

Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (119049 => 119050)


--- trunk/Source/WebCore/dom/ContainerNode.cpp	2012-05-31 04:23:37 UTC (rev 119049)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp	2012-05-31 04:25:01 UTC (rev 119050)
@@ -584,10 +584,8 @@
     
     allowEventDispatch();
 
-    // FIXME: Why doesn't this use notify(newChild.get()) instead?
-    if (inDocument())
-        ChildNodeInsertionNotifier(this).notifyInsertedIntoDocument(newChild.get());
     childrenChanged(true, last, 0, 1);
+    ChildNodeInsertionNotifier(this).notify(newChild.get());
 }
 
 void ContainerNode::suspendPostAttachCallbacks()
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to