1115379 - Streamline DownloadsViewItemController construction and remove now unneeded identifiers

Status: RESOLVED FIXED

(Firefox :: Downloads Panel, defect)

Description

[:Paolo Amadini]

Attachment: The patch

We can now efficiently keep DownloadsViewItemController instances in a Map. Currently there are at most three of them for each Downloads Panel.

The comment saying we must keep track of deleted instances is not true with the new Downloads API anymore.

These changes allow us to remove the dummy download identifiers.

Comment 1

[Marco Bonardo [:mak]]

Comment on attachment 8541264 [details] [diff] [review]
The patch

Review of attachment 8541264 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/downloads/content/downloads.js
@@ +830,5 @@
>      }
>    },
>  
>    /**
> +   * Associates each richlistbox Element for a download with its corresponding

maybe it would make more sense to speak about "richlistitem" instead of "richlistbox element"

@@ +973,1 @@
>      let localFile = controller.dataItem.localFile;

nit: could be merged into:
let localFile = DownloadsView.controllerForElement(element)
                             .dataItem.localFile;

@@ +1333,5 @@
>  /**
>   * Handles all the user interaction events, in particular the "commands",
>   * related to a single item in the downloads list widgets.
>   */
> +function DownloadsViewItemController(aDataItem) {

looks like this change breaks http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/test/browser/browser_basic_functionality.js#52

Comment 2

[:Paolo Amadini]

Attachment: Updated patch

Comment 3

[Marco Mucci [:MarcoM]]

Hi Paolo, can you provide a point value.

Comment 4

[:Paolo Amadini]

https://hg.mozilla.org/integration/fx-team/rev/85a8c3b8ad38

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/85a8c3b8ad38