Closed Bug 595333 Opened 9 years ago Closed 9 years ago
Downloads started during Download
View initialization are added twice
This is only noticeable in Fennec after the patch in Bug 594504. The patch opens the downloads manager, and sends a progress event at around the same time for a new queued download. As a result, both the download progress listener, and the nsIDownloadManager create richlistitems for the download, and it shows up twice.
Here I just check every time an item is created to make sure one with the same id doesn't already exist. I am a bit worried about its performance implications if the list is huge, but it seems like an easy way to solve any related bugs.
Attachment #474192 - Flags: review?(mark.finkle)
Comment on attachment 474192 [details] [diff] [review] Check before creating This patch would work OK, but I was curious to figure out why this was happening at all. Here is my guess: * _createItem is called in two places: when we fill the full list and in downloadStarted, called via a DownloadProgressListener * The listener is not connected until the DownloadsView is fully initialize - which happens when the view is made visible. * Tapping the alert notification starts a download and then makes the view visible. * In the view initialization code, the listener is connected first then the full list is loaded async. I think the listener is in a race with loading the full list, and it might be winning. I want to check to see if just putting the check for the existing item in the downloadStarted method is enough to fix this bug or not. If it gets more complicated, we can use this patch. But if we can move the check out of _createItem, it would mitigate the performance risk Wes pointed out.
It is a race between the listener and the full list code. Because the full list code is async, it is hard to simply delay the listener. We could add a state flag to let us know when loading begins and ends, but the patch here is simpler and works without much overhead.
Comment on attachment 474192 [details] [diff] [review] Check before creating if( -> if ( (must break Wes of this)
Attachment #474192 - Flags: review?(mark.finkle) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee: nobody → wjohnston
Verified fixed on: Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110913 Firefox/9.0a1 Fennec/9.0a1 Device: Samsung Galaxy S OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.