Last Comment Bug 664116 - AllTabs.jsm needs to remove event listeners from browser windows
: AllTabs.jsm needs to remove event listeners from browser windows
Status: RESOLVED FIXED
: mlk
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 7
Assigned To: Dão Gottwald [:dao]
:
Mentors:
Depends on:
Blocks: bc-leaks
  Show dependency treegraph
 
Reported: 2011-06-14 05:13 PDT by Dão Gottwald [:dao]
Modified: 2016-04-12 14:00 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (4.33 KB, patch)
2011-06-14 05:13 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Review
patch v2 (4.24 KB, patch)
2011-06-16 06:24 PDT, Dão Gottwald [:dao]
dietrich: review+
Details | Diff | Review

Description Dão Gottwald [:dao] 2011-06-14 05:13:20 PDT
Created attachment 539178 [details] [diff] [review]
patch

AllTabs.jsm seems to be one of the culprits keeping closed browser windows around.
Comment 1 Dão Gottwald [:dao] 2011-06-14 05:28:23 PDT
passed tests on try: http://tbpl.mozilla.org/?tree=Try&rev=c86516044ad9
Comment 2 Tim Taubert [:ttaubert] 2011-06-14 13:33:21 PDT
Uh, we didn't do that? :/ The patch looks good.
Comment 3 Dietrich Ayala (:dietrich) 2011-06-16 05:59:02 PDT
Comment on attachment 539178 [details] [diff] [review]
patch

mockEvents seems unnecessary. why not just build realEvents manually and iterate over that? would be a little bit simpler and more readable.
Comment 4 Dão Gottwald [:dao] 2011-06-16 06:24:52 PDT
Created attachment 539780 [details] [diff] [review]
patch v2

> mockEvents seems unnecessary. why not just build realEvents manually and
> iterate over that? would be a little bit simpler and more readable.

done
Comment 5 Dietrich Ayala (:dietrich) 2011-06-16 09:07:53 PDT
Comment on attachment 539780 [details] [diff] [review]
patch v2

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

thanks! this needs a test still. Tim, what'd be the best way to go about that?
Comment 6 Dão Gottwald [:dao] 2011-06-16 09:58:44 PDT
Various existing tabview tests verify that the event registration continues to work (http://tbpl.mozilla.org/?tree=Try&rev=c97acee70c78). I don't think the removal can be verified in a sensible way.
Comment 7 Dietrich Ayala (:dietrich) 2011-06-16 10:26:19 PDT
Comment on attachment 539780 [details] [diff] [review]
patch v2

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

ok, r=me!
Comment 8 Dão Gottwald [:dao] 2011-06-16 23:10:18 PDT
http://hg.mozilla.org/mozilla-central/rev/c68a54bad24e
Comment 9 Virgil Dicu [:virgil] [QA] 2011-08-23 01:39:23 PDT
Could you please provide some simple STRs in order to have this issue verified in QA?
Comment 10 Dão Gottwald [:dao] 2011-08-23 01:40:00 PDT
This doesn't need verification.

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