Last Comment Bug 731875 - Geolocation doorhanger might cause a zombie compartment
: Geolocation doorhanger might cause a zombie compartment
Product: Core
Classification: Components
Component: Geolocation (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
: Andrew Overholt [:overholt]
: 744817 (view as bug list)
Depends on:
Blocks: leakychrome 745744
  Show dependency treegraph
Reported: 2012-02-29 17:16 PST by Josh Matthews [:jdm] (on vacation until Dec 5)
Modified: 2012-04-16 12:24 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (16.19 KB, patch)
2012-04-16 07:39 PDT, Olli Pettay [:smaug]
doug.turner: review+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] (on vacation until Dec 5) 2012-02-29 17:16:03 PST
I see a zombie compartment from sitting in about:compartments, and I haven't visited that test page for more than a day. I'll try to find some time to try with a blank profile, as this one has lots of addons present.
Comment 1 Andrew McCreight [:mccr8] 2012-04-12 10:29:47 PDT
*** Bug 744817 has been marked as a duplicate of this bug. ***
Comment 2 Andrew McCreight [:mccr8] 2012-04-12 10:31:56 PDT
I had a similar thing happen on Google Maps.   The documents ended up being held alive like so:

0x12a3e9000 [rc=6] nsDocument normal (xhtml) (etc)
    Roots: 1
        0x132fa0f90 [rc=2] root nsXPCWrappedJS (nsIDOMGeoPositionOptions)
0x124cb5800 [rc=6] nsDocument normal (xhtml)  (etc)
    Roots: 1
        0x132fa0f90 [rc=2] root nsXPCWrappedJS (nsIDOMGeoPositionOptions)
Comment 3 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-04-12 10:40:15 PDT
It's very interesting that the nsIDOMGeoPositionOptions object is at fault here; it looks like we would have to be holding on to an nsGeolocationRequest object somewhere for that to be the case. That backs up my theory that the doorhanger is at fault, so we should be looking at (which takes a request object and initiates a prompt that holds on to it) and (which actually creates the prompt).
Comment 4 Olli Pettay [:smaug] 2012-04-12 10:42:46 PDT
I'm going to remove nsIDOMGeoPositionOptions
Comment 5 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-04-12 10:48:51 PDT
Here's my working theory: _refreshPanel ( creates extra menu items based on secondaryActions (that hold on to the request) that are only removed when showing another prompt. _remove ( doesn't remove these elements, so they can stick around for a very long time.
Comment 6 Olli Pettay [:smaug] 2012-04-16 07:39:46 PDT
Created attachment 615329 [details] [diff] [review]

Need to still fix nsIDOMGeoPositionErrorCallback and nsIDOMGeoPositionCallback
handling, since those cause leaks.
Comment 7 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-04-16 07:48:34 PDT
I strongly suspect that with this patch applied we'll just see other member objects of the request (such as the callbacks) causing the callbacks. I think we need to address comment 5 to fix all the possible leaks from the doorhanger.
Comment 8 Olli Pettay [:smaug] 2012-04-16 07:54:34 PDT
...which is why I filed bug 745744.
We need the patch in this bug for correctness.
Comment 9 Doug Turner (:dougt) 2012-04-16 12:01:51 PDT
Comment on attachment 615329 [details] [diff] [review]

Review of attachment 615329 [details] [diff] [review]:

fix nsIDOMGeoPositionErrorCallback and nsIDOMGeoPositionCallback in another patch or followup.
Comment 10 Olli Pettay [:smaug] 2012-04-16 12:24:50 PDT

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