Title: [152707] trunk
Revision
152707
Author
yu...@chromium.org
Date
2013-07-15 23:55:31 -0700 (Mon, 15 Jul 2013)

Log Message

Source/WebCore: 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):

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (152706 => 152707)


--- trunk/LayoutTests/ChangeLog	2013-07-16 06:32:43 UTC (rev 152706)
+++ trunk/LayoutTests/ChangeLog	2013-07-16 06:55:31 UTC (rev 152707)
@@ -1,3 +1,17 @@
+2013-07-15  Yuta Kitamura  <yu...@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  <rn...@webkit.org>
 
         Input element that becomes visible during a simulated click event from an associated label element doesn't get focused

Added: trunk/LayoutTests/fast/dom/Range/detach-range-during-deletecontents-expected.txt (0 => 152707)


--- trunk/LayoutTests/fast/dom/Range/detach-range-during-deletecontents-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/detach-range-during-deletecontents-expected.txt	2013-07-16 06:55:31 UTC (rev 152707)
@@ -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
+

Added: trunk/LayoutTests/fast/dom/Range/detach-range-during-deletecontents.html (0 => 152707)


--- trunk/LayoutTests/fast/dom/Range/detach-range-during-deletecontents.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/detach-range-during-deletecontents.html	2013-07-16 06:55:31 UTC (rev 152707)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</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 = '';
+document.body.appendChild(iframe);
+</script>
+</body>
+<script src=""
+</html>

Added: trunk/LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml (0 => 152707)


--- trunk/LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml	2013-07-16 06:55:31 UTC (rev 152707)
@@ -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>

Modified: trunk/Source/WebCore/ChangeLog (152706 => 152707)


--- trunk/Source/WebCore/ChangeLog	2013-07-16 06:32:43 UTC (rev 152706)
+++ trunk/Source/WebCore/ChangeLog	2013-07-16 06:55:31 UTC (rev 152707)
@@ -1,3 +1,26 @@
+2013-07-15  Yuta Kitamura  <yu...@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  <rn...@webkit.org>
 
         Input element that becomes visible during a simulated click event from an associated label element doesn't get focused

Modified: trunk/Source/WebCore/dom/Range.cpp (152706 => 152707)


--- trunk/Source/WebCore/dom/Range.cpp	2013-07-16 06:32:43 UTC (rev 152706)
+++ trunk/Source/WebCore/dom/Range.cpp	2013-07-16 06:55:31 UTC (rev 152707)
@@ -693,9 +693,13 @@
         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 @@
     // 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) {

Modified: trunk/Source/WebCore/dom/RangeBoundaryPoint.h (152706 => 152707)


--- trunk/Source/WebCore/dom/RangeBoundaryPoint.h	2013-07-16 06:32:43 UTC (rev 152706)
+++ trunk/Source/WebCore/dom/RangeBoundaryPoint.h	2013-07-16 06:55:31 UTC (rev 152707)
@@ -35,6 +35,8 @@
 public:
     explicit RangeBoundaryPoint(PassRefPtr<Node> container);
 
+    explicit RangeBoundaryPoint(const RangeBoundaryPoint&);
+
     const Position toPosition() const;
 
     Node* container() const;
@@ -56,7 +58,7 @@
 
 private:
     static const int invalidOffset = -1;
-    
+
     RefPtr<Node> m_containerNode;
     mutable int m_offsetInContainer;
     RefPtr<Node> m_childBeforeBoundary;
@@ -70,6 +72,13 @@
     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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to