494550 - Get nsIDownloadManager initialization out of startup path

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Bug 489686 landed some fixes to reduce a Ts regression. The result seemed to have no affect on Ts. This might be due to the initialization of the nsIDownloadManager in the DownloadsView.init() method. We add a listener so we can display popup alerts when a file is downloaded.

We should try to use the observer notification fired by the nsIDownloadManager ("download-started") to display the initial popup alert and then attach the listener so we get the download complete trigger too. This should keep the download manager out of the startup path.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch gets the DM out of our explicit startup patch. There might be other ways the DM is being intialized, but we can track those down as we find them.

This patch uses the ObserverService to listen for notifications sent by the DM when a file download starts/ends. The patch removes the download progress listener we were using to monitor downloads for alerts.

The patch adds nsISupportsWeakReference to downloads and extensions for the ObserverService.

The patch adjusts the alert XUL so long text breaks correctly and doesn't just cause the alert to get wider.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 379681 [details] [diff] [review]
patch

>diff --git a/chrome/content/downloads.js b/chrome/content/downloads.js

>   init: function dv_init() {

>+    var os = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
>+    os.addObserver(this, "dl-start", true);
>+    os.addObserver(this, "dl-failed", true);
>+    os.addObserver(this, "dl-done", true);

what about dl-blocked/dl-dirty/dl-cancel? I guess blocked/dirty can't currently happen since we don't have a backend for scanning, and cancel can't happen outside of our UI? Probably wouldn't hurt to listen for these anyways, though...

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Added the other dl-* notifications. Pushed:
http://hg.mozilla.org/mobile-browser/rev/5bbb4c203890