745744 - Geolocation doorhanger might cause zombie compartments because it keeps callbacks alive

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

+++ 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.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

All the zombies cause major problems to responsiveness, so [Snappy]

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

See also https://bugzilla.mozilla.org/show_bug.cgi?id=731875#c5

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Hmm, yeah, we probably need to clean up the panel when it's hidden, or something.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: 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.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

This should be simple fix for a rather major snappy-ness problem, so we should try to get this fixed for FF14.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 7

[:Gavin Sharp [email: [e-mail]]]

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!)

Comment 8

[:Gavin Sharp [email: [e-mail]]]

I think we should wait until we get some small amount of central feedback before landing this on Aurora, FWIW.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/218a9ef79151

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 615793 [details] [diff] [review]
WIP

[triage comment]
snappy, been on central a few days, low risk.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6f27de794daa

Comment 12

[Simona Badau, Desktop 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.