Bug 822343 - Downloads view takes far too long to render. Part 4 - Cache the download state so command-updating doesn't check the file size over and over. r=mconley.
authorAsaf Romano <mano@mozilla.com>
Sat, 22 Dec 2012 22:13:25 +0200
changeset 126003 ad84d41d41456205126875f11def790d0c5b4464
parent 126002 eee94feb23acb76e1d8d11d58bf90d4d0e13ce0c
child 126004 178bc511dec97ec9d828f02fc15aa38ab6898961
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
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 4 - Cache the download state so command-updating doesn't check the file size over and over. r=mconley.
browser/components/downloads/content/allDownloadsViewOverlay.js
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -91,17 +91,17 @@ DownloadElementShell.prototype = {
 
   set dataItem(aValue) {
     if ((this._dataItem = aValue)) {
       this._wasDone = this._dataItem.done;
       this._wasInProgress = this._dataItem.inProgress;
     }
     else if (this._placesNode) {
       this._wasInProgress = false;
-      this._wasDone = this._state == nsIDM.DOWNLOAD_FINISHED;
+      this._wasDone = this.getDownloadState(true) == nsIDM.DOWNLOAD_FINISHED;
     }
 
     this._updateStatusUI();
     return aValue;
   },
 
   _placesNode: null,
   get placesNode() this._placesNode,
@@ -110,17 +110,17 @@ DownloadElementShell.prototype = {
       // 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;
       if (!this._dataItem && this._placesNode) {
         this._wasInProgress = false;
-        this._wasDone = this._state == nsIDM.DOWNLOAD_FINISHED;
+        this._wasDone = this.getDownloadState(true) == nsIDM.DOWNLOAD_FINISHED;
         this._updateStatusUI();
       }
     }
     return aNode;
   },
 
   // The download uri (as a string)
   get downloadURI() {
@@ -211,52 +211,63 @@ DownloadElementShell.prototype = {
       }
     }
     return this.__file;
   },
 
   // 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 (!this._file || !this._file.exists())
-      return 0;
-    try {
-      return this._file.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;
+      }
     }
-    catch(ex) {
-      Cu.reportError(ex);
-      return 0;
-    }
+    return this.__fileSize;
   },
 
+  /**
+   * Get the state of the download
+   * @param [optional] aForceUpdate
+   *        Whether to force update the cached download state. Default: false.
+   */
   // The download state (see nsIDownloadManager).
-  get _state() {
-    if (this._dataItem)
-      return this._dataItem.state;
-
-    let state = -1;
-    try {
-      return this._getAnnotation(DOWNLOAD_STATE_ANNO);
-    }
-    catch (ex) {
-      // The state annotation didn't exist in past releases.
-      if (!this._file) {
-        state = nsIDM.DOWNLOAD_FAILED;
-      }
-      else if (this._file.exists()) {
-        state = this._fileSize > 0 ?
-          nsIDM.DOWNLOAD_FINISHED : nsIDM.DOWNLOAD_FAILED;
+  getDownloadState: function DES_getDownloadState(aForceUpdate = false) {
+    if (aForceUpdate || !("_state" in this)) {
+      if (this._dataItem) {
+        this._state = this._dataItem.state;
       }
       else {
-        // XXXmano I'm not sure if this right. We should probably show no
-        // status text at all in this case.
-        state = nsIDM.DOWNLOAD_CANCELED;
+        try {
+          this._state = this._getAnnotation(DOWNLOAD_STATE_ANNO);
+        }
+        catch (ex) {
+          // The state annotation didn't exist in past releases.
+          if (!this._file) {
+            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 state;
+    return this._state;
   },
 
   // The status text for the download
   get _statusText() {
     let s = DownloadsCommon.strings;
     if (this._dataItem && this._dataItem.inProgress) {
       if (this._dataItem.paused) {
         let transfer =
@@ -287,17 +298,17 @@ DownloadElementShell.prototype = {
         DownloadUtils.getURIHost(this._dataItem.referrer ||
                                  this._dataItem.uri);
 
       let end = new Date(this.dataItem.endTime);
       let [displayDate, fullDate] = DownloadUtils.getReadableDates(end);
       return s.statusSeparator(fullHost, fullDate);
     }
 
-    switch (this._state) {
+    switch (this.getDownloadState()) {
       case nsIDM.DOWNLOAD_FAILED:
         return s.stateFailed;
       case nsIDM.DOWNLOAD_CANCELED:
         return s.stateCanceled;
       case nsIDM.DOWNLOAD_BLOCKED_PARENTAL:
         return s.stateBlockedParentalControls;
       case nsIDM.DOWNLOAD_BLOCKED_POLICY:
         return s.stateBlockedPolicy;
@@ -326,17 +337,17 @@ 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._state);
+    this._element.setAttribute("state", this.getDownloadState(true));
     this._element.setAttribute("status", this._statusText);
 
     // 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.
@@ -424,17 +435,17 @@ DownloadElementShell.prototype = {
   },
 
   /* nsIController */
   isCommandEnabled: function DES_isCommandEnabled(aCommand) {
     switch (aCommand) {
       case "downloadsCmd_open": {
         return this._file.exists() &&
                ((this._dataItem && this._dataItem.openable) ||
-                (this._state == nsIDM.DOWNLOAD_FINISHED));
+                (this.getDownloadState() == nsIDM.DOWNLOAD_FINISHED));
       }
       case "downloadsCmd_show": {
         return this._getTargetFileOrPartFileIfExists() != null;
       }
       case "downloadsCmd_pauseResume":
         return this._dataItem && this._dataItem.inProgress && this._dataItem.resumable;
       case "downloadsCmd_retry":
         return ((this._dataItem && this._dataItem.canRetry) ||
@@ -542,17 +553,17 @@ DownloadElementShell.prototype = {
           return "downloadsCmd_show";
         case nsIDM.DOWNLOAD_BLOCKED_PARENTAL:
         case nsIDM.DOWNLOAD_DIRTY:
         case nsIDM.DOWNLOAD_BLOCKED_POLICY:
           return "downloadsCmd_openReferrer";
       }
       return "";
     }
-    let command = getDefaultCommandForState(this._state);
+    let command = getDefaultCommandForState(this.getDownloadState());
     if (this.isCommandEnabled(command))
       this.doCommand(command);
   }
 };
 
 /**
  * A Downloads Places View is a places view designed to show a places query
  * for history donwloads alongside the current "session"-downloads.
@@ -1074,17 +1085,17 @@ 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._state);
+    contextMenu.setAttribute("state", element._shell.getDownloadState());
     return true;
   },
 
   onKeyPress: function DPV_onKeyPress(aEvent) {
     let selectedElements = this._richlistbox.selectedItems;
     if (!selectedElements)
       return;