Reviewed by Darin.
Fixed <rdar://problem/
5592485> Safari crashed trying to get a motorcycle insurance quote
on Geico.com WebCore::Document::inPageCache()
Calling Node::willRemove on the focusedNode would immediately tell the document to remove
the focused node, and trigger JS events. This means that the document is mutated while
the engine is trying to tell all child nodes that it's about to removed. To avoid
crashing, we need to hold off on mutating the document until node traversal is finished.
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeChild):
(WebCore::ContainerNode::removeChildren):
* dom/Node.cpp:
* dom/Node.h:
(WebCore::Node::willRemove):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clear):
LayoutTests:
Reviewed by Darin.
Fixed <rdar://problem/
5592485> Safari crashed trying to get a motorcycle insurance quote
on Geico.com WebCore::Document::inPageCache()
* fast/events/nested-event-remove-node-crash-expected.txt: Added.
* fast/events/nested-event-remove-node-crash.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@28875
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-12-19 Alice Liu <alice.liu@apple.com>
+
+ Reviewed by Darin.
+
+ Fixed <rdar://problem/5592485> Safari crashed trying to get a motorcycle insurance quote
+ on Geico.com WebCore::Document::inPageCache()
+
+ * fast/events/nested-event-remove-node-crash-expected.txt: Added.
+ * fast/events/nested-event-remove-node-crash.html: Added.
+
2007-12-19 Andre Boule <aboule@apple.com>
Reviewed by Dan Bernstein.
--- /dev/null
+<html>
+<head>
+<script>
+function sendXHR()
+{
+ XHR[numXHRs] = new XMLHttpRequest();
+ XHR[numXHRs].onreadystatechange = handleStateChange;
+ XHR[numXHRs].open("GET", "nested-event-remove-node-crash-expected.txt", true);
+ XHR[numXHRs].send(null);
+ numXHRs = numXHRs + 1;
+}
+
+function callback(response)
+{
+ document.getElementById("replaceMe").innerHTML = "";
+ document.getElementById("replaceMe").innerHTML = response;
+ if (window.layoutTestController && (run == 2))
+ layoutTestController.notifyDone();
+}
+
+function handleStateChange()
+{
+ if ((XHR[0].readyState == 4) && (run < 2)) { // yes this looks wrong but it's how to reproduce the bug
+ run = run + 1;
+ callback(XHR[0].responseText);
+ }
+}
+
+function test()
+{
+/*
+ 1. focus a node
+ 2. send an XHR who's handler will remove the node
+ 3. the focused node's onblur will fire
+ 4. the onblur event handler will send off an XHR who's handler will remove the node
+*/
+ document.getElementById("theSelect").focus();
+ sendXHR();
+
+ if (window.layoutTestController) {
+ layoutTestController.waitUntilDone();
+ layoutTestController.dumpAsText();
+ }
+}
+
+function GC()
+{
+ // Force GC.
+ if (window.GCController)
+ GCController.collect();
+ else {
+ for (var i = 0; i < 10000; ++i) {
+ ({ });
+ }
+ }
+}
+
+/* GLOBALS */
+var XHR = new Array();
+var numXHRs = 0;
+var run = 0;
+
+</script>
+</head>
+<body onload="test()">
+
+<div id="replaceMe">
+
+<div>
+<select id="theSelect" onblur="sendXHR();GC();">
+</select>
+</div>
+
+</div>
+</body>
+</html>
+2007-12-19 Alice Liu <alice.liu@apple.com>
+
+ Reviewed by Darin.
+
+ Fixed <rdar://problem/5592485> Safari crashed trying to get a motorcycle insurance quote
+ on Geico.com WebCore::Document::inPageCache()
+
+ Calling Node::willRemove on the focusedNode would immediately tell the document to remove
+ the focused node, and trigger JS events. This means that the document is mutated while
+ the engine is trying to tell all child nodes that it's about to removed. To avoid
+ crashing, we need to hold off on mutating the document until node traversal is finished.
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::removeChild):
+ (WebCore::ContainerNode::removeChildren):
+ * dom/Node.cpp:
+ * dom/Node.h:
+ (WebCore::Node::willRemove):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::clear):
+
2007-12-19 Andre Boule <aboule@apple.com>
Reviewed by Dan Bernstein.
return false;
}
+ document()->removeFocusedNodeOfSubtree(child.get());
+
// FIXME: After sending the mutation events, "this" could be destroyed.
// We can prevent that by doing a "ref", but first we have to make sure
// that no callers call with ref count == 0 and parent = 0 (as of this
for (n = m_firstChild; n; n = n->nextSibling())
willRemoveChild(n);
+ // exclude this node when looking for removed focusedNode since only children will be removed
+ document()->removeFocusedNodeOfSubtree(this, true);
+
forbidEventDispatch();
while ((n = m_firstChild) != 0) {
Node *next = n->nextSibling();
m_activeNode = newActiveNode;
}
-void Document::focusedNodeRemoved(Node* node)
+void Document::focusedNodeRemoved()
{
setFocusedNode(0);
}
+void Document::removeFocusedNodeOfSubtree(Node* node, bool amongChildrenOnly)
+{
+ if (!m_focusedNode || this->inPageCache()) // If the document is in the page cache, then we don't need to clear out the focused node.
+ return;
+
+ bool nodeInSubtree = false;
+ if (amongChildrenOnly)
+ nodeInSubtree = m_focusedNode->isDescendantOf(node);
+ else
+ nodeInSubtree = (m_focusedNode == node) || m_focusedNode->isDescendantOf(node);
+
+ if (nodeInSubtree)
+ document()->focusedNodeRemoved();
+}
+
void Document::hoveredNodeDetached(Node* node)
{
if (!m_hoverNode || (node != m_hoverNode && (!m_hoverNode->isTextNode() || node != m_hoverNode->parent())))
void setActiveNode(PassRefPtr<Node>);
Node* activeNode() const { return m_activeNode.get(); }
- void focusedNodeRemoved(Node*);
+ void focusedNodeRemoved();
+ void removeFocusedNodeOfSubtree(Node*, bool amongChildrenOnly = false);
void hoveredNodeDetached(Node*);
void activeChainNodeDetached(Node*);
void Node::willRemove()
{
- // If the document is in the page cache, then we don't need to clear out the focused node.
- if (!document()->inPageCache() && m_focused)
- document()->focusedNodeRemoved(this);
}
void Node::detach()
if (m_frame->document()->attached()) {
m_frame->document()->willRemove();
m_frame->document()->detach();
+
+ m_frame->document()->removeFocusedNodeOfSubtree(m_frame->document());
}
}
git://git.webkit.org / WebKit-https.git / commitdiff