Crash in Range::processAncestorsAndTheirSiblings.
authorinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Sep 2011 05:22:36 +0000 (05:22 +0000)
committerinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Sep 2011 05:22:36 +0000 (05:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=67556

Reviewed by Ryosuke Niwa.

Source/WebCore:

Create a temporary RefPtr Node vector to keep all the ancestor's
childs so that we don't access removed child nodes.

Test: fast/dom/Range/range-delete-contents-event-fire-crash.html

* dom/Range.cpp:
(WebCore::Range::processContents):
(WebCore::Range::processAncestorsAndTheirSiblings):

LayoutTests:

Tests that we do not crash when removing contents of
a range from the document.

* fast/dom/Range/range-delete-contents-event-fire-crash-expected.txt: Added.
* fast/dom/Range/range-delete-contents-event-fire-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@94511 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/Range/range-delete-contents-event-fire-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Range/range-delete-contents-event-fire-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Range.cpp

index 9026d51..ce224ad 100644 (file)
@@ -1,3 +1,16 @@
+2011-09-04  Abhishek Arya  <inferno@chromium.org>
+
+        Crash in Range::processAncestorsAndTheirSiblings.
+        https://bugs.webkit.org/show_bug.cgi?id=67556
+
+        Reviewed by Ryosuke Niwa.
+
+        Tests that we do not crash when removing contents of
+        a range from the document.
+
+        * fast/dom/Range/range-delete-contents-event-fire-crash-expected.txt: Added.
+        * fast/dom/Range/range-delete-contents-event-fire-crash.html: Added.
+
 2011-09-04  Jeremy Apthorp  <jeremya@google.com>
 
         Don't detach elements from the render tree when entering fullscreen mode
 2011-09-04  Jeremy Apthorp  <jeremya@google.com>
 
         Don't detach elements from the render tree when entering fullscreen mode
diff --git a/LayoutTests/fast/dom/Range/range-delete-contents-event-fire-crash-expected.txt b/LayoutTests/fast/dom/Range/range-delete-contents-event-fire-crash-expected.txt
new file mode 100644 (file)
index 0000000..3be5dec
--- /dev/null
@@ -0,0 +1,2 @@
+PASS
diff --git a/LayoutTests/fast/dom/Range/range-delete-contents-event-fire-crash.html b/LayoutTests/fast/dom/Range/range-delete-contents-event-fire-crash.html
new file mode 100644 (file)
index 0000000..fec43ec
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>\r
+<html>\r
+<div id="test1">\r
+<input id="test2"/>\r
+<input id="test3"/>\r
+<ol></ol>\r
+</div>\r
+<script>\r
+if (window.layoutTestController)\r
+    layoutTestController.dumpAsText();\r
+\r
+function runTest() {\r
+    var range = document.createRange();\r
+    var test1 = document.getElementById("test1");\r
+    var test2 = document.getElementById("test2");\r
+    var test3 = document.getElementById("test3");\r
+    \r
+    range.setStartBefore(test2);\r
+    range.selectNodeContents(test3);\r
+    range.setEndAfter(test1);\r
+    range.commonAncestorContainer;\r
+    range.deleteContents();\r
+}\r
+\r
+document.addEventListener("DOMSubtreeModified", runTest, true);\r
+document.body.appendChild(document.createTextNode("PASS"));\r
+</script>\r
+</html>
\ No newline at end of file
index 99ce731..ed51e8b 100644 (file)
@@ -1,3 +1,19 @@
+2011-09-04  Abhishek Arya  <inferno@chromium.org>
+
+        Crash in Range::processAncestorsAndTheirSiblings.
+        https://bugs.webkit.org/show_bug.cgi?id=67556
+
+        Reviewed by Ryosuke Niwa.
+
+        Create a temporary RefPtr Node vector to keep all the ancestor's
+        childs so that we don't access removed child nodes.
+
+        Test: fast/dom/Range/range-delete-contents-event-fire-crash.html
+
+        * dom/Range.cpp:
+        (WebCore::Range::processContents):
+        (WebCore::Range::processAncestorsAndTheirSiblings):
+
 2011-09-04  Jeremy Apthorp  <jeremya@google.com>
 
         Don't detach elements from the render tree when entering fullscreen mode
 2011-09-04  Jeremy Apthorp  <jeremya@google.com>
 
         Don't detach elements from the render tree when entering fullscreen mode
index e18066e..29bb018 100644 (file)
@@ -58,6 +58,8 @@ using namespace std;
 static WTF::RefCountedLeakCounter rangeCounter("Range");
 #endif
 
 static WTF::RefCountedLeakCounter rangeCounter("Range");
 #endif
 
+typedef Vector<RefPtr<Node> > NodeVector;
+
 inline Range::Range(PassRefPtr<Document> ownerDocument)
     : m_ownerDocument(ownerDocument)
     , m_start(m_ownerDocument)
 inline Range::Range(PassRefPtr<Document> ownerDocument)
     : m_ownerDocument(ownerDocument)
     , m_start(m_ownerDocument)
@@ -681,8 +683,6 @@ static inline unsigned lengthOfContentsInNode(Node* node)
 
 PassRefPtr<DocumentFragment> Range::processContents(ActionType action, ExceptionCode& ec)
 {
 
 PassRefPtr<DocumentFragment> Range::processContents(ActionType action, ExceptionCode& ec)
 {
-    typedef Vector<RefPtr<Node> > NodeVector;
-
     RefPtr<DocumentFragment> fragment;
     if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS)
         fragment = DocumentFragment::create(m_ownerDocument.get());
     RefPtr<DocumentFragment> fragment;
     if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS)
         fragment = DocumentFragment::create(m_ownerDocument.get());
@@ -896,9 +896,14 @@ PassRefPtr<Node> Range::processAncestorsAndTheirSiblings(ActionType action, Node
         // FIXME: This assertion may fail if DOM is modified during mutation event
         // FIXME: Share code with Range::processNodes
         ASSERT(!firstChildInAncestorToProcess || firstChildInAncestorToProcess->parentNode() == ancestor);
         // FIXME: This assertion may fail if DOM is modified during mutation event
         // FIXME: Share code with Range::processNodes
         ASSERT(!firstChildInAncestorToProcess || firstChildInAncestorToProcess->parentNode() == ancestor);
-        RefPtr<Node> next;
-        for (Node* child = firstChildInAncestorToProcess.get(); child; child = next.get()) {
-            next = direction == ProcessContentsForward ? child->nextSibling() : child->previousSibling();
+        
+        NodeVector nodes;
+        for (Node* child = firstChildInAncestorToProcess.get(); child;
+            child = (direction == ProcessContentsForward) ? child->nextSibling() : child->previousSibling())
+            nodes.append(child);
+
+        for (NodeVector::const_iterator it = nodes.begin(); it != nodes.end(); it++) {
+            Node* child = it->get();
             switch (action) {
             case DELETE_CONTENTS:
                 ancestor->removeChild(child, ec);
             switch (action) {
             case DELETE_CONTENTS:
                 ancestor->removeChild(child, ec);