[webkit-changes] cvs commit: WebCore/khtml/ecma kjs_dom.cpp
Adele
adele at opensource.apple.com
Tue Jun 7 15:43:00 PDT 2005
adele 05/06/07 15:43:00
Modified: . Tag: Safari-2-0-branch ChangeLog
khtml/ecma Tag: Safari-2-0-branch kjs_dom.cpp
Log:
Merged fix for <rdar://problem/4139800> from TOT to Safari-2-0-branch
2005-06-07 Adele Peterson <adele at apple.com>
Change by Darin, reviewed by Maciej and me.
- fixed <rdar://problem/4139800> REGRESSION: Safari crashes at bebe.com
There were cases where the DOMNode mark function would end up not even
marking the node that it was called on. The old code tried to skip any
subtrees that were already marked, but that code was wrong.
* khtml/ecma/kjs_dom.cpp: (KJS::DOMNode::mark): Changed three things:
1) Instead of a boolean, keep a set of trees that we are marking, that
prevents problems if we mark things that have references between
separate trees of DOM nodes.
2) Don't do the "outermost node with a wrapper" check, just start marking
at the root of the entire tree, because there's no way to have an
unreachable node pointing to a reachable node.
3) Handle the unusual case where the document's wrapper dictionary is
gone by marking just this node explicitly.
This passes the layout tests, still fixes the Dashboard memory leak from
bug 4125222, and makes bebe.com work again.
Revision Changes Path
No revision
No revision
1.4104.2.32 +26 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.4104.2.31
retrieving revision 1.4104.2.32
diff -u -r1.4104.2.31 -r1.4104.2.32
--- ChangeLog 3 Jun 2005 21:34:45 -0000 1.4104.2.31
+++ ChangeLog 7 Jun 2005 22:42:54 -0000 1.4104.2.32
@@ -1,3 +1,29 @@
+2005-06-07 Adele Peterson <adele at apple.com>
+
+ Merged fix for <rdar://problem/4139800> from TOT to Safari-2-0-branch
+
+ 2005-06-07 Adele Peterson <adele at apple.com>
+
+ Change by Darin, reviewed by Maciej and me.
+
+ - fixed <rdar://problem/4139800> REGRESSION: Safari crashes at bebe.com
+
+ There were cases where the DOMNode mark function would end up not even
+ marking the node that it was called on. The old code tried to skip any
+ subtrees that were already marked, but that code was wrong.
+
+ * khtml/ecma/kjs_dom.cpp: (KJS::DOMNode::mark): Changed three things:
+ 1) Instead of a boolean, keep a set of trees that we are marking, that
+ prevents problems if we mark things that have references between
+ separate trees of DOM nodes.
+ 2) Don't do the "outermost node with a wrapper" check, just start marking
+ at the root of the entire tree, because there's no way to have an
+ unreachable node pointing to a reachable node.
+ 3) Handle the unusual case where the document's wrapper dictionary is
+ gone by marking just this node explicitly.
+ This passes the layout tests, still fixes the Dashboard memory leak from
+ bug 4125222, and makes bebe.com work again.
+
=== WebCore-415.9 ===
2005-06-03 Adele Peterson <adele at apple.com>
No revision
No revision
1.66.6.6 +38 -18 WebCore/khtml/ecma/kjs_dom.cpp
Index: kjs_dom.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/ecma/kjs_dom.cpp,v
retrieving revision 1.66.6.5
retrieving revision 1.66.6.6
diff -u -r1.66.6.5 -r1.66.6.6
--- kjs_dom.cpp 31 May 2005 23:48:22 -0000 1.66.6.5
+++ kjs_dom.cpp 7 Jun 2005 22:42:59 -0000 1.66.6.6
@@ -104,36 +104,56 @@
void DOMNode::mark()
{
- static bool markingTree = false;
+ assert(!marked());
- if (node.isNull() || node.handle()->inDocument() || markingTree) {
+
+ // Nodes in the document are kept alive by ScriptInterpreter::mark,
+ // so we have no special responsibilities and can just call the base class here.
+ if (node.isNull() || node.handle()->inDocument()) {
DOMObject::mark();
return;
}
+
+ // This is a node outside the document, so find the root of the tree it is in,
+ // and start marking from there.
+ NodeImpl *root = node.handle();
+ for (NodeImpl *current = node.handle(); current; current = current->parentNode()) {
+ root = current;
+ }
- DocumentImpl *document = node.handle()->getDocument();
- NodeImpl *outermostWrappedNode = node.handle();
- for (NodeImpl *current = node.handle()->parentNode(); current; current = current->parentNode()) {
- if (ScriptInterpreter::getDOMNodeForDocument(document, current))
- outermostWrappedNode = current;
+ static QPtrDict<NodeImpl> markingRoots;
+
+ // If we're already marking this tree, then we can simply mark this wrapper
+ // by calling the base class; our caller is iterating the tree.
+ if (markingRoots.find(root)) {
+ DOMObject::mark();
+ return;
}
- markingTree = true;
+ DocumentImpl *document = node.handle()->getDocument();
- NodeImpl *nodeToMark = outermostWrappedNode;
- while (nodeToMark) {
+ // Mark the whole tree; use the global set of roots to avoid reentering.
+ markingRoots.insert(root, root);
+ for (NodeImpl *nodeToMark = root; nodeToMark; nodeToMark = nodeToMark->traverseNextNode()) {
DOMNode *wrapper = ScriptInterpreter::getDOMNodeForDocument(document, nodeToMark);
- if (!wrapper)
- nodeToMark = nodeToMark->traverseNextNode();
- else if (wrapper->marked())
- nodeToMark = nodeToMark->traverseNextSibling();
- else {
- wrapper->mark();
- nodeToMark = nodeToMark->traverseNextNode();
+ if (wrapper) {
+ if (!wrapper->marked())
+ wrapper->mark();
+ } else if (nodeToMark == node.handle()) {
+ // This is the case where the map from the document to wrappers has
+ // been cleared out, but a wrapper is being marked. For now, we'll
+ // let the rest of the tree of wrappers get collected, because we have
+ // no good way of finding them. Later we should test behavior of other
+ // browsers and see if we need to preserve other wrappers in this case.
+ if (!marked())
+ mark();
}
}
+ markingRoots.remove(root);
+
+ // Double check that we actually ended up marked. This assert caught problems in the past.
+ assert(marked());
- markingTree = false;
}
bool DOMNode::toBoolean(ExecState *) const
More information about the webkit-changes
mailing list