Geolocation doorhanger might cause zombie compartments because it keeps callbacks alive

RESOLVED FIXED in Firefox 13

Status

()

Toolkit
General
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox13 verified)

Details

(Whiteboard: [Snappy][MemShrink:P2][qa+])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
+++ This bug was initially created as a clone of Bug #731875 +++

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

Comment 1

5 years ago
All the zombies cause major problems to responsiveness, so [Snappy]
Whiteboard: [Snappy]
(Assignee)

Comment 2

5 years ago
See also https://bugzilla.mozilla.org/show_bug.cgi?id=731875#c5
Hmm, yeah, we probably need to clean up the panel when it's hidden, or something.
(Assignee)

Updated

5 years ago
Assignee: nobody → bugs
(Assignee)

Comment 4

5 years ago
Created attachment 615793 [details] [diff] [review]
WIP

Fixes the leak, and hopefully even the tests
https://tbpl.mozilla.org/?tree=Try&rev=cb23b9cca906
I can't figure out any way to test this, since this is a runtime leak, and
we don't have anything like about:cc running during tests.
Attachment #615793 - Flags: review?(gavin.sharp)
(Assignee)

Comment 5

5 years ago
This should be simple fix for a rather major snappy-ness problem, so we should try to get this fixed for FF14.
tracking-firefox14: --- → ?
Attachment #615793 - Flags: review?(gavin.sharp) → review+
Whiteboard: [Snappy] → [Snappy][MemShrink]
Whiteboard: [Snappy][MemShrink] → [Snappy][MemShrink:P2]
(Assignee)

Updated

5 years ago
Attachment #615793 - Flags: approval-mozilla-central?
(Assignee)

Comment 6

5 years ago
Comment on attachment 615793 [details] [diff] [review]
WIP

We should probably take this to FF13

[Approval Request Comment]
Regression caused by (bug #): NA
User impact if declined: Possibly higher cycle collection times
Testing completed (on m-c, etc.): passes tests on tryserver
Risk to taking this patch (and alternatives if risky): should be low risk
String changes made by this patch: NA
Attachment #615793 - Flags: approval-mozilla-aurora?
Comment on attachment 615793 [details] [diff] [review]
WIP

(I think you could use a=desktop-only, but you can have a=gavin if you want!)
Attachment #615793 - Flags: approval-mozilla-central? → approval-mozilla-central+
I think we should wait until we get some small amount of central feedback before landing this on Aurora, FWIW.
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/218a9ef79151
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
tracking-firefox14: ? → ---
Target Milestone: --- → mozilla14
Comment on attachment 615793 [details] [diff] [review]
WIP

[triage comment]
snappy, been on central a few days, low risk.
Attachment #615793 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/6f27de794daa
status-firefox13: --- → fixed
Whiteboard: [Snappy][MemShrink:P2] → [Snappy][MemShrink:P2][qa+]
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0

Verified on Firefox 13 beta 3 - no zombie compartment is left after visiting www.joshmatthews.net/position.html.
status-firefox13: fixed → verified
Depends on: 758822

Updated

3 years ago
Depends on: 1036016
You need to log in before you can comment on or make changes to this bug.