Bug 835875 - Add the ability to cancel downloads. r=enn
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 19 Feb 2013 10:10:50 +0100
changeset 132182 9b8cdf7f47b0abf12a9fb8fa71e901e11cf2f0ee
parent 132181 a7a57be02f88b21de5d586a5fc059565996ced7e
child 132183 6bd304c9f9920c2ffe347befe2f4f8e93952c558
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersenn
bugs835875
milestone21.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 835875 - Add the ability to cancel downloads. r=enn
toolkit/components/jsdownloads/src/DownloadCore.jsm
toolkit/components/jsdownloads/test/unit/head.js
toolkit/components/jsdownloads/test/unit/test_DownloadCore.js
--- a/toolkit/components/jsdownloads/src/DownloadCore.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm
@@ -66,17 +66,17 @@ const BackgroundFileSaverStreamListener 
 
 /**
  * Represents a single download, with associated state and actions.  This object
  * is transient, though it can be included in a DownloadList so that it can be
  * managed by the user interface and persisted across sessions.
  */
 function Download()
 {
-  this._deferDone = Promise.defer();
+  this._deferStopped = Promise.defer();
 }
 
 Download.prototype = {
   /**
    * DownloadSource object associated with this download.
    */
   source: null,
 
@@ -88,20 +88,25 @@ Download.prototype = {
   /**
    * DownloadSaver object associated with this download.
    */
   saver: null,
 
   /**
    * Becomes true when the download has been completed successfully, failed, or
    * has been canceled.  This property can become true, then it can be reset to
-   * false when a failed or canceled download is resumed.  This property remains
-   * false while the download is paused.
+   * false when a failed or canceled download is restarted.
    */
-  done: false,
+  stopped: false,
+
+  /**
+   * Indicates that the download has been canceled.  This property can become
+   * true, then it can be reset to false when a canceled download is restarted.
+   */
+  canceled: false,
 
   /**
    * When the download fails, this is set to a DownloadError instance indicating
    * the cause of the failure.  If the download has been completed successfully
    * or has been canceled, this property is null.
    */
   error: null,
 
@@ -136,17 +141,17 @@ Download.prototype = {
    *
    * @note You shouldn't rely on this property being equal to totalBytes to
    *       determine whether the download is completed.  You should use the
    *       individual state properties instead.
    */
   currentBytes: 0,
 
   /**
-   * This can be set to a function that is called when other properties change.
+   * This can be set to a function that is called after other properties change.
    */
   onchange: null,
 
   /**
    * Raises the onchange notification.
    */
   _notifyChange: function D_notifyChange() {
     try {
@@ -157,52 +162,57 @@ Download.prototype = {
       Cu.reportError(ex);
     }
   },
 
   /**
    * This deferred object is resolved when this download finishes successfully,
    * and rejected if this download fails.
    */
-  _deferDone: null,
+  _deferStopped: null,
 
   /**
    * Starts the download.
    *
    * @return {Promise}
    * @resolves When the download has finished successfully.
    * @rejects JavaScript exception if the download failed.
    */
   start: function D_start()
   {
-    this._deferDone.resolve(Task.spawn(function task_D_start() {
+    this._deferStopped.resolve(Task.spawn(function task_D_start() {
       try {
         yield this.saver.execute();
         this.progress = 100;
       } catch (ex) {
+        if (this.canceled) {
+          throw new DownloadError(Cr.NS_ERROR_FAILURE, "Download canceled.");
+        }
         this.error = ex;
         throw ex;
       } finally {
-        this.done = true;
+        this.stopped = true;
         this._notifyChange();
       }
     }.bind(this)));
 
-    return this.whenDone();
+    return this._deferStopped.promise;
   },
 
   /**
-   * Waits for the download to finish.
-   *
-   * @return {Promise}
-   * @resolves When the download has finished successfully.
-   * @rejects JavaScript exception if the download failed.
+   * Cancels the download.
    */
-  whenDone: function D_whenDone() {
-    return this._deferDone.promise;
+  cancel: function D_cancel()
+  {
+    if (this.stopped || this.canceled) {
+      return;
+    }
+
+    this.canceled = true;
+    this.saver.cancel();
   },
 
   /**
    * Updates progress notifications based on the number of bytes transferred.
    *
    * @param aCurrentBytes
    *        Number of bytes transferred until now.
    * @param aTotalBytes
@@ -329,31 +339,44 @@ DownloadSaver.prototype = {
    *
    * @return {Promise}
    * @resolves When the download has finished successfully.
    * @rejects JavaScript exception if the download failed.
    */
   execute: function DS_execute()
   {
     throw new Error("Not implemented.");
-  }
+  },
+
+  /**
+   * Cancels the download.
+   */
+  cancel: function DS_cancel()
+  {
+    throw new Error("Not implemented.");
+  },
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 //// DownloadCopySaver
 
 /**
  * Saver object that simply copies the entire source file to the target.
  */
 function DownloadCopySaver() { }
 
 DownloadCopySaver.prototype = {
   __proto__: DownloadSaver.prototype,
 
   /**
+   * BackgroundFileSaver object currently handling the download.
+   */
+  _backgroundFileSaver: null,
+
+  /**
    * Implements "DownloadSaver.execute".
    */
   execute: function DCS_execute()
   {
     let deferred = Promise.defer();
     let download = this.download;
 
     // Create the object that will save the file in a background thread.
@@ -419,17 +442,31 @@ DownloadCopySaver.prototype = {
         onDataAvailable: function DCSE_onDataAvailable(aRequest, aContext,
                                                        aInputStream, aOffset,
                                                        aCount)
         {
           backgroundFileSaver.onDataAvailable(aRequest, aContext, aInputStream,
                                               aOffset, aCount);
         },
       }, null);
+
+      // If the operation succeeded, store the object to allow cancellation.
+      this._backgroundFileSaver = backgroundFileSaver;
     } catch (ex) {
       // In case an error occurs while setting up the chain of objects for the
       // download, ensure that we release the resources of the background saver.
       deferred.reject(ex);
       backgroundFileSaver.finish(Cr.NS_ERROR_FAILURE);
     }
     return deferred.promise;
   },
+
+  /**
+   * Implements "DownloadSaver.cancel".
+   */
+  cancel: function DCS_cancel()
+  {
+    if (this._backgroundFileSaver) {
+      this._backgroundFileSaver.finish(Cr.NS_ERROR_FAILURE);
+      this._backgroundFileSaver = null;
+    }
+  },
 };
--- a/toolkit/components/jsdownloads/test/unit/head.js
+++ b/toolkit/components/jsdownloads/test/unit/head.js
@@ -43,16 +43,20 @@ const HTTP_SERVER_PORT = 4444;
 const HTTP_BASE = "http://localhost:" + HTTP_SERVER_PORT;
 
 const FAKE_SERVER_PORT = 4445;
 const FAKE_BASE = "http://localhost:" + FAKE_SERVER_PORT;
 
 const TEST_SOURCE_URI = NetUtil.newURI(HTTP_BASE + "/source.txt");
 const TEST_FAKE_SOURCE_URI = NetUtil.newURI(FAKE_BASE + "/source.txt");
 
+const TEST_INTERRUPTIBLE_PATH = "/interruptible.txt";
+const TEST_INTERRUPTIBLE_URI = NetUtil.newURI(HTTP_BASE +
+                                              TEST_INTERRUPTIBLE_PATH);
+
 const TEST_TARGET_FILE_NAME = "test-download.txt";
 const TEST_DATA_SHORT = "This test string is downloaded.";
 
 /**
  * All the tests are implemented with add_task, this starts them automatically.
  */
 function run_test()
 {
@@ -130,16 +134,58 @@ function startFakeServer()
     onSocketAccepted: function (aServ, aTransport) {
       aTransport.close(Cr.NS_BINDING_ABORTED);
     },
     onStopListening: function () { },
   });
   return serverSocket;
 }
 
+/**
+ * This function allows testing events or actions that need to happen in the
+ * middle of a download.
+ *
+ * Calling this function registers a new request handler in the internal HTTP
+ * server, accessible at the TEST_INTERRUPTIBLE_URI address.  The HTTP handler
+ * returns the TEST_DATA_SHORT text, then waits until the "resolve" method is
+ * called on the object returned by the function.  At this point, the handler
+ * sends the TEST_DATA_SHORT text again to complete the response.
+ *
+ * You can also call the "reject" method on the returned object to interrupt the
+ * response midway.  Because of how the network layer is implemented, this does
+ * not cause the socket to return an error.
+ *
+ * The handler is unregistered when the response finishes or is interrupted.
+ *
+ * @returns Deferred object used to control the response.
+ */
+function startInterruptibleResponseHandler()
+{
+  let deferResponse = Promise.defer();
+  gHttpServer.registerPathHandler(TEST_INTERRUPTIBLE_PATH,
+    function (aRequest, aResponse)
+    {
+      aResponse.processAsync();
+      aResponse.setHeader("Content-Type", "text/plain", false);
+      aResponse.setHeader("Content-Length", "" + (TEST_DATA_SHORT.length * 2),
+                          false);
+      aResponse.write(TEST_DATA_SHORT);
+
+      deferResponse.promise.then(function SIRH_onSuccess() {
+        aResponse.write(TEST_DATA_SHORT);
+        aResponse.finish();
+        gHttpServer.registerPathHandler(TEST_INTERRUPTIBLE_PATH, null);
+      }, function SIRH_onFailure() {
+        aResponse.abort();
+        gHttpServer.registerPathHandler(TEST_INTERRUPTIBLE_PATH, null);
+      });
+    });
+  return deferResponse;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Initialization functions common to all tests
 
 let gHttpServer;
 
 add_task(function test_common_initialize()
 {
   // Start the HTTP server.
--- a/toolkit/components/jsdownloads/test/unit/test_DownloadCore.js
+++ b/toolkit/components/jsdownloads/test/unit/test_DownloadCore.js
@@ -7,19 +7,30 @@
  * Tests the objects defined in the "DownloadCore" module.
  */
 
 "use strict";
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Globals
 
-function promiseSimpleDownload() {
+/**
+ * Creates a new Download object, using TEST_TARGET_FILE_NAME as the target.
+ * The target is deleted by getTempFile when this function is called.
+ *
+ * @param aSourceURI
+ *        The nsIURI for the download source, or null to use TEST_SOURCE_URI.
+ *
+ * @return {Promise}
+ * @resolves The newly created Download object.
+ * @rejects JavaScript exception.
+ */
+function promiseSimpleDownload(aSourceURI) {
   return Downloads.createDownload({
-    source: { uri: TEST_SOURCE_URI },
+    source: { uri: aSourceURI || TEST_SOURCE_URI },
     target: { file: getTempFile(TEST_TARGET_FILE_NAME) },
     saver: { type: "copy" },
   });
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Tests
 
@@ -31,169 +42,207 @@ add_task(function test_download_construc
   let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
 
   let download = yield Downloads.createDownload({
     source: { uri: TEST_SOURCE_URI },
     target: { file: targetFile },
     saver: { type: "copy" },
   });
 
+  // Checks the generated DownloadSource and DownloadTarget properties.
+  do_check_true(download.source.uri.equals(TEST_SOURCE_URI));
+  do_check_eq(download.target.file, targetFile);
+
   // Starts the download and waits for completion.
   yield download.start();
 
   yield promiseVerifyContents(targetFile, TEST_DATA_SHORT);
 });
 
 /**
  * Checks initial and final state and progress for a successful download.
  */
 add_task(function test_download_initial_final_state()
 {
   let download = yield promiseSimpleDownload();
 
-  do_check_false(download.done);
+  do_check_false(download.stopped);
   do_check_eq(download.progress, 0);
 
   // Starts the download and waits for completion.
   yield download.start();
 
-  do_check_true(download.done);
+  do_check_true(download.stopped);
   do_check_eq(download.progress, 100);
 });
 
 /**
  * Checks that the view is notified of the final download state.
  */
 add_task(function test_download_view_final_notified()
 {
   let download = yield promiseSimpleDownload();
 
   let onchangeNotified = false;
-  let lastNotifiedDone;
+  let lastNotifiedStopped;
   let lastNotifiedProgress;
   download.onchange = function () {
     onchangeNotified = true;
-    lastNotifiedDone = download.done;
+    lastNotifiedStopped = download.stopped;
     lastNotifiedProgress = download.progress;
   };
 
   // Starts the download and waits for completion.
   yield download.start();
 
   // The view should have been notified before the download completes.
   do_check_true(onchangeNotified);
-  do_check_true(lastNotifiedDone);
+  do_check_true(lastNotifiedStopped);
   do_check_eq(lastNotifiedProgress, 100);
 });
 
 /**
  * Checks intermediate progress for a successful download.
  */
 add_task(function test_download_intermediate_progress()
 {
-  let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
-
-  let deferUntilHalfProgress = Promise.defer();
-  gHttpServer.registerPathHandler("/test_download_intermediate_progress",
-    function (aRequest, aResponse)
-    {
-      aResponse.processAsync();
-      aResponse.setHeader("Content-Type", "text/plain", false);
-      aResponse.setHeader("Content-Length", "" + (TEST_DATA_SHORT.length * 2),
-                          false);
-      aResponse.write(TEST_DATA_SHORT);
+  let interruptible = startInterruptibleResponseHandler();
 
-      deferUntilHalfProgress.promise.then(function () {
-        aResponse.write(TEST_DATA_SHORT);
-        aResponse.finish();
-      });
-    });
-
-  let download = yield Downloads.createDownload({
-    source: { uri: NetUtil.newURI(HTTP_BASE +
-                                  "/test_download_intermediate_progress") },
-    target: { file: targetFile },
-    saver: { type: "copy" },
-  });
+  let download = yield promiseSimpleDownload(TEST_INTERRUPTIBLE_URI);
 
   download.onchange = function () {
     if (download.progress == 50) {
       do_check_true(download.hasProgress);
       do_check_eq(download.currentBytes, TEST_DATA_SHORT.length);
       do_check_eq(download.totalBytes, TEST_DATA_SHORT.length * 2);
 
       // Continue after the first chunk of data is fully received.
-      deferUntilHalfProgress.resolve();
+      interruptible.resolve();
     }
   };
 
   // Starts the download and waits for completion.
   yield download.start();
 
-  do_check_true(download.done);
+  do_check_true(download.stopped);
   do_check_eq(download.progress, 100);
 
-  yield promiseVerifyContents(targetFile, TEST_DATA_SHORT + TEST_DATA_SHORT);
+  yield promiseVerifyContents(download.target.file,
+                              TEST_DATA_SHORT + TEST_DATA_SHORT);
+});
+
+/**
+ * Cancels a download and verifies that its state is reported correctly.
+ */
+add_task(function test_download_cancel()
+{
+  let interruptible = startInterruptibleResponseHandler();
+  try {
+    let download = yield promiseSimpleDownload(TEST_INTERRUPTIBLE_URI);
+
+    // Cancel the download after receiving the first part of the response.
+    download.onchange = function () {
+      if (!download.stopped && download.progress == 50) {
+        download.cancel();
+      }
+    };
+
+    try {
+      yield download.start();
+      do_throw("The download should have been canceled.");
+    } catch (ex if ex instanceof Downloads.Error) {
+      do_check_false(ex.becauseSourceFailed);
+      do_check_false(ex.becauseTargetFailed);
+    }
+
+    do_check_true(download.stopped);
+    do_check_true(download.canceled);
+    do_check_true(download.error === null);
+
+    do_check_false(download.target.file.exists());
+  } finally {
+    interruptible.reject();
+  }
+});
+
+/**
+ * Cancels a download right after starting it.
+ */
+add_task(function test_download_cancel_immediately()
+{
+  let interruptible = startInterruptibleResponseHandler();
+  try {
+    let download = yield promiseSimpleDownload(TEST_INTERRUPTIBLE_URI);
+
+    let promiseStopped = download.start();
+    download.cancel();
+
+    try {
+      yield promiseStopped;
+      do_throw("The download should have been canceled.");
+    } catch (ex if ex instanceof Downloads.Error) {
+      do_check_false(ex.becauseSourceFailed);
+      do_check_false(ex.becauseTargetFailed);
+    }
+
+    do_check_true(download.stopped);
+    do_check_true(download.canceled);
+    do_check_true(download.error === null);
+
+    do_check_false(download.target.file.exists());
+  } finally {
+    interruptible.reject();
+  }
 });
 
 /**
  * Ensures download error details are reported on network failures.
  */
 add_task(function test_download_error_source()
 {
-  let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
-
   let serverSocket = startFakeServer();
   try {
-    let download = yield Downloads.createDownload({
-      source: { uri: TEST_FAKE_SOURCE_URI },
-      target: { file: targetFile },
-      saver: { type: "copy" },
-    });
+    let download = yield promiseSimpleDownload(TEST_FAKE_SOURCE_URI);
 
     do_check_true(download.error === null);
 
     try {
       yield download.start();
       do_throw("The download should have failed.");
     } catch (ex if ex instanceof Downloads.Error && ex.becauseSourceFailed) {
       // A specific error object is thrown when reading from the source fails.
     }
 
-    do_check_true(download.done);
+    do_check_true(download.stopped);
+    do_check_false(download.canceled);
     do_check_true(download.error !== null);
     do_check_true(download.error.becauseSourceFailed);
     do_check_false(download.error.becauseTargetFailed);
   } finally {
     serverSocket.close();
   }
 });
 
 /**
  * Ensures download error details are reported on local writing failures.
  */
 add_task(function test_download_error_target()
 {
-  let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
-
-  // Create a file without write access permissions.
-  targetFile.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0);
-
-  let download = yield Downloads.createDownload({
-    source: { uri: TEST_SOURCE_URI },
-    target: { file: targetFile },
-    saver: { type: "copy" },
-  });
+  let download = yield promiseSimpleDownload();
 
   do_check_true(download.error === null);
 
+  // Create a file without write access permissions before downloading.
+  download.target.file.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0);
+
   try {
     yield download.start();
     do_throw("The download should have failed.");
   } catch (ex if ex instanceof Downloads.Error && ex.becauseTargetFailed) {
     // A specific error object is thrown when writing to the target fails.
   }
 
-  do_check_true(download.done);
+  do_check_true(download.stopped);
+  do_check_false(download.canceled);
   do_check_true(download.error !== null);
   do_check_true(download.error.becauseTargetFailed);
   do_check_false(download.error.becauseSourceFailed);
 });