Last Comment Bug 894147 - Don't leak BrowserElementParent by holding a ref to it from an event handler registered on its window
: Don't leak BrowserElementParent by holding a ref to it from an event handler ...
Status: RESOLVED FIXED
[MemShrink][LeoVB+] QARegressExclude
:
Product: Firefox OS
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: 1.1 QE4 (15jul)
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
Depends on:
Blocks: 893012
  Show dependency treegraph
 
Reported: 2013-07-15 17:40 PDT by Justin Lebar (not reading bugmail)
Modified: 2013-11-01 12:14 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
leo+
wontfix
wontfix
fixed
fixed
wontfix
wontfix
fixed


Attachments
Patch, v1: Don't leak BrowserElementParent due to an event listener on the window which contains it. (4.00 KB, patch)
2013-07-15 17:42 PDT, Justin Lebar (not reading bugmail)
khuey: review+
Details | Diff | Review

Description Justin Lebar (not reading bugmail) 2013-07-15 17:40:41 PDT
I put the Gaia fixes for bug 893012 into their own bug, so we might as well put the Gecko fix for that bug into its own bug.  I will move the attachment over here.
Comment 1 Justin Lebar (not reading bugmail) 2013-07-15 17:42:35 PDT
Created attachment 776068 [details] [diff] [review]
Patch, v1: Don't leak BrowserElementParent due to an event listener on the window which contains it.

khuey, want to do the honors here?
Comment 2 Justin Lebar (not reading bugmail) 2013-07-17 14:33:49 PDT
https://hg.mozilla.org/projects/birch/rev/d23e2b6fb808
Comment 3 Ryan VanderMeulen [:RyanVM] 2013-07-18 12:08:01 PDT
https://hg.mozilla.org/mozilla-central/rev/d23e2b6fb808

"[jlebar will uplift]" - <3
Comment 4 Justin Lebar (not reading bugmail) 2013-07-18 12:19:04 PDT
https://hg.mozilla.org/releases/mozilla-b2g18/rev/d95eb0b797a6
Comment 5 Justin Lebar (not reading bugmail) 2013-07-18 13:57:58 PDT
Ah, crap, this has debugging stuff in it that we should *definitely* get rid of.

>     1.81 +  _gotLocationChange: function(data) {
>     1.82 +    this[data._payload_] = 'BEP location';
>     1.83 +    this._fireEventFromMsg(data);
>     1.84 +  },
>     1.85 +

This causes us to leak every URI until we close the frame.  :(
Comment 7 Justin Lebar (not reading bugmail) 2013-07-18 14:59:28 PDT
Dang, I got the tricky part of this merge right, but then I messed up the simple part.  This push should work.

I got rid of the debugging stuff from comment 5 in this push.  I'll push to m-c to get rid of it there, too.

https://hg.mozilla.org/releases/mozilla-b2g18/rev/54acb89975b5
Comment 8 Justin Lebar (not reading bugmail) 2013-07-18 15:08:08 PDT
And pushed the debug-code removal to birch: https://hg.mozilla.org/projects/birch/rev/0d19bdaf4f2b
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-07-19 06:55:48 PDT
(In reply to Justin Lebar [:jlebar] from comment #8)
> And pushed the debug-code removal to birch:
> https://hg.mozilla.org/projects/birch/rev/0d19bdaf4f2b

https://hg.mozilla.org/mozilla-central/rev/0d19bdaf4f2b
Comment 11 dkumar 2013-07-22 11:20:32 PDT
Can you please provide steps to verify this fix? - as we can perform blackbox testing from the UI

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