Security

(public)

User Story

In trying to fix bug 893012, we (mccr8, khuey, mrbkap, dbaron) discovered that the |apploaded| function in window_manager.js leaks homescreenFrame, even though it doesn't touch that variable.
Changing the closure so it doesn't touch |document| fixes the problem.
Patch in a moment.

This is the PR, which also includes bug 894081.
https://github.com/mozilla-b2g/gaia/pull/10991
We just need a gaia person to sign off that we're not breaking anything here. Note that I also changed the event listener from living on |frame| to living on |iframe|; this isn't strictly necessary, but given bug 894081, I think it's a good example to set.

(In reply to Alive Kuo [:alive] from comment #5)
> Justin, I want to know more about this to avoid the same thing happens again.
> Is this a general leaking pattern if we touch 'document' in any event
> handler?
We currently think this is a bug in the JS engine. You may want to follow along in Bug 894149.

(In reply to Alive Kuo [:alive] from comment #5)
> Justin, I want to know more about this to avoid the same thing happens again.
> Is this a general leaking pattern if we touch 'document' in any event
> handler?
But until we fix that bug, I think it's probably wise to avoid touching |document| from within event handlers.
In fixing this bug we've now found three leaks caused by event handlers (one in Gecko, two in Gaia), so I definitely owe the mailing lists an e-mail about this. None of the bugs was obvious, I think; we just have to be careful.

(In reply to Justin Lebar [:jlebar] from comment #7)
> so I definitely owe the mailing lists an e-mail
> about this. None of the bugs was obvious, I think; we just have to be
> careful.
Agree, it's tricky...thanks.

fwiw we now believe that pulling |document| out of the closure and getting it off the event target has no effect, but I'm pretty sure that registering the listener on |iframe| instead of |frame| is necessary. But I don't want to churn this code unnecessarily right now, so if it's OK with you guys, I'm going to leave it alone until things calm down.