Source/WebCore: Fix a crash in Range::processContents().
authoryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jul 2013 06:55:31 +0000 (06:55 +0000)
committeryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jul 2013 06:55:31 +0000 (06:55 +0000)
NULL ptr in WebCore::Range::processAncestorsAndTheirSiblings
https://bugs.webkit.org/show_bug.cgi?id=77614

Reviewed by Ryosuke Niwa.

This change is ported from Blink revision 153483:
https://src.chromium.org/viewvc/blink?revision=153483&view=revision

This crash can be initiated by calling Range.detach() while deleteContents()
is processing the same range. Range::processContents() should save the state
of the range since mutation events can change the state of the range.

Test: fast/dom/Range/detach-range-during-deletecontents.html

* dom/Range.cpp:
(WebCore::Range::processContents):
* dom/RangeBoundaryPoint.h:
(WebCore::RangeBoundaryPoint::RangeBoundaryPoint):

LayoutTests: NULL ptr in WebCore::Range::processAncestorsAndTheirSiblings
https://bugs.webkit.org/show_bug.cgi?id=77614

Reviewed by Ryosuke Niwa.

This change is ported from Blink revision 153483:
https://src.chromium.org/viewvc/blink?revision=153483&view=revision

* fast/dom/Range/detach-range-during-deletecontents-expected.txt: Added.
* fast/dom/Range/detach-range-during-deletecontents.html: Added.
* fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/Range/detach-range-during-deletecontents-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Range/detach-range-during-deletecontents.html [new file with mode: 0644]
LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Range.cpp
Source/WebCore/dom/RangeBoundaryPoint.h

index 1660652..0d6f711 100644 (file)
@@ -1,3 +1,17 @@
+2013-07-15  Yuta Kitamura  <yutak@chromium.org>
+
+        NULL ptr in WebCore::Range::processAncestorsAndTheirSiblings
+        https://bugs.webkit.org/show_bug.cgi?id=77614
+
+        Reviewed by Ryosuke Niwa.
+
+        This change is ported from Blink revision 153483:
+        https://src.chromium.org/viewvc/blink?revision=153483&view=revision
+
+        * fast/dom/Range/detach-range-during-deletecontents-expected.txt: Added.
+        * fast/dom/Range/detach-range-during-deletecontents.html: Added.
+        * fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml: Added.
+
 2013-07-15  Ryosuke Niwa  <rniwa@webkit.org>
 
         Input element that becomes visible during a simulated click event from an associated label element doesn't get focused
diff --git a/LayoutTests/fast/dom/Range/detach-range-during-deletecontents-expected.txt b/LayoutTests/fast/dom/Range/detach-range-during-deletecontents-expected.txt
new file mode 100644 (file)
index 0000000..85d1d35
--- /dev/null
@@ -0,0 +1,11 @@
+CONSOLE MESSAGE: line 12: InvalidStateError: DOM Exception 11: An attempt was made to use an object that is not, or is no longer, usable.
+Detaching a Range while deleteContents() is running should not cause a crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS The browser did not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Range/detach-range-during-deletecontents.html b/LayoutTests/fast/dom/Range/detach-range-during-deletecontents.html
new file mode 100644 (file)
index 0000000..40658f4
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+description('Detaching a Range while deleteContents() is running should not cause a crash.');
+
+window.jsTestIsAsync = true;
+
+window.addEventListener('message', function (event) {
+    if (event.data === 'done') {
+        testPassed('The browser did not crash.');
+        document.body.removeChild(iframe);
+        finishJSTest();
+    }
+}, false);
+
+var iframe = document.createElement('iframe');
+iframe.src = 'resources/detach-range-during-deletecontents-iframe.xhtml';
+document.body.appendChild(iframe);
+</script>
+</body>
+<script src="../../js/resources/js-test-post.js"></script>
+</html>
diff --git a/LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml b/LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml
new file mode 100644 (file)
index 0000000..69a4459
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<!-- This file is intentionally malformed. -->
+<html xmlns="http://www.w3.org/1999/xhtml">
+<script>
+<![CDATA[
+var done = false;
+var range = null;
+function expandAndDeleteRange()
+{
+    range = document.createRange();
+    range.expand("sentence");
+    range.deleteContents();
+}
+var repeat = 0;
+function detach()
+{
+    if (done)
+        return;
+    ++repeat;
+    if (repeat >= 2) {
+        done = true;
+        document.removeEventListener("DOMSubtreeModified", listener, true);
+        window.parent.postMessage("done", "*");
+        return;
+    }
+    range.detach();
+}
+
+var firstCall = true;
+function listener()
+{
+    if (firstCall) {
+        firstCall = false;
+        expandAndDeleteRange();
+    } else
+        detach();
+}
+document.addEventListener("DOMSubtreeModified", listener, true);
+
+document.addEventListener("DOMContentLoaded", expandAndDeleteRange);
+]]>
+</script>
index 4ce68b3..e88d705 100644 (file)
@@ -1,3 +1,26 @@
+2013-07-15  Yuta Kitamura  <yutak@chromium.org>
+
+        Fix a crash in Range::processContents().
+
+        NULL ptr in WebCore::Range::processAncestorsAndTheirSiblings
+        https://bugs.webkit.org/show_bug.cgi?id=77614
+
+        Reviewed by Ryosuke Niwa.
+
+        This change is ported from Blink revision 153483:
+        https://src.chromium.org/viewvc/blink?revision=153483&view=revision
+
+        This crash can be initiated by calling Range.detach() while deleteContents()
+        is processing the same range. Range::processContents() should save the state
+        of the range since mutation events can change the state of the range.
+
+        Test: fast/dom/Range/detach-range-during-deletecontents.html
+
+        * dom/Range.cpp:
+        (WebCore::Range::processContents):
+        * dom/RangeBoundaryPoint.h:
+        (WebCore::RangeBoundaryPoint::RangeBoundaryPoint):
+
 2013-07-15  Ryosuke Niwa  <rniwa@webkit.org>
 
         Input element that becomes visible during a simulated click event from an associated label element doesn't get focused
index a90872c..ba1ff19 100644 (file)
@@ -693,9 +693,13 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
         return fragment;
     }
 
+    // Since mutation events can modify the range during the process, the boundary points need to be saved.
+    RangeBoundaryPoint originalStart(m_start);
+    RangeBoundaryPoint originalEnd(m_end);
+
     // what is the highest node that partially selects the start / end of the range?
-    RefPtr<Node> partialStart = highestAncestorUnderCommonRoot(m_start.container(), commonRoot.get());
-    RefPtr<Node> partialEnd = highestAncestorUnderCommonRoot(m_end.container(), commonRoot.get());
+    RefPtr<Node> partialStart = highestAncestorUnderCommonRoot(originalStart.container(), commonRoot.get());
+    RefPtr<Node> partialEnd = highestAncestorUnderCommonRoot(originalEnd.container(), commonRoot.get());
 
     // Start and end containers are different.
     // There are three possibilities here:
@@ -718,22 +722,22 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
     // after any DOM mutation event, at various stages below. See webkit bug 60350.
 
     RefPtr<Node> leftContents;
-    if (m_start.container() != commonRoot && commonRoot->contains(m_start.container())) {
-        leftContents = processContentsBetweenOffsets(action, 0, m_start.container(), m_start.offset(), lengthOfContentsInNode(m_start.container()), ec);
-        leftContents = processAncestorsAndTheirSiblings(action, m_start.container(), ProcessContentsForward, leftContents, commonRoot.get(), ec);
+    if (originalStart.container() != commonRoot && commonRoot->contains(originalStart.container())) {
+        leftContents = processContentsBetweenOffsets(action, 0, originalStart.container(), originalStart.offset(), lengthOfContentsInNode(originalStart.container()), ec);
+        leftContents = processAncestorsAndTheirSiblings(action, originalStart.container(), ProcessContentsForward, leftContents, commonRoot.get(), ec);
     }
 
     RefPtr<Node> rightContents;
-    if (m_end.container() != commonRoot && commonRoot->contains(m_end.container())) {
-        rightContents = processContentsBetweenOffsets(action, 0, m_end.container(), 0, m_end.offset(), ec);
-        rightContents = processAncestorsAndTheirSiblings(action, m_end.container(), ProcessContentsBackward, rightContents, commonRoot.get(), ec);
+    if (m_end.container() != commonRoot && commonRoot->contains(originalEnd.container())) {
+        rightContents = processContentsBetweenOffsets(action, 0, originalEnd.container(), 0, originalEnd.offset(), ec);
+        rightContents = processAncestorsAndTheirSiblings(action, originalEnd.container(), ProcessContentsBackward, rightContents, commonRoot.get(), ec);
     }
 
     // delete all children of commonRoot between the start and end container
-    RefPtr<Node> processStart = childOfCommonRootBeforeOffset(m_start.container(), m_start.offset(), commonRoot.get());
-    if (processStart && m_start.container() != commonRoot) // processStart contains nodes before m_start.
+    RefPtr<Node> processStart = childOfCommonRootBeforeOffset(originalStart.container(), originalStart.offset(), commonRoot.get());
+    if (processStart && originalStart.container() != commonRoot) // processStart contains nodes before m_start.
         processStart = processStart->nextSibling();
-    RefPtr<Node> processEnd = childOfCommonRootBeforeOffset(m_end.container(), m_end.offset(), commonRoot.get());
+    RefPtr<Node> processEnd = childOfCommonRootBeforeOffset(originalEnd.container(), originalEnd.offset(), commonRoot.get());
 
     // Collapse the range, making sure that the result is not within a node that was partially selected.
     if (action == EXTRACT_CONTENTS || action == DELETE_CONTENTS) {
index 117bc93..9925019 100644 (file)
@@ -35,6 +35,8 @@ class RangeBoundaryPoint {
 public:
     explicit RangeBoundaryPoint(PassRefPtr<Node> container);
 
+    explicit RangeBoundaryPoint(const RangeBoundaryPoint&);
+
     const Position toPosition() const;
 
     Node* container() const;
@@ -56,7 +58,7 @@ public:
 
 private:
     static const int invalidOffset = -1;
-    
+
     RefPtr<Node> m_containerNode;
     mutable int m_offsetInContainer;
     RefPtr<Node> m_childBeforeBoundary;
@@ -70,6 +72,13 @@ inline RangeBoundaryPoint::RangeBoundaryPoint(PassRefPtr<Node> container)
     ASSERT(m_containerNode);
 }
 
+inline RangeBoundaryPoint::RangeBoundaryPoint(const RangeBoundaryPoint& other)
+    : m_containerNode(other.container())
+    , m_offsetInContainer(other.offset())
+    , m_childBeforeBoundary(other.childBefore())
+{
+}
+
 inline Node* RangeBoundaryPoint::container() const
 {
     return m_containerNode.get();