Commit e0fc2216 authored by ggaren@apple.com's avatar ggaren@apple.com

If Node X is reachable from JavaScript, all Nodes in the same tree should be kept alive

https://bugs.webkit.org/show_bug.cgi?id=88834

Reviewed by Gavin Barraclough.

Source/WebCore: 

* bindings/js/JSNodeCustom.cpp:
(WebCore::isObservable): Clarified this comment, since it seems to have
misled some folks. 

* bindings/js/JSNodeCustom.h:
(WebCore::willCreatePossiblyOrphanedTreeByRemoval): New helper function
to ensure that a disconnected tree is visible to JavaScript.

* bindings/js/ScriptState.cpp:
(WebCore::mainWorldScriptState): Need to check for null because a document's
frame can be null.

* dom/ContainerNode.cpp:
(WebCore::dispatchChildRemovalEvents): When we remove a subtree from the
document, we sever the reference that JavaScript previously held by
referencing its root. So, we give JavaScript an opportunity to establish
a reference to the new root.

LayoutTests: 

* fast/dom/gc-12-expected.txt: Added.
* fast/dom/gc-12.html: Added. Test case matches an example cited by
Kentaro Hara <haraken@chromium.org> in bugzilla.

* fast/dom/gc-3-expected.txt:
* fast/dom/gc-3.html:
* fast/dom/gc-5-expected.txt:
* fast/dom/gc-5.html: Updated these tests to reflect new expected behavior.
We've decided that disconnected trees should persist in memory. This risks
a programmer accidentally retaining more memory than expected, but it
also makes the API more obvious.

* fast/dom/gc-dom-tree-lifetime-expected.txt: Added.
* fast/dom/gc-dom-tree-lifetime.html: Added. Test case written by
Kentaro Hara <haraken@chromium.org>.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@130584 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 32bec245
2012-10-04 Geoffrey Garen <ggaren@apple.com>
If Node X is reachable from JavaScript, all Nodes in the same tree should be kept alive
https://bugs.webkit.org/show_bug.cgi?id=88834
Reviewed by Gavin Barraclough.
* fast/dom/gc-12-expected.txt: Added.
* fast/dom/gc-12.html: Added. Test case matches an example cited by
Kentaro Hara <haraken@chromium.org> in bugzilla.
* fast/dom/gc-3-expected.txt:
* fast/dom/gc-3.html:
* fast/dom/gc-5-expected.txt:
* fast/dom/gc-5.html: Updated these tests to reflect new expected behavior.
We've decided that disconnected trees should persist in memory. This risks
a programmer accidentally retaining more memory than expected, but it
also makes the API more obvious.
* fast/dom/gc-dom-tree-lifetime-expected.txt: Added.
* fast/dom/gc-dom-tree-lifetime.html: Added. Test case written by
Kentaro Hara <haraken@chromium.org>.
2012-10-06 Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
[EFL] Gardening.
......
This test verifies that DOM nodes are not garbage collected as long as a node in the tree is referenced from JavaScript.
PASS: span should be a Node and is.
PASS: span.parentNode should be a Node and is.
PASS: span.firstChild should be a Node and is.
PASS: span should be a Node and is.
PASS: span.parentNode should be a Node and is.
<p>
This test verifies that DOM nodes are not garbage collected as long as a node in the
tree is referenced from JavaScript.
</p>
<pre id="console"></pre>
<script>
function log(s)
{
document.getElementById("console").appendChild(document.createTextNode(s + "\n"));
}
function shouldBeNode(a, aDescription)
{
if (!(a instanceof Node)) {
log("FAIL: " + aDescription + " should be a Node but instead is " + a + ".");
return;
}
log("PASS: " + aDescription + " should be a Node and is.");
}
function gc()
{
if (window.GCController)
return GCController.collect();
for (var i = 0; i < 10000; i++)
var s = new String("");
}
(function () {
if (window.testRunner) {
testRunner.dumpAsText();
}
(function() {
// Try to create an orphan tree by removal.
var p = document.createElement("p");
document.body.appendChild(p);
p.innerHTML ='<div><span id="span"><br></span></div>';
var span = document.getElementById("span");
p.innerHTML = "";
gc();
shouldBeNode(span, "span");
shouldBeNode(span.parentNode, "span.parentNode");
shouldBeNode(span.firstChild, "span.firstChild");
})();
(function() {
// Try to create an orphan tree by insertion.
var p = document.createElement("p");
var span = document.createElement("span");
p.appendChild(span);
p = null;
gc();
shouldBeNode(span, "span");
shouldBeNode(span.parentNode, "span.parentNode");
})();
})();
</script>
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.
This test verifies that DOM nodes are retained because a wrapper exists for a descendant. A wrapper need not 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".
The output should be the following pieces of text on lines by themselves: "replacement content", "B", "parent node exists", "child node exists".
replacement content
B
parent node exists
child node exists
......@@ -36,10 +36,10 @@ if (window.testRunner) {
<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.
This test verifies that DOM nodes are retained because a wrapper exists for a descendant. A wrapper need not 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".
The output should be the following pieces of text on lines by themselves: "replacement content", "B", "parent node exists", "child node exists".
</p>
</div>
<div id="div">
......
This test verifies that DOM nodes are not retained just because a wrapper exists and is protected for a sibling. A wrapper must exist for the node itself or for an ancestor.
This test verifies that DOM nodes are retained because a wrapper exists and is protected for a sibling. A wrapper need not 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".
The output should be the following pieces of text on lines by themselves: "replacement content", "B", "D".
replacement content
B
D
......@@ -34,10 +34,10 @@ if (window.testRunner) {
<body onload="doit()">
<div style="border: 1px solid red">
<p>
This test verifies that DOM nodes are not retained just because a wrapper exists and is protected for a sibling. A wrapper must exist for the node itself or for an ancestor.
This test verifies that DOM nodes are retained because a wrapper exists and is protected for a sibling. A wrapper need not 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".
The output should be the following pieces of text on lines by themselves: "replacement content", "B", "D".
</p>
</div>
<div id="div">
......
=== Initial state ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML and divX ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML, divX and divY ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML, divX, divY and divZ ===
PASS All <div> objects in a DOM tree are successfully destructed.
=== Initial state ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML and divX ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML, divX and divY ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML, divX, divY and divZ ===
PASS All <div> objects in a DOM tree are successfully destructed.
=== Initial state ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML and divX ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML, divX and divY ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
=== After clearing innerHTML, divX, divY and divZ ===
PASS All <div> objects in a DOM tree are successfully destructed.
=== Initial state ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML and divX ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML, divX and divY ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML, divX, divY and divZ ===
PASS All <div> objects in a DOM tree are successfully destructed.
=== Initial state ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML and divX ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML, divX and divY ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
=== After clearing innerHTML, divX, divY and divZ ===
PASS All <div> objects in a DOM tree are successfully destructed.
=== Initial state ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML ===
PASS globalDiv.id is "div2"
PASS globalDiv.parentNode.id is "div2-parent"
PASS globalDiv.firstChild.id is "div2-child"
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML and divX ===
PASS globalDiv.id is "div1"
PASS globalDiv.parentNode.id is "div1-parent"
PASS globalDiv.firstChild.id is "div1-child"
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML, divX and divY ===
PASS globalDiv.id is "div0"
PASS globalDiv.parentNode.id is "div0-parent"
PASS globalDiv.firstChild.id is "div0-child"
=== After clearing innerHTML, divX, divY and divZ ===
PASS All <div> objects in a DOM tree are successfully destructed.
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE html>
<html>
<head>
<script src="../js/resources/js-test-pre.js"></script>
</head>
<body>
<script>
var testCases = [["div0", "div1", "div2"],
["div0", "div2", "div1"],
["div1", "div0", "div2"],
["div1", "div2", "div0"],
["div2", "div0", "div1"],
["div2", "div1", "div0"]];
var rootDiv = document.createElement("div");
document.body.appendChild(rootDiv);
var testHtml = "<div id='div0-parent'><div id='div0'><div id='div0-child'></div></div><div id='div1-parent'><div id='div1'><div id='div1-child'></div></div><div id='div2-parent'><div id='div2'><div id='div2-child'></div></div></div></div></div>";
testCases.forEach(function (test) {
var divX, divY, divZ;
debug("=== Initial state ===");
rootDiv.innerHTML = testHtml;
divX = document.getElementById(test[0]);
divY = document.getElementById(test[1]);
divZ = document.getElementById(test[2]);
checkParentAndChildAlive(divX, test[0]);
checkParentAndChildAlive(divY, test[1]);
checkParentAndChildAlive(divZ, test[2]);
debug("=== After clearing innerHTML ===");
rootDiv.innerHTML = testHtml;
divX = document.getElementById(test[0]);
divY = document.getElementById(test[1]);
divZ = document.getElementById(test[2]);
rootDiv.innerHTML = "";
checkParentAndChildAlive(divX, test[0]);
checkParentAndChildAlive(divY, test[1]);
checkParentAndChildAlive(divZ, test[2]);
debug("=== After clearing innerHTML and divX ===");
rootDiv.innerHTML = testHtml;
divX = document.getElementById(test[0]);
divY = document.getElementById(test[1]);
divZ = document.getElementById(test[2]);
rootDiv.innerHTML = "";
divX = null;
gc();
checkParentAndChildAlive(divY, test[1]);
checkParentAndChildAlive(divZ, test[2]);
debug("=== After clearing innerHTML, divX and divY ===");
rootDiv.innerHTML = testHtml;
divX = document.getElementById(test[0]);
divY = document.getElementById(test[1]);
divZ = document.getElementById(test[2]);
rootDiv.innerHTML = "";
divX = null;
divY = null;
gc();
checkParentAndChildAlive(divZ, test[2]);
debug("=== After clearing innerHTML, divX, divY and divZ ===");
rootDiv.innerHTML = testHtml;
divX = document.getElementById(test[0]);
divY = document.getElementById(test[1]);
divZ = document.getElementById(test[2]);
if (window.internals)
prevNodes = window.internals.numberOfLiveNodes();
rootDiv.innerHTML = "";
divX = null;
divY = null;
divZ = null;
gc();
if (window.internals) {
// If all the Node objects in testHtml are successfully destructed,
// at least 9 <div>s objects will be removed.
// (Actually, since testHtml rendering requires more than 9 Node objects.)
if (window.internals.numberOfLiveNodes() <= prevNodes - 9)
testPassed("All <div> objects in a DOM tree are successfully destructed.");
else
testFailed("<div> objects in a DOM tree are not destructed.");
}
});
function checkParentAndChildAlive(div, name) {
globalDiv = div;
shouldBeEqualToString('globalDiv.id', name);
shouldBeEqualToString('globalDiv.parentNode.id', name + "-parent");
shouldBeEqualToString('globalDiv.firstChild.id', name + "-child");
globalDiv = null;
gc();
}
var successfullyParsed = true;
</script>
<script src="../js/resources/js-test-post.js"></script>
</body>
</html>
2012-10-04 Geoffrey Garen <ggaren@apple.com>
If Node X is reachable from JavaScript, all Nodes in the same tree should be kept alive
https://bugs.webkit.org/show_bug.cgi?id=88834
Reviewed by Gavin Barraclough.
* bindings/js/JSNodeCustom.cpp:
(WebCore::isObservable): Clarified this comment, since it seems to have
misled some folks.
* bindings/js/JSNodeCustom.h:
(WebCore::willCreatePossiblyOrphanedTreeByRemoval): New helper function
to ensure that a disconnected tree is visible to JavaScript.
* bindings/js/ScriptState.cpp:
(WebCore::mainWorldScriptState): Need to check for null because a document's
frame can be null.
* dom/ContainerNode.cpp:
(WebCore::dispatchChildRemovalEvents): When we remove a subtree from the
document, we sever the reference that JavaScript previously held by
referencing its root. So, we give JavaScript an opportunity to establish
a reference to the new root.
2012-10-06 Byungwoo Lee <bw80.lee@samsung.com>
Fix build warning : -Wunused-parameter.
......
......@@ -85,10 +85,7 @@ using namespace HTMLNames;
static inline bool isObservable(JSNode* jsNode, Node* node)
{
// The DOM doesn't know how to keep a tree of nodes alive without the root
// being explicitly referenced. So, we artificially treat the root of
// every tree as observable.
// FIXME: Resolve this lifetime issue in the DOM, and remove this.
// The root node keeps the tree intact.
if (!node->parentNode())
return true;
......
......@@ -27,6 +27,7 @@
#define JSNodeCustom_h
#include "JSDOMBinding.h"
#include "ScriptState.h"
#include <wtf/AlwaysInline.h>
namespace WebCore {
......@@ -68,6 +69,29 @@ inline JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject,
return createWrapper(exec, globalObject, node);
}
// In the C++ DOM, a node tree survives as long as there is a reference to its
// root. In the JavaScript DOM, a node tree survives as long as there is a
// reference to any node in the tree. To model the JavaScript DOM on top of
// the C++ DOM, we ensure that the root of every tree has a JavaScript
// wrapper.
inline void willCreatePossiblyOrphanedTreeByRemoval(Node* root)
{
if (root->wrapper())
return;
if (!root->isContainerNode())
return;
if (!toContainerNode(root)->hasChildNodes())
return;
ScriptState* scriptState = mainWorldScriptState(root->document()->frame());
if (!scriptState)
return;
toJS(scriptState, static_cast<JSDOMGlobalObject*>(scriptState->lexicalGlobalObject()), root);
}
} // namespace WebCore
#endif // JSDOMNodeCustom_h
......@@ -93,6 +93,8 @@ void setEvalEnabled(ScriptState* scriptState, bool enabled)
ScriptState* mainWorldScriptState(Frame* frame)
{
if (!frame)
return 0;
JSDOMWindowShell* shell = frame->script()->windowShell(mainThreadNormalWorld());
return shell->window()->globalExec();
}
......
......@@ -46,6 +46,10 @@
#include <wtf/CurrentTime.h>
#include <wtf/Vector.h>
#if USE(JSC)
#include "JSNode.h"
#endif
using namespace std;
namespace WebCore {
......@@ -977,6 +981,9 @@ static void dispatchChildRemovalEvents(Node* child)
ASSERT(!NoEventDispatchAssertion::isEventDispatchForbidden());
#if USE(JSC)
willCreatePossiblyOrphanedTreeByRemoval(child);
#endif
InspectorInstrumentation::willRemoveDOMNode(child->document(), child);
RefPtr<Node> c = child;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment