WebappsApplication objects are leaked until their corresponding window dies

RESOLVED FIXED in Firefox 25

Status

RESOLVED FIXED
6 years ago
a year ago

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

Tracking

Trunk
mozilla25
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [MemShrink][qa-])

Attachments

(3 attachments, 1 obsolete attachment)

WebappsApplication inherits from
Oops, hit enter too early.  :)

WebappsApplication inherits from DOMRequestIpcHelper.

DOMRequestIpcHelper adds some strong refs to itself in initHelper.

Those strong refs are manually unlinked on inner-window-destroyed.
(Assignee)

Updated

6 years ago
Blocks: 851626
Oh gosh, we may leak them forever.

DOMRequestIpcHelper has a QI method.  You can QI to nsIObserver.  This observer is the thing that unlinks the DOMRequestIpcHelper when the window closes.

But suppose an object "inherits" from DOMRequestIpcHelper and declares its own QI method, and suppose that doesn't include nsIObserver.  I'd expect that the observer never fires, at this point.

Both WebappsRegistry and WebappsApplication have this problem.
Even after (I think) fixing the leaks in Gecko, it seems like Gaia is leaking all app objects forever, or something.

I traced the roots graph back to

via nsXPCWrappedJS::mJSObj :
0x48c362c0 [Function]
    --[**UNKNOWN SLOT 0**]-> 0x48c345b0 [Object <no private>]
    --[app]-> 0x43dc3c40 [XPCWrappedNative_NoHelper 4721f7f0]

via nsXPCWrappedJS::mJSObj :
0x43da2820 [Object <no private>]
    --[updatableApps]-> 0x43da6bf0 [Array <no private>]
    --[objectElements[40]]-> 0x48c345b0 [Object <no private>]
    --[app]-> 0x43dc3c40 [XPCWrappedNative_NoHelper 4721f7f0]

I don't know how to tell what the first one is, but in the second one, "updatableApps" comes from Gaia.
Here are the roots for another one of these leaked objects.

Again, this is all Gaia, afaict.

> $ python gc/find_roots.py ~/code/moz/B2G/tools/gc-logs-0/gc-edges.3239.13729013
> 32.log 0x43d394e0
> Parsing /home/jlebar/code/moz/B2G/tools/gc-logs-0/gc-edges.3239.1372901332.log. Done loading graph. Reversing graph. Done.
> 
> via nsXPCWrappedJS::mJSObj :
> 0x43daab60 [Function ai_handleApplicationInstallEvent]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daac60 [Function ai_handleCancelDownloadCancel]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via nsXPCWrappedJS::mJSObj :
> 0x43daab80 [Function ai_handleApplicationUninstall]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daabc0 [Function ai_showInstallCancelDialog]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via nsXPCWrappedJS::mJSObj :
> 0x43daab40 [Function ai_handleChromeEvent]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daac20 [Function ai_showDownloadCancelDialog]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daac00 [Function ai_hideInstallCancelDialog]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daaba0 [Function ai_handleInstall]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daac40 [Function ai_handleConfirmDownloadCancel]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via mCallback :
> 0x43daabe0 [Function ai_handleInstallCancel]
>     --[**UNKNOWN SLOT 0**]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via XPCWrappedNative::mFlatJSObject :
> 0x43d240d0 [Window 45da4b20]
>     --[AppInstallManager]-> 0x43da2430 [Object <no private>]
>     --[handleApplicationReady]-> 0x43daad00 [Function ai_handleApplicationReady]
>     --[shape]-> 0x43dab370 [shape]
>     --[base]-> 0x43da5a10 [base_shape]
>     --[parent]-> 0x43daa7a0 [Function ai_handleApplicationReady]
>     --[script]-> 0x43d2ce80 [script app://system.gaiamobile.org/js/app_install_manager.js:73]
>     --[objects[0]]-> 0x43daa3a0 [Function ai_handleApplicationReady/<]
>     --[lazyScript]-> 0x43da87f0 [lazyscript]
>     --[realScript]-> 0x43d2cf00 [script app://system.gaiamobile.org/js/app_install_manager.js:80]
>     --[function]-> 0x43d33860 [Function ai_handleApplicationReady/<]
>     --[fun_callscope]-> 0x48c05cd0 [Call <no private>]
>     --[apps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via nsXPCWrappedJS::mJSObj :
> 0x43d54d80 [Function a_uninstall]
>     --[fun_callscope]-> 0x43d55370 [Call <no private>]
>     --[self]-> 0x43d51920 [Object <no private>]
>     --[installedApps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
> 
> via nsXPCWrappedJS::mJSObj :
> 0x43d54d60 [Function a_install]
>     --[lazyScript]-> 0x43d581f0 [lazyscript]
>     --[enclosingScope]-> 0x43d54ba0 [Function a_init]
>     --[script]-> 0x43d28600 [script app://system.gaiamobile.org/js/applications.js:12]
>     --[objects[0]]-> 0x43d54a60 [Function getAllApps]
>     --[lazyScript]-> 0x43d58190 [lazyscript]
>     --[realScript]-> 0x48c39b00 [script app://system.gaiamobile.org/js/applications.js:16]
>     --[objects[0]]-> 0x43d54a80 [Function mozAppGotAll]
>     --[lazyScript]-> 0x43d58160 [lazyscript]
>     --[lazyScriptInnerFunction]-> 0x43d54aa0 [Function mozAppGotAll/<]
>     --[lazyScript]-> 0x43d58130 [lazyscript]
>     --[realScript]-> 0x43d2cd80 [script app://system.gaiamobile.org/js/applications.js:19]
>     --[function]-> 0x43d337e0 [Function mozAppGotAll/<]
>     --[fun_callscope]-> 0x43d55370 [Call <no private>]
>     --[self]-> 0x43d51920 [Object <no private>]
>     --[installedApps]-> 0x43d55340 [Object <no private>]
>     --[https://marketplace.firefox.com/packaged.webapp]-> 0x43d394e0 [XPCWrappedNative_NoHelper 48861550]
So...I think we should take the patch I've written for this, which will make it so Gecko no longer leaks these objects forever.

Then we should probably focus our efforts on bug 889990, which will make Gaia's leaking of these objects not a huge deal.
Created attachment 771134 [details]
gc edges from comments above (fix applied)
(Assignee)

Updated

6 years ago
Attachment #771134 - Attachment description: gc edges from comments above → gc edges from comments above (fix applied)
Created attachment 771135 [details]
cc edges from comments above (fix applied)
Created attachment 771139 [details] [diff] [review]
Patch, v1
Attachment #771139 - Flags: review?(fabrice)
(In reply to Justin Lebar [:jlebar] from comment #8)
> Created attachment 771139 [details] [diff] [review]
> Patch, v1

Whoops, ignore that hunk in nsMemoryInfoDumper.cpp.  That is for a separate bug.
Comment on attachment 771139 [details] [diff] [review]
Patch, v1

Review of attachment 771139 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/DOMRequestHelper.jsm
@@ +160,1 @@
>        return;

It looks like we will always hit this return case.
> It looks like we will always hit this return case.

Could you give a little more context?  I'm not sure which return you're talking about.
Comment on attachment 771139 [details] [diff] [review]
Patch, v1

Review of attachment 771139 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/DOMRequestHelper.jsm
@@ +160,1 @@
>        return;

this._destroyed = true;
if (this._destroyed) {
 return;
}

Setting _destroyed, then immediately checking if it's true?
Ah, I see; yes, those lines should be swapped, of course.
Comment on attachment 771139 [details] [diff] [review]
Patch, v1

Review of attachment 771139 [details] [diff] [review]:
-----------------------------------------------------------------

That looks mostly good, but I'd like to see a new version with comments addressed.

::: dom/apps/src/Webapps.js
@@ +249,5 @@
>  
>      let util = this._window.QueryInterface(Ci.nsIInterfaceRequestor)
>                             .getInterface(Ci.nsIDOMWindowUtils);
>      this._id = util.outerWindowID;
>      cpmm.sendAsyncMessage("Webapps:RegisterForMessages",

Didn't you forget to add Ci.nsISupportsWeakReference to WebappsRegistry object QueryInterface?

::: dom/base/DOMRequestHelper.jsm
@@ +9,1 @@
>  const Cu = Components.utils; 

Nit: feel free to clean this trailing whitespace.

@@ +35,5 @@
> + * appropriate window is destroyed.  We use DOMRequestIpcHelperMessageListener
> + * for this, too.
> + */
> +this.DOMRequestIpcHelperMessageListener = function(aHelper, aWindow, aMessages) {
> +  this._weakHelper = Cu.getWeakReference(aHelper);

You're only using _weakHelper.get() so you could just keep that.

::: dom/push/src/Push.js
@@ +57,5 @@
>      let perm = Services.perms.testExactPermissionFromPrincipal(principal,
>                                                                 "push");
>      if (perm != Ci.nsIPermissionManager.ALLOW_ACTION)
>        return null;
>  

Is is ok to not call initDomRequestHelper() at all? We won't hook up the inner-window-destroyed observer.
Attachment #771139 - Flags: review?(fabrice) → feedback+
::: dom/apps/src/Webapps.js
@@ +249,5 @@
>
>      let util = this._window.QueryInterface(Ci.nsIInterfaceRequestor)
>                             .getInterface(Ci.nsIDOMWindowUtils);
>      this._id = util.outerWindowID;
>      cpmm.sendAsyncMessage("Webapps:RegisterForMessages",

> Didn't you forget to add Ci.nsISupportsWeakReference to WebappsRegistry object
> QueryInterface?

Yeah, thanks.

@@ +35,5 @@
> + * appropriate window is destroyed.  We use
DOMRequestIpcHelperMessageListener
> + * for this, too.
> + */
> +this.DOMRequestIpcHelperMessageListener = function(aHelper, aWindow,
aMessages) {
> +  this._weakHelper = Cu.getWeakReference(aHelper);
>
> You're only using _weakHelper.get() so you could just keep that.

I don't understand; what do you mean?

::: dom/push/src/Push.js
@@ +57,5 @@
>      let perm = Services.perms.testExactPermissionFromPrincipal(principal,
>                                                                 "push");
>      if (perm != Ci.nsIPermissionManager.ALLOW_ACTION)
>        return null;
>

> Is is ok to not call initDomRequestHelper() at all? We won't hook up the
> inner-window-destroyed observer.

It should be initDOMRequestHelper here, not initMessageListener, yes.  That function doesn't even exist anymore.

JS is hard.
(In reply to Justin Lebar [:jlebar] from comment #15)

> @@ +35,5 @@
> > + * appropriate window is destroyed.  We use
> DOMRequestIpcHelperMessageListener
> > + * for this, too.
> > + */
> > +this.DOMRequestIpcHelperMessageListener = function(aHelper, aWindow,
> aMessages) {
> > +  this._weakHelper = Cu.getWeakReference(aHelper);
> >
> > You're only using _weakHelper.get() so you could just keep that.
> 
> I don't understand; what do you mean?

instead of |this._weakHelper = Cu.getWeakReference(aHelper)|, doing |this._weakHelper = Cu.getWeakReference(aHelper).get()|.

That's a really minor change though.
> instead of |this._weakHelper = Cu.getWeakReference(aHelper)|, doing |this._weakHelper = 
> Cu.getWeakReference(aHelper).get()|.

Isn't that totally different?  The latter makes this._weakHelper a strong reference, right?
(In reply to Justin Lebar [:jlebar] from comment #17)
> > instead of |this._weakHelper = Cu.getWeakReference(aHelper)|, doing |this._weakHelper = 
> > Cu.getWeakReference(aHelper).get()|.
> 
> Isn't that totally different?  The latter makes this._weakHelper a strong
> reference, right?

You're right, my bad.
Created attachment 772300 [details] [diff] [review]
Patch, v2
Attachment #771139 - Attachment is obsolete: true
Attachment #772300 - Flags: review?(fabrice)
Attachment #772300 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/4b00bb32356d
Assignee: nobody → justin.lebar+bug
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Depends on: 893188
I think we forgot to leo? this this.  This blocks two blockers.
Blocks: 886217, 897684
blocking-b2g: --- → leo+
Needs a branch-specific patch for uplift.
status-b2g18: --- → affected
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
Flags: needinfo?(justin.lebar+bug)
Keywords: branch-patch-needed
This will be pain in the rear to backport, and we had multiple regressions when landing on trunk.

I may have a safer fix in bug 900221.  Stay tuned.
Flags: needinfo?(justin.lebar+bug)
(Assignee)

Updated

6 years ago
Flags: needinfo?(justin.lebar+bug)
Given bug 901759, I think we'd just need to change {add,remove}MessageListener to {add,remove}WeakMessageListener in DOMRequestHelpers.
Flags: needinfo?(justin.lebar+bug)
> This will be pain in the rear to backport, and we had multiple regressions when landing on trunk.

And moreover, backporting this bug isn't going to fix the entire problem, because we'll still leak message listeners, and that's going to slow lots of things down (e.g. dispatching messages).
Here's a git branch with the alternative approach we've discussed in bug 900221 and dependent bugs.

It would be good to get some testing of these patches before we land them.
I'm going to move the leo+ here over to bug 900221, which intends to fix the same problem, but in a way that's safer for branches.  In fact, if everything works as planned there, we should be able to back out this change.
blocking-b2g: leo+ → ---
(Assignee)

Updated

6 years ago
status-b2g18: affected → wontfix
status-b2g-v1.1hd: affected → wontfix
Keywords: branch-patch-needed
(Assignee)

Updated

6 years ago
No longer blocks: 897684
(Assignee)

Updated

6 years ago
status-b2g18: wontfix → fixed
status-b2g-v1.1hd: wontfix → affected
Sorry for the flag spam.
status-b2g18: fixed → wontfix
status-b2g-v1.1hd: affected → wontfix
Fabrice, you may want to back out these DOMRequestHelper changes now that we've switched too {add,remove}WeakMessageListener; it's not necessary anymore to split up the DOMRequestHelper class like this.
Whiteboard: [MemShrink] → [MemShrink][qa-]

Updated

a year ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.