Log Message
Merge 90130 BUG=87925 Review URL: http://codereview.chromium.org/7344019
Modified Paths
- branches/chromium/782/LayoutTests/fast/dom/Range/range-extractContents.html
- branches/chromium/782/Source/WebCore/dom/Range.cpp
Added Paths
- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt
- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html
- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt
- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html
Diff
Copied: branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt (from rev 90130, trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt) (0 => 90845)
--- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt (rev 0)
+++ branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash-expected.txt 2011-07-12 20:57:29 UTC (rev 90845)
@@ -0,0 +1 @@
+PASS: does not crash
Copied: branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html (from rev 90130, trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html) (0 => 90845)
--- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html (rev 0)
+++ branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash.html 2011-07-12 20:57:29 UTC (rev 90845)
@@ -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>
Copied: branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt (from rev 90130, trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt) (0 => 90845)
--- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt (rev 0)
+++ branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2-expected.txt 2011-07-12 20:57:29 UTC (rev 90845)
@@ -0,0 +1 @@
+PASS: does not crash
Copied: branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html (from rev 90130, trunk/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html) (0 => 90845)
--- branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html (rev 0)
+++ branches/chromium/782/LayoutTests/fast/dom/Range/range-extract-contents-event-fire-crash2.html 2011-07-12 20:57:29 UTC (rev 90845)
@@ -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: branches/chromium/782/LayoutTests/fast/dom/Range/range-extractContents.html (90844 => 90845)
--- branches/chromium/782/LayoutTests/fast/dom/Range/range-extractContents.html 2011-07-12 20:25:22 UTC (rev 90844)
+++ branches/chromium/782/LayoutTests/fast/dom/Range/range-extractContents.html 2011-07-12 20:57:29 UTC (rev 90845)
@@ -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: branches/chromium/782/Source/WebCore/dom/Range.cpp (90844 => 90845)
--- branches/chromium/782/Source/WebCore/dom/Range.cpp 2011-07-12 20:25:22 UTC (rev 90844)
+++ branches/chromium/782/Source/WebCore/dom/Range.cpp 2011-07-12 20:57:29 UTC (rev 90845)
@@ -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
