[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.2.2-27-g91dab87
Gustavo Noronha Silva
gns at gnome.org
Thu Jul 15 21:13:27 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 9884ee5a6aba4c8c09f56713c9c3969d8317c93c
Author: ap at apple.com <ap at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon May 10 22:13:36 2010 +0000
Reviewed by Darin Adler.
Based on a patch by Eric Seidel.
https://bugs.webkit.org/show_bug.cgi?id=28697
<rdar://problem/7946578> WebKit crash on WebCore::Node::nodeIndex()
It's not OK to call ContainerNode::willRemoveChild() in a loop, because Range code assumes
that it can adjust start and end position to any node except for the one being removed -
so these notifications cannot be batched.
Test: fast/dom/Range/remove-all-children-crash.html
* dom/ContainerNode.cpp:
(WebCore::willRemoveChild): Removed unused ExceptionCode.
(WebCore::willRemoveChildren): New function, used in removeChildren() case.
(WebCore::ContainerNode::removeChild): ExceptionCode return was always 0, don't bother with it.
(WebCore::ContainerNode::removeChildren): Call willRemoveChildrenFromNode.
(WebCore::dispatchChildRemovalEvents): Moved some logic out into willRemoveChildrenFromNode
and willRemoveChild.
* dom/Document.cpp:
(WebCore::Document::nodeChildrenWillBeRemoved): New function, used in removeChildren() case.
* dom/Document.h:
(WebCore::Document::nodeChildrenWillBeRemoved): New function, used in removeChildren() case.
* dom/Range.h:
* dom/Range.cpp:
(WebCore::boundaryNodeChildrenWillBeRemoved): New function, used in removeChildren() case.
(WebCore::Range::nodeChildrenWillBeRemoved): Ditto.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@59098 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 887287b..fe8950f 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-05-10 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Darin Adler.
+
+ https://bugs.webkit.org/show_bug.cgi?id=28697
+ <rdar://problem/7946578> WebKit crash on WebCore::Node::nodeIndex()
+
+ * fast/dom/Range/remove-all-children-crash-expected.txt: Added.
+ * fast/dom/Range/remove-all-children-crash.html: Added.
+
2010-04-20 Justin Schuh <jschuh at chromium.org>
Reviewed by Adam Barth.
diff --git a/LayoutTests/fast/dom/Range/remove-all-children-crash-expected.txt b/LayoutTests/fast/dom/Range/remove-all-children-crash-expected.txt
new file mode 100644
index 0000000..fdf7370
--- /dev/null
+++ b/LayoutTests/fast/dom/Range/remove-all-children-crash-expected.txt
@@ -0,0 +1,3 @@
+Test for bug 28697.
+
+PASS, the test did not crash.
diff --git a/LayoutTests/fast/dom/Range/remove-all-children-crash.html b/LayoutTests/fast/dom/Range/remove-all-children-crash.html
new file mode 100644
index 0000000..e40fce0
--- /dev/null
+++ b/LayoutTests/fast/dom/Range/remove-all-children-crash.html
@@ -0,0 +1,39 @@
+<body>
+<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=28697">bug 28697</a>.</p>
+<div id="div"><p id="one"></p><p id="two">FAIL, the test did not start.</p></div>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function gc() {
+ if (typeof GCController !== "undefined")
+ GCController.collect();
+ else {
+ function gcRec(n) {
+ if (n < 1)
+ return {};
+ var temp = {i: "ab" + i + (i / 100000)};
+ temp += "foo";
+ gcRec(n-1);
+ }
+ for (var i = 0; i < 1000; i++)
+ gcRec(10)
+ }
+}
+
+var div = document.getElementById("div");
+var two = document.getElementById("two");
+var range = document.createRange();
+range.setStart(two, 0);
+range.setEnd(two, 0);
+
+div.innerHTML = "FAIL, the test did not complete.";
+
+gc();
+
+range.startOffset;
+div.innerHTML = "PASS, the test did not crash.";
+if (window.layoutTestController) {
+ layoutTestController.notifyDone();
+}
+</script>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 75fabc9..eb5063b 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2010-05-10 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Darin Adler.
+
+ Based on a patch by Eric Seidel.
+
+ https://bugs.webkit.org/show_bug.cgi?id=28697
+ <rdar://problem/7946578> WebKit crash on WebCore::Node::nodeIndex()
+
+ It's not OK to call ContainerNode::willRemoveChild() in a loop, because Range code assumes
+ that it can adjust start and end position to any node except for the one being removed -
+ so these notifications cannot be batched.
+
+ Test: fast/dom/Range/remove-all-children-crash.html
+
+ * dom/ContainerNode.cpp:
+ (WebCore::willRemoveChild): Removed unused ExceptionCode.
+ (WebCore::willRemoveChildren): New function, used in removeChildren() case.
+ (WebCore::ContainerNode::removeChild): ExceptionCode return was always 0, don't bother with it.
+ (WebCore::ContainerNode::removeChildren): Call willRemoveChildrenFromNode.
+ (WebCore::dispatchChildRemovalEvents): Moved some logic out into willRemoveChildrenFromNode
+ and willRemoveChild.
+
+ * dom/Document.cpp:
+ (WebCore::Document::nodeChildrenWillBeRemoved): New function, used in removeChildren() case.
+
+ * dom/Document.h:
+ (WebCore::Document::nodeChildrenWillBeRemoved): New function, used in removeChildren() case.
+
+ * dom/Range.h:
+ * dom/Range.cpp:
+ (WebCore::boundaryNodeChildrenWillBeRemoved): New function, used in removeChildren() case.
+ (WebCore::Range::nodeChildrenWillBeRemoved): Ditto.
+
2010-04-20 Justin Schuh <jschuh at chromium.org>
Reviewed by Adam Barth.
diff --git a/WebCore/dom/ContainerNode.cpp b/WebCore/dom/ContainerNode.cpp
index fb2852f..c17489a 100644
--- a/WebCore/dom/ContainerNode.cpp
+++ b/WebCore/dom/ContainerNode.cpp
@@ -292,19 +292,32 @@ void ContainerNode::willRemove()
Node::willRemove();
}
-static ExceptionCode willRemoveChild(Node *child)
+static void willRemoveChild(Node* child)
{
- ExceptionCode ec = 0;
+ // update auxiliary doc info (e.g. iterators) to note that node is being removed
+ child->document()->nodeWillBeRemoved(child);
+ child->document()->incDOMTreeVersion();
// fire removed from document mutation events.
dispatchChildRemovalEvents(child);
- if (ec)
- return ec;
if (child->attached())
child->willRemove();
-
- return 0;
+}
+
+static void willRemoveChildren(ContainerNode* container)
+{
+ container->document()->nodeChildrenWillBeRemoved(container);
+ container->document()->incDOMTreeVersion();
+
+ // FIXME: Adding new children from event handlers can cause an infinite loop here.
+ for (RefPtr<Node> child = container->firstChild(); child; child = child->nextSibling()) {
+ // fire removed from document mutation events.
+ dispatchChildRemovalEvents(child.get());
+
+ if (child->attached())
+ child->willRemove();
+ }
}
bool ContainerNode::removeChild(Node* oldChild, ExceptionCode& ec)
@@ -328,10 +341,7 @@ bool ContainerNode::removeChild(Node* oldChild, ExceptionCode& ec)
}
RefPtr<Node> child = oldChild;
-
- ec = willRemoveChild(child.get());
- if (ec)
- return false;
+ willRemoveChild(child.get());
// Mutation events might have moved this child into a different parent.
if (child->parentNode() != this) {
@@ -399,14 +409,12 @@ bool ContainerNode::removeChildren()
return false;
// The container node can be removed from event handlers.
- RefPtr<Node> protect(this);
-
+ RefPtr<ContainerNode> protect(this);
+
// Do any prep work needed before actually starting to detach
// and remove... e.g. stop loading frames, fire unload events.
- // FIXME: Adding new children from event handlers can cause an infinite loop here.
- for (RefPtr<Node> n = m_firstChild; n; n = n->nextSibling())
- willRemoveChild(n.get());
-
+ willRemoveChildren(protect.get());
+
// exclude this node when looking for removed focusedNode since only children will be removed
document()->removeFocusedNodeOfSubtree(this, true);
@@ -936,6 +944,8 @@ static void dispatchChildInsertionEvents(Node* child)
static void dispatchChildRemovalEvents(Node* child)
{
+ ASSERT(!eventDispatchForbidden());
+
#if ENABLE(INSPECTOR)
if (Page* page = child->document()->page()) {
if (InspectorController* inspectorController = page->inspectorController())
@@ -946,11 +956,6 @@ static void dispatchChildRemovalEvents(Node* child)
RefPtr<Node> c = child;
RefPtr<Document> document = child->document();
- // update auxiliary doc info (e.g. iterators) to note that node is being removed
- document->nodeWillBeRemoved(child);
-
- document->incDOMTreeVersion();
-
// dispatch pre-removal mutation events
if (c->parentNode() && document->hasListenerType(Document::DOMNODEREMOVED_LISTENER))
c->dispatchEvent(MutationEvent::create(eventNames().DOMNodeRemovedEvent, true, c->parentNode()));
diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index 59fefa3..060ef37 100644
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -2879,6 +2879,28 @@ void Document::nodeChildrenChanged(ContainerNode* container)
}
}
+void Document::nodeChildrenWillBeRemoved(ContainerNode* container)
+{
+ if (!disableRangeMutation(page())) {
+ HashSet<Range*>::const_iterator end = m_ranges.end();
+ for (HashSet<Range*>::const_iterator it = m_ranges.begin(); it != end; ++it)
+ (*it)->nodeChildrenWillBeRemoved(container);
+ }
+
+ HashSet<NodeIterator*>::const_iterator nodeIteratorsEnd = m_nodeIterators.end();
+ for (HashSet<NodeIterator*>::const_iterator it = m_nodeIterators.begin(); it != nodeIteratorsEnd; ++it) {
+ for (Node* n = container->firstChild(); n; n = n->nextSibling())
+ (*it)->nodeWillBeRemoved(n);
+ }
+
+ if (Frame* frame = this->frame()) {
+ for (Node* n = container->firstChild(); n; n = n->nextSibling()) {
+ frame->selection()->nodeWillBeRemoved(n);
+ frame->dragCaretController()->nodeWillBeRemoved(n);
+ }
+ }
+}
+
void Document::nodeWillBeRemoved(Node* n)
{
HashSet<NodeIterator*>::const_iterator nodeIteratorsEnd = m_nodeIterators.end();
diff --git a/WebCore/dom/Document.h b/WebCore/dom/Document.h
index df87ebd..1f4e22c 100644
--- a/WebCore/dom/Document.h
+++ b/WebCore/dom/Document.h
@@ -611,6 +611,9 @@ public:
void detachRange(Range*);
void nodeChildrenChanged(ContainerNode*);
+ // nodeChildrenWillBeRemoved is used when removing all node children at once.
+ void nodeChildrenWillBeRemoved(ContainerNode*);
+ // nodeWillBeRemoved is only safe when removing one node at a time.
void nodeWillBeRemoved(Node*);
void textInserted(Node*, unsigned offset, unsigned length);
diff --git a/WebCore/dom/Range.cpp b/WebCore/dom/Range.cpp
index 52d1785..689b590 100644
--- a/WebCore/dom/Range.cpp
+++ b/WebCore/dom/Range.cpp
@@ -1716,6 +1716,31 @@ void Range::nodeChildrenChanged(ContainerNode* container)
boundaryNodeChildrenChanged(m_end, container);
}
+static inline void boundaryNodeChildrenWillBeRemoved(RangeBoundaryPoint& boundary, ContainerNode* container)
+{
+ for (Node* nodeToBeRemoved = container->firstChild(); nodeToBeRemoved; nodeToBeRemoved = nodeToBeRemoved->nextSibling()) {
+ if (boundary.childBefore() == nodeToBeRemoved) {
+ boundary.setToStartOfNode(container);
+ return;
+ }
+
+ for (Node* n = boundary.container(); n; n = n->parentNode()) {
+ if (n == nodeToBeRemoved) {
+ boundary.setToStartOfNode(container);
+ return;
+ }
+ }
+ }
+}
+
+void Range::nodeChildrenWillBeRemoved(ContainerNode* container)
+{
+ ASSERT(container);
+ ASSERT(container->document() == m_ownerDocument);
+ boundaryNodeChildrenWillBeRemoved(m_start, container);
+ boundaryNodeChildrenWillBeRemoved(m_end, container);
+}
+
static inline void boundaryNodeWillBeRemoved(RangeBoundaryPoint& boundary, Node* nodeToBeRemoved)
{
if (boundary.childBefore() == nodeToBeRemoved) {
diff --git a/WebCore/dom/Range.h b/WebCore/dom/Range.h
index fd0f66a..bfddd32 100644
--- a/WebCore/dom/Range.h
+++ b/WebCore/dom/Range.h
@@ -111,6 +111,7 @@ public:
void textQuads(Vector<FloatQuad>&, bool useSelectionHeight = false);
void nodeChildrenChanged(ContainerNode*);
+ void nodeChildrenWillBeRemoved(ContainerNode*);
void nodeWillBeRemoved(Node*);
void textInserted(Node*, unsigned offset, unsigned length);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list