Bug 883959: Part 2: Rework download progress. r=sfoster.
authorMarina Samuel <msamuel@mozilla.com>
Thu, 01 Aug 2013 15:18:02 -0400
changeset 141193 9aac8dc7a9b9af2fa8ca155e728f35f1c4a936bb
parent 141192 9ac580e78e1092d73f72c4e9fc1cdb068c55506e
child 141194 7579823b624ad51e7695163520ade3eb20bc6786
push id25049
push userryanvm@gmail.com
push dateFri, 02 Aug 2013 23:53:35 +0000
treeherdermozilla-central@d0edf8086809 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfoster
bugs883959
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 883959: Part 2: Rework download progress. r=sfoster.
browser/metro/base/content/downloads.js
browser/metro/base/tests/mochitest/browser_downloads.js
--- a/browser/metro/base/content/downloads.js
+++ b/browser/metro/base/content/downloads.js
@@ -7,16 +7,17 @@ const URI_GENERIC_ICON_DOWNLOAD = "chrom
 
 var Downloads = {
   /**
    * _downloadCount keeps track of the number of downloads that a single
    * notification bar groups together. A download is grouped with other
    * downloads if it starts before other downloads have completed.
    */
   _downloadCount: 0,
+  _downloadsInProgress: 0,
   _inited: false,
   _progressAlert: null,
   _lastSec: Infinity,
   _notificationBox: null,
   _progressNotification: null,
   _progressNotificationInfo: new Map(),
   _runDownloadBooleanMap: new Map(),
 
@@ -94,35 +95,36 @@ var Downloads = {
     let download = this.manager.getDownload(id);
 
     if (download) {
       this.manager.removeDownload(id);
     }
   },
 
   cancelDownload: function dh_cancelDownload(aDownload) {
-    this._progressNotificationInfo.delete(aDownload.guid);
-    this._runDownloadBooleanMap.delete(aDownload.targetFile.path);
-    this._downloadCount--;
-    if (this._progressNotificationInfo.size == 0) {
-      this._notificationBox.removeNotification(this._progressNotification);
-      this._progressNotification = null;
-    }
-
     let fileURI = aDownload.target;
     if (!(fileURI && fileURI.spec)) {
       Util.dumpLn("Cant remove download file for: "+aDownload.id+", fileURI is invalid");
-      return;
     }
 
-    let file = this._getLocalFile(fileURI);
     try {
-      this.manager.cancelDownload(aDownload.id);
+      let file = this._getLocalFile(fileURI);
       if (file && file.exists())
         file.remove(false);
+      this.manager.cancelDownload(aDownload.id);
+
+      // If cancelling was successful, stop tracking the download.
+      this._progressNotificationInfo.delete(aDownload.guid);
+      this._runDownloadBooleanMap.delete(aDownload.targetFile.path);
+      this._downloadCount--;
+      this._downloadsInProgress--;
+      if (this._downloadsInProgress <= 0) {
+        this._notificationBox.removeNotification(this._progressNotification);
+        this._progressNotification = null;
+      }
     } catch (ex) {
       Util.dumpLn("Failed to cancel download, with id: "+aDownload.id+", download target URI spec: " + fileURI.spec);
       Util.dumpLn("Failed download source:"+(aDownload.source && aDownload.source.spec));
     }
   },
 
   // Cancels all downloads.
   cancelDownloads: function dh_cancelDownloads() {
@@ -362,46 +364,49 @@ var Downloads = {
 
     switch (aTopic) {
       case "dl-run":
         let file = aSubject.QueryInterface(Ci.nsIFile);
         this._runDownloadBooleanMap.set(file.path, (aData == 'true'));
         break;
       case "dl-start":
         this._downloadCount++;
+        this._downloadsInProgress++;
         let download = aSubject.QueryInterface(Ci.nsIDownload);
         if (!this._progressNotificationInfo.get(download.guid)) {
           this._progressNotificationInfo.set(download.guid, {});
         }
         if (!this._progressAlert) {
           this._progressAlert = new AlertDownloadProgressListener();
           this.manager.addListener(this._progressAlert);
         }
         this.updateInfobar(download);
         break;
       case "dl-done":
+        this._downloadsInProgress--;
         download = aSubject.QueryInterface(Ci.nsIDownload);
         let runAfterDownload = this._runDownloadBooleanMap.get(download.targetFile.path);
         if (runAfterDownload) {
           this.openDownload(download);
         }
 
-        this._progressNotificationInfo.delete(download.guid);
         this._runDownloadBooleanMap.delete(download.targetFile.path);
-        if (this._progressNotificationInfo.size == 0) {
+        if (this._downloadsInProgress == 0) {
           if (this._downloadCount > 1 || !runAfterDownload) {
             this._showDownloadCompleteNotification(download);
           }
+          this._progressNotificationInfo.clear();
           this._downloadCount = 0;
           this._notificationBox.removeNotification(this._progressNotification);
           this._progressNotification = null;
         }
         break;
       case "dl-failed":
         download = aSubject.QueryInterface(Ci.nsIDownload);
+        this._showDownloadFailedNotification(download);
         break;
     }
   },
 
   QueryInterface: function (aIID) {
     if (!aIID.equals(Ci.nsIObserver) &&
         !aIID.equals(Ci.nsISupportsWeakReference) &&
         !aIID.equals(Ci.nsISupports))
--- a/browser/metro/base/tests/mochitest/browser_downloads.js
+++ b/browser/metro/base/tests/mochitest/browser_downloads.js
@@ -326,9 +326,19 @@ gTests.push({
     }
     finally {
       // Clean up when the test finishes.
       yield resetDownloads();
     }
   }
 });
 
-
+/**
+ * Make sure the cancelled/aborted downloads are handled correctly.
+ */
+gTests.push({
+  desc: "Cancel/Abort Downloads",
+  run: function(){
+    todo(false, "Ensure that a cancelled/aborted download is in the correct state \
+      including correct values for state variables (e.g. _downloadCount, _downloadsInProgress) \
+      and the existence of the downloaded file.");
+  }
+});
\ No newline at end of file