Last Comment Bug 416856 - Download status listener keeps windows alive until app shutdown
: Download status listener keeps windows alive until app shutdown
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Download Manager (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.9beta4
Assigned To: Ben Turner (not reading bugmail, use the needinfo flag!)
:
Mentors:
Depends on:
Blocks: 402278
  Show dependency treegraph
 
Reported: 2008-02-11 11:01 PST by Ben Turner (not reading bugmail, use the needinfo flag!)
Modified: 2008-07-31 04:30 PDT (History)
7 users (show)
jonas: blocking1.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, v1 (3.24 KB, patch)
2008-02-11 11:01 PST, Ben Turner (not reading bugmail, use the needinfo flag!)
asaf: review-
Details | Diff | Review
Patch, v2 (1.38 KB, patch)
2008-02-13 16:11 PST, Ben Turner (not reading bugmail, use the needinfo flag!)
asaf: review+
Details | Diff | Review

Description Ben Turner (not reading bugmail, use the needinfo flag!) 2008-02-11 11:01:49 PST
Created attachment 302630 [details] [diff] [review]
Patch, v1

Working on making the cycle collector free closed DOM windows I found that the download status panel listener is never removed when the browser closes. This keeps all DOM windows open until the app shuts down and the download manager clears its listener list.

We need to figure out a generic way to prevent this sort of thing, but that's for another bug.
Comment 1 Shawn Wilsher :sdwilsh 2008-02-11 11:19:14 PST
Wouldn't it be better to just unregister the listener on window unload?
Comment 2 Ben Turner (not reading bugmail, use the needinfo flag!) 2008-02-11 11:35:59 PST
onunload="BrowserShutdown()", so I think this is doing what you're talking about?
Comment 3 Shawn Wilsher :sdwilsh 2008-02-11 12:32:38 PST
right, sorry.  Missed that.
Comment 4 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2008-02-13 08:06:41 PST
Comment on attachment 302630 [details] [diff] [review]
Patch, v1

I would rather add an unit method to the listener object, instead of exposing isListening.
Comment 5 Ben Turner (not reading bugmail, use the needinfo flag!) 2008-02-13 16:11:33 PST
Created attachment 303159 [details] [diff] [review]
Patch, v2

Ok, with uninit.
Comment 6 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2008-02-13 16:19:08 PST
Comment on attachment 303159 [details] [diff] [review]
Patch, v2

nit: remove the braces in uninit.

r=mano otherwise.
Comment 7 Ben Turner (not reading bugmail, use the needinfo flag!) 2008-02-13 16:29:10 PST
Fixed, thanks!
Comment 8 Mike Lierman 2008-03-07 17:01:00 PST
*** VERIFIED
I even 100% remember this bug, it's fixed now.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008030607 Minefield/3.0b5pre

-Mike

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