Last Comment Bug 745744 - Geolocation doorhanger might cause zombie compartments because it keeps callbacks alive
: Geolocation doorhanger might cause zombie compartments because it keeps callb...
Status: RESOLVED FIXED
[Snappy][MemShrink:P2][qa+]
:
Product: Toolkit
Classification: Components
Component: General (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla14
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on: 758822 731875 1036016
Blocks: leakychrome
  Show dependency treegraph
 
Reported: 2012-04-16 07:45 PDT by Olli Pettay [:smaug]
Modified: 2014-07-22 11:28 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified


Attachments
WIP (941 bytes, patch)
2012-04-17 11:02 PDT, Olli Pettay [:smaug]
gavin.sharp: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
gavin.sharp: approval‑mozilla‑central+
Details | Diff | Review

Description Olli Pettay [:smaug] 2012-04-16 07:45:29 PDT
+++ 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] 2012-04-16 07:57:39 PDT
All the zombies cause major problems to responsiveness, so [Snappy]
Comment 2 Olli Pettay [:smaug] 2012-04-16 08:01:03 PDT
See also https://bugzilla.mozilla.org/show_bug.cgi?id=731875#c5
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-04-16 17:31:52 PDT
Hmm, yeah, we probably need to clean up the panel when it's hidden, or something.
Comment 4 Olli Pettay [:smaug] 2012-04-17 11:02:09 PDT
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.
Comment 5 Olli Pettay [:smaug] 2012-04-17 12:03:14 PDT
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] 2012-04-18 07:47:12 PDT
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: gavin@gavinsharp.com] 2012-04-18 09:06:57 PDT
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: gavin@gavinsharp.com] 2012-04-18 09:07:46 PDT
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] 2012-04-18 09:33:51 PDT
https://hg.mozilla.org/mozilla-central/rev/218a9ef79151
Comment 10 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-20 16:08:33 PDT
Comment on attachment 615793 [details] [diff] [review]
WIP

[triage comment]
snappy, been on central a few days, low risk.
Comment 12 Simona B [:simonab] 2012-05-09 03:52:13 PDT
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.

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