Closed Bug 889984 Opened 11 years ago Closed 11 years ago

WebappsApplication objects are leaked until their corresponding window dies

Categories

(Core Graveyard :: DOM: Apps, defect)

defect
Not set
normal

Tracking

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

RESOLVED FIXED
mozilla25
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- fixed
b2g18 --- wontfix
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- wontfix

People

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

References

Details

(Whiteboard: [MemShrink][qa-])

Attachments

(3 files, 1 obsolete file)

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.
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.
Attachment #771134 - Attachment description: gc edges from comments above → gc edges from comments above (fix applied)
Attached patch Patch, v1 (obsolete) — Splinter Review
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.
Attached patch Patch, v2Splinter Review
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
Closed: 11 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.
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)
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+ → ---
No longer blocks: 897684
Sorry for the flag spam.
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-]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: