Bug 1395663 - Allow retrieving DownloadHistoryList objects that include a limited number of history results. r?Paolo draft
authorMike de Boer <mdeboer@mozilla.com>
Fri, 08 Sep 2017 11:33:11 +0200
changeset 661343 0bf4b66766c685f485b891749a07819b269147ca
parent 661103 b4c1ad9565ee9d00d96501c4a83083daf25c1413
child 661379 5448ee4027bdbd6801314801bb52f99a40c33bf8
push id78719
push usermdeboer@mozilla.com
push dateFri, 08 Sep 2017 09:38:50 +0000
reviewersPaolo
bugs1395663
milestone57.0a1
Bug 1395663 - Allow retrieving DownloadHistoryList objects that include a limited number of history results. r?Paolo MozReview-Commit-ID: AaqCEPNntUb
toolkit/components/jsdownloads/src/DownloadHistory.jsm
toolkit/components/jsdownloads/test/unit/test_DownloadHistory.js
--- a/toolkit/components/jsdownloads/src/DownloadHistory.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadHistory.jsm
@@ -28,17 +28,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 
 // Places query used to retrieve all history downloads for the related list.
 const HISTORY_PLACES_QUERY =
       "place:transition=" + Ci.nsINavHistoryService.TRANSITION_DOWNLOAD +
-      "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_ASCENDING;
+      "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_DESCENDING;
 
 const DESTINATIONFILEURI_ANNO = "downloads/destinationFileURI";
 const METADATA_ANNO = "downloads/metaData";
 
 const METADATA_STATE_FINISHED = 1;
 const METADATA_STATE_FAILED = 2;
 const METADATA_STATE_CANCELED = 3;
 const METADATA_STATE_PAUSED = 4;
@@ -53,29 +53,37 @@ this.DownloadHistory = {
   /**
    * Retrieves the main DownloadHistoryList object which provides a unified view
    * on downloads from both previous browsing sessions and this session.
    *
    * @param type
    *        Determines which type of downloads from this session should be
    *        included in the list. This is Downloads.PUBLIC by default, but can
    *        also be Downloads.PRIVATE or Downloads.ALL.
+   * @param maxHistoryResults
+   *        Optional number that limits the amount of results the history query
+   *        may return.
    *
    * @return {Promise}
    * @resolves The requested DownloadHistoryList object.
    * @rejects JavaScript exception.
    */
-  getList({type = Downloads.PUBLIC} = {}) {
-    if (!this._listPromises[type]) {
-      this._listPromises[type] = Downloads.getList(type).then(list => {
-        return new DownloadHistoryList(list, HISTORY_PLACES_QUERY);
+  getList({type = Downloads.PUBLIC, maxHistoryResults} = {}) {
+    let key = `${type}|${maxHistoryResults ? maxHistoryResults : -1}`;
+    if (!this._listPromises[key]) {
+      this._listPromises[key] = Downloads.getList(type).then(list => {
+        // When the amount of history downloads is capped, we request the list in
+        // descending order, to make sure that the list can apply the limit.
+        let query = HISTORY_PLACES_QUERY +
+          (maxHistoryResults ? "&maxResults=" + maxHistoryResults : "");
+        return new DownloadHistoryList(list, query);
       });
     }
 
-    return this._listPromises[type];
+    return this._listPromises[key];
   },
 
   /**
    * This object is populated with one key for each type of download list that
    * can be returned by the getList method. The values are promises that resolve
    * to DownloadHistoryList objects.
    */
   _listPromises: {},
@@ -553,17 +561,17 @@ this.DownloadHistoryList.prototype = {
         slot.historyDownload = null;
       } else {
         let slotsForUrl = this._slotsForUrl.get(slot.download.source.url);
         this._removeSlot({ slot, slotsForUrl });
       }
     }
 
     // Add new slots or reuse existing ones for history downloads.
-    for (let index = 0; index < container.childCount; index++) {
+    for (let index = container.childCount - 1; index >= 0; --index) {
       try {
         this._insertPlacesNode(container.getChild(index));
       } catch (ex) {
         Cu.reportError(ex);
       }
     }
 
     this._notifyAllViews("onDownloadBatchEnded");
--- a/toolkit/components/jsdownloads/test/unit/test_DownloadHistory.js
+++ b/toolkit/components/jsdownloads/test/unit/test_DownloadHistory.js
@@ -244,16 +244,25 @@ add_task(async function test_DownloadHis
   // Add a new private download and verify it appears only on the complete list.
   let privateDownloadToAdd = { offset: NEXT_OFFSET + 10, inSession: true,
                                succeeded: true, isPrivate: true };
   allView.expected.push(privateDownloadToAdd);
   await addTestDownload(privateDownloadToAdd);
   await view.waitForExpected();
   await allView.waitForExpected();
 
+  // Now test the maxHistoryResults parameter.
+  let allHistoryList2 = await DownloadHistory.getList({ type: Downloads.ALL,
+    maxHistoryResults: 3 });
+  // Prepare the set of downloads to contain fewer history downloads by removing
+  // the oldest ones.
+  let allView2 = new TestView(allView.expected.slice(3));
+  await allHistoryList2.addView(allView2);
+  await allView2.waitForExpected();
+
   // Clear history and check that session downloads with partial data remain.
   // Private downloads are also not cleared when clearing history.
   view.expected = view.expected.filter(d => d.hasPartialData);
   allView.expected = allView.expected.filter(d => d.hasPartialData ||
                                                   d.isPrivate);
   await PlacesUtils.history.clear();
   await view.waitForExpected();
   await allView.waitForExpected();