- fixed <rdar://problem/
4125222> Dashboard heap size grows over time (leak caused by JavaScript DOM node wrappers?)
- changed per-document DOMObject caching to work with DOMNodes, since that is all it was used for anyway.
Test cases added: (these tests actually ensure that DOM wrappers
are sufficiently protected from GC to match other browsers, they
do not verify that the leak is fixed since there's no way to test
that with layout tests).
* layout-tests/fast/dom/gc-1-expected.txt: Added.
* layout-tests/fast/dom/gc-1.html: Added.
* layout-tests/fast/dom/gc-2-expected.txt: Added.
* layout-tests/fast/dom/gc-2.html: Added.
* layout-tests/fast/dom/gc-3-expected.txt: Added.
* layout-tests/fast/dom/gc-3.html: Added.
* khtml/ecma/kjs_binding.cpp:
(KJS::ScriptInterpreter::forgetDOMObjectForDocument): New function - allows nodes
that get removed from the document to go away from the cache if not referenced.
(KJS::ScriptInterpreter::mark): Don't mark nodes that aren't in the document,
they can stay in the cache but only if they have another source of life.
(KJS::ScriptInterpreter::domNodesPerDocument): Renamed and changed parameter types.
(KJS::ScriptInterpreter::getDOMNodeForDocument): Renamed and changed parameter types.
(KJS::ScriptInterpreter::forgetDOMNodeForDocument): Renamed and changed parameter types.
(KJS::ScriptInterpreter::putDOMNodeForDocument): Renamed and changed parameter types.
(KJS::ScriptInterpreter::forgetAllDOMNodesForDocument): Renamed and changed parameter types.
(KJS::ScriptInterpreter::updateDOMNodeDocument): Renamed and changed parameter types.
* khtml/ecma/kjs_binding.h:
* khtml/ecma/kjs_dom.cpp:
(KJS::DOMNode::~DOMNode): call forgetDOMObjectForDocument.
(KJS::DOMNode::mark): If the node is not in the document, make sure to mark
all wrappers in its detached piece of the tree.
(KJS::getDOMNode): Updated for renames
* khtml/ecma/kjs_dom.h:
* khtml/xml/dom_docimpl.cpp:
(DocumentImpl::~DocumentImpl): Updated for renames.
* khtml/xml/dom_nodeimpl.cpp:
(NodeImpl::checkAddChild): Updated for renames.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@9230
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
--- /dev/null
+This test verifies that JavaScript wrappers for DOM nodes are protected against garbage collection as long as the node remains in the document (so custom properties are preserved).
+
+The output should be the following pieces of text on lines by themselves: "original span", "B":
+
+original span
+B
+
--- /dev/null
+<head>
+<script>
+function doit()
+{
+ document.getElementById("span-B").customProperty = "B";
+
+ // create lots of objects to force a garbage collection
+ var i = 0;
+ while (i < 1000) {
+ i = i+1;
+ }
+
+ var output= document.getElementById("output");
+
+ output.innerHTML += document.getElementById("span-B").customProperty + "<BR>";
+}
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+}
+
+</script>
+</head>
+
+<body onload="doit()">
+<div style="border: 1px solid red">
+<p>
+This test verifies that JavaScript wrappers for DOM nodes are
+protected against garbage collection as long as the node remains in
+the document (so custom properties are preserved).
+</p>
+<p>
+The output should be the following pieces of text on lines by themselves: "original span", "B":
+</p>
+</div>
+<div id="div">
+<span id="span-A"><span id="span-B"><span id="span-C">original span</span></span></span>
+</div>
+<div id="output">
+</div>
+</body>
--- /dev/null
+This test verifies that JavaScript wrappers for DOM nodes are protected against garbage collection for node trees that are outside the document, so long as there's a reference to at least one node in the tree.
+
+The output should be the following pieces of text on lines by themselves: "replacement content", "B", "A", "C".
+
+replacement content
+B
+A
+C
+
--- /dev/null
+<html>
+<head>
+<script>
+function doit()
+{
+ var B = document.getElementById("span-B");
+ B.customProperty = "B";
+ B.parentNode.customProperty = "A";
+ B.firstChild.customProperty = "C";
+
+ document.getElementById("div").innerHTML = "<span>replacement content</span>";
+
+ // create lots of objects to force a garbage collection
+ var i = "";
+ while (i < 1000) {
+ i = i + "a";
+ }
+
+ var output= document.getElementById("output");
+
+ output.innerHTML += B.customProperty + "<BR>";
+ if (B.parentNode) {
+ output.innerHTML += B.parentNode.customProperty + "<BR>";
+ }
+ if (B.firstChild) {
+ output.innerHTML += B.firstChild.customProperty + "<BR>";
+ }
+}
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+}
+
+</script>
+</head>
+
+<body onload="doit()">
+<div style="border: 1px solid red">
+<p>
+This test verifies that JavaScript wrappers for DOM nodes are
+protected against garbage collection for node trees that are outside
+the document, so long as there's a reference to at least one node in
+the tree.
+</p>
+<p>
+The output should be the following pieces of text on lines by themselves: "replacement content", "B", "A", "C".
+</p>
+</div>
+<div id="div">
+<span id="span-A"><span id="span-B"><span id="span-C">original span</span></span></span>
+</div>
+<div id="output">
+</div>
+</body>
+</html>
\ No newline at end of file
--- /dev/null
+This test verifies that DOM nodes are not retained just because a wrapper exists for a descendant. A wrapper must exist for the node itself or for an ancestor.
+
+The output should be the following pieces of text on lines by themselves: "replacement content", "B", "child node exists".
+
+replacement content
+B
+child node exists
+
--- /dev/null
+<head>
+<script>
+function doit()
+{
+ var B = document.getElementById("span-B");
+ B.customProperty = "B";
+
+ document.getElementById("div").innerHTML = "<span>replacement content</span>";
+
+ // create lots of objects to force a garbage collection
+ var i = 0;
+ while (i < 1000) {
+ i = i+1;
+ }
+
+ var output= document.getElementById("output");
+
+ output.innerHTML += B.customProperty + "<BR>";
+ if (B.parentNode) {
+ output.innerHTML += "parent node exists<BR>";
+ }
+ if (B.firstChild) {
+ output.innerHTML += "child node exists<BR>";
+ }
+}
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+}
+
+</script>
+</head>
+
+<body onload="doit()">
+<div style="border: 1px solid red">
+<p>
+This test verifies that DOM nodes are not retained just because a wrapper exists for a descendant. A wrapper must exist for the node itself or for an ancestor.
+</p>
+<p>
+The output should be the following pieces of text on lines by themselves: "replacement content", "B", "child node exists".
+</p>
+</div>
+<div id="div">
+<span id="span-A"><span id="span-B"><span id="span-C">original span</span></span></span>
+</div>
+<div id="output">
+</div>
+</body>
+2005-05-30 Maciej Stachowiak <mjs@apple.com>
+
+ Reviewed by Darin.
+
+ - fixed <rdar://problem/4125222> Dashboard heap size grows over time (leak caused by JavaScript DOM node wrappers?)
+
+ - changed per-document DOMObject caching to work with DOMNodes, since that is all it was used for anyway.
+
+ Test cases added: (these tests actually ensure that DOM wrappers
+ are sufficiently protected from GC to match other browsers, they
+ do not verify that the leak is fixed since there's no way to test
+ that with layout tests).
+
+ * layout-tests/fast/dom/gc-1-expected.txt: Added.
+ * layout-tests/fast/dom/gc-1.html: Added.
+ * layout-tests/fast/dom/gc-2-expected.txt: Added.
+ * layout-tests/fast/dom/gc-2.html: Added.
+ * layout-tests/fast/dom/gc-3-expected.txt: Added.
+ * layout-tests/fast/dom/gc-3.html: Added.
+
+ * khtml/ecma/kjs_binding.cpp:
+ (KJS::ScriptInterpreter::forgetDOMObjectForDocument): New function - allows nodes
+ that get removed from the document to go away from the cache if not referenced.
+ (KJS::ScriptInterpreter::mark): Don't mark nodes that aren't in the document,
+ they can stay in the cache but only if they have another source of life.
+ (KJS::ScriptInterpreter::domNodesPerDocument): Renamed and changed parameter types.
+ (KJS::ScriptInterpreter::getDOMNodeForDocument): Renamed and changed parameter types.
+ (KJS::ScriptInterpreter::forgetDOMNodeForDocument): Renamed and changed parameter types.
+ (KJS::ScriptInterpreter::putDOMNodeForDocument): Renamed and changed parameter types.
+ (KJS::ScriptInterpreter::forgetAllDOMNodesForDocument): Renamed and changed parameter types.
+ (KJS::ScriptInterpreter::updateDOMNodeDocument): Renamed and changed parameter types.
+ * khtml/ecma/kjs_binding.h:
+ * khtml/ecma/kjs_dom.cpp:
+ (KJS::DOMNode::~DOMNode): call forgetDOMObjectForDocument.
+ (KJS::DOMNode::mark): If the node is not in the document, make sure to mark
+ all wrappers in its detached piece of the tree.
+ (KJS::getDOMNode): Updated for renames
+ * khtml/ecma/kjs_dom.h:
+ * khtml/xml/dom_docimpl.cpp:
+ (DocumentImpl::~DocumentImpl): Updated for renames.
+ * khtml/xml/dom_nodeimpl.cpp:
+ (NodeImpl::checkAddChild): Updated for renames.
+
2005-05-30 Darin Adler <darin@apple.com>
Reviewed by John.
#include "kjs_dom.h"
#include "kjs_window.h"
#include <kjs/internal.h> // for InterpreterImp
+#include <kjs/collector.h>
#include "dom/dom_exception.h"
#include "dom/dom2_events.h"
}
static QPtrDict<DOMObject> * staticDomObjects = 0;
-QPtrDict< QPtrDict<DOMObject> > * staticDomObjectsPerDocument = 0;
+QPtrDict< QPtrDict<DOMNode> > * staticDOMNodesPerDocument = 0;
QPtrDict<DOMObject> & ScriptInterpreter::domObjects()
{
return *staticDomObjects;
}
-QPtrDict< QPtrDict<DOMObject> > & ScriptInterpreter::domObjectsPerDocument()
+QPtrDict< QPtrDict<DOMNode> > & ScriptInterpreter::domNodesPerDocument()
{
- if (!staticDomObjectsPerDocument) {
- staticDomObjectsPerDocument = new QPtrDict<QPtrDict<DOMObject> >();
- staticDomObjectsPerDocument->setAutoDelete(true);
+ if (!staticDOMNodesPerDocument) {
+ staticDOMNodesPerDocument = new QPtrDict<QPtrDict<DOMNode> >();
+ staticDOMNodesPerDocument->setAutoDelete(true);
}
- return *staticDomObjectsPerDocument;
+ return *staticDOMNodesPerDocument;
}
deleteDOMObject( objectHandle );
}
-DOMObject* ScriptInterpreter::getDOMObjectForDocument( DOM::DocumentImpl* documentHandle, void *objectHandle )
+DOMNode *ScriptInterpreter::getDOMNodeForDocument(DOM::DocumentImpl *document, DOM::NodeImpl *node)
{
- QPtrDict<DOMObject> *documentDict = (QPtrDict<DOMObject> *)domObjectsPerDocument()[documentHandle];
- if (documentDict) {
- return (*documentDict)[objectHandle];
- }
+ QPtrDict<DOMNode> *documentDict = (QPtrDict<DOMNode> *)domNodesPerDocument()[document];
+ if (documentDict)
+ return (*documentDict)[node];
return NULL;
}
-void ScriptInterpreter::putDOMObjectForDocument( DOM::DocumentImpl* documentHandle, void *objectHandle, DOMObject *obj )
+void ScriptInterpreter::forgetDOMNodeForDocument(DOM::DocumentImpl *document, NodeImpl *node)
+{
+ QPtrDict<DOMNode> *documentDict = domNodesPerDocument()[document];
+ if (documentDict)
+ documentDict->remove(node);
+}
+
+void ScriptInterpreter::putDOMNodeForDocument(DOM::DocumentImpl *document, NodeImpl *nodeHandle, DOMNode *nodeWrapper)
{
- QPtrDict<DOMObject> *documentDict = (QPtrDict<DOMObject> *)domObjectsPerDocument()[documentHandle];
+ QPtrDict<DOMNode> *documentDict = domNodesPerDocument()[document];
if (!documentDict) {
- documentDict = new QPtrDict<DOMObject>();
- domObjectsPerDocument().insert(documentHandle, documentDict);
+ documentDict = new QPtrDict<DOMNode>();
+ domNodesPerDocument().insert(document, documentDict);
}
-
- documentDict->insert( objectHandle, obj );
+
+ documentDict->insert(nodeHandle, nodeWrapper);
}
-bool ScriptInterpreter::deleteDOMObjectsForDocument( DOM::DocumentImpl* documentHandle )
+void ScriptInterpreter::forgetAllDOMNodesForDocument(DOM::DocumentImpl *document)
{
- return domObjectsPerDocument().remove( documentHandle );
+ domNodesPerDocument().remove(document);
}
void ScriptInterpreter::mark()
{
- QPtrDictIterator<QPtrDict<DOMObject> > dictIterator(domObjectsPerDocument());
-
- QPtrDict<DOMObject> *objectDict;
- while ((objectDict = dictIterator.current())) {
- QPtrDictIterator<DOMObject> objectIterator(*objectDict);
-
- DOMObject *obj;
- while ((obj = objectIterator.current())) {
- if (!obj->marked()) {
- obj->mark();
- }
- ++objectIterator;
+ QPtrDictIterator<QPtrDict<DOMNode> > dictIterator(domNodesPerDocument());
+
+ QPtrDict<DOMNode> *nodeDict;
+ while ((nodeDict = dictIterator.current())) {
+ QPtrDictIterator<DOMNode> nodeIterator(*nodeDict);
+
+ DOMNode *node;
+ while ((node = nodeIterator.current())) {
+ // don't mark wrappers for nodes that are no longer in the
+ // document - they should not be saved if the node is not
+ // otherwise reachable from JS.
+ if (node->impl()->inDocument() && !node->marked())
+ node->mark();
+
+ ++nodeIterator;
}
++dictIterator;
}
}
-void ScriptInterpreter::forgetDOMObjectsForDocument( DOM::DocumentImpl* documentHandle )
-{
- deleteDOMObjectsForDocument( documentHandle );
-}
-
-void ScriptInterpreter::updateDOMObjectDocument(void *objectHandle, DOM::DocumentImpl *oldDoc, DOM::DocumentImpl *newDoc)
+void ScriptInterpreter::updateDOMNodeDocument(NodeImpl *node, DOM::DocumentImpl *oldDoc, DOM::DocumentImpl *newDoc)
{
- DOMObject* cachedObject = getDOMObjectForDocument(oldDoc, objectHandle);
- if (cachedObject) {
- putDOMObjectForDocument(newDoc, objectHandle, cachedObject);
- }
+ DOMNode *cachedObject = getDOMNodeForDocument(oldDoc, node);
+ if (cachedObject)
+ putDOMNodeForDocument(newDoc, node, cachedObject);
}
bool ScriptInterpreter::wasRunByUserGesture() const
namespace DOM {
class DocumentImpl;
class EventImpl;
+ class NodeImpl;
}
namespace KJS {
virtual UString toString(ExecState *) const { return UString("[function]"); }
};
+ class DOMNode;
+
/**
* We inherit from Interpreter, to save a pointer to the HTML part
* that the interpreter runs for.
return domObjects().remove( objectHandle );
}
- static DOMObject* getDOMObjectForDocument( DOM::DocumentImpl* documentHandle, void *objectHandle );
- static void putDOMObjectForDocument( DOM::DocumentImpl* documentHandle, void *objectHandle, DOMObject *obj );
- static bool deleteDOMObjectsForDocument( DOM::DocumentImpl* documentHandle );
-
- /**
- * Static method. Makes all interpreters forget about the object
- */
static void forgetDOMObject( void* objectHandle );
- static void forgetDOMObjectsForDocument( DOM::DocumentImpl* documentHandle );
- static void updateDOMObjectDocument(void *objectHandle, DOM::DocumentImpl *oldDoc, DOM::DocumentImpl *newDoc);
+
+ static DOMNode *getDOMNodeForDocument(DOM::DocumentImpl *document, DOM::NodeImpl *node);
+ static void putDOMNodeForDocument(DOM::DocumentImpl *document, DOM::NodeImpl *nodeHandle, DOMNode *nodeWrapper);
+ static void forgetDOMNodeForDocument(DOM::DocumentImpl *document, DOM::NodeImpl *node);
+ static void forgetAllDOMNodesForDocument(DOM::DocumentImpl *document);
+ static void updateDOMNodeDocument(DOM::NodeImpl *nodeHandle, DOM::DocumentImpl *oldDoc, DOM::DocumentImpl *newDoc);
+
KHTMLPart* part() const { return m_part; }
KHTMLPart* m_part;
static QPtrDict<DOMObject> &domObjects();
- static QPtrDict<QPtrDict<DOMObject> > &domObjectsPerDocument();
+ static QPtrDict<QPtrDict<DOMNode> > &domNodesPerDocument();
DOM::EventImpl *m_evt;
bool m_inlineCode;
{
}
+DOMNode::~DOMNode()
+{
+ ScriptInterpreter::forgetDOMNodeForDocument(m_impl->getDocument(), m_impl.get());
+}
+
+void DOMNode::mark()
+{
+ static bool markingTree = false;
+
+ if (m_impl->inDocument() || markingTree) {
+ DOMObject::mark();
+ return;
+ }
+
+ DocumentImpl *document = m_impl->getDocument();
+ NodeImpl *outermostWrappedNode = m_impl.get();
+ for (NodeImpl *current = m_impl->parentNode(); current; current = current->parentNode()) {
+ if (ScriptInterpreter::getDOMNodeForDocument(document, current))
+ outermostWrappedNode = current;
+ }
+
+ markingTree = true;
+
+ NodeImpl *nodeToMark = outermostWrappedNode;
+ while (nodeToMark) {
+ DOMNode *wrapper = ScriptInterpreter::getDOMNodeForDocument(document, nodeToMark);
+ if (!wrapper)
+ nodeToMark = nodeToMark->traverseNextNode();
+ else if (wrapper->marked())
+ nodeToMark = nodeToMark->traverseNextSibling();
+ else {
+ wrapper->mark();
+ nodeToMark = nodeToMark->traverseNextNode();
+ }
+ }
+
+ markingTree = false;
+}
+
bool DOMNode::toBoolean(ExecState *) const
{
return m_impl.notNull();
ValueImp *getDOMNode(ExecState *exec, NodeImpl *n)
{
- DOMObject *ret = 0;
+ DOMNode *ret = 0;
if (!n)
return Null();
ScriptInterpreter* interp = static_cast<ScriptInterpreter *>(exec->dynamicInterpreter());
DocumentImpl *doc = n->getDocument();
- if ((ret = interp->getDOMObjectForDocument(doc, n)))
+ if ((ret = interp->getDOMNodeForDocument(doc, n)))
return ret;
switch (n->nodeType()) {
ret = new DOMNode(exec, n);
}
- interp->putDOMObjectForDocument(doc, n, ret);
+ interp->putDOMNodeForDocument(doc, n, ret);
return ret;
}
class DOMNode : public DOMObject {
public:
DOMNode(ExecState *exec, DOM::NodeImpl *n);
+ virtual ~DOMNode();
virtual bool toBoolean(ExecState *) const;
virtual Value tryGet(ExecState *exec, const Identifier &propertyName) const;
Value getValueProperty(ExecState *exec, int token) const;
-
+ virtual void mark();
virtual void tryPut(ExecState *exec, const Identifier &propertyName, const Value& value, int attr = None);
void putValue(ExecState *exec, int token, const Value& value, int attr);
DOM::NodeImpl *impl() const { return m_impl.get(); }
assert(m_savedRenderer == 0);
#endif
- KJS::ScriptInterpreter::forgetDOMObjectsForDocument(this);
+ KJS::ScriptInterpreter::forgetAllDOMNodesForDocument(this);
if (changedDocuments && m_docChanged)
changedDocuments->remove(this);
// only do this once we know there won't be an exception
if (shouldAdoptChild) {
- KJS::ScriptInterpreter::updateDOMObjectDocument(newChild, newChild->getDocument(), getDocument());
+ KJS::ScriptInterpreter::updateDOMNodeDocument(newChild, newChild->getDocument(), getDocument());
newChild->setDocument(getDocument()->docPtr());
}
}
git://git.webkit.org / WebKit-https.git / commitdiff