Geolocation doorhanger might cause a zombie compartment

RESOLVED FIXED

Status

()

Core
Geolocation
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jdm, Assigned: smaug)

Tracking

(Blocks: 1 bug)

Trunk
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
I see a zombie compartment from www.joshmatthews.net/position.html 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.
Component: General → Geolocation
Product: Firefox → Core
QA Contact: general → geolocation
Duplicate of this bug: 744817
Blocks: 728407
Whiteboard: [MemShrink]
I had a similar thing happen on Google Maps.   The documents ended up being held alive like so:

0x12a3e9000 [rc=6] nsDocument normal (xhtml) http://maps.google.com/maps (etc)
    Roots: 1
        0x132fa0f90 [rc=2] root nsXPCWrappedJS (nsIDOMGeoPositionOptions)
0x124cb5800 [rc=6] nsDocument normal (xhtml) http://maps.google.com/maps  (etc)
    Roots: 1
        0x132fa0f90 [rc=2] root nsXPCWrappedJS (nsIDOMGeoPositionOptions)
(Reporter)

Comment 3

5 years ago
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 http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#1552 (which takes a request object and initiates a prompt that holds on to it) and http://mxr.mozilla.org/mozilla-central/source/toolkit/content/PopupNotifications.jsm#234 (which actually creates the prompt).
(Reporter)

Updated

5 years ago
Component: Geolocation → General
Product: Core → Firefox
QA Contact: geolocation → general
(Assignee)

Comment 4

5 years ago
I'm going to remove nsIDOMGeoPositionOptions
(Reporter)

Comment 5

5 years ago
Here's my working theory: _refreshPanel (http://mxr.mozilla.org/mozilla-central/source/toolkit/content/PopupNotifications.jsm#386) creates extra menu items based on secondaryActions (that hold on to the request) that are only removed when showing another prompt. _remove (http://mxr.mozilla.org/mozilla-central/source/toolkit/content/PopupNotifications.jsm#351) doesn't remove these elements, so they can stick around for a very long time.
Component: General → General
Product: Firefox → Toolkit
QA Contact: general → general
(Assignee)

Comment 6

5 years ago
Created attachment 615329 [details] [diff] [review]
patch

Need to still fix nsIDOMGeoPositionErrorCallback and nsIDOMGeoPositionCallback
handling, since those cause leaks.
Assignee: nobody → bugs
Attachment #615329 - Flags: review?(doug.turner)
(Assignee)

Updated

5 years ago
Blocks: 745744
(Reporter)

Comment 7

5 years ago
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.
(Assignee)

Comment 8

5 years ago
...which is why I filed bug 745744.
We need the patch in this bug for correctness.
(Assignee)

Updated

5 years ago
Component: General → Geolocation
Product: Toolkit → Core
QA Contact: general → geolocation

Comment 9

5 years ago
Comment on attachment 615329 [details] [diff] [review]
patch

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

fix nsIDOMGeoPositionErrorCallback and nsIDOMGeoPositionCallback in another patch or followup.
Attachment #615329 - Flags: review?(doug.turner) → review+
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/mozilla-central/rev/686e5bcf747b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.