Bug 1381409 - Part 3 - Use DownloadList notifications directly in front-end download views. r=mak draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 17 Jul 2017 12:05:12 +0100
changeset 609738 81ff4f6b59235c5fba5639d2aa149df7d432a6a5
parent 609737 3c66d1d7d8a75b414d317294adb18c21d565a90b
child 637655 28177708c9976222405fde013542fb736d63e715
push id68670
push userpaolo.mozmail@amadzone.org
push dateMon, 17 Jul 2017 12:26:21 +0000
reviewersmak
bugs1381409
milestone56.0a1
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
@@ -1099,18 +1099,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.
    *