Bug 828243 - Cleanup wasDone, wasInProgress and endTime usage.
authorAsaf Romano <mano@mozilla.com>
Wed, 09 Jan 2013 14:45:08 +0200
changeset 127054 50f3e4082f971cc77940c25d6fe0ac5b3c91aba0
parent 127053 6ec8ebb4f02e3efe94d948c5837b1b54c67e55a2
child 127055 2bdd85edca10c27df7226761ca9d666f3dda4442
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)
bugs828243
milestone20.0a2
Bug 828243 - Cleanup wasDone, wasInProgress and endTime usage. r=mak a=gavin
browser/components/downloads/content/allDownloadsViewOverlay.js
browser/components/downloads/content/downloads.js
browser/components/downloads/src/DownloadsCommon.jsm
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -109,30 +109,26 @@ DownloadElementShell.prototype = {
   // The data item for the download
   _dataItem: null,
   get dataItem() this._dataItem,
 
   set dataItem(aValue) {
     this._dataItem = aValue;
     if (this._dataItem) {
       this._active = true;
-      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;
       if (this.active)
         this._fetchTargetFileInfo();
     }
 
-    it (this.active)
+    if (this.active)
       this._updateStatusUI();
     return aValue;
   },
 
   _placesNode: null,
   get placesNode() this._placesNode,
   set placesNode(aNode) {
     if (this._placesNode != aNode) {
@@ -141,18 +137,16 @@ DownloadElementShell.prototype = {
       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;
         if (this.active) {
           this._updateStatusUI();
           this._fetchTargetFileInfo();
         }
       }
     }
     return aNode;
@@ -466,36 +460,27 @@ DownloadElementShell.prototype = {
         this._updateDownloadStatusUI();
         if (this._element.selected)
           goUpdateDownloadCommands();
       }
     }
   },
 
   /* DownloadView */
-  onStateChange: function DES_onStateChange() {
-    if (!this._wasDone && this._dataItem.done) {
+  onStateChange: function DES_onStateChange(aOldState) {
+    if (aOldState != nsIDM.DOWNLOAD_FINISHED &&
+        aOldState != this.dataItem.state) {
       // See comment in DVI_onStateChange in downloads.js (the panel-view)
       this._element.setAttribute("image", this._icon + "&state=normal");
 
       this._targetFileInfoFetched = false;
       if (this.active)
         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();
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -851,18 +851,16 @@ const DownloadsView = {
  * @param aElement
  *        XUL element corresponding to the single download item in the view.
  */
 function DownloadsViewItem(aDataItem, aElement)
 {
   this._element = aElement;
   this.dataItem = aDataItem;
 
-  this.wasDone = this.dataItem.done;
-  this.wasInProgress = this.dataItem.inProgress;
   this.lastEstimatedSecondsLeft = Infinity;
 
   // Set the URI that represents the correct icon for the target file.  As soon
   // as bug 239948 comment 12 is handled, the "file" property will be always a
   // file URL rather than a file name.  At that point we should remove the "//"
   // (double slash) from the icon URI specification (see test_moz_icon_uri.js).
   this.image = "moz-icon://" + this.dataItem.file + "?size=32";
 
@@ -905,36 +903,29 @@ DownloadsViewItem.prototype = {
   //////////////////////////////////////////////////////////////////////////////
   //// Callback functions from DownloadsData
 
   /**
    * Called when the download state might have changed.  Sometimes the state of
    * the download might be the same as before, if the data layer received
    * multiple events for the same download.
    */
-  onStateChange: function DVI_onStateChange()
+  onStateChange: function DVI_onStateChange(aOldState)
   {
     // If a download just finished successfully, it means that the target file
     // now exists and we can extract its specific icon.  To ensure that the icon
     // is reloaded, we must change the URI used by the XUL image element, for
     // example by adding a query parameter.  Since this URI has a "moz-icon"
     // scheme, this only works if we add one of the parameters explicitly
     // supported by the nsIMozIconURI interface.
-    if (!this.wasDone && this.dataItem.openable) {
+    if (aOldState != Ci.nsIDownloadManager.DOWNLOAD_FINISHED &&
+        aOldState != this.dataItem.state) {
       this._element.setAttribute("image", this.image + "&state=normal");
     }
 
-    // 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;
-
     // Update the user interface after switching states.
     this._element.setAttribute("state", this.dataItem.state);
     this._updateProgress();
     this._updateStatusLine();
   },
 
   /**
    * Called when the download progress has changed.
--- a/browser/components/downloads/src/DownloadsCommon.jsm
+++ b/browser/components/downloads/src/DownloadsCommon.jsm
@@ -935,57 +935,63 @@ DownloadsDataCtor.prototype = {
         break;
 #endif
     }
   },
 
   //////////////////////////////////////////////////////////////////////////////
   //// nsIDownloadProgressListener
 
-  onDownloadStateChange: function DD_onDownloadStateChange(aState, aDownload)
+  onDownloadStateChange: function DD_onDownloadStateChange(aOldState, aDownload)
   {
 #ifdef MOZ_PER_WINDOW_PRIVATE_BROWSING
     if (aDownload.isPrivate != this._isPrivate) {
       // Ignore the downloads with a privacy status other than what we are
       // tracking.
       return;
     }
 #endif
 
     // When a new download is added, it may have the same identifier of a
     // download that we previously deleted during this session, and we also
     // want to provide a visible indication that the download started.
-    let isNew = aState == nsIDM.DOWNLOAD_NOTSTARTED ||
-                aState == nsIDM.DOWNLOAD_QUEUED;
+    let isNew = aOldState == nsIDM.DOWNLOAD_NOTSTARTED ||
+                aOldState == nsIDM.DOWNLOAD_QUEUED;
 
     let dataItem = this._getOrAddDataItem(aDownload, isNew);
     if (!dataItem) {
       return;
     }
 
+    let wasInProgress = dataItem.inProgress;
+
     dataItem.state = aDownload.state;
     dataItem.referrer = aDownload.referrer && aDownload.referrer.spec;
     dataItem.resumable = aDownload.resumable;
     dataItem.startTime = Math.round(aDownload.startTime / 1000);
     dataItem.currBytes = aDownload.amountTransferred;
     dataItem.maxBytes = aDownload.size;
 
+    if (wasInProgress && !dataItem.inProgress) {
+      dataItem.endTime = Date.now();
+    }
+
     // When a download is retried, we create a different download object from
     // the database with the same ID as before. This means that the nsIDownload
     // that the dataItem holds might now need updating.
     //
     // We only overwrite this in the event that _download exists, because if it
     // doesn't, that means that no caller ever tried to get the nsIDownload,
     // which means it was never retrieved and doesn't need to be overwritten.
     if (dataItem._download) {
       dataItem._download = aDownload;
     }
 
     this._views.forEach(
-      function (view) view.getViewItem(dataItem).onStateChange()
+      function (view) view.getViewItem(dataItem).onStateChange(aOldState)
     );
 
     if (isNew && !dataItem.newDownloadNotified) {
       dataItem.newDownloadNotified = true;
       this._notifyDownloadEvent("start");
     }
 
     // This is a final state of which we are only notified once.
@@ -1723,17 +1729,17 @@ DownloadsIndicatorDataCtor.prototype = {
    *
    * @return Object that can be used to notify item status events.
    */
   getViewItem: function DID_getViewItem(aDataItem)
   {
     let data = this._isPrivate ? PrivateDownloadsIndicatorData
                                : DownloadsIndicatorData;
     return Object.freeze({
-      onStateChange: function DIVI_onStateChange()
+      onStateChange: function DIVI_onStateChange(aOldState)
       {
         if (aDataItem.state == nsIDM.DOWNLOAD_FINISHED ||
             aDataItem.state == nsIDM.DOWNLOAD_FAILED) {
           data.attention = true;
         }
 
         // Since the state of a download changed, reset the estimated time left.
         data._lastRawTimeLeft = -1;
@@ -1998,17 +2004,17 @@ DownloadsSummaryData.prototype = {
     this._dataItems.splice(itemIndex, 1);
     this._updateViews();
   },
 
   getViewItem: function DSD_getViewItem(aDataItem)
   {
     let self = this;
     return Object.freeze({
-      onStateChange: function DIVI_onStateChange()
+      onStateChange: function DIVI_onStateChange(aOldState)
       {
         // Since the state of a download changed, reset the estimated time left.
         self._lastRawTimeLeft = -1;
         self._lastTimeLeft = -1;
         self._updateViews();
       },
       onProgressChange: function DIVI_onProgressChange()
       {