Bug 822343 - Downloads view takes far too long to render. Part 2 - async I/O. r=mak
authorAsaf Romano <mano@mozilla.com>
Sat, 05 Jan 2013 16:08:45 +0200
changeset 117736 07fa18b4c450d3289519e60fa3592a644857d2c3
parent 117735 eaede79d4d42ed2d0d856ed2d9f91c085ca68c07
child 117737 a55044631d140b83ad2f5cd07037972b850d59db
push id24110
push userphilringnalda@gmail.com
push dateSat, 05 Jan 2013 23:57:49 +0000
treeherdermozilla-central@20d1a5916ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs822343
milestone20.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 822343 - Downloads view takes far too long to render. Part 2 - async I/O. r=mak
browser/components/downloads/content/allDownloadsViewOverlay.css
browser/components/downloads/content/allDownloadsViewOverlay.js
browser/components/downloads/content/download.css
--- a/browser/components/downloads/content/allDownloadsViewOverlay.css
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.css
@@ -21,14 +21,13 @@ richlistitem.download {
                               [state="9"]) /* Blocked (policy)   */)
                                            .downloadRemoveFromHistoryMenuItem,
 .download-state:not(:-moz-any([state="-1"],/* Starting (initial) */
                               [state="0"], /* Downloading        */
                               [state="1"], /* Finished           */
                               [state="4"], /* Paused             */
                               [state="5"]) /* Starting (queued)  */)
                                            .downloadShowMenuItem,
-
-.download-state[state="7"]                 .downloadCommandsSeparator
-
+.download-state[state="7"]                 .downloadCommandsSeparator,
+.download-state:not([state])                 .downloadCommandsSeparator
 {
   display: none;
 }
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -13,16 +13,17 @@ let Ci = Components.interfaces;
 let Cc = Components.classes;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://gre/modules/DownloadUtils.jsm");
 Cu.import("resource:///modules/DownloadsCommon.jsm");
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/osfile.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 const nsIDM = Ci.nsIDownloadManager;
 
 const DESTINATION_FILE_URI_ANNO  = "downloads/destinationFileURI";
 const DESTINATION_FILE_NAME_ANNO = "downloads/destinationFileName";
@@ -92,40 +93,49 @@ DownloadElementShell.prototype = {
   // The data item for the download
   _dataItem: null,
   get dataItem() this._dataItem,
 
   set dataItem(aValue) {
     if ((this._dataItem = aValue)) {
       this._wasDone = this._dataItem.done;
       this._wasInProgress = this._dataItem.inProgress;
+      this._targetFileInfoFetched = false;
+      this._fetchTargetFileInfo();
     }
     else if (this._placesNode) {
       this._wasInProgress = false;
       this._wasDone = this.getDownloadState(true) == nsIDM.DOWNLOAD_FINISHED;
+      this._targetFileInfoFetched = false;
+      this._fetchTargetFileInfo();
     }
 
     this._updateStatusUI();
     return aValue;
   },
 
   _placesNode: null,
   get placesNode() this._placesNode,
   set placesNode(aNode) {
     if (this._placesNode != aNode) {
       // Preserve the annotations map if this is the first loading and we got
       // cached values.
       if (this._placesNode || !this._annotations) {
         this._annotations = new Map();
       }
       this._placesNode = aNode;
+
+      // We don't need to update the UI if we had a data item, because
+      // the places information isn't used in this case.
       if (!this._dataItem && this._placesNode) {
         this._wasInProgress = false;
         this._wasDone = this.getDownloadState(true) == nsIDM.DOWNLOAD_FINISHED;
+        this._targetFileInfoFetched = false;
         this._updateStatusUI();
+        this._fetchTargetFileInfo();
       }
     }
     return aNode;
   },
 
   // The download uri (as a string)
   get downloadURI() {
     if (this._dataItem)
@@ -176,106 +186,118 @@ DownloadElementShell.prototype = {
       }
       value = aDefaultValue;
     }
 
     this._annotations.set(aAnnotation, value);
     return value;
   },
 
-  // The uri (as a string) of the target file, if any.
-  get _targetFileURI() {
-    if (this._dataItem)
-      return this._dataItem.file;
-
-    return this._getAnnotation(DESTINATION_FILE_URI_ANNO, "");
-  },
-
   // The label for the download
   get _displayName() {
     if (this._dataItem)
       return this._dataItem.target;
 
     try {
       return this._getAnnotation(DESTINATION_FILE_NAME_ANNO);
     }
     catch(ex) { }
 
     // Fallback to the places title, or, at last, to the download uri.
     return this._placesNode.title || this.downloadURI;
   },
 
-  // If there's a target file for the download, this is its nsIFile object.
-  get _file() {
-    if (!("__file" in this)) {
-      if (this._dataItem) {
-        this.__file = this._dataItem.localFile;
-      }
-      else {
-        this.__file = this._targetFileURI ?
-          GetFileForFileURI(this._targetFileURI) : null;
-      }
-    }
-    return this.__file;
+  // The uri (as a string) of the target file, if any.
+  get _targetFileURI() {
+    if (this._dataItem)
+      return this._dataItem.file;
+
+    return this._getAnnotation(DESTINATION_FILE_URI_ANNO, "");
+  },
+
+  get _targetFilePath() {
+    let fileURI = this._targetFileURI;
+    if (fileURI)
+      return GetFileForFileURI(fileURI).path;
+    return "";
   },
 
-  // The target's file size in bytes. If there's no target file, or If we
-  // cannot determine its size, 0 is returned.
-  get _fileSize() {
-    if (!("__fileSize" in this)) {
-      if (!this._file || !this._file.exists())
-        this.__fileSize = 0;
-      try {
-        this.__fileSize = this._file.fileSize;
-      }
-      catch(ex) {
-        Cu.reportError(ex);
-        this.__fileSize = 0;
-      }
+  _fetchTargetFileInfo: function DES__fetchTargetFileInfo() {
+    if (this._targetFileInfoFetched)
+      throw new Error("_fetchTargetFileInfo should not be called if the information was already fetched");
+
+    let path = this._targetFilePath;
+
+    // In previous version, the target file annotations were not set,
+    // so we cannot where is the file.
+    if (!path) {
+      this._targetFileInfoFetched = true;
+      this._targetFileExists = false;
+      return;
     }
-    return this.__fileSize;
+
+    OS.File.stat(path).then(
+      function onSuccess(fileInfo) {
+        this._targetFileInfoFetched = true;
+        this._targetFileExists = true;
+        this._targetFileSize = fileInfo.size;
+        delete this._state;
+        this._updateDownloadStatusUI();
+      }.bind(this),
+
+      function onFailure(aReason) {
+        if (reason instanceof OS.File.Error && reason.becauseNoSuchFile) {
+          this._targetFileInfoFetched = true;
+          this._targetFileExists = false;
+        }
+        else {
+          Cu.reportError("Could not fetch info for target file (reason: " +
+                         aReason + ")");
+        }
+
+        this._updateDownloadStatusUI();
+      }.bind(this)
+    );
   },
 
   /**
-   * Get the state of the download
+   * Get the state of the download (see nsIDownloadManager).
+   * For past downloads, for which we don't know the state at first,
+   * |undefined| is returned until we have info for the target file,
+   * indicating the state is unknown. |undefined| is also returned
+   * if the file was not found at last.
+   *
    * @param [optional] aForceUpdate
    *        Whether to force update the cached download state. Default: false.
+   * @return the download state if available, |undefined| otherwise.
    */
-  // The download state (see nsIDownloadManager).
   getDownloadState: function DES_getDownloadState(aForceUpdate = false) {
     if (aForceUpdate || !("_state" in this)) {
       if (this._dataItem) {
         this._state = this._dataItem.state;
       }
       else {
         try {
           this._state = this._getAnnotation(DOWNLOAD_STATE_ANNO);
         }
         catch (ex) {
-          // The state annotation didn't exist in past releases.
-          if (!this._file) {
+          if (!this._targetFileInfoFetched || !this._targetFileExists)
+            this._state = undefined;
+          else if (this._targetFileSize > 0)
+            this._state = nsIDM.DOWNLOAD_FINISHED;
+          else
             this._state = nsIDM.DOWNLOAD_FAILED;
-          }
-          else if (this._file.exists()) {
-            this._state = this._fileSize > 0 ?
-              nsIDM.DOWNLOAD_FINISHED : nsIDM.DOWNLOAD_FAILED;
-          }
-          else {
-            // XXXmano I'm not sure if this right. We should probably show no
-            // status text at all in this case.
-            this._state = nsIDM.DOWNLOAD_CANCELED;
-          }
         }
       }
     }
     return this._state;
   },
 
   // The status text for the download
-  get _statusText() {
+  _getStatusText: function DES__getStatusText() {
     let s = DownloadsCommon.strings;
     if (this._dataItem && this._dataItem.inProgress) {
       if (this._dataItem.paused) {
         let transfer =
           DownloadUtils.getTransferTotal(this._dataItem.currBytes,
                                          this._dataItem.maxBytes);
 
          // We use the same XUL label to display both the state and the amount
@@ -315,25 +337,25 @@ DownloadElementShell.prototype = {
       case nsIDM.DOWNLOAD_BLOCKED_PARENTAL:
         return s.stateBlockedParentalControls;
       case nsIDM.DOWNLOAD_BLOCKED_POLICY:
         return s.stateBlockedPolicy;
       case nsIDM.DOWNLOAD_DIRTY:
         return s.stateDirty;
       case nsIDM.DOWNLOAD_FINISHED:{
         // For completed downloads, show the file size (e.g. "1.5 MB")
-        if (this._fileSize > 0) {
-          let [size, unit] = DownloadUtils.convertByteUnits(this._fileSize);
+        if (this._targetFileInfoFetched && this._targetFileExists) {
+          let [size, unit] = DownloadUtils.convertByteUnits(this._targetFileSize);
           return s.sizeWithUnits(size, unit);
         }
         break;
       }
     }
 
-    return "";
+    return s.sizeUnknown;
   },
 
   // The progressmeter element for the download
   get _progressElement() {
     let progressElement = document.getAnonymousElementByAttribute(
       this._element, "anonid", "progressmeter");
     if (progressElement) {
       delete this._progressElement;
@@ -341,18 +363,21 @@ DownloadElementShell.prototype = {
     }
     return null;
   },
 
   // Updates the download state attribute (and by that hide/unhide the
   // appropriate buttons and context menu items), the status text label,
   // and the progress meter.
   _updateDownloadStatusUI: function  DES__updateDownloadStatusUI() {
-    this._element.setAttribute("state", this.getDownloadState(true));
-    this._element.setAttribute("status", this._statusText);
+    let state = this.getDownloadState(true);
+    if (state !== undefined)
+      this._element.setAttribute("state", state);
+
+    this._element.setAttribute("status", this._getStatusText());
 
     // For past-downloads, we're done. For session-downloads, we may also need
     // to update the progress-meter.
     if (!this._dataItem)
       return;
 
     // Copied from updateProgress in downloads.js.
     if (this._dataItem.starting) {
@@ -373,24 +398,22 @@ DownloadElementShell.prototype = {
     }
 
     // Dispatch the ValueChange event for accessibility, if possible.
     if (this._progressElement) {
       let event = document.createEvent("Events");
       event.initEvent("ValueChange", true, true);
       this._progressElement.dispatchEvent(event);
     }
-
-    goUpdateDownloadCommands();
   },
 
   _updateStatusUI: function DES__updateStatusUI() {
-    this._updateDownloadStatusUI();
+    this._element.setAttribute("displayName", this._displayName);
     this._element.setAttribute("image", this._icon);
-    this._element.setAttribute("displayName", this._displayName);
+    this._updateDownloadStatusUI();
   },
 
   placesNodeIconChanged: function DES_placesNodeIconChanged() {
     if (!this._dataItem)
       this._element.setAttribute("image", this._icon);
   },
 
   placesNodeTitleChanged: function DES_placesNodeTitleChanged() {
@@ -405,102 +428,122 @@ DownloadElementShell.prototype = {
         this._element.setAttribute("image", this._icon);
         this._updateDownloadStatusUI();
       }
       else if (aAnnoName == DESTINATION_FILE_NAME_ANNO) {
         this._element.setAttribute("displayName", this._displayName);
       }
       else if (aAnnoName == DOWNLOAD_STATE_ANNO) {
         this._updateDownloadStatusUI();
+        if (this._element.selected)
+          goUpdateDownloadCommands();
       }
     }
   },
 
   /* DownloadView */
   onStateChange: function DES_onStateChange() {
-    // See comment in DVI_onStateChange in downloads.js (the panel-view)
-    if (!this._wasDone && this._dataItem.done)
+    if (!this._wasDone && this._dataItem.done) {
+      // See comment in DVI_onStateChange in downloads.js (the panel-view)
       this._element.setAttribute("image", this._icon + "&state=normal");
 
+      this._targetFileInfoFetched = false;
+      this._fetchTargetFileInfo();
+    }
+
     this._wasDone = this._dataItem.done;
 
     // Update the end time using the current time if required.
     if (this._wasInProgress && !this._dataItem.inProgress) {
       this._endTime = Date.now();
     }
 
     this._wasDone = this._dataItem.done;
     this._wasInProgress = this._dataItem.inProgress;
 
     this._updateDownloadStatusUI();
+    if (this._element.selected)
+      goUpdateDownloadCommands();
   },
 
   /* DownloadView */
   onProgressChange: function DES_onProgressChange() {
     this._updateDownloadStatusUI();
   },
 
   /* nsIController */
   isCommandEnabled: function DES_isCommandEnabled(aCommand) {
     switch (aCommand) {
       case "downloadsCmd_open": {
-        return this._file.exists() &&
-               ((this._dataItem && this._dataItem.openable) ||
-                (this.getDownloadState() == nsIDM.DOWNLOAD_FINISHED));
+        // We cannot open a session dowload file unless it's done ("openable").
+        // If it's finished, we need to make sure the file was not removed,
+        // as we do for past downloads.
+        if (this._dataItem && !this._dataItem.openable)
+          return false;
+
+        // Disable the command until we can yet tell whether
+        // or not the file is there.
+        if (!this._targetFileInfoFetched)
+          return false;
+
+        return this._targetFileExists;
       }
       case "downloadsCmd_show": {
-        return this._getTargetFileOrPartFileIfExists() != null;
+        // TODO: Bug 827010 - Handle part-file asynchronously. 
+        if (this._dataItem &&
+            this._dataItem.partFile && this._dataItem.partFile.exists())
+          return true;
+
+        // Disable the command until we can yet tell whether
+        // or not the file is there.
+        if (!this._targetFileInfoFetched)
+          return false;
+
+        return this._targetFileExists;
       }
       case "downloadsCmd_pauseResume":
         return this._dataItem && this._dataItem.inProgress && this._dataItem.resumable;
       case "downloadsCmd_retry":
-        return ((this._dataItem && this._dataItem.canRetry) ||
-                (!this._dataItem && this._file))
+        // Disable the retry command for past downloads until it's fully implemented.
+        return this._dataItem && this._dataItem.canRetry;
       case "downloadsCmd_openReferrer":
         return this._dataItem && !!this._dataItem.referrer;
       case "cmd_delete":
         // The behavior in this case is somewhat unexpected, so we disallow that.
         if (this._placesNode && this._dataItem && this._dataItem.inProgress)
           return false;
         return true;
       case "downloadsCmd_cancel":
         return this._dataItem != null;
     }
     return false;
   },
 
-  _getTargetFileOrPartFileIfExists: function DES__getTargetFileOrPartFileIfExists() {
-    if (this._file && this._file.exists())
-      return this._file;
-    if (this._dataItem &&
-        this._dataItem.partFile && this._dataItem.partFile.exists())
-      return this._dataItem.partFile;
-    return null;
-  },
-
   _retryAsHistoryDownload: function DES__retryAsHistoryDownload() {
     // TODO: save in the right location (the current saveURL api does not allow this)
     saveURL(this.downloadURI, this._displayName, null, true, true, undefined, document);
   },
 
   /* nsIController */
   doCommand: function DES_doCommand(aCommand) {
     switch (aCommand) {
       case "downloadsCmd_open": {
         if (this._dateItem)
           this._dataItem.openLocalFile(window);
         else
-          DownloadsCommon.openDownloadedFile(this._file, null, window);
+          DownloadsCommon.openDownloadedFile(
+            GetFileForFileURI(this._targetFileURI), null, window);
         break;
       }
       case "downloadsCmd_show": {
         if (this._dataItem)
           this._dataItem.showLocalFile();
         else
-          DownloadsCommon.showDownloadedFile(this._getTargetFileOrPartFileIfExists());
+          DownloadsCommon.showDownloadedFile(
+            GetFileForFileURI(this._targetFileURI));
         break;
       }
       case "downloadsCmd_openReferrer": {
         openURL(this._dataItem.referrer);
         break;
       }
       case "downloadsCmd_cancel": {
         this._dataItem.cancel();
@@ -1106,17 +1149,22 @@ DownloadsPlacesView.prototype = {
   onContextMenu: function DPV_onContextMenu(aEvent)
   {
     let element = this._richlistbox.selectedItem;
     if (!element || !element._shell)
       return false;
 
     // Set the state attribute so that only the appropriate items are displayed.
     let contextMenu = document.getElementById("downloadsContextMenu");
-    contextMenu.setAttribute("state", element._shell.getDownloadState());
+    let state = element._shell.getDownloadState();
+    if (state !== undefined)
+      contextMenu.setAttribute("state", state);
+    else
+      contextMenu.removeAttribute("state");
+
     return true;
   },
 
   onKeyPress: function DPV_onKeyPress(aEvent) {
     let selectedElements = this._richlistbox.selectedItems;
     if (!selectedElements)
       return;
 
--- a/browser/components/downloads/content/download.css
+++ b/browser/components/downloads/content/download.css
@@ -34,12 +34,14 @@ richlistitem.download button {
                               [state="4"]) /* Paused             */)
                                            .downloadCancel,
 
 .download-state:not(:-moz-any([state="2"], /* Failed             */
                               [state="3"]) /* Canceled           */)
                                            .downloadRetry,
 
 .download-state:not(          [state="1"]  /* Finished           */)
-                                           .downloadShow
+                                           .downloadShow,
+
+.download-state:not([state]) > button
 {
   display: none;
 }