Bug 916430 - General Downloads API review. r=enn

browser/base/content/test/browser_sanitize-timespans.js

@@ -620,7 +620,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 10 * kMsecPerMin), // 10 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   download = yield Downloads.createDownload({
     source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440",
@@ -628,7 +628,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 45 * kMsecPerMin), // 45 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   download = yield Downloads.createDownload({
     source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169",
@@ -636,7 +636,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 70 * kMsecPerMin), // 70 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   download = yield Downloads.createDownload({
     source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440",
@@ -644,7 +644,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 90 * kMsecPerMin), // 90 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   download = yield Downloads.createDownload({
     source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169",
@@ -652,7 +652,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 130 * kMsecPerMin), // 130 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   download = yield Downloads.createDownload({
     source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440",
@@ -660,7 +660,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 180 * kMsecPerMin), // 180 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   download = yield Downloads.createDownload({
     source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169",
@@ -668,7 +668,7 @@ function setupDownloads() {
   });
   download.startTime = new Date(now_mSec - 250 * kMsecPerMin), // 250 minutes ago
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
 
   // Add "today" download
   let today = new Date();
@@ -682,7 +682,7 @@ function setupDownloads() {
   });
   download.startTime = today, // 12:00:01 AM this morning
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
   
   // Add "before today" download
   let lastYear = new Date();
@@ -694,7 +694,7 @@ function setupDownloads() {
   });
   download.startTime = lastYear,
   download.canceled = true;
-  publicList.add(download);
+  yield publicList.add(download);
   
   // Confirm everything worked
   let downloads = yield publicList.getAll();

browser/base/content/test/browser_sanitizeDialog.js

@@ -987,7 +987,7 @@ function blankSlate() {
     let publicList = yield Downloads.getList(Downloads.PUBLIC);
     let downloads = yield publicList.getAll();
     for (let download of downloads) {
-      publicList.remove(download);
+      yield publicList.remove(download);
       yield download.finalize(true);
     }
     downloadsDone = true;

toolkit/components/jsdownloads/src/DownloadImport.jsm

@@ -170,7 +170,7 @@ this.DownloadImport.prototype = {
 
             let download = yield Downloads.createDownload(downloadOptions);
 
-            this.list.add(download);
+            yield this.list.add(download);
 
             if (resumeDownload) {
               download.start();

toolkit/components/jsdownloads/src/DownloadIntegration.jsm

@@ -216,8 +216,11 @@ this.DownloadIntegration = {
       // After the list of persistent downloads has been loaded, add the
       // DownloadAutoSaveView and the DownloadHistoryObserver (even if the load
       // operation failed).  These objects are kept alive by the underlying
-      // DownloadList and by the history service respectively.
+      // DownloadList and by the history service respectively.  We wait for a
-      new DownloadAutoSaveView(aList, this._store);
+      // complete initialization of the view used for detecting changes to
+      // downloads to be persisted, before other callers get a chance to modify
+      // the list without being detected.
+      yield new DownloadAutoSaveView(aList, this._store).initialize();
       new DownloadHistoryObserver(aList);
     }.bind(this));
   },
@@ -769,7 +772,8 @@ this.DownloadObserver = {
       }
     };
 
-    aList.addView(downloadsView);
+    // We register the view asynchronously.
+    aList.addView(downloadsView).then(null, Cu.reportError);
   },
 
   /**
@@ -828,8 +832,9 @@ this.DownloadObserver = {
           let list = yield Downloads.getList(Downloads.PRIVATE);
           let downloads = yield list.getAll();
 
+          // We can remove the downloads and finalize them in parallel.
           for (let download of downloads) {
-            list.remove(download);
+            list.remove(download).then(null, Cu.reportError);
             download.finalize(true).then(null, Cu.reportError);
           }
         });
@@ -904,7 +909,9 @@ DownloadHistoryObserver.prototype = {
 
 /**
  * This view can be added to a DownloadList object to trigger a save operation
- * in the given DownloadStore object when a relevant change occurs.
+ * in the given DownloadStore object when a relevant change occurs.  You should
+ * call the "initialize" method in order to register the view and load the
+ * current state from disk.
  *
  * You do not need to keep a reference to this object in order to keep it alive,
  * because the DownloadList object already keeps a strong reference to it.
@@ -915,25 +922,40 @@ DownloadHistoryObserver.prototype = {
  *        The DownloadStore object used for saving.
  */
 function DownloadAutoSaveView(aList, aStore) {
+  this._list = aList;
   this._store = aStore;
   this._downloadsMap = new Map();
-
-  // We set _initialized to true after adding the view, so that onDownloadAdded
-  // doesn't cause a save to occur.
-  aList.addView(this);
-  this._initialized = true;
 }
 
 DownloadAutoSaveView.prototype = {
+  /**
+   * DownloadList object linked to this view.
+   */
+  _list: null,
+
+  /**
+   * The DownloadStore object used for saving.
+   */
+  _store: null,
+
   /**
    * True when the initial state of the downloads has been loaded.
    */
   _initialized: false,
 
   /**
-   * The DownloadStore object used for saving.
+   * Registers the view and loads the current state from disk.
+   *
+   * @return {Promise}
+   * @resolves When the view has been registered.
+   * @rejects JavaScript exception.
    */
-  _store: null,
+  initialize: function ()
+  {
+    // We set _initialized to true after adding the view, so that
+    // onDownloadAdded doesn't cause a save to occur.
+    return this._list.addView(this).then(() => this._initialized = true);
+  },
 
   /**
    * This map contains only Download objects that should be saved to disk, and

toolkit/components/jsdownloads/src/DownloadList.jsm

@@ -83,11 +83,17 @@ DownloadList.prototype = {
    *
    * @param aDownload
    *        The Download object to add.
+   *
+   * @return {Promise}
+   * @resolves When the download has been added.
+   * @rejects JavaScript exception.
    */
   add: function DL_add(aDownload) {
     this._downloads.push(aDownload);
     aDownload.onchange = this._change.bind(this, aDownload);
     this._notifyAllViews("onDownloadAdded", aDownload);
+
+    return Promise.resolve();
   },
 
   /**
@@ -101,6 +107,10 @@ DownloadList.prototype = {
    *
    * @param aDownload
    *        The Download object to remove.
+   *
+   * @return {Promise}
+   * @resolves When the download has been removed.
+   * @rejects JavaScript exception.
    */
   remove: function DL_remove(aDownload) {
     let index = this._downloads.indexOf(aDownload);
@@ -109,6 +119,8 @@ DownloadList.prototype = {
       aDownload.onchange = null;
       this._notifyAllViews("onDownloadRemoved", aDownload);
     }
+
+    return Promise.resolve();
   },
 
   /**
@@ -145,10 +157,10 @@ DownloadList.prototype = {
    *          },
    *        }
    *
-   * @note The onDownloadAdded notifications are sent synchronously.  This
+   * @return {Promise}
-   *       allows for a complete initialization of the view used for detecting
+   * @resolves When the view has been registered and all the onDownloadAdded
-   *       changes to downloads to be persisted, before other callers get a
+   *           notifications for the existing downloads have been sent.
-   *       chance to modify them.
+   * @rejects JavaScript exception.
    */
   addView: function DL_addView(aView)
   {
@@ -163,18 +175,26 @@ DownloadList.prototype = {
         }
       }
     }
+
+    return Promise.resolve();
   },
 
   /**
-   * Removes a view that was previously added using addView.  The removed view
+   * Removes a view that was previously added using addView.
-   * will not receive any more notifications after this method returns.
    *
    * @param aView
    *        The view object to remove.
+   *
+   * @return {Promise}
+   * @resolves When the view has been removed.  At this point, the removed view
+   *           will not receive any more notifications.
+   * @rejects JavaScript exception.
    */
   removeView: function DL_removeView(aView)
   {
     this._views.delete(aView);
+
+    return Promise.resolve();
   },
 
   /**
@@ -222,7 +242,7 @@ DownloadList.prototype = {
             (!aFilterFn || aFilterFn(download))) {
           // Remove the download first, so that the views don't get the change
           // notifications that may occur during finalization.
-          this.remove(download);
+          yield this.remove(download);
           // Ensure that the download is stopped and no partial data is kept.
           // This works even if the download state has changed meanwhile.  We
           // don't need to wait for the procedure to be complete before
@@ -254,8 +274,8 @@ function DownloadCombinedList(aPublicList, aPrivateList)
   DownloadList.call(this);
   this._publicList = aPublicList;
   this._privateList = aPrivateList;
-  aPublicList.addView(this);
+  aPublicList.addView(this).then(null, Cu.reportError);
-  aPrivateList.addView(this);
+  aPrivateList.addView(this).then(null, Cu.reportError);
 }
 
 DownloadCombinedList.prototype = {
@@ -282,13 +302,17 @@ DownloadCombinedList.prototype = {
    *
    * @param aDownload
    *        The Download object to add.
+   *
+   * @return {Promise}
+   * @resolves When the download has been added.
+   * @rejects JavaScript exception.
    */
   add: function (aDownload)
   {
     if (aDownload.source.isPrivate) {
-      this._privateList.add(aDownload);
+      return this._privateList.add(aDownload);
     } else {
-      this._publicList.add(aDownload);
+      return this._publicList.add(aDownload);
     }
   },
 
@@ -303,13 +327,17 @@ DownloadCombinedList.prototype = {
    *
    * @param aDownload
    *        The Download object to remove.
+   *
+   * @return {Promise}
+   * @resolves When the download has been removed.
+   * @rejects JavaScript exception.
    */
   remove: function (aDownload)
   {
     if (aDownload.source.isPrivate) {
-      this._privateList.remove(aDownload);
+      return this._privateList.remove(aDownload);
     } else {
-      this._publicList.remove(aDownload);
+      return this._publicList.remove(aDownload);
     }
   },
 
@@ -368,6 +396,10 @@ DownloadSummary.prototype = {
    *
    * @param aList
    *        Underlying DownloadList whose contents should be summarized.
+   *
+   * @return {Promise}
+   * @resolves When the view on the underlying list has been registered.
+   * @rejects JavaScript exception.
    */
   bindToList: function (aList)
   {
@@ -375,12 +407,12 @@ DownloadSummary.prototype = {
       throw new Error("bindToList may be called only once.");
     }
 
-    aList.addView(this);
+    return aList.addView(this).then(() => {
-
+      // Set the list reference only after addView has returned, so that we don't
-    // Set the list reference only after addView has returned, so that we don't
+      // send a notification to our views for each download that is added.
-    // send a notification to our views for each download that is added.
+      this._list = aList;
-    this._list = aList;
+      this._onListChanged();
-    this._onListChanged();
+    });
   },
 
   /**
@@ -399,6 +431,11 @@ DownloadSummary.prototype = {
    *            // Called after any property of the summary has changed.
    *          },
    *        }
+   *
+   * @return {Promise}
+   * @resolves When the view has been registered and the onSummaryChanged
+   *           notification has been sent.
+   * @rejects JavaScript exception.
    */
   addView: function (aView)
   {
@@ -411,18 +448,26 @@ DownloadSummary.prototype = {
         Cu.reportError(ex);
       }
     }
+
+    return Promise.resolve();
   },
 
   /**
-   * Removes a view that was previously added using addView.  The removed view
+   * Removes a view that was previously added using addView.
-   * will not receive any more notifications after this method returns.
    *
    * @param aView
    *        The view object to remove.
+   *
+   * @return {Promise}
+   * @resolves When the view has been removed.  At this point, the removed view
+   *           will not receive any more notifications.
+   * @rejects JavaScript exception.
    */
   removeView: function (aView)
   {
     this._views.delete(aView);
+
+    return Promise.resolve();
   },
 
   /**

toolkit/components/jsdownloads/src/DownloadStore.jsm

@@ -132,7 +132,7 @@ DownloadStore.prototype = {
           } finally {
             // Add the download to the list if we succeeded in creating it,
             // after we have updated its initial state.
-            this.list.add(download);
+            yield this.list.add(download);
           }
         } catch (ex) {
           // If an item is unrecognized, don't prevent others from being loaded.

toolkit/components/jsdownloads/src/Downloads.jsm

@@ -132,7 +132,7 @@ this.Downloads = {
    * @resolves When the download has finished successfully.
    * @rejects JavaScript exception if the download failed.
    */
-  simpleDownload: function D_simpleDownload(aSource, aTarget, aOptions) {
+  startDirect: function (aSource, aTarget, aOptions) {
     return this.createDownload({
       source: aSource,
       target: aTarget,
@@ -182,10 +182,10 @@ this.Downloads = {
         let publicSummary = yield this.getSummary(Downloads.PUBLIC);
         let privateSummary = yield this.getSummary(Downloads.PRIVATE);
         let combinedSummary = yield this.getSummary(Downloads.ALL);
-        
+
-        publicSummary.bindToList(publicList);
+        yield publicSummary.bindToList(publicList);
-        privateSummary.bindToList(privateList);
+        yield privateSummary.bindToList(privateList);
-        combinedSummary.bindToList(combinedList);
+        yield combinedSummary.bindToList(combinedList);
 
         this._lists[Downloads.PUBLIC] = publicList;
         this._lists[Downloads.PRIVATE] = privateList;

toolkit/components/jsdownloads/test/unit/common_test_Download.js

@@ -1212,8 +1212,8 @@ add_task(function test_public_and_private()
   });
 
   let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
-  yield Downloads.simpleDownload(sourceUrl, targetFile);
+  yield Downloads.startDirect(sourceUrl, targetFile);
-  yield Downloads.simpleDownload(sourceUrl, targetFile);
+  yield Downloads.startDirect(sourceUrl, targetFile);
 
   if (!gUseLegacySaver) {
     let download = yield Downloads.createDownload({

toolkit/components/jsdownloads/test/unit/head.js

@@ -369,16 +369,17 @@ function promiseStartLegacyDownload(aSourceUrl, aOptions) {
     // are controlling becomes visible in the list of downloads.
     aList.addView({
       onDownloadAdded: function (aDownload) {
-        aList.removeView(this);
+        aList.removeView(this).then(null, do_report_unexpected_exception);
 
         // Remove the download to keep the list empty for the next test.  This
         // also allows the caller to register the "onchange" event directly.
-        aList.remove(aDownload);
+        let promise = aList.remove(aDownload);
 
         // When the download object is ready, make it available to the caller.
-        deferred.resolve(aDownload);
+        promise.then(() => deferred.resolve(aDownload),
+                     do_report_unexpected_exception);
       },
-    });
+    }).then(null, do_report_unexpected_exception);
 
     let isPrivate = aOptions && aOptions.isPrivate;
 
@@ -421,16 +422,17 @@ function promiseStartExternalHelperAppServiceDownload(aSourceUrl) {
     // are controlling becomes visible in the list of downloads.
     aList.addView({
       onDownloadAdded: function (aDownload) {
-        aList.removeView(this);
+        aList.removeView(this).then(null, do_report_unexpected_exception);
 
         // Remove the download to keep the list empty for the next test.  This
         // also allows the caller to register the "onchange" event directly.
-        aList.remove(aDownload);
+        let promise = aList.remove(aDownload);
 
         // When the download object is ready, make it available to the caller.
-        deferred.resolve(aDownload);
+        promise.then(() => deferred.resolve(aDownload),
+                     do_report_unexpected_exception);
       },
-    });
+    }).then(null, do_report_unexpected_exception);
 
     let channel = NetUtil.newChannel(sourceURI);
 

toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js

@@ -218,9 +218,9 @@ add_task(function test_notifications()
     download3.start();
 
     // Add downloads to list.
-    list.add(download1);
+    yield list.add(download1);
-    list.add(download2);
+    yield list.add(download2);
-    list.add(download3);
+    yield list.add(download3);
     // Cancel third download
     yield download3.cancel();
 
@@ -232,9 +232,9 @@ add_task(function test_notifications()
     yield promiseAttempt2;
 
     // Clean up.
-    list.remove(download1);
+    yield list.remove(download1);
-    list.remove(download2);
+    yield list.remove(download2);
-    list.remove(download3);
+    yield list.remove(download3);
   }
 });
 
@@ -254,8 +254,8 @@ add_task(function test_no_notifications()
     download2.start();
 
     // Add downloads to list.
-    list.add(download1);
+    yield list.add(download1);
-    list.add(download2);
+    yield list.add(download2);
 
     yield download1.cancel();
     yield download2.cancel();
@@ -263,8 +263,8 @@ add_task(function test_no_notifications()
     notifyPromptObservers(isPrivate, 0, 0);
 
     // Clean up.
-    list.remove(download1);
+    yield list.remove(download1);
-    list.remove(download2);
+    yield list.remove(download2);
   }
 });
 
@@ -285,8 +285,8 @@ add_task(function test_mix_notifications()
   let promiseAttempt2 = download2.start();
 
   // Add downloads to lists.
-  publicList.add(download1);
+  yield publicList.add(download1);
-  privateList.add(download2);
+  yield privateList.add(download2);
 
   notifyPromptObservers(true, 2, 1);
 
@@ -296,8 +296,8 @@ add_task(function test_mix_notifications()
   yield promiseAttempt2;
 
   // Clean up.
-  publicList.remove(download1);
+  yield publicList.remove(download1);
-  privateList.remove(download2);
+  yield privateList.remove(download2);
 });
 
 /**
@@ -316,8 +316,8 @@ add_task(function test_exit_private_browsing()
   let promiseAttempt2 = download2.start();
 
   // Add downloads to list.
-  privateList.add(download1);
+  yield privateList.add(download1);
-  privateList.add(download2);
+  yield privateList.add(download2);
 
   // Complete the download.
   yield promiseAttempt1;

toolkit/components/jsdownloads/test/unit/test_DownloadList.js

@@ -91,14 +91,14 @@ add_task(function test_add_getAll()
   let list = yield promiseNewList();
 
   let downloadOne = yield promiseNewDownload();
-  list.add(downloadOne);
+  yield list.add(downloadOne);
 
   let itemsOne = yield list.getAll();
   do_check_eq(itemsOne.length, 1);
   do_check_eq(itemsOne[0], downloadOne);
 
   let downloadTwo = yield promiseNewDownload();
-  list.add(downloadTwo);
+  yield list.add(downloadTwo);
 
   let itemsTwo = yield list.getAll();
   do_check_eq(itemsTwo.length, 2);
@@ -116,14 +116,14 @@ add_task(function test_remove()
 {
   let list = yield promiseNewList();
 
-  list.add(yield promiseNewDownload());
+  yield list.add(yield promiseNewDownload());
-  list.add(yield promiseNewDownload());
+  yield list.add(yield promiseNewDownload());
 
   let items = yield list.getAll();
-  list.remove(items[0]);
+  yield list.remove(items[0]);
 
   // Removing an item that was never added should not raise an error.
-  list.remove(yield promiseNewDownload());
+  yield list.remove(yield promiseNewDownload());
 
   items = yield list.getAll();
   do_check_eq(items.length, 1);
@@ -146,25 +146,25 @@ add_task(function test_DownloadCombinedList_add_remove_getAll()
     target: getTempFile(TEST_TARGET_FILE_NAME).path,
   });
 
-  publicList.add(publicDownload);
+  yield publicList.add(publicDownload);
-  privateList.add(privateDownload);
+  yield privateList.add(privateDownload);
 
   do_check_eq((yield combinedList.getAll()).length, 2);
 
-  combinedList.remove(publicDownload);
+  yield combinedList.remove(publicDownload);
-  combinedList.remove(privateDownload);
+  yield combinedList.remove(privateDownload);
 
   do_check_eq((yield combinedList.getAll()).length, 0);
 
-  combinedList.add(publicDownload);
+  yield combinedList.add(publicDownload);
-  combinedList.add(privateDownload);
+  yield combinedList.add(privateDownload);
 
   do_check_eq((yield publicList.getAll()).length, 1);
   do_check_eq((yield privateList.getAll()).length, 1);
   do_check_eq((yield combinedList.getAll()).length, 2);
 
-  publicList.remove(publicDownload);
+  yield publicList.remove(publicDownload);
-  privateList.remove(privateDownload);
+  yield privateList.remove(privateDownload);
 
   do_check_eq((yield combinedList.getAll()).length, 0);
 });
@@ -188,8 +188,8 @@ add_task(function test_notifications_add_remove()
       source: { url: httpUrl("source.txt"), isPrivate: true },
       target: getTempFile(TEST_TARGET_FILE_NAME).path,
     });
-    list.add(downloadOne);
+    yield list.add(downloadOne);
-    list.add(downloadTwo);
+    yield list.add(downloadTwo);
 
     // Check that we receive add notifications for existing elements.
     let addNotifications = 0;
@@ -204,11 +204,11 @@ add_task(function test_notifications_add_remove()
         addNotifications++;
       },
     };
-    list.addView(viewOne);
+    yield list.addView(viewOne);
     do_check_eq(addNotifications, 2);
 
     // Check that we receive add notifications for new elements.
-    list.add(yield promiseNewDownload());
+    yield list.add(yield promiseNewDownload());
     do_check_eq(addNotifications, 3);
 
     // Check that we receive remove notifications.
@@ -219,18 +219,18 @@ add_task(function test_notifications_add_remove()
         removeNotifications++;
       },
     };
-    list.addView(viewTwo);
+    yield list.addView(viewTwo);
-    list.remove(downloadOne);
+    yield list.remove(downloadOne);
     do_check_eq(removeNotifications, 1);
 
     // We should not receive remove notifications after the view is removed.
-    list.removeView(viewTwo);
+    yield list.removeView(viewTwo);
-    list.remove(downloadTwo);
+    yield list.remove(downloadTwo);
     do_check_eq(removeNotifications, 1);
 
     // We should not receive add notifications after the view is removed.
-    list.removeView(viewOne);
+    yield list.removeView(viewOne);
-    list.add(yield promiseNewDownload());
+    yield list.add(yield promiseNewDownload());
     do_check_eq(addNotifications, 3);
   }
 });
@@ -253,12 +253,12 @@ add_task(function test_notifications_change()
       source: { url: httpUrl("source.txt"), isPrivate: true },
       target: getTempFile(TEST_TARGET_FILE_NAME).path,
     });
-    list.add(downloadOne);
+    yield list.add(downloadOne);
-    list.add(downloadTwo);
+    yield list.add(downloadTwo);
 
     // Check that we receive change notifications.
     let receivedOnDownloadChanged = false;
-    list.addView({
+    yield list.addView({
       onDownloadChanged: function (aDownload) {
         do_check_eq(aDownload, downloadOne);
         receivedOnDownloadChanged = true;
@@ -269,7 +269,7 @@ add_task(function test_notifications_change()
 
     // We should not receive change notifications after a download is removed.
     receivedOnDownloadChanged = false;
-    list.remove(downloadTwo);
+    yield list.remove(downloadTwo);
     yield downloadTwo.start();
     do_check_false(receivedOnDownloadChanged);
   }
@@ -303,12 +303,12 @@ add_task(function test_notifications_this()
       receivedOnDownloadRemoved = true;
     },
   };
-  list.addView(view);
+  yield list.addView(view);
 
   let download = yield promiseNewDownload();
-  list.add(download);
+  yield list.add(download);
   yield download.start();
-  list.remove(download);
+  yield list.remove(download);
 
   // Verify that we executed the checks.
   do_check_true(receivedOnDownloadAdded);
@@ -344,7 +344,7 @@ add_task(function test_history_expiration()
       }
     },
   };
-  list.addView(downloadView);
+  yield list.addView(downloadView);
 
   // Work with one finished download and one canceled download.
   yield downloadOne.start();
@@ -358,8 +358,8 @@ add_task(function test_history_expiration()
   yield promiseExpirableDownloadVisit(httpUrl("interruptible.txt"));
 
   // After clearing history, we can add the downloads to be removed to the list.
-  list.add(downloadOne);
+  yield list.add(downloadOne);
-  list.add(downloadTwo);
+  yield list.add(downloadTwo);
 
   // Force a history expiration.
   let expire = Cc["@mozilla.org/places/expiration;1"]
@@ -380,8 +380,8 @@ add_task(function test_history_clear()
   let list = yield promiseNewList();
   let downloadOne = yield promiseNewDownload();
   let downloadTwo = yield promiseNewDownload();
-  list.add(downloadOne);
+  yield list.add(downloadOne);
-  list.add(downloadTwo);
+  yield list.add(downloadTwo);
 
   let deferred = Promise.defer();
   let removeNotifications = 0;
@@ -392,7 +392,7 @@ add_task(function test_history_clear()
       }
     },
   };
-  list.addView(downloadView);
+  yield list.addView(downloadView);
 
   yield downloadOne.start();
   yield downloadTwo.start();
@@ -414,10 +414,10 @@ add_task(function test_removeFinished()
   let downloadTwo = yield promiseNewDownload();
   let downloadThree = yield promiseNewDownload();
   let downloadFour = yield promiseNewDownload();
-  list.add(downloadOne);
+  yield list.add(downloadOne);
-  list.add(downloadTwo);
+  yield list.add(downloadTwo);
-  list.add(downloadThree);
+  yield list.add(downloadThree);
-  list.add(downloadFour);
+  yield list.add(downloadFour);
 
   let deferred = Promise.defer();
   let removeNotifications = 0;
@@ -432,7 +432,7 @@ add_task(function test_removeFinished()
       }
     },
   };
-  list.addView(downloadView);
+  yield list.addView(downloadView);
 
   // Start three of the downloads, but don't start downloadTwo, then set
   // downloadFour to have partial data. All downloads except downloadFour
@@ -467,7 +467,7 @@ add_task(function test_DownloadSummary()
   // Add a public download that has succeeded.
   let succeededPublicDownload = yield promiseNewDownload();
   yield succeededPublicDownload.start();
-  publicList.add(succeededPublicDownload);
+  yield publicList.add(succeededPublicDownload);
 
   // Add a public download that has been canceled midway.
   let canceledPublicDownload =
@@ -475,14 +475,14 @@ add_task(function test_DownloadSummary()
   canceledPublicDownload.start();
   yield promiseDownloadMidway(canceledPublicDownload);
   yield canceledPublicDownload.cancel();
-  publicList.add(canceledPublicDownload);
+  yield publicList.add(canceledPublicDownload);
 
   // Add a public download that is in progress.
   let inProgressPublicDownload =
       yield promiseNewDownload(httpUrl("interruptible.txt"));
   inProgressPublicDownload.start();
   yield promiseDownloadMidway(inProgressPublicDownload);
-  publicList.add(inProgressPublicDownload);
+  yield publicList.add(inProgressPublicDownload);
 
   // Add a private download that is in progress.
   let inProgressPrivateDownload = yield Downloads.createDownload({
@@ -491,7 +491,7 @@ add_task(function test_DownloadSummary()
   });
   inProgressPrivateDownload.start();
   yield promiseDownloadMidway(inProgressPrivateDownload);
-  privateList.add(inProgressPrivateDownload);
+  yield privateList.add(inProgressPrivateDownload);
 
   // Verify that the summary includes the total number of bytes and the
   // currently transferred bytes only for the downloads that are not stopped.
@@ -553,11 +553,11 @@ add_task(function test_DownloadSummary_notifications()
   let summary = yield Downloads.getSummary(Downloads.ALL);
 
   let download = yield promiseNewDownload();
-  list.add(download);
+  yield list.add(download);
 
   // Check that we receive change notifications.
   let receivedOnSummaryChanged = false;
-  summary.addView({
+  yield summary.addView({
     onSummaryChanged: function () {
       receivedOnSummaryChanged = true;
     },

toolkit/components/jsdownloads/test/unit/test_Downloads.js

@@ -61,40 +61,40 @@ add_task(function test_createDownload_public()
 });
 
 /**
- * Tests simpleDownload with nsIURI and nsIFile as arguments.
+ * Tests startDirect with nsIURI and nsIFile as arguments.
  */
-add_task(function test_simpleDownload_uri_file_arguments()
+add_task(function test_startDirect_uri_file_arguments()
 {
   let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
-  yield Downloads.simpleDownload(NetUtil.newURI(httpUrl("source.txt")),
+  yield Downloads.startDirect(NetUtil.newURI(httpUrl("source.txt")),
-                                 targetFile);
+                              targetFile);
   yield promiseVerifyContents(targetFile.path, TEST_DATA_SHORT);
 });
 
 /**
- * Tests simpleDownload with DownloadSource and DownloadTarget as arguments.
+ * Tests startDirect with DownloadSource and DownloadTarget as arguments.
  */
-add_task(function test_simpleDownload_object_arguments()
+add_task(function test_startDirect_object_arguments()
 {
   let targetPath = getTempFile(TEST_TARGET_FILE_NAME).path;
-  yield Downloads.simpleDownload({ url: httpUrl("source.txt") },
+  yield Downloads.startDirect({ url: httpUrl("source.txt") },
-                                 { path: targetPath });
+                              { path: targetPath });
   yield promiseVerifyContents(targetPath, TEST_DATA_SHORT);
 });
 
 /**
- * Tests simpleDownload with string arguments.
+ * Tests startDirect with string arguments.
  */
-add_task(function test_simpleDownload_string_arguments()
+add_task(function test_startDirect_string_arguments()
 {
   let targetPath = getTempFile(TEST_TARGET_FILE_NAME).path;
-  yield Downloads.simpleDownload(httpUrl("source.txt"),
+  yield Downloads.startDirect(httpUrl("source.txt"),
-                                 targetPath);
+                              targetPath);
   yield promiseVerifyContents(targetPath, TEST_DATA_SHORT);
 
   targetPath = getTempFile(TEST_TARGET_FILE_NAME).path;
-  yield Downloads.simpleDownload(new String(httpUrl("source.txt")),
+  yield Downloads.startDirect(new String(httpUrl("source.txt")),
-                                 new String(targetPath));
+                              new String(targetPath));
   yield promiseVerifyContents(targetPath, TEST_DATA_SHORT);
 });