Bug 1396581 - Allow retrieving DownloadHistoryList objects that show private downloads. r=mak
authorMike de Boer <mdeboer@mozilla.com>
Tue, 05 Sep 2017 11:44:07 +0200
changeset 428382 f12d13ff72de26051890ebd07d423b28579c8087
parent 428381 666cff43b2c3eea70c7751752546cdf3740c9177
child 428383 6f458218dda9687d1405cb6a894f89ddfc2336f8
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1396581
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,44 @@ 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} = {}) {
+    if (!this._listPromises[type]) {
+      this._listPromises[type] = Downloads.getList(type).then(list => {
         return new DownloadHistoryList(list, HISTORY_PLACES_QUERY);
       });
     }
 
-    return this._promiseList;
+    return this._listPromises[type];
   },
-  _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();
 });