We can use opaqueRootForGC() instead. By this change, we can remove all AddImplicitReferences() from V8 bindings.
Created attachment 177899 [details] Patch
Comment on attachment 177899 [details] Patch I tried this before but ran into trouble (some tests failed). It's likely I screwed it up though.
Comment on attachment 177899 [details] Patch There is a subtle difference here: Previously when someone was hold an event listener function, they wouldn't hold the node it was attached to (because AddImplicitReferences are one-way connections). Now we'll hold the node as well (because addToGroup is a two-way connection). I can't see how that would matter in practice.
(In reply to comment #3) > (From update of attachment 177899 [details]) > There is a subtle difference here: Previously when someone was hold an event listener function, they wouldn't hold the node it was attached to (because AddImplicitReferences are one-way connections). Now we'll hold the node as well (because addToGroup is a two-way connection). I can't see how that would matter in practice. Good point. In theory, the following situation would be possible: - There is a tree T. - A node P is in T. P has an event listener L. - A JS object X has a reference to L. X is reachable from JS roots. - All nodes in T become unreachable. - T is expected to be collected. However, due to the reference from X to L, T is not collected. Maybe we shouldn't make this change?
> Maybe we shouldn't make this change? I don't think we should worry about it.
Comment on attachment 177899 [details] Patch ok
Comment on attachment 177899 [details] Patch Clearing flags on attachment: 177899 Committed r136794: <http://trac.webkit.org/changeset/136794>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > > Maybe we shouldn't make this change? > > I don't think we should worry about it. Could you elaborate on this? Now adding a listener to a node will keep the tree alive forever unless the listener is also collected. I can easily imagine a case when the same function is reused as an event listener for different DOM trees and in this case all the trees will never be GCed. What is the benefit of this patch given that it introduces this regression? I think it should be rolled out.
(In reply to comment #9) > (In reply to comment #5) > > > Maybe we shouldn't make this change? > > > > I don't think we should worry about it. > > Could you elaborate on this? Now adding a listener to a node will keep the tree alive forever unless the listener is also collected. I can easily imagine a case when the same function is reused as an event listener for different DOM trees and in this case all the trees will never be GCed. > > What is the benefit of this patch given that it introduces this regression? I think it should be rolled out. Adam is on vacation. I'll roll out the patch for now.
Rolled out in r136794.
(In reply to comment #11) > Rolled out in r136794. You probably meant r137885