Don't leak BrowserElementParent by holding a ref to it from an event handler registered on its window

RESOLVED FIXED in Firefox 25, Firefox OS v1.1hd


Firefox OS
4 years ago
4 years ago


(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))


1.1 QE4 (15jul)

Firefox Tracking Flags

(blocking-b2g:leo+, firefox23 wontfix, firefox24 wontfix, firefox25 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)


(Whiteboard: [MemShrink][LeoVB+] QARegressExclude)


(1 attachment)



4 years ago
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.


4 years ago
Whiteboard: [MemShrink]

Comment 1

4 years ago
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?
Attachment #776068 - Flags: review?(khuey)
Attachment #776068 - Flags: review?(khuey) → review+
Assignee: nobody → justin.lebar+bug


4 years ago
blocking-b2g: --- → leo+

Comment 2

4 years ago
Whiteboard: [MemShrink] → [MemShrink][jlebar will uplift]

"[jlebar will uplift]" - <3
Last Resolved: 4 years ago
Resolution: --- → FIXED

Comment 4

4 years ago
Whiteboard: [MemShrink][jlebar will uplift] → [MemShrink]
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-b2g-v1.1hd: --- → affected
status-firefox23: --- → wontfix
status-firefox24: --- → wontfix
status-firefox25: --- → fixed
Target Milestone: --- → 1.1 QE4 (15jul)

Comment 5

4 years ago
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.  :(
Backed out due to debug mochitest orange.
status-b2g18: fixed → affected

Comment 7

4 years ago
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.

Comment 8

4 years ago
And pushed the debug-code removal to birch:
status-b2g18: affected → fixed
status-b2g-v1.1hd: affected → fixed
(In reply to Justin Lebar [:jlebar] from comment #8)
> And pushed the debug-code removal to birch:

Comment 11

4 years ago
Can you please provide steps to verify this fix? - as we can perform blackbox testing from the UI


4 years ago
Whiteboard: [MemShrink] → [MemShrink][LeoVB+]


4 years ago
Whiteboard: [MemShrink][LeoVB+] → [MemShrink][LeoVB+] QARegressExclude
You need to log in before you can comment on or make changes to this bug.