Static NodeList needs to to keep item wrappers alive

[Erik Arvidsson]

Created attachment 151772 [details]
Layout test

Currently we do not keep the wrappers of static node lists alive. Both the JSC and V8 bindings get this wrong.

document.body.innerHTML = '<b></b>';
var elements = document.body.querySelectorAll('*');
elements[0].customProperty = 42;
document.body.textContent = '';
gc();
shouldBe('elements[0].customProperty', '42');

[Geoffrey Garen]

Does this problem only apply to StaticNodeList? What about StaticHashSetNodeList and DynamicNodeList?

In JSC, the way to fix this is to make the node list wrapper's visit function add its document to the opaque root set. This will require giving all NodeLists, and not just DynamicNodeLists, owner document pointers.

[Erik Arvidsson]

(In reply to comment #1)
> Does this problem only apply to StaticNodeList? What about StaticHashSetNodeList and DynamicNodeList?

Most (all?) of the DynamicNodeLists keep the wrappers alive though other means. For example the nodes in a childNodes NodeList are kept alive as long as the parent node is kept alive.

[Geoffrey Garen]

How do they keep the parent node alive?

[Ryosuke Niwa]

(In reply to comment #3)
> How do they keep the parent node alive?

DynamicNodeList has a RefPtr to its owner.

[Geoffrey Garen]

But what keeps the JavaScript wrapper alive?

[Adam Klein]

*** Bug 105666 has been marked as a duplicate of this bug. ***