Bug 1127867 - Use the new back-end property to get the size of downloads asynchronously. r=mak
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 16 Feb 2015 18:49:52 +0000
changeset 229504 acb012724bf0b36cb9be64c91b7dd00b94dfd36f
parent 229503 448f00fe77e1474b1576eb06141a1ffaaa9aeb8a
child 229505 c41b520cb4a83e166afb6104ca090e95ddfb236b
push id55709
push userryanvm@gmail.com
push dateTue, 17 Feb 2015 19:27:34 +0000
treeherdermozilla-inbound@ebd50d4250b2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1127867
milestone38.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 1127867 - Use the new back-end property to get the size of downloads asynchronously. r=mak
browser/components/downloads/DownloadsCommon.jsm
browser/components/downloads/content/allDownloadsViewOverlay.js
browser/components/downloads/content/downloads.js
browser/components/downloads/content/downloadsViewCommon.js
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -319,36 +319,16 @@ this.DownloadsCommon = {
         return nsIDM.DOWNLOAD_PAUSED;
       }
       return nsIDM.DOWNLOAD_CANCELED;
     }
     return nsIDM.DOWNLOAD_NOTSTARTED;
   },
 
   /**
-   * Returns the highest number of bytes transferred or the known size of the
-   * given Download object, or -1 if the size is not available. Callers should
-   * use Download properties directly when possible.
-   */
-  maxBytesOfDownload(download) {
-    if (download.succeeded) {
-      // If the download succeeded, show the final size if available, otherwise
-      // use the last known number of bytes transferred.  The final size on disk
-      // will be available when bug 941063 is resolved.
-      return download.hasProgress ? download.totalBytes : download.currentBytes;
-    } else if (download.hasProgress) {
-      // If the final size and progress are known, use them.
-      return download.totalBytes;
-    } else {
-      // The download final size and progress percentage is unknown.
-      return -1;
-    }
-  },
-
-  /**
    * Given an iterable collection of Download objects, generates and returns
    * statistics about that collection.
    *
    * @param downloads An iterable collection of Download objects.
    *
    * @return Object whose properties are the generated statistics. Currently,
    *         we return the following properties:
    *
@@ -377,48 +357,42 @@ this.DownloadsCommon = {
       // it's still at Infinity by the time we're done iterating all
       // download.
       slowestSpeed: Infinity,
       rawTimeLeft: -1,
       percentComplete: -1
     }
 
     for (let download of downloads) {
-      let state = DownloadsCommon.stateOfDownload(download);
-      let maxBytes = DownloadsCommon.maxBytesOfDownload(download);
+      summary.numActive++;
 
-      summary.numActive++;
-      switch (state) {
-        case nsIDM.DOWNLOAD_PAUSED:
-          summary.numPaused++;
-          break;
-        case nsIDM.DOWNLOAD_SCANNING:
-          summary.numScanning++;
-          break;
-        case nsIDM.DOWNLOAD_DOWNLOADING:
-          summary.numDownloading++;
-          if (maxBytes > 0 && download.speed > 0) {
-            let sizeLeft = maxBytes - download.currentBytes;
-            summary.rawTimeLeft = Math.max(summary.rawTimeLeft,
-                                           sizeLeft / download.speed);
-            summary.slowestSpeed = Math.min(summary.slowestSpeed,
-                                            download.speed);
-          }
-          break;
+      if (!download.stopped) {
+        summary.numDownloading++;
+        if (download.hasProgress && download.speed > 0) {
+          let sizeLeft = download.totalBytes - download.currentBytes;
+          summary.rawTimeLeft = Math.max(summary.rawTimeLeft,
+                                         sizeLeft / download.speed);
+          summary.slowestSpeed = Math.min(summary.slowestSpeed,
+                                          download.speed);
+        }
+      } else if (download.canceled && download.hasPartialData) {
+        summary.numPaused++;
       }
+
       // Only add to total values if we actually know the download size.
-      if (maxBytes > 0 && state != nsIDM.DOWNLOAD_CANCELED &&
-                          state != nsIDM.DOWNLOAD_FAILED) {
-        summary.totalSize += maxBytes;
+      if (download.succeeded) {
+        summary.totalSize += download.target.size;
+        summary.totalTransferred += download.target.size;
+      } else if (download.hasProgress) {
+        summary.totalSize += download.totalBytes;
         summary.totalTransferred += download.currentBytes;
       }
     }
 
-    if (summary.numActive != 0 && summary.totalSize != 0 &&
-        summary.numActive != summary.numScanning) {
+    if (summary.totalSize != 0) {
       summary.percentComplete = (summary.totalTransferred /
                                  summary.totalSize) * 100;
     }
 
     if (summary.slowestSpeed == Infinity) {
       summary.slowestSpeed = 0;
     }
 
@@ -738,20 +712,18 @@ DownloadsDataCtor.prototype = {
         // annotations should be ideally hidden behind methods of
         // nsIDownloadHistory (bug 830415).
         if (!this._isPrivate) {
           try {
             let downloadMetaData = {
               state: DownloadsCommon.stateOfDownload(download),
               endTime: download.endTime,
             };
-            if (download.succeeded ||
-                (download.error && download.error.becauseBlocked)) {
-              downloadMetaData.fileSize =
-                DownloadsCommon.maxBytesOfDownload(download);
+            if (download.succeeded) {
+              downloadMetaData.fileSize = download.target.size;
             }
   
             PlacesUtils.annotations.setPageAnnotation(
                           NetUtil.newURI(download.source.url),
                           "downloads/metaData",
                           JSON.stringify(downloadMetaData), 0,
                           PlacesUtils.annotations.EXPIRE_WITH_HISTORY);
           } catch (ex) {
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -23,18 +23,24 @@ const DOWNLOAD_VIEW_SUPPORTED_COMMANDS =
  * @param url
  *        URI string for the download source.
  * @param endTime
  *        Timestamp with the end time for the download, used if there is no
  *        additional metadata available.
  */
 function HistoryDownload(aPlacesNode) {
   // TODO (bug 829201): history downloads should get the referrer from Places.
-  this.source = { url: aPlacesNode.uri };
-  this.target = { path: undefined, size: undefined };
+  this.source = {
+    url: aPlacesNode.uri,
+  };
+  this.target = {
+    path: undefined,
+    exists: false,
+    size: undefined,
+  };
 
   // In case this download cannot obtain its end time from the Places metadata,
   // use the time from the Places node, that is the start time of the download.
   this.endTime = aPlacesNode.time / 1000;
 }
 
 HistoryDownload.prototype = {
   /**
@@ -58,38 +64,40 @@ HistoryDownload.prototype = {
                    : metaData.state == nsIDM.DOWNLOAD_BLOCKED_PARENTAL
                    ? { becauseBlockedByParentalControls: true }
                    : metaData.state == nsIDM.DOWNLOAD_DIRTY
                    ? { becauseBlockedByReputationCheck: true }
                    : null;
       this.canceled = metaData.state == nsIDM.DOWNLOAD_CANCELED ||
                       metaData.state == nsIDM.DOWNLOAD_PAUSED;
       this.endTime = metaData.endTime;
+
+      // Normal history downloads are assumed to exist until the user interface
+      // is refreshed, at which point these values may be updated.
+      this.target.exists = true;
       this.target.size = metaData.fileSize;
     } catch (ex) {
       // Metadata might be missing from a download that has started but hasn't
       // stopped already. Normally, this state is overridden with the one from
       // the corresponding in-progress session download. But if the browser is
       // terminated abruptly and additionally the file with information about
       // in-progress downloads is lost, we may end up using this state. We use
       // the failed state to allow the download to be restarted.
       //
       // On the other hand, if the download is missing the target file
       // annotation as well, it is just a very old one, and we can assume it
       // succeeded.
       this.succeeded = !this.target.path;
       this.error = this.target.path ? { message: "Unstarted download." } : null;
       this.canceled = false;
-      this.target.size = -1;
+
+      // These properties may be updated if the user interface is refreshed.
+      this.exists = false;
+      this.target.size = undefined;
     }
-
-    // This property is currently used to get the size of downloads, but will be
-    // replaced by download.target.size when available for session downloads.
-    this.totalBytes = this.target.size;
-    this.currentBytes = this.target.size;
   },
 
   /**
    * History downloads are never in progress.
    */
   stopped: true,
 
   /**
@@ -118,16 +126,30 @@ HistoryDownload.prototype = {
     let initiatingDoc = browserWin ? browserWin.document : document;
 
     // Do not suggest a file name if we don't know the original target.
     let leafName = this.target.path ? OS.Path.basename(this.target.path) : null;
     DownloadURL(this.source.url, leafName, initiatingDoc);
 
     return Promise.resolve();
   },
+
+  /**
+   * This method mimicks the "refresh" method of session downloads, except that
+   * it cannot notify that the data changed to the Downloads View.
+   */
+  refresh: Task.async(function* () {
+    try {
+      this.target.size = (yield OS.File.stat(this.target.path)).size;
+      this.target.exists = true;
+    } catch (ex) {
+      // We keep the known file size from the metadata, if any.
+      this.target.exists = false;
+    }
+  }),
 };
 
 /**
  * A download element shell is responsible for handling the commands and the
  * displayed data for a single download view element.
  *
  * The shell may contain a session download, a history download, or both.  When
  * both a history and a current download are present, the current download gets
@@ -263,46 +285,29 @@ HistoryDownloadElementShell.prototype = 
   /* nsIController */
   isCommandEnabled(aCommand) {
     // The only valid command for inactive elements is cmd_delete.
     if (!this.active && aCommand != "cmd_delete") {
       return false;
     }
     switch (aCommand) {
       case "downloadsCmd_open":
-        // We cannot open a session download file unless it's succeeded.
-        // If it's succeeded, we need to make sure the file was not removed,
-        // as we do for past downloads.
-        if (this._sessionDownload && !this.download.succeeded) {
-          return false;
-        }
-
-        if (this._targetFileChecked) {
-          return this._targetFileExists;
-        }
-
-        // If the target file information is not yet fetched,
-        // temporarily assume that the file is in place.
-        return this.download.succeeded;
+        // This property is false if the download did not succeed.
+        return this.download.target.exists;
       case "downloadsCmd_show":
         // TODO: Bug 827010 - Handle part-file asynchronously.
         if (this._sessionDownload && this.download.target.partFilePath) {
           let partFile = new FileUtils.File(this.download.target.partFilePath);
           if (partFile.exists()) {
             return true;
           }
         }
 
-        if (this._targetFileChecked) {
-          return this._targetFileExists;
-        }
-
-        // If the target file information is not yet fetched,
-        // temporarily assume that the file is in place.
-        return this.download.succeeded;
+        // This property is false if the download did not succeed.
+        return this.download.target.exists;
       case "downloadsCmd_pauseResume":
         return this.download.hasPartialData && !this.download.error;
       case "downloadsCmd_retry":
         return this.download.canceled || this.download.error;
       case "downloadsCmd_openReferrer":
         return !!this.download.source.referrer;
       case "cmd_delete":
         // We don't want in-progress downloads to be removed accidentally.
@@ -429,26 +434,30 @@ HistoryDownloadElementShell.prototype = 
     // called again before the information is collected.
     if (!this._targetFileChecked) {
       this._checkTargetFileOnSelect().catch(Cu.reportError);
     }
   },
 
   _checkTargetFileOnSelect: Task.async(function* () {
     try {
-      this._targetFileExists = yield OS.File.exists(this.download.target.path);
+      yield this.download.refresh();
     } finally {
       // Do not try to check for existence again if this failed once.
       this._targetFileChecked = true;
     }
 
     // Update the commands only if the element is still selected.
     if (this.element.selected) {
       goUpdateDownloadCommands();
     }
+
+    // Ensure the interface has been updated based on the new values. We need to
+    // do this because history downloads can't trigger update notifications.
+    this._updateProgress();
   }),
 };
 
 /**
  * A Downloads Places View is a places view designed to show a places query
  * for history downloads alongside the current "session"-downloads.
  *
  * As we don't use the places controller, some methods implemented by other
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -534,17 +534,17 @@ const DownloadsPanel = {
         return;
       }
 
       // When the panel is opened, we check if the target files of visible items
       // still exist, and update the allowed items interactions accordingly.  We
       // do these checks on a background thread, and don't prevent the panel to
       // be displayed while these checks are being performed.
       for (let viewItem of DownloadsView._visibleViewItems.values()) {
-        viewItem.verifyTargetExists();
+        viewItem.download.refresh().catch(Cu.reportError);
       }
 
       DownloadsCommon.log("Opening downloads panel popup.");
       this.panel.openPopup(anchor, "bottomcenter topright", 0, 0, false, null);
     });
   },
 };
 
@@ -986,66 +986,35 @@ function DownloadsViewItem(download, aEl
   this.download = download;
   this.element = aElement;
   this.element._shell = this;
 
   this.element.setAttribute("type", "download");
   this.element.classList.add("download-state");
 
   this._updateState();
-  this.verifyTargetExists();
 }
 
 DownloadsViewItem.prototype = {
   __proto__: DownloadElementShell.prototype,
 
   /**
    * The XUL element corresponding to the associated richlistbox item.
    */
   _element: null,
 
   onStateChanged() {
     this.element.setAttribute("image", this.image);
     this.element.setAttribute("state",
                               DownloadsCommon.stateOfDownload(this.download));
-
-    if (this.download.succeeded) {
-      // We assume the existence of the target of a download that just completed
-      // successfully, without checking the condition in the background.  If the
-      // panel is already open, this will take effect immediately.  If the panel
-      // is opened later, a new background existence check will be performed.
-      this.element.setAttribute("exists", "true");
-    }
   },
 
   onChanged() {
     this._updateProgress();
   },
-
-  /**
-   * Starts checking whether the target file of a finished download is still
-   * available on disk, and sets an attribute that controls how the item is
-   * presented visually.
-   *
-   * The existence check is executed on a background thread.
-   */
-  verifyTargetExists() {
-    // We don't need to check if the download is not finished successfully.
-    if (!this.download.succeeded) {
-      return;
-    }
-
-    OS.File.exists(this.download.target.path).then(aExists => {
-      if (aExists) {
-        this.element.setAttribute("exists", "true");
-      } else {
-        this.element.removeAttribute("exists");
-      }
-    }).catch(Cu.reportError);
-  },
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 //// DownloadsViewController
 
 /**
  * Handles part of the user interaction events raised by the downloads list
  * widget, in particular the "commands" that apply to multiple items, and
--- a/browser/components/downloads/content/downloadsViewCommon.js
+++ b/browser/components/downloads/content/downloadsViewCommon.js
@@ -119,16 +119,25 @@ DownloadElementShell.prototype = {
     this._updateProgress();
   },
 
   /**
    * Updates the elements that change regularly for in-progress downloads,
    * namely the progress bar and the status line.
    */
   _updateProgress() {
+    if (this.download.succeeded) {
+      // We only need to add or remove this attribute for succeeded downloads.
+      if (this.download.target.exists) {
+        this.element.setAttribute("exists", "true");
+      } else {
+        this.element.removeAttribute("exists");
+      }
+    }
+
     // The progress bar is only displayed for in-progress downloads.
     if (this.download.hasProgress) {
       this.element.setAttribute("progressmode", "normal");
       this.element.setAttribute("progress", this.download.progress);
     } else {
       this.element.setAttribute("progressmode", "undetermined");
     }
 
@@ -159,53 +168,54 @@ DownloadElementShell.prototype = {
   get rawStatusTextAndTip() {
     const nsIDM = Ci.nsIDownloadManager;
     let s = DownloadsCommon.strings;
 
     let text = "";
     let tip = "";
 
     if (!this.download.stopped) {
-      let maxBytes = DownloadsCommon.maxBytesOfDownload(this.download);
+      let total = this.download.hasProgress ? this.download.totalBytes : -1;
       // By default, extended status information including the individual
       // download rate is displayed in the tooltip. The history view overrides
       // the getter and displays the detials in the main area instead.
       [text] = DownloadUtils.getDownloadStatusNoRate(
                                           this.download.currentBytes,
-                                          maxBytes,
+                                          total,
                                           this.download.speed,
                                           this.lastEstimatedSecondsLeft);
       let newEstimatedSecondsLeft;
       [tip, newEstimatedSecondsLeft] = DownloadUtils.getDownloadStatus(
                                           this.download.currentBytes,
-                                          maxBytes,
+                                          total,
                                           this.download.speed,
                                           this.lastEstimatedSecondsLeft);
       this.lastEstimatedSecondsLeft = newEstimatedSecondsLeft;
     } else if (this.download.canceled && this.download.hasPartialData) {
-      let maxBytes = DownloadsCommon.maxBytesOfDownload(this.download);
+      let total = this.download.hasProgress ? this.download.totalBytes : -1;
       let transfer = DownloadUtils.getTransferTotal(this.download.currentBytes,
-                                                    maxBytes);
+                                                    total);
 
       // We use the same XUL label to display both the state and the amount
       // transferred, for example "Paused -  1.1 MB".
       text = s.statusSeparatorBeforeNumber(s.statePaused, transfer);
     } else if (!this.download.succeeded && !this.download.canceled &&
                !this.download.error) {
       text = s.stateStarting;
     } else {
       let stateLabel;
 
       if (this.download.succeeded) {
-        // For completed downloads, show the file size (e.g. "1.5 MB")
-        let maxBytes = DownloadsCommon.maxBytesOfDownload(this.download);
-        if (maxBytes >= 0) {
-          let [size, unit] = DownloadUtils.convertByteUnits(maxBytes);
+        // For completed downloads, show the file size (e.g. "1.5 MB").
+        if (this.download.target.size !== undefined) {
+          let [size, unit] = DownloadUtils.convertByteUnits(
+                                                  this.download.target.size);
           stateLabel = s.sizeWithUnits(size, unit);
         } else {
+          // History downloads may not have a size defined.
           stateLabel = s.sizeUnknown;
         }
       } else if (this.download.canceled) {
         stateLabel = s.stateCanceled;
       } else if (this.download.error.becauseBlockedByParentalControls) {
         stateLabel = s.stateBlockedParentalControls;
       } else if (this.download.error.becauseBlockedByReputationCheck) {
         stateLabel = s.stateDirty;