Bug 1396581 - Allow retrieving DownloadHistoryList objects that show private downloads. r=mak draft
authorMike de Boer <mdeboer@mozilla.com>
Mon, 04 Sep 2017 14:38:06 +0100
changeset 658622 04b0ff0364a53dddf3005230f4e4222d53244ded
parent 655774 ab2d700fda2b4934d24227216972dce9fac19b74
child 729712 271d0589df196be35c7a7e1987e3373eeda929cc
push id77830
push userpaolo.mozmail@amadzone.org
push dateMon, 04 Sep 2017 13:45:36 +0000
reviewersmak
bugs1396581
milestone57.0a1
Bug 1396581 - Allow retrieving DownloadHistoryList objects that show private downloads. r=mak MozReview-Commit-ID: 7lFuZQRmwWI
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
@@ -46,34 +46,46 @@ const METADATA_STATE_BLOCKED_PARENTAL = 
 const METADATA_STATE_DIRTY = 8;
 
 /**
  * Provides methods to retrieve downloads from previous sessions and store
  * downloads for future sessions.
  */
 this.DownloadHistory = {
   /**
-   * Retrieves the main DownloadHistoryList object which provides a view on
-   * downloads from previous browsing sessions, as well as downloads from this
-   * session that were not started from a private browsing window.
+   * 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.
    *
    * @return {Promise}
    * @resolves The requested DownloadHistoryList object.
    * @rejects JavaScript exception.
    */
-  getList() {
-    if (!this._promiseList) {
-      this._promiseList = Downloads.getList(Downloads.PUBLIC).then(list => {
+  getList({type = Downloads.PUBLIC} = {}) {
+    let key = type;
+
+    if (!this._listPromises[key]) {
+      this._listPromises[key] = Downloads.getList(type).then(list => {
         return new DownloadHistoryList(list, HISTORY_PLACES_QUERY);
       });
     }
 
-    return this._promiseList;
+    return this._listPromises[key];
   },
-  _promiseList: null,
+
+  /**
+   * 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: {},
 
   /**
    * Stores new detailed metadata for the given download in history. This is
    * normally called after a download finishes, fails, or is canceled.
    *
    * Failed or canceled downloads with partial data are not stored as paused,
    * because the information from the session download is required for resuming.
    *
--- a/toolkit/components/jsdownloads/test/unit/test_DownloadHistory.js
+++ b/toolkit/components/jsdownloads/test/unit/test_DownloadHistory.js
@@ -58,16 +58,88 @@ function areEqual(a, b) {
     Assert.equal(a, b);
     return true;
   }
   do_print(a + " !== " + b);
   return false;
 }
 
 /**
+ * This allows waiting for an expected list at various points during the test.
+ */
+class TestView {
+  constructor(expected) {
+    this.expected = [...expected];
+    this.downloads = [];
+    this.resolveWhenExpected = () => {};
+  }
+  onDownloadAdded(download, options = {}) {
+    if (options.insertBefore) {
+      let index = this.downloads.indexOf(options.insertBefore);
+      this.downloads.splice(index, 0, download);
+    } else {
+      this.downloads.push(download);
+    }
+    this.checkForExpectedDownloads();
+  }
+  onDownloadChanged(download) {
+    this.checkForExpectedDownloads();
+  }
+  onDownloadRemoved(download) {
+    let index = this.downloads.indexOf(download);
+    this.downloads.splice(index, 1);
+    this.checkForExpectedDownloads();
+  }
+  checkForExpectedDownloads() {
+    // Wait for all the expected downloads to be added or removed before doing
+    // the detailed tests. This is done to avoid creating irrelevant output.
+    if (this.downloads.length != this.expected.length) {
+      return;
+    }
+    for (let i = 0; i < this.downloads.length; i++) {
+      if (this.downloads[i].source.url != this.expected[i].source.url ||
+          this.downloads[i].target.path != this.expected[i].target.path) {
+        return;
+      }
+    }
+    // Check and report the actual state of the downloads. Even if the items
+    // are in the expected order, the metadata for history downloads might not
+    // have been updated to the final state yet.
+    for (let i = 0; i < this.downloads.length; i++) {
+      let download = this.downloads[i];
+      let testDownload = this.expected[i];
+      do_print("Checking download source " + download.source.url +
+               " with target " + download.target.path);
+      if (!areEqual(download.succeeded, !!testDownload.succeeded) ||
+          !areEqual(download.canceled, !!testDownload.canceled) ||
+          !areEqual(download.hasPartialData, !!testDownload.hasPartialData) ||
+          !areEqual(!!download.error, !!testDownload.error)) {
+        return;
+      }
+      // If the above properties match, the error details should be correct.
+      if (download.error) {
+        if (testDownload.error.becauseSourceFailed) {
+          Assert.equal(download.error.message, "History download failed.");
+        }
+        Assert.equal(download.error.becauseBlockedByParentalControls,
+                     testDownload.error.becauseBlockedByParentalControls);
+        Assert.equal(download.error.becauseBlockedByReputationCheck,
+                     testDownload.error.becauseBlockedByReputationCheck);
+      }
+    }
+    this.resolveWhenExpected();
+  }
+  async waitForExpected() {
+    let promise = new Promise(resolve => this.resolveWhenExpected = resolve);
+    this.checkForExpectedDownloads();
+    await promise;
+  }
+}
+
+/**
  * Tests that various operations on session and history downloads are reflected
  * by the DownloadHistoryList object, and that the order of results is correct.
  */
 add_task(async function test_DownloadHistory() {
   // Clean up at the beginning and at the end of the test.
   async function cleanup() {
     await PlacesUtils.history.clear();
   }
@@ -84,143 +156,105 @@ add_task(async function test_DownloadHis
     // Session downloads should show up after all the history download, in the
     // same order as they were added.
     { offset: 45, canceled: true, inSession: true },
     { offset: 35, canceled: true, hasPartialData: true, inSession: true },
     { offset: 55, succeeded: true, inSession: true },
   ];
   const NEXT_OFFSET = 60;
 
+  let publicList = await promiseNewList();
+  let allList = await Downloads.getList(Downloads.ALL);
+
   async function addTestDownload(properties) {
-    properties.source = { url: httpUrl("source" + properties.offset) };
+    properties.source = {
+      url: httpUrl("source" + properties.offset),
+      isPrivate: properties.isPrivate,
+    };
     let targetFile = getTempFile(TEST_TARGET_FILE_NAME + properties.offset);
     properties.target = { path: targetFile.path };
     properties.startTime = new Date(baseDate.getTime() + properties.offset);
 
     let download = await Downloads.createDownload(properties);
     if (properties.inSession) {
-      await publicList.add(download);
+      await allList.add(download);
+    }
+
+    if (properties.isPrivate) {
+      return;
     }
 
     // Add the download to history using the XPCOM service, then use the
     // DownloadHistory module to save the associated metadata.
     let promiseAnnotations = waitForAnnotations(properties.source.url);
     let promiseVisit = promiseWaitForVisit(properties.source.url);
     gDownloadHistory.addDownload(Services.io.newURI(properties.source.url),
                                  null,
                                  properties.startTime.getTime() * 1000,
                                  NetUtil.newURI(targetFile));
     await promiseVisit;
     DownloadHistory.updateMetaData(download);
     await promiseAnnotations;
   }
 
   // Add all the test downloads to history.
-  let publicList = await promiseNewList();
   for (let properties of testDownloads) {
     await addTestDownload(properties);
   }
 
-  // This allows waiting for an expected list at various points during the test.
-  let view = {
-    downloads: [],
-    onDownloadAdded(download, options = {}) {
-      if (options.insertBefore) {
-        let index = this.downloads.indexOf(options.insertBefore);
-        this.downloads.splice(index, 0, download);
-      } else {
-        this.downloads.push(download);
-      }
-      this.checkForExpectedDownloads();
-    },
-    onDownloadChanged(download) {
-      this.checkForExpectedDownloads();
-    },
-    onDownloadRemoved(download) {
-      let index = this.downloads.indexOf(download);
-      this.downloads.splice(index, 1);
-      this.checkForExpectedDownloads();
-    },
-    checkForExpectedDownloads() {
-      // Wait for all the expected downloads to be added or removed before doing
-      // the detailed tests. This is done to avoid creating irrelevant output.
-      if (this.downloads.length != testDownloads.length) {
-        return;
-      }
-      for (let i = 0; i < this.downloads.length; i++) {
-        if (this.downloads[i].source.url != testDownloads[i].source.url ||
-            this.downloads[i].target.path != testDownloads[i].target.path) {
-          return;
-        }
-      }
-      // Check and report the actual state of the downloads. Even if the items
-      // are in the expected order, the metadata for history downloads might not
-      // have been updated to the final state yet.
-      for (let i = 0; i < view.downloads.length; i++) {
-        let download = view.downloads[i];
-        let testDownload = testDownloads[i];
-        do_print("Checking download source " + download.source.url +
-                 " with target " + download.target.path);
-        if (!areEqual(download.succeeded, !!testDownload.succeeded) ||
-            !areEqual(download.canceled, !!testDownload.canceled) ||
-            !areEqual(download.hasPartialData, !!testDownload.hasPartialData) ||
-            !areEqual(!!download.error, !!testDownload.error)) {
-          return;
-        }
-        // If the above properties match, the error details should be correct.
-        if (download.error) {
-          if (testDownload.error.becauseSourceFailed) {
-            Assert.equal(download.error.message, "History download failed.");
-          }
-          Assert.equal(download.error.becauseBlockedByParentalControls,
-                       testDownload.error.becauseBlockedByParentalControls);
-          Assert.equal(download.error.becauseBlockedByReputationCheck,
-                       testDownload.error.becauseBlockedByReputationCheck);
-        }
-      }
-      this.resolveWhenExpected();
-    },
-    resolveWhenExpected: () => {},
-    async waitForExpected() {
-      let promise = new Promise(resolve => this.resolveWhenExpected = resolve);
-      this.checkForExpectedDownloads();
-      await promise;
-    },
-  };
-
   // Initialize DownloadHistoryList only after having added the history and
   // session downloads, and check that they are loaded in the correct order.
-  let list = await DownloadHistory.getList();
-  await list.addView(view);
+  let historyList = await DownloadHistory.getList();
+  let view = new TestView(testDownloads);
+  await historyList.addView(view);
   await view.waitForExpected();
 
   // Remove a download from history and verify that the change is reflected.
-  let downloadToRemove = testDownloads[1];
-  testDownloads.splice(1, 1);
+  let downloadToRemove = view.expected[1];
+  view.expected.splice(1, 1);
   await PlacesUtils.history.remove(downloadToRemove.source.url);
   await view.waitForExpected();
 
   // Add a download to history and verify it's placed before session downloads,
   // even if the start date is more recent.
   let downloadToAdd = { offset: NEXT_OFFSET, canceled: true };
-  testDownloads.splice(testDownloads.findIndex(d => d.inSession), 0,
+  view.expected.splice(view.expected.findIndex(d => d.inSession), 0,
                        downloadToAdd);
   await addTestDownload(downloadToAdd);
   await view.waitForExpected();
 
   // Add a session download and verify it's placed after all session downloads,
   // even if the start date is less recent.
   let sessionDownloadToAdd = { offset: 0, inSession: true, succeeded: true };
-  testDownloads.push(sessionDownloadToAdd);
+  view.expected.push(sessionDownloadToAdd);
   await addTestDownload(sessionDownloadToAdd);
   await view.waitForExpected();
 
   // Add a session download for the same URI without a history entry, and verify
   // it's visible and placed after all session downloads.
-  testDownloads.push(sessionDownloadToAdd);
+  view.expected.push(sessionDownloadToAdd);
   await publicList.add(await Downloads.createDownload(sessionDownloadToAdd));
   await view.waitForExpected();
 
+  // Create a new DownloadHistoryList that also shows private downloads. Since
+  // we only have public downloads, the two lists should contain the same items.
+  let allHistoryList = await DownloadHistory.getList({ type: Downloads.ALL });
+  let allView = new TestView(view.expected);
+  await allHistoryList.addView(allView);
+  await allView.waitForExpected();
+
+  // 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();
+
   // Clear history and check that session downloads with partial data remain.
-  testDownloads = testDownloads.filter(d => d.hasPartialData);
+  // 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();
 });