Le 01/11/2013 21:13, Andrew McCreight a écrit :
----- Original Message -----
Why so?
function f(e){}
(function(){
var iframe = document.getElementByTagName('iframe')[0];
iframe.addEventListener('someEvent', f);
})()
In this case, f doesn't hold a reference to anything besides itself, not
to the iframe at least. In the case of DOM events, it can look at its
argument to find e.target which will dynamically (!), only for the time
of the listener call, refer to the iframe.
In any case, when an event doesn't need to be listened to anymore, there
is a time in your application where you can remove the listener. The
leak is not removing the listener, not the listener itself.
Well, you can look at the patch. I guess I described it poorly. You have some
thing (I said 'iframe' above, but I guess it isn't really one) x that wants to
listen to an event, so you create an event listener for it, that holds alive x.
The situation is more like this:
function f(e){tellSomebodyAboutTheEvent(x);}
(function(){
var iframe = document.getElementByTagName('iframe')[0];
iframe.addEventListener('someEvent', f);
})()
f is now keeping x alive, and if f is the only thing keeping x alive, then
that's basically a leak as far as we are concerned as a JS programmer.
There is a point in your program in which you know that 'iframe' won't
be needed anymore and you do something about that. Maybe you clear a
given data structure, maybe you remove the iframe from the document
without assigning it to a variable, maybe you do something else, but you
do do something.
It's at this point that either you want to remove the listener or
reassign x to another value.
(There's also the problem that f stays alive forever if the event never fires,
but that's a smaller leak...)
It's only a leak if you *know* that the event will never fire. If the
event may never fire, that's not a leak, just bad luck.
In case being concrete helps
(it does :-) )
in our case, x was the parent-process representation of some app, and the event
was some kind of visibility change event, so you can tell the app if the screen
is turned off or on. Obviously, you want to get rid of x rather than keep it
around to ensure that we can tell this zombie app thing that the screen is
going to turn off.
Ok. I think I understand better the patch now. Thanks.
I noticed a bug in the patch [1]. It's been interestingly fixed as a
side-effect of bug 899354 [2], but sharing for those interested.
In the patch [1], the event listener is
visibilityChangeHandler.bind(/*args*/). The listener that is attempted
to be removed is visibilityChangeHandler which is a different function
and was never itself registered as a listener, so the listener was never
removed and the removeEventListener call failed silently.
Even with the new version, as application author, you know when you
don't want a given browserElementParent (x) any longer, so you know when
to break all the references. The only thing that's lacking is API bits
all over the place for you to break these references. It's more work
than weak references, but it's doable.
David
[1] https://hg.mozilla.org/mozilla-central/rev/d23e2b6fb808
[2] https://hg.mozilla.org/mozilla-central/rev/602e8b21a0ff
_______________________________________________
dev-tech-js-engine-internals mailing list
dev-tech-js-engine-internals@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
