Log Message
2011-06-30 Abhishek Arya <[email protected]> Reviewed by Ryosuke Niwa.
Crash when calling DOMSubtreeModified event when extracting range
contents.
https://bugs.webkit.org/show_bug.cgi?id=63650
Convert a few nodes to RefPtrs and add commonRoot verification checks
for Range::processContents.
Tests: fast/dom/Range/range-extract-contents-event-fire-crash.html
fast/dom/Range/range-extract-contents-event-fire-crash2.html
* dom/Range.cpp:
(WebCore::childOfCommonRootBeforeOffset):
(WebCore::Range::processContents):
(WebCore::Range::processContentsBetweenOffsets):
(WebCore::Range::processAncestorsAndTheirSiblings):
2011-06-29 Abhishek Arya <[email protected]>
Reviewed by Ryosuke Niwa.
Crash when calling DOMSubtreeModified event when extracting range
contents.
https://bugs.webkit.org/show_bug.cgi?id=63650
* fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt: Added.
* fast/dom/Range/range-extract-contents-event-fire-crash.html: Added.
* fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt: Added.
* fast/dom/Range/range-extract-contents-event-fire-crash2.html: Added.
* fast/dom/Range/range-extractContents.html: remove the appending of fragment
in this crasher test since we now refptr the nodes and leftContents will be visible.
This crasher test does not need to show the extractContents fragment.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/fast/dom/Range/range-extractContents.html
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/dom/Range.cpp
Added Paths
- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt
- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html
- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt
- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html
Diff
Modified: trunk/LayoutTests/ChangeLog (90129 => 90130)
--- trunk/LayoutTests/ChangeLog 2011-06-30 16:53:52 UTC (rev 90129)
+++ trunk/LayoutTests/ChangeLog 2011-06-30 17:00:48 UTC (rev 90130)
@@ -1,3 +1,19 @@
+2011-06-29 Abhishek Arya <[email protected]>
+
+ Reviewed by Ryosuke Niwa.
+
+ Crash when calling DOMSubtreeModified event when extracting range
+ contents.
+ https://bugs.webkit.org/show_bug.cgi?id=63650
+
+ * fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt: Added.
+ * fast/dom/Range/range-extract-contents-event-fire-crash.html: Added.
+ * fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt: Added.
+ * fast/dom/Range/range-extract-contents-event-fire-crash2.html: Added.
+ * fast/dom/Range/range-extractContents.html: remove the appending of fragment
+ in this crasher test since we now refptr the nodes and leftContents will be visible.
+ This crasher test does not need to show the extractContents fragment.
+
2011-06-30 Nate Chapin <[email protected]>
Unreviewed, chromium test expectations update after r90101.
Added: trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt (0 => 90130)
--- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt 2011-06-30 17:00:48 UTC (rev 90130)
@@ -0,0 +1 @@
+PASS: does not crash
Added: trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html (0 => 90130)
--- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html 2011-06-30 17:00:48 UTC (rev 90130)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>
+ <span>
+ <span id="start"></span>
+ </span>
+</p>
+<p>
+ <span>
+ <span id="end"></span>
+ </span>
+</p>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function runTest()
+{
+ document.removeEventListener("DOMSubtreeModified", runTest, false);
+ document.body.innerHTML = 'PASS: does not crash';
+}
+
+document.addEventListener("DOMSubtreeModified", runTest, false);
+
+var r = document.createRange();
+r.setStartBefore(document.getElementById('start'));
+r.setEndAfter(document.getElementById('end'));
+r.extractContents();
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt (0 => 90130)
--- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt 2011-06-30 17:00:48 UTC (rev 90130)
@@ -0,0 +1 @@
+PASS: does not crash
Added: trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html (0 => 90130)
--- trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html 2011-06-30 17:00:48 UTC (rev 90130)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>
+ <span>
+ <span id="start"></span>
+ </span>
+ <span>
+ <span id="end"></span>
+ </span>
+</p>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function runTest()
+{
+ document.removeEventListener("DOMSubtreeModified", runTest, false);
+ document.body.innerHTML = 'PASS: does not crash';
+}
+
+document.addEventListener("DOMSubtreeModified", runTest, false);
+
+var r = document.createRange();
+r.setStartBefore(document.getElementById('start'));
+r.setEndAfter(document.getElementById('end'));
+r.extractContents();
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/fast/dom/Range/range-extractContents.html (90129 => 90130)
--- trunk/LayoutTests/fast/dom/Range/range-extractContents.html 2011-06-30 16:53:52 UTC (rev 90129)
+++ trunk/LayoutTests/fast/dom/Range/range-extractContents.html 2011-06-30 17:00:48 UTC (rev 90130)
@@ -21,7 +21,6 @@
r.setStartBefore(document.getElementById('start'));
r.setEndAfter(document.getElementById('end'));
var fragment = r.extractContents();
- document.body.appendChild(fragment);
} catch(e) {
}
log('PASS: No crash.');
Modified: trunk/Source/WebCore/ChangeLog (90129 => 90130)
--- trunk/Source/WebCore/ChangeLog 2011-06-30 16:53:52 UTC (rev 90129)
+++ trunk/Source/WebCore/ChangeLog 2011-06-30 17:00:48 UTC (rev 90130)
@@ -1,3 +1,23 @@
+2011-06-30 Abhishek Arya <[email protected]>
+
+ Reviewed by Ryosuke Niwa.
+
+ Crash when calling DOMSubtreeModified event when extracting range
+ contents.
+ https://bugs.webkit.org/show_bug.cgi?id=63650
+
+ Convert a few nodes to RefPtrs and add commonRoot verification checks
+ for Range::processContents.
+
+ Tests: fast/dom/Range/range-extract-contents-event-fire-crash.html
+ fast/dom/Range/range-extract-contents-event-fire-crash2.html
+
+ * dom/Range.cpp:
+ (WebCore::childOfCommonRootBeforeOffset):
+ (WebCore::Range::processContents):
+ (WebCore::Range::processContentsBetweenOffsets):
+ (WebCore::Range::processAncestorsAndTheirSiblings):
+
2011-06-30 Dan Bernstein <[email protected]>
Reviewed by Adele Peterson.
Modified: trunk/Source/WebCore/dom/Range.cpp (90129 => 90130)
--- trunk/Source/WebCore/dom/Range.cpp 2011-06-30 16:53:52 UTC (rev 90129)
+++ trunk/Source/WebCore/dom/Range.cpp 2011-06-30 17:00:48 UTC (rev 90130)
@@ -628,7 +628,9 @@
{
ASSERT(container);
ASSERT(commonRoot);
- ASSERT(commonRoot->contains(container));
+
+ if (!commonRoot->contains(container))
+ return 0;
if (container == commonRoot) {
container = container->firstChild();
@@ -682,7 +684,7 @@
if (ec)
return 0;
- Node* commonRoot = commonAncestorContainer(ec);
+ RefPtr<Node> commonRoot = commonAncestorContainer(ec);
if (ec)
return 0;
ASSERT(commonRoot);
@@ -693,8 +695,8 @@
}
// what is the highest node that partially selects the start / end of the range?
- Node* partialStart = highestAncestorUnderCommonRoot(m_start.container(), commonRoot);
- Node* partialEnd = highestAncestorUnderCommonRoot(m_end.container(), commonRoot);
+ RefPtr<Node> partialStart = highestAncestorUnderCommonRoot(m_start.container(), commonRoot.get());
+ RefPtr<Node> partialEnd = highestAncestorUnderCommonRoot(m_end.container(), commonRoot.get());
// Start and end containers are different.
// There are three possibilities here:
@@ -713,29 +715,32 @@
//
// These are deleted, cloned, or extracted (i.e. both) depending on action.
+ // Note that we are verifying that our common root hierarchy is still intact
+ // after any DOM mutation event, at various stages below. See webkit bug 60350.
+
RefPtr<Node> leftContents;
- if (m_start.container() != commonRoot) {
+ 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, ec);
+ leftContents = processAncestorsAndTheirSiblings(action, m_start.container(), ProcessContentsForward, leftContents, commonRoot.get(), ec);
}
RefPtr<Node> rightContents;
- if (m_end.container() != commonRoot) {
+ 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, ec);
+ rightContents = processAncestorsAndTheirSiblings(action, m_end.container(), ProcessContentsBackward, rightContents, commonRoot.get(), ec);
}
// delete all children of commonRoot between the start and end container
- Node* processStart = childOfCommonRootBeforeOffset(m_start.container(), m_start.offset(), commonRoot);
- if (m_start.container() != commonRoot) // processStart contains nodes before m_start.
+ RefPtr<Node> processStart = childOfCommonRootBeforeOffset(m_start.container(), m_start.offset(), commonRoot.get());
+ if (processStart && m_start.container() != commonRoot) // processStart contains nodes before m_start.
processStart = processStart->nextSibling();
- Node* processEnd = childOfCommonRootBeforeOffset(m_end.container(), m_end.offset(), commonRoot);
+ RefPtr<Node> processEnd = childOfCommonRootBeforeOffset(m_end.container(), m_end.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) {
- if (partialStart)
+ if (partialStart && commonRoot->contains(partialStart.get()))
setStart(partialStart->parentNode(), partialStart->nodeIndex() + 1, ec);
- else if (partialEnd)
+ else if (partialEnd && commonRoot->contains(partialEnd.get()))
setStart(partialEnd->parentNode(), partialEnd->nodeIndex(), ec);
if (ec)
return 0;
@@ -750,7 +755,7 @@
if (processStart) {
NodeVector nodes;
- for (Node* n = processStart; n && n != processEnd; n = n->nextSibling())
+ for (Node* n = processStart.get(); n && n != processEnd; n = n->nextSibling())
nodes.append(n);
processNodes(action, nodes, commonRoot, fragment, ec);
}
@@ -841,7 +846,7 @@
break;
}
- return result;
+ return result.release();
}
void Range::processNodes(ActionType action, Vector<RefPtr<Node> >& nodes, PassRefPtr<Node> oldContainer, PassRefPtr<Node> newContainer, ExceptionCode& ec)
@@ -906,7 +911,7 @@
firstChildInAncestorToProcess = direction == ProcessContentsForward ? ancestor->nextSibling() : ancestor->previousSibling();
}
- return clonedContainer;
+ return clonedContainer.release();
}
PassRefPtr<DocumentFragment> Range::extractContents(ExceptionCode& ec)
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
