Bug 1381409 - Part 3 - Use DownloadList notifications directly in front-end download views. r=mak
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 17 Jul 2017 12:05:12 +0100
changeset 369303 4a2fd40a1239e375bc759fdf9b4b860162e38e0a
parent 369302 a529720f06eecf154d4784bbe845984c5efcb338
child 369304 9acc3b67079572f45bab4d124bc8db63fa1aaf2f
push id92682
push userpaolo.mozmail@amadzone.org
push dateTue, 18 Jul 2017 13:46:32 +0000
treeherdermozilla-inbound@4a2fd40a1239 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1381409
milestone56.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 1381409 - Part 3 - Use DownloadList notifications directly in front-end download views. r=mak The DownloadList object now provides batch notifications directly, in preparation for linking front-end views to other types of download lists without having to use the DownloadsData indirection. MozReview-Commit-ID: FOTz1YwGRE1
browser/components/downloads/DownloadsCommon.jsm
browser/components/downloads/content/allDownloadsViewOverlay.js
browser/components/downloads/content/downloads.js
toolkit/components/jsdownloads/src/DownloadList.jsm
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -654,34 +654,40 @@ XPCOMUtils.defineLazyGetter(DownloadsCom
  * ones.
  */
 function DownloadsDataCtor(aPrivate) {
   this._isPrivate = aPrivate;
 
   // Contains all the available Download objects and their integer state.
   this.oldDownloadStates = new Map();
 
-  // Array of view objects that should be notified when the available download
-  // data changes.
-  this._views = [];
+  // This defines "initializeDataLink" and "_promiseList" synchronously, then
+  // continues execution only when "initializeDataLink" is called, allowing the
+  // underlying data to be loaded only when actually needed.
+  this._promiseList = (async () => {
+    await new Promise(resolve => this.initializeDataLink = resolve);
+
+    let list = await Downloads.getList(this._isPrivate ? Downloads.PRIVATE
+                                                       : Downloads.PUBLIC);
+    await list.addView(this);
+    return list;
+  })();
 }
 
 DownloadsDataCtor.prototype = {
   /**
    * Starts receiving events for current downloads.
    */
-  initializeDataLink() {
-    if (!this._dataLinkInitialized) {
-      let promiseList = Downloads.getList(this._isPrivate ? Downloads.PRIVATE
-                                                          : Downloads.PUBLIC);
-      promiseList.then(list => list.addView(this)).catch(Cu.reportError);
-      this._dataLinkInitialized = true;
-    }
-  },
-  _dataLinkInitialized: false,
+  initializeDataLink() {},
+
+  /**
+   * Promise resolved with the underlying DownloadList object once we started
+   * receiving events for current downloads.
+   */
+  _promiseList: null,
 
   /**
    * Iterator for all the available Download objects. This is empty until the
    * data has been loaded using the JavaScript API for downloads.
    */
   get downloads() {
     return this.oldDownloadStates.keys();
   },
@@ -695,43 +701,37 @@ DownloadsDataCtor.prototype = {
       if (download.stopped && !(download.canceled && download.hasPartialData)) {
         return true;
       }
     }
     return false;
   },
 
   /**
-   * Asks the back-end to remove finished downloads from the list.
+   * Asks the back-end to remove finished downloads from the list. This method
+   * is only called after the data link has been initialized.
    */
   removeFinished() {
-    let promiseList = Downloads.getList(this._isPrivate ? Downloads.PRIVATE
-                                                        : Downloads.PUBLIC);
-    promiseList.then(list => list.removeFinished())
-               .catch(Cu.reportError);
+    this._promiseList.then(list => list.removeFinished()).catch(Cu.reportError);
     let indicatorData = this._isPrivate ? PrivateDownloadsIndicatorData
                                         : DownloadsIndicatorData;
     indicatorData.attention = DownloadsCommon.ATTENTION_NONE;
   },
 
   // Integration with the asynchronous Downloads back-end
 
   onDownloadAdded(download) {
     // Download objects do not store the end time of downloads, as the Downloads
     // API does not need to persist this information for all platforms. Once a
     // download terminates on a Desktop browser, it becomes a history download,
     // for which the end time is stored differently, as a Places annotation.
     download.endTime = Date.now();
 
     this.oldDownloadStates.set(download,
                                DownloadsCommon.stateOfDownload(download));
-
-    for (let view of this._views) {
-      view.onDownloadAdded(download);
-    }
   },
 
   onDownloadChanged(download) {
     let oldState = this.oldDownloadStates.get(download);
     let newState = DownloadsCommon.stateOfDownload(download);
     this.oldDownloadStates.set(download, newState);
 
     if (oldState != newState) {
@@ -775,75 +775,46 @@ DownloadsDataCtor.prototype = {
         this._notifyDownloadEvent("finish");
       }
     }
 
     if (!download.newDownloadNotified) {
       download.newDownloadNotified = true;
       this._notifyDownloadEvent("start");
     }
-
-    for (let view of this._views) {
-      view.onDownloadChanged(download);
-    }
   },
 
   onDownloadRemoved(download) {
     this.oldDownloadStates.delete(download);
-
-    for (let view of this._views) {
-      view.onDownloadRemoved(download);
-    }
   },
 
   // Registration of views
 
   /**
    * Adds an object to be notified when the available download data changes.
    * The specified object is initialized with the currently available downloads.
    *
    * @param aView
    *        DownloadsView object to be added.  This reference must be passed to
    *        removeView before termination.
    */
   addView(aView) {
-    this._views.push(aView);
-    this._updateView(aView);
+    this._promiseList.then(list => list.addView(aView))
+                     .catch(Cu.reportError);
   },
 
   /**
    * Removes an object previously added using addView.
    *
    * @param aView
    *        DownloadsView object to be removed.
    */
   removeView(aView) {
-    let index = this._views.indexOf(aView);
-    if (index != -1) {
-      this._views.splice(index, 1);
-    }
-  },
-
-  /**
-   * Ensures that the currently loaded data is added to the specified view.
-   *
-   * @param aView
-   *        DownloadsView object to be initialized.
-   */
-  _updateView(aView) {
-    // Indicate to the view that a batch loading operation is in progress.
-    aView.onDataLoadStarting();
-
-    // This iterates downloads in the same order as they were originally added.
-    for (let download of this.downloads) {
-      aView.onDownloadAdded(download);
-    }
-
-    // Notify the view that all data is available.
-    aView.onDataLoadCompleted();
+    this._promiseList.then(list => list.removeView(aView))
+                     .catch(Cu.reportError);
   },
 
   // Notifications sent to the most recent browser window only
 
   /**
    * Set to true after the first download causes the downloads panel to be
    * displayed.
    */
@@ -980,34 +951,34 @@ const DownloadsViewPrototype = {
       if (this._isPrivate) {
         PrivateDownloadsData.removeView(this);
       } else {
         DownloadsData.removeView(this);
       }
     }
   },
 
-  // Callback functions from DownloadsData
+  // Callback functions from DownloadList
 
   /**
    * Indicates whether we are still loading downloads data asynchronously.
    */
   _loading: false,
 
   /**
    * Called before multiple downloads are about to be loaded.
    */
-  onDataLoadStarting() {
+  onDownloadBatchStarting() {
     this._loading = true;
   },
 
   /**
    * Called after data loading finished.
    */
-  onDataLoadCompleted() {
+  onDownloadBatchEnded() {
     this._loading = false;
   },
 
   /**
    * Called when a new download data item is available, either during the
    * asynchronous data load or when a new download is started.
    *
    * @param download
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -1100,18 +1100,17 @@ DownloadsPlacesView.prototype = {
           this._richlistbox.selectedItem = firstDownloadElement;
           this._richlistbox.currentItem = firstDownloadElement;
           this._initiallySelectedElement = firstDownloadElement;
         });
       }
     }
   },
 
-  onDataLoadStarting() {},
-  onDataLoadCompleted() {
+  onDownloadBatchEnded() {
     this._ensureInitialSelection();
   },
 
   onDownloadAdded(download) {
     this._addDownloadData(download, null, true);
   },
 
   onDownloadChanged(download) {
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -734,26 +734,26 @@ const DownloadsView = {
     return this.downloadsHistory = document.getElementById("downloadsHistory");
   },
 
   // Callback functions from DownloadsData
 
   /**
    * Called before multiple downloads are about to be loaded.
    */
-  onDataLoadStarting() {
-    DownloadsCommon.log("onDataLoadStarting called for DownloadsView.");
+  onDownloadBatchStarting() {
+    DownloadsCommon.log("onDownloadBatchStarting called for DownloadsView.");
     this.loading = true;
   },
 
   /**
    * Called after data loading finished.
    */
-  onDataLoadCompleted() {
-    DownloadsCommon.log("onDataLoadCompleted called for DownloadsView.");
+  onDownloadBatchEnded() {
+    DownloadsCommon.log("onDownloadBatchEnded called for DownloadsView.");
 
     this.loading = false;
 
     // We suppressed item count change notifications during the batch load, at
     // this point we should just call the function once.
     this._itemCountChanged();
 
     // Notify the panel that all the initially available downloads have been
--- a/toolkit/components/jsdownloads/src/DownloadList.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadList.jsm
@@ -144,34 +144,42 @@ this.DownloadList.prototype = {
    *            // Called after aDownload is added to the end of the list.
    *          },
    *          onDownloadChanged: function (aDownload) {
    *            // Called after the properties of aDownload change.
    *          },
    *          onDownloadRemoved: function (aDownload) {
    *            // Called after aDownload is removed from the list.
    *          },
+   *          onDownloadBatchStarting: function () {
+   *            // Called before multiple changes are made at the same time.
+   *          },
+   *          onDownloadBatchEnded: function () {
+   *            // Called after all the changes have been made.
+   *          },
    *        }
    *
    * @return {Promise}
    * @resolves When the view has been registered and all the onDownloadAdded
    *           notifications for the existing downloads have been sent.
    * @rejects JavaScript exception.
    */
   addView: function DL_addView(aView) {
     this._views.add(aView);
 
     if ("onDownloadAdded" in aView) {
+      this._notifyAllViews("onDownloadBatchStarting");
       for (let download of this._downloads) {
         try {
           aView.onDownloadAdded(download);
         } catch (ex) {
           Cu.reportError(ex);
         }
       }
+      this._notifyAllViews("onDownloadBatchEnded");
     }
 
     return Promise.resolve();
   },
 
   /**
    * Removes a view that was previously added using addView.
    *