Last Comment Bug 668000 - browser_pageInfo.js leaks
: browser_pageInfo.js leaks
Status: RESOLVED FIXED
: mlk
Product: Firefox
Classification: Client Software
Component: Page Info Window (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Dão Gottwald [:dao]
:
Mentors:
Depends on:
Blocks: bc-leaks 767896
  Show dependency treegraph
 
Reported: 2011-06-28 13:01 PDT by Dão Gottwald [:dao]
Modified: 2012-06-25 01:53 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.63 KB, patch)
2011-06-28 13:01 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2011-06-28 13:01:35 PDT
Created attachment 542562 [details] [diff] [review]
patch

pageInfo.js never removes its callbacks (nor do the consumers remove themsevles), and browser_pageInfo.js doesn't remove its observer.
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-28 13:09:10 PDT
I don't really understand why the pageInfo.js changes are needed. How are those arrays special such that the need to be explicitly released on unload?
Comment 2 Dão Gottwald [:dao] 2011-06-28 13:24:36 PDT
Anyone can add functions to the arrays. Add-ons do it and browser_bug517902.js does it.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-28 15:23:34 PDT
I still don't see why that is a problem. Once the page info window goes away those references should die, whether you null them out explicitly or not.
Comment 4 Dão Gottwald [:dao] 2011-06-28 15:38:43 PDT
browser_bug517902.js also holds a reference to the window. I don't know how reliably such cycles are resolved. It's also possible that the observer part alone gets rid of the leaks.
Comment 5 Dão Gottwald [:dao] 2011-06-29 07:45:29 PDT
It looked like the pageInfo.js part fixed a bunch of about:blank leaks, but I'm getting unstable results now, so I just landed the browser_pageInfo.js fix.

http://hg.mozilla.org/mozilla-central/rev/03f27e11397f

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