[PALEMOON] Bug 1115369 - Use notifications instead of getViewItem for DownloadsView

3 files changed, 78 insertions, 98 deletions

diff --git a/application/palemoon/components/downloads/DownloadsCommon.jsm b/application/palemoon/components/downloads/DownloadsCommon.jsm
index bd5d55a73..0a3af7878 100644
--- a/application/palemoon/components/downloads/DownloadsCommon.jsm
+++ b/application/palemoon/components/downloads/DownloadsCommon.jsm
@@ -773,7 +773,7 @@ DownloadsDataCtor.prototype = {
     if (oldState != aDataItem.state) {
       for (let view of this._views) {
         try {
-          view.getViewItem(aDataItem).onStateChange(oldState);
+          view.onDataItemStateChanged(aDataItem, oldState);
         } catch (ex) {
           Cu.reportError(ex);
         }
@@ -812,7 +812,7 @@ DownloadsDataCtor.prototype = {
     }
 
     for (let view of this._views) {
-      view.getViewItem(aDataItem).onProgressChange();
+      view.onDataItemChanged(aDataItem);
     }
   },
 
@@ -1889,17 +1889,26 @@ const DownloadsViewPrototype = {
   },
 
   /**
-   * Returns the view item associated with the provided data item for this view.
+   * Called when the "state" property of a DownloadsDataItem has changed.
    *
-   * @param aDataItem
-   *        DownloadsDataItem object for which the view item is requested.
+   * The onDataItemChanged notification will be sent afterwards.
    *
-   * @return Object that can be used to notify item status events.
+   * @note Subclasses should override this.
+   */
+  onDataItemStateChanged(aDataItem) {
+    throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
+  },
+
+  /**
+   * Called every time any state property of a DownloadsDataItem may have
+   * changed, including progress properties and the "state" property.
+   *
+   * Note that progress notification changes are throttled at the Downloads.jsm
+   * API level, and there is no throttling mechanism in the front-end.
    *
    * @note Subclasses should override this.
    */
-  getViewItem: function DID_getViewItem(aDataItem)
-  {
+  onDataItemChanged(aDataItem) {
     throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
   },
 
@@ -2014,37 +2023,21 @@ DownloadsIndicatorDataCtor.prototype = {
     this._updateViews();
   },
 
-  /**
-   * Returns the view item associated with the provided data item for this view.
-   *
-   * @param aDataItem
-   *        DownloadsDataItem object for which the view item is requested.
-   *
-   * @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(aOldState)
-      {
-        if (aDataItem.state == nsIDM.DOWNLOAD_FINISHED ||
-            aDataItem.state == nsIDM.DOWNLOAD_FAILED) {
-          data.attention = true;
-        }
+  // DownloadsView
+  onDataItemStateChanged(aDataItem, aOldState) {
+    if (aDataItem.state == nsIDM.DOWNLOAD_FINISHED ||
+        aDataItem.state == nsIDM.DOWNLOAD_FAILED) {
+      this.attention = true;
+    }
 
-        // Since the state of a download changed, reset the estimated time left.
-        data._lastRawTimeLeft = -1;
-        data._lastTimeLeft = -1;
+    // Since the state of a download changed, reset the estimated time left.
+    this._lastRawTimeLeft = -1;
+    this._lastTimeLeft = -1;
+  },
 
-        data._updateViews();
-      },
-      onProgressChange: function DIVI_onProgressChange()
-      {
-        data._updateViews();
-      }
-    });
+  // DownloadsView
+  onDataItemChanged() {
+    this._updateViews();
   },
 
   //////////////////////////////////////////////////////////////////////////////
@@ -2298,22 +2291,16 @@ DownloadsSummaryData.prototype = {
     this._updateViews();
   },
 
-  getViewItem: function DSD_getViewItem(aDataItem)
-  {
-    let self = this;
-    return Object.freeze({
-      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()
-      {
-        self._updateViews();
-      }
-    });
+  // DownloadsView
+  onDataItemStateChanged(aOldState) {
+    // Since the state of a download changed, reset the estimated time left.
+    this._lastRawTimeLeft = -1;
+    this._lastTimeLeft = -1;
+  },
+
+  // DownloadsView
+  onDataItemChanged() {
+    this._updateViews();
   },
 
   //////////////////////////////////////////////////////////////////////////////
diff --git a/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js b/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js
index 054f0405f..ba1aa6092 100644
--- a/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js
+++ b/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js
@@ -55,9 +55,8 @@ const NOT_AVAILABLE = Number.MAX_VALUE;
  *
  * The caller is also responsible for "passing over" notification from both the
  * download-view and the places-result-observer, in the following manner:
- *  - The DownloadsPlacesView object implements getViewItem of the download-view
- *    pseudo interface.  It returns this object (therefore we implement
- *    onStateChangea and onProgressChange here).
+ *  - The DownloadsPlacesView object implements onDataItemStateChanged and
+ *    onDataItemChanged of the DownloadsView pseudo interface.
  *  - The DownloadsPlacesView object adds itself as a places result observer and
  *    calls this object's placesNodeIconChanged, placesNodeTitleChanged and
  *    placeNodeAnnotationChanged from its callbacks.
@@ -557,8 +556,7 @@ DownloadElementShell.prototype = {
     }
   },
 
-  /* DownloadView */
-  onStateChange: function DES_onStateChange(aOldState) {
+  onStateChanged(aOldState) {
     let metaData = this.getDownloadMetaData();
     metaData.state = this.dataItem.state;
     if (aOldState != nsIDM.DOWNLOAD_FINISHED && aOldState != metaData.state) {
@@ -572,14 +570,14 @@ DownloadElementShell.prototype = {
     }
 
     this._updateDownloadStatusUI();
+
     if (this._element.selected)
       goUpdateDownloadCommands();
     else
       goUpdateCommand("downloadsCmd_clearDownloads");
   },
 
-  /* DownloadView */
-  onProgressChange: function DES_onProgressChange() {
+  onChanged() {
     this._updateDownloadStatusUI();
   },
 
@@ -915,7 +913,7 @@ DownloadsPlacesView.prototype = {
     // data item. Thus, we also check that we make sure we don't have a view item
     // already.
     if (!shouldCreateShell &&
-        aDataItem && this.getViewItem(aDataItem) == null) {
+        aDataItem && !this._viewItemsForDataItems.has(aDataItem)) {
       // If there's a past-download-only shell for this download-uri with no
       // associated data item, use it for the new data item. Otherwise, go ahead
       // and create another shell.
@@ -1034,7 +1032,7 @@ DownloadsPlacesView.prototype = {
     if (shells.size == 0)
       throw new Error("Should have had at leaat one shell for this uri");
 
-    let shell = this.getViewItem(aDataItem);
+    let shell = this._viewItemsForDataItems.get(aDataItem);
     if (!shells.has(shell))
       throw new Error("Missing download element shell in shells list for url");
 
@@ -1342,8 +1340,15 @@ DownloadsPlacesView.prototype = {
     this._removeSessionDownloadFromView(aDataItem);
   },
 
-  getViewItem: function(aDataItem)
-    this._viewItemsForDataItems.get(aDataItem, null),
+  // DownloadsView
+  onDataItemStateChanged(aDataItem, aOldState) {
+    this._viewItemsForDataItems.get(aDataItem).onStateChanged(aOldState);
+  },
+
+  // DownloadsView
+  onDataItemChanged(aDataItem) {
+    this._viewItemsForDataItems.get(aDataItem).onChanged();
+  },
 
   supportsCommand: function DPV_supportsCommand(aCommand) {
     if (DOWNLOAD_VIEW_SUPPORTED_COMMANDS.indexOf(aCommand) != -1) {
diff --git a/application/palemoon/components/downloads/content/downloads.js b/application/palemoon/components/downloads/content/downloads.js
index 833d7d72f..44457e711 100644
--- a/application/palemoon/components/downloads/content/downloads.js
+++ b/application/palemoon/components/downloads/content/downloads.js
@@ -570,7 +570,7 @@ const DownloadsPanel = {
       // still exist, and update the allowed items interactions accordingly.  We
       // do these checks on a background thread, and don't prevent the panel to
       // be displayed while these checks are being performed.
-      for each (let viewItem in DownloadsView._viewItems) {
+      for (let viewItem of DownloadsView._visibleViewItems.values()) {
         viewItem.verifyTargetExists();
       }
 
@@ -711,11 +711,11 @@ const DownloadsView = {
   _dataItems: [],
 
   /**
-   * Object containing the available DownloadsViewItem objects, indexed by their
-   * numeric download identifier.  There is a limited number of view items in
-   * the panel at any given time.
+   * Associates the visible DownloadsDataItem objects with their corresponding
+   * DownloadsViewItem object.  There is a limited number of view items in the
+   * panel at any given time.
    */
-  _viewItems: {},
+  _visibleViewItems: new Map(),
 
   /**
    * Called when the number of items in the list changes.
@@ -879,31 +879,21 @@ const DownloadsView = {
     this._itemCountChanged();
   },
 
-  /**
-   * Returns the view item associated with the provided data item for this view.
-   *
-   * @param aDataItem
-   *        DownloadsDataItem object for which the view item is requested.
-   *
-   * @return Object that can be used to notify item status events.
-   */
-  getViewItem: function DV_getViewItem(aDataItem)
-  {
-    // If the item is visible, just return it, otherwise return a mock object
-    // that doesn't react to notifications.
-    if (aDataItem.downloadGuid in this._viewItems) {
-      return this._viewItems[aDataItem.downloadGuid];
+  // DownloadsView
+  onDataItemStateChanged(aDataItem, aOldState) {
+    let viewItem = this._visibleViewItems.get(aDataItem);
+    if (viewItem) {
+      viewItem.onStateChanged(aOldState);
     }
-    return this._invisibleViewItem;
   },
 
-  /**
-   * Mock DownloadsDataItem object that doesn't react to notifications.
-   */
-  _invisibleViewItem: Object.freeze({
-    onStateChange: function () { },
-    onProgressChange: function () { }
-  }),
+  // DownloadsView
+  onDataItemChanged(aDataItem) {
+    let viewItem = this._visibleViewItems.get(aDataItem);
+    if (viewItem) {
+      viewItem.onChanged();
+    }
+  },
 
   /**
    * Creates a new view item associated with the specified data item, and adds
@@ -916,7 +906,7 @@ const DownloadsView = {
 
     let element = document.createElement("richlistitem");
     let viewItem = new DownloadsViewItem(aDataItem, element);
-    this._viewItems[aDataItem.downloadGuid] = viewItem;
+    this._visibleViewItems.set(aDataItem, viewItem);
     if (aNewest) {
       this.richListBox.insertBefore(element, this.richListBox.firstChild);
     } else {
@@ -930,14 +920,14 @@ const DownloadsView = {
   _removeViewItem: function DV_removeViewItem(aDataItem)
   {
     DownloadsCommon.log("Removing a DownloadsViewItem from the downloads list.");
-    let element = this.getViewItem(aDataItem)._element;
+    let element = this._visibleViewItems.get(aDataItem)._element;
     let previousSelectedIndex = this.richListBox.selectedIndex;
     this.richListBox.removeChild(element);
     if (previousSelectedIndex != -1) {
       this.richListBox.selectedIndex = Math.min(previousSelectedIndex,
                                                 this.richListBox.itemCount - 1);
     }
-    delete this._viewItems[aDataItem.downloadGuid];
+    this._visibleViewItems.delete(aDataItem);
   },
 
   //////////////////////////////////////////////////////////////////////////////
@@ -1134,7 +1124,7 @@ DownloadsViewItem.prototype = {
    * the download might be the same as before, if the data layer received
    * multiple events for the same download.
    */
-  onStateChange: function DVI_onStateChange(aOldState)
+  onStateChanged(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
@@ -1155,14 +1145,12 @@ DownloadsViewItem.prototype = {
 
     // 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.
    */
-  onProgressChange: function DVI_onProgressChange() {
+  onChanged() {
     this._updateProgress();
     this._updateStatusLine();
   },