Last Comment Bug 896825 - "Fix" bug 896776 in a way that is safe for leo
: "Fix" bug 896776 in a way that is safe for leo
Status: RESOLVED FIXED
[MemShrink:P1][LeoVB+]
:
Product: Firefox OS
Classification: Client Software
Component: Gaia::System (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Kyle Huey [:khuey] (khuey@mozilla.com)
:
Mentors:
Depends on:
Blocks: 893012
  Show dependency treegraph
 
Reported: 2013-07-22 18:34 PDT by Kyle Huey [:khuey] (khuey@mozilla.com)
Modified: 2013-08-07 20:16 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
leo+
fixed
fixed


Attachments
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11107 (358 bytes, text/html)
2013-07-22 21:48 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
alegnadise+moz: review+
Details

Description Kyle Huey [:khuey] (khuey@mozilla.com) 2013-07-22 18:34:10 PDT
+++ This bug was initially created as a clone of Bug #896776 +++

What I really want to do in bug 896776 is probably too invasive for leo.  So let's do something simpler.
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) 2013-07-22 21:48:25 PDT
Created attachment 779594 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11107

Pointer to Github pull-request
Comment 2 Justin Lebar (not reading bugmail) 2013-07-22 22:37:52 PDT
Blocks a blocker.
Comment 3 (inactive after 6/18) Alive Kuo [:alive] 2013-07-23 01:20:33 PDT
As I said I'm working on a new system to resolve this kind of problems.
I want to know:
* The problem is closeCallback isn't reset to null or isn't invoked?
* How if we have a static closeCallback for each frame and won't modify that callback after transition?
Comment 4 Kyle Huey [:khuey] (khuey@mozilla.com) 2013-07-23 06:31:39 PDT
(In reply to Alive Kuo [:alive][Paris work week 7/22-26] from comment #3)
> As I said I'm working on a new system to resolve this kind of problems.
> I want to know:
> * The problem is closeCallback isn't reset to null or isn't invoked?

It is not invoked.  closeCallback calls removeFrame, and if it is never invoked the iframe is never removed from the DOM.

> * How if we have a static closeCallback for each frame and won't modify that
> callback after transition?

We need to ensure that we invoke the callback.  That is the real problem here.
Comment 5 (inactive after 6/18) Alive Kuo [:alive] 2013-07-23 06:39:03 PDT
Yeah I know this problem...sigh it's not totally resolved by some workaround. So I'm working a brand-new transition control to fix all this kind of problems. Thanks for investigation.
Comment 6 Justin Lebar (not reading bugmail) 2013-07-23 08:00:43 PDT
> How about if we have a static closeCallback for each frame and won't modify that callback 
> after transition?

That could work, but you would have to be careful not to leak closeCallback, because closeCallback would probably keep the iframe alive.
Comment 7 Justin Lebar (not reading bugmail) 2013-07-23 08:03:53 PDT
In bug 896776 Alive says he has a plan to change a lot of code so as to fix this on master.  That's great, but then I think we should land this on master as well as b2g18, so both trees are leak-free.
Comment 8 (inactive after 6/18) Alive Kuo [:alive] 2013-07-23 08:04:39 PDT
(In reply to Justin Lebar [:jlebar] from comment #7)
> In bug 896776 Alive says he has a plan to change a lot of code so as to fix
> this on master.  That's great, but then I think we should land this on
> master as well as b2g18, so both trees are leak-free.

Sure!
Comment 9 [:fabrice] Fabrice Desré 2013-07-24 14:39:15 PDT
Pushed to master: https://github.com/mozilla-b2g/gaia/commit/3d64d2aefaa80919d183c95c2fd95a94f2b7702a
Comment 10 John Ford [:jhford] 2013-08-01 00:17:24 PDT
Uplifted 3d64d2aefaa80919d183c95c2fd95a94f2b7702a to:
v1-train: 25c7466682f1148da685abc1f2177a3221290d84
Comment 11 Tim Guan-tin Chien [:timdream] (please needinfo; OOO and on leave until July) 2013-08-07 20:16:19 PDT
v1.1.0hd: 25c7466682f1148da685abc1f2177a3221290d84

Note You need to log in before you can comment on or make changes to this bug.