Bug 847191 - Integration with legacy interfaces to start new downloads. r=enn

browser/installer/package-manifest.in

@@ -355,6 +355,10 @@
 @BINPATH@/browser/components/DownloadsStartup.js
 @BINPATH@/browser/components/DownloadsUI.js
 @BINPATH@/browser/components/BrowserPlaces.manifest
+#ifdef MOZ_JSDOWNLOADS
+@BINPATH@/components/Downloads.manifest
+@BINPATH@/components/DownloadLegacy.js
+#endif
 @BINPATH@/components/BrowserPageThumbs.manifest
 @BINPATH@/components/SiteSpecificUserAgent.js
 @BINPATH@/components/SiteSpecificUserAgent.manifest

toolkit/components/build/nsToolkitCompsModule.cpp

@@ -111,7 +111,9 @@ static const mozilla::Module::CIDEntry kToolkitCIDs[] = {
   { &kNS_PARENTALCONTROLSSERVICE_CID, false, NULL, nsParentalControlsServiceWinConstructor },
 #endif
   { &kNS_DOWNLOADMANAGER_CID, false, NULL, nsDownloadManagerConstructor },
+#ifndef MOZ_JSDOWNLOADS
   { &kNS_DOWNLOAD_CID, false, NULL, nsDownloadProxyConstructor },
+#endif
   { &kNS_FIND_SERVICE_CID, false, NULL, nsFindServiceConstructor },
   { &kNS_TYPEAHEADFIND_CID, false, NULL, nsTypeAheadFindConstructor },
 #ifdef MOZ_URL_CLASSIFIER
@@ -136,7 +138,9 @@ static const mozilla::Module::ContractIDEntry kToolkitContracts[] = {
   { NS_PARENTALCONTROLSSERVICE_CONTRACTID, &kNS_PARENTALCONTROLSSERVICE_CID },
 #endif
   { NS_DOWNLOADMANAGER_CONTRACTID, &kNS_DOWNLOADMANAGER_CID },
+#ifndef MOZ_JSDOWNLOADS
   { NS_TRANSFER_CONTRACTID, &kNS_DOWNLOAD_CID },
+#endif
   { NS_FIND_SERVICE_CONTRACTID, &kNS_FIND_SERVICE_CID },
   { NS_TYPEAHEADFIND_CONTRACTID, &kNS_TYPEAHEADFIND_CID },
 #ifdef MOZ_URL_CLASSIFIER

toolkit/components/jsdownloads/src/DownloadCore.jsm

@@ -27,6 +27,9 @@
  *
  * DownloadCopySaver
  * Saver object that simply copies the entire source file to the target.
+ *
+ * DownloadLegacySaver
+ * Saver object that integrates with the legacy nsITransfer interface.
  */
 
 "use strict";
@@ -38,6 +41,7 @@ this.EXPORTED_SYMBOLS = [
   "DownloadError",
   "DownloadSaver",
   "DownloadCopySaver",
+  "DownloadLegacySaver",
 ];
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -52,6 +56,8 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "OS",
+                                  "resource://gre/modules/osfile.jsm")
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
                                   "resource://gre/modules/commonjs/sdk/core/promise.js");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
@@ -654,3 +660,147 @@ DownloadCopySaver.prototype = {
     }
   },
 };
+
+////////////////////////////////////////////////////////////////////////////////
+//// DownloadLegacySaver
+
+/**
+ * Saver object that integrates with the legacy nsITransfer interface.
+ *
+ * For more background on the process, see the DownloadLegacyTransfer object.
+ */
+function DownloadLegacySaver()
+{
+  this.deferExecuted = Promise.defer();
+  this.deferCanceled = Promise.defer();
+}
+
+DownloadLegacySaver.prototype = {
+  __proto__: DownloadSaver.prototype,
+
+  /**
+   * nsIRequest object associated to the status and progress updates we
+   * received.  This object is null before we receive the first status and
+   * progress update, and is also reset to null when the download is stopped.
+   */
+  request: null,
+
+  /**
+   * This deferred object contains a promise that is resolved as soon as this
+   * download finishes successfully, and is rejected in case the download is
+   * canceled or receives a failure notification through nsITransfer.
+   */
+  deferExecuted: null,
+
+  /**
+   * This deferred object contains a promise that is resolved if the download
+   * receives a cancellation request through the "cancel" method, and is never
+   * rejected.  The nsITransfer implementation will register a handler that
+   * actually causes the download cancellation.
+   */
+  deferCanceled: null,
+
+  /**
+   * This is populated with the value of the aSetProgressBytesFn argument of the
+   * "execute" method, and is null before the method is called.
+   */
+  setProgressBytesFn: null,
+
+  /**
+   * Called by the nsITransfer implementation while the download progresses.
+   *
+   * @param aCurrentBytes
+   *        Number of bytes transferred until now.
+   * @param aTotalBytes
+   *        Total number of bytes to be transferred, or -1 if unknown.
+   */
+  onProgressBytes: function DLS_onProgressBytes(aCurrentBytes, aTotalBytes)
+  {
+    // Ignore progress notifications until we are ready to process them.
+    if (!this.setProgressBytesFn) {
+      return;
+    }
+
+    this.progressWasNotified = true;
+    this.setProgressBytesFn(aCurrentBytes, aTotalBytes);
+  },
+
+  /**
+   * Whether the onProgressBytes function has been called at least once.
+   */
+  progressWasNotified: false,
+
+  /**
+   * Called by the nsITransfer implementation when the request has finished.
+   *
+   * @param aRequest
+   *        nsIRequest associated to the status update.
+   * @param aStatus
+   *        Status code received by the nsITransfer implementation.
+   */
+  onTransferFinished: function DLS_onTransferFinished(aRequest, aStatus)
+  {
+    // Store a reference to the request, used when handling completion.
+    this.request = aRequest;
+
+    if (Components.isSuccessCode(aStatus)) {
+      this.deferExecuted.resolve();
+    } else {
+      // Infer the origin of the error from the failure code, because more
+      // specific data is not available through the nsITransfer implementation.
+      this.deferExecuted.reject(new DownloadError(aStatus, null, true));
+    }
+  },
+
+  /**
+   * Implements "DownloadSaver.execute".
+   */
+  execute: function DLS_execute(aSetProgressBytesFn)
+  {
+    this.setProgressBytesFn = aSetProgressBytesFn;
+
+    return Task.spawn(function task_DLS_execute() {
+      try {
+        // Wait for the component that executes the download to finish.
+        yield this.deferExecuted.promise;
+
+        // At this point, the "request" property has been populated.  Ensure we
+        // report the value of "Content-Length", if available, even if the
+        // download didn't generate any progress events.
+        if (!this.progressWasNotified &&
+            this.request instanceof Ci.nsIChannel &&
+            this.request.contentLength >= 0) {
+          aSetProgressBytesFn(0, this.request.contentLength);
+        }
+
+        // The download implementation may not have created the target file if
+        // no data was received from the source.  In this case, ensure that an
+        // empty file is created as expected.
+        try {
+          // This atomic operation is more efficient than an existence check.
+          let file = yield OS.File.open(this.download.target.file.path,
+                                        { create: true });
+          yield file.close();
+        } catch (ex if ex instanceof OS.File.Error && ex.becauseExists) { }
+      } finally {
+        // We don't need the reference to the request anymore.
+        this.request = null;
+      }
+    }.bind(this));
+  },
+
+  /**
+   * Implements "DownloadSaver.cancel".
+   */
+  cancel: function DLS_cancel()
+  {
+    // Synchronously cancel the operation as soon as the object is connected.
+    this.deferCanceled.resolve();
+
+    // We don't necessarily receive status notifications after we call "cancel",
+    // but cancellation through nsICancelable should be synchronous, thus force
+    // the rejection of the execution promise immediately.
+    this.deferExecuted.reject(new DownloadError(Cr.NS_ERROR_FAILURE,
+                                                "Download canceled."));
+  },
+};

toolkit/components/jsdownloads/src/DownloadLegacy.js

@@ -0,0 +1,192 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * This component implements the XPCOM interfaces required for integration with
+ * the legacy download components.
+ *
+ * New code is expected to use the "Downloads.jsm" module directly, without
+ * going through the interfaces implemented in this XPCOM component.  These
+ * interfaces are only maintained for backwards compatibility with components
+ * that still work synchronously on the main thread.
+ */
+
+"use strict";
+
+////////////////////////////////////////////////////////////////////////////////
+//// Globals
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cu = Components.utils;
+const Cr = Components.results;
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "Downloads",
+                                  "resource://gre/modules/Downloads.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Promise",
+                                  "resource://gre/modules/commonjs/sdk/core/promise.js");
+
+////////////////////////////////////////////////////////////////////////////////
+//// DownloadLegacyTransfer
+
+/**
+ * nsITransfer implementation that provides a bridge to a Download object.
+ *
+ * Legacy downloads work differently than the JavaScript implementation.  In the
+ * latter, the caller only provides the properties for the Download object and
+ * the entire process is handled by the "start" method.  In the legacy
+ * implementation, the caller must create a separate object to execute the
+ * download, and then make the download visible to the user by hooking it up to
+ * an nsITransfer instance.
+ *
+ * Since nsITransfer instances may be created before the download system is
+ * initialized, and initialization as well as other operations are asynchronous,
+ * this implementation is able to delay all progress and status notifications it
+ * receives until the associated Download object is finally created.
+ *
+ * Conversely, the DownloadLegacySaver object can also receive execution and
+ * cancellation requests asynchronously, before or after it is connected to
+ * this nsITransfer instance.  For that reason, those requests are communicated
+ * in a potentially deferred way, using promise objects.
+ *
+ * The component that executes the download implements nsICancelable to receive
+ * cancellation requests, but after cancellation it cannot be reused again.
+ *
+ * Since the components that execute the download may be different and they
+ * don't always give consistent results, this bridge takes care of enforcing the
+ * expectations, for example by ensuring the target file exists when the
+ * download is successful, even if the source has a size of zero bytes.
+ */
+function DownloadLegacyTransfer()
+{
+  this._deferDownload = Promise.defer();
+}
+
+DownloadLegacyTransfer.prototype = {
+  classID: Components.ID("{1b4c85df-cbdd-4bb6-b04e-613caece083c}"),
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// nsISupports
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIWebProgressListener,
+                                         Ci.nsIWebProgressListener2,
+                                         Ci.nsITransfer]),
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// nsIWebProgressListener
+
+  onStateChange: function DLT_onStateChange(aWebProgress, aRequest, aStateFlags,
+                                            aStatus)
+  {
+    // Detect when the last file has been received, or the download failed.
+    if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
+        (aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK)) {
+      // Wait for the associated Download object to be available.
+      this._deferDownload.promise.then(function DLT_OSC_onDownload(aDownload) {
+        aDownload.saver.onTransferFinished(aRequest, aStatus);
+      }).then(null, Cu.reportError);
+    }
+  },
+
+  onProgressChange: function DLT_onProgressChange(aWebProgress, aRequest,
+                                                  aCurSelfProgress,
+                                                  aMaxSelfProgress,
+                                                  aCurTotalProgress,
+                                                  aMaxTotalProgress)
+  {
+    return onProgressChange64(aWebProgress, aRequest, aCurSelfProgress,
+                              aMaxSelfProgress, aCurTotalProgress,
+                              aMaxTotalProgress);
+  },
+
+  onLocationChange: function () { },
+
+  onStatusChange: function DLT_onStatusChange(aWebProgress, aRequest, aStatus,
+                                              aMessage)
+  {
+    // The status change may optionally be received in addition to the state
+    // change, but if no network request actually started, it is possible that
+    // we only receive a status change with an error status code.
+    if (!Components.isSuccessCode(aStatus)) {
+      // Wait for the associated Download object to be available.
+      this._deferDownload.promise.then(function DLT_OSC_onDownload(aDownload) {
+        aDownload.saver.onTransferFinished(aRequest, aStatus);
+      }).then(null, Cu.reportError);
+    }
+  },
+
+  onSecurityChange: function () { },
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// nsIWebProgressListener2
+
+  onProgressChange64: function DLT_onProgressChange64(aWebProgress, aRequest,
+                                                      aCurSelfProgress,
+                                                      aMaxSelfProgress,
+                                                      aCurTotalProgress,
+                                                      aMaxTotalProgress)
+  {
+    // Wait for the associated Download object to be available.
+    this._deferDownload.promise.then(function DLT_OPC64_onDownload(aDownload) {
+      aDownload.saver.onProgressBytes(aCurTotalProgress, aMaxTotalProgress);
+    }).then(null, Cu.reportError);
+  },
+
+  onRefreshAttempted: function DLT_onRefreshAttempted(aWebProgress, aRefreshURI,
+                                                      aMillis, aSameURI)
+  {
+    // Indicate that refreshes and redirects are allowed by default.  However,
+    // note that download components don't usually call this method at all.
+    return true;
+  },
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// nsITransfer
+
+  init: function DLT_init(aSource, aTarget, aDisplayName, aMIMEInfo, aStartTime,
+                          aTempFile, aCancelable, aIsPrivate)
+  {
+    // Create a new Download object associated to a DownloadLegacySaver, and
+    // wait for it to be available.  This operation may cause the entire
+    // download system to initialize before the object is created.
+    Downloads.createDownload({
+      source: { uri: aSource },
+      target: { file: aTarget.QueryInterface(Ci.nsIFileURL).file },
+      saver: { type: "legacy" },
+    }).then(function DLT_I_onDownload(aDownload) {
+      // Now that the saver is available, hook up the cancellation handler.
+      aDownload.saver.deferCanceled.promise
+                     .then(function () aCancelable.cancel(Cr.NS_ERROR_ABORT))
+                     .then(null, Cu.reportError);
+
+      // Start the download before allowing it to be controlled.
+      aDownload.start();
+
+      // Start processing all the other events received through nsITransfer.
+      this._deferDownload.resolve(aDownload);
+
+      // Add the download to the list, allowing it to be seen and canceled.
+      return Downloads.getPublicDownloadList()
+                      .then(function (aList) aList.add(aDownload));
+    }.bind(this)).then(null, Cu.reportError);
+  },
+
+  //////////////////////////////////////////////////////////////////////////////
+  //// Private methods and properties
+
+  /**
+   * This deferred object contains a promise that is resolved with the Download
+   * object associated with this nsITransfer instance, when it is available.
+   */
+  _deferDownload: null,
+};
+
+////////////////////////////////////////////////////////////////////////////////
+//// Module
+
+this.NSGetFactory = XPCOMUtils.generateNSGetFactory([DownloadLegacyTransfer]);

toolkit/components/jsdownloads/src/Downloads.jsm

@@ -81,7 +81,9 @@ this.Downloads = {
       download.target.file = aProperties.target.file;
 
       // Support for different aProperties.saver values isn't implemented yet.
-      download.saver = new DownloadCopySaver();
+      download.saver = aProperties.saver.type == "legacy"
+                       ? new DownloadLegacySaver()
+                       : new DownloadCopySaver();
       download.saver.download = download;
 
       // This explicitly makes this function a generator for Task.jsm, so that

toolkit/components/jsdownloads/src/Downloads.manifest

@@ -0,0 +1,2 @@
+component {1b4c85df-cbdd-4bb6-b04e-613caece083c} DownloadLegacy.js
+contract @mozilla.org/transfer;1 {1b4c85df-cbdd-4bb6-b04e-613caece083c}

toolkit/components/jsdownloads/src/Makefile.in

@@ -9,6 +9,11 @@ VPATH     = @srcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
+EXTRA_COMPONENTS = \
+  Downloads.manifest \
+  DownloadLegacy.js \
+  $(NULL)
+
 EXTRA_JS_MODULES = \
   Downloads.jsm \
   DownloadCore.jsm \

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

@@ -0,0 +1,280 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests the integration with legacy interfaces for downloads.
+ */
+
+"use strict";
+
+////////////////////////////////////////////////////////////////////////////////
+//// Globals
+
+/**
+ * Starts a new download using the nsIWebBrowserPersist interface, and controls
+ * it using the legacy nsITransfer interface.
+ *
+ * @param aSourceURI
+ *        The nsIURI for the download source, or null to use TEST_SOURCE_URI.
+ * @param aOutPersist
+ *        Optional object that receives a reference to the created
+ *        nsIWebBrowserPersist instance in the "value" property.
+ *
+ * @return {Promise}
+ * @resolves The Download object created as a consequence of controlling the
+ *           download through the legacy nsITransfer interface.
+ * @rejects Never.  The current test fails in case of exceptions.
+ */
+function promiseStartLegacyDownload(aSourceURI, aOutPersist) {
+  let sourceURI = aSourceURI || TEST_SOURCE_URI;
+  let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
+
+  let persist = Cc["@mozilla.org/embedding/browser/nsWebBrowserPersist;1"]
+                  .createInstance(Ci.nsIWebBrowserPersist);
+  let transfer = Cc["@mozilla.org/transfer;1"].createInstance(Ci.nsITransfer);
+
+  if (aOutPersist) {
+    aOutPersist.value = persist;
+  }
+
+  let deferred = Promise.defer();
+
+  Downloads.getPublicDownloadList().then(function (aList) {
+    // Temporarily register a view that will get notified when the download we
+    // are controlling becomes visible in the list of public downloads.
+    aList.addView({
+      onDownloadAdded: function (aDownload) {
+        aList.removeView(this);
+
+        // 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);
+
+        // When the download object is ready, make it available to the caller.
+        deferred.resolve(aDownload);
+      },
+    });
+
+    // Initialize the components so they reference each other.  This will cause
+    // the Download object to be created and added to the public downloads.
+    transfer.init(sourceURI, NetUtil.newURI(targetFile), null, null, null, null,
+                  persist, false);
+    persist.progressListener = transfer;
+
+    // Start the actual download process.
+    persist.saveURI(sourceURI, null, null, null, null, targetFile, null);
+  }.bind(this)).then(null, do_report_unexpected_exception);
+
+  return deferred.promise;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//// Tests
+
+/**
+ * Executes a download controlled by the legacy nsITransfer interface.
+ */
+add_task(function test_basic()
+{
+  let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
+
+  let download = yield promiseStartLegacyDownload();
+
+  // Checks the generated DownloadSource and DownloadTarget properties.
+  do_check_true(download.source.uri.equals(TEST_SOURCE_URI));
+  do_check_true(download.target.file.equals(targetFile));
+
+  // The download is already started, just wait for completion.
+  yield download.whenSucceeded();
+
+  yield promiseVerifyContents(targetFile, TEST_DATA_SHORT);
+});
+
+/**
+ * Checks final state and progress for a successful download.
+ */
+add_task(function test_final_state()
+{
+  let download = yield promiseStartLegacyDownload();
+
+  // The download is already started, just wait for completion.
+  yield download.whenSucceeded();
+
+  do_check_true(download.stopped);
+  do_check_true(download.succeeded);
+  do_check_false(download.canceled);
+  do_check_true(download.error === null);
+  do_check_eq(download.progress, 100);
+});
+
+/**
+ * Checks intermediate progress for a successful download.
+ */
+add_task(function test_intermediate_progress()
+{
+  let deferResponse = deferNextResponse();
+
+  let download = yield promiseStartLegacyDownload(TEST_INTERRUPTIBLE_URI);
+
+  let 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.
+      deferResponse.resolve();
+    }
+  };
+
+  // Register for the notification, but also call the function directly in case
+  // the download already reached the expected progress.
+  download.onchange = onchange;
+  onchange();
+
+  // The download is already started, just wait for completion.
+  yield download.whenSucceeded();
+
+  do_check_true(download.stopped);
+  do_check_eq(download.progress, 100);
+
+  yield promiseVerifyContents(download.target.file,
+                              TEST_DATA_SHORT + TEST_DATA_SHORT);
+});
+
+/**
+ * Downloads a file with a "Content-Length" of 0 and checks the progress.
+ */
+add_task(function test_empty_progress()
+{
+  let download = yield promiseStartLegacyDownload(TEST_EMPTY_URI);
+
+  yield download.whenSucceeded();
+
+  do_check_true(download.stopped);
+  do_check_true(download.hasProgress);
+  do_check_eq(download.progress, 100);
+  do_check_eq(download.currentBytes, 0);
+  do_check_eq(download.totalBytes, 0);
+
+  do_check_eq(download.target.file.fileSize, 0);
+});
+
+/**
+ * Downloads an empty file with no "Content-Length" and checks the progress.
+ */
+add_task(function test_empty_noprogress()
+{
+  let deferResponse = deferNextResponse();
+  let promiseEmptyRequestReceived = promiseNextRequestReceived();
+
+  let download = yield promiseStartLegacyDownload(TEST_EMPTY_NOPROGRESS_URI);
+
+  // Wait for the request to be received by the HTTP server, but don't allow the
+  // request to finish yet.  Before checking the download state, wait for the
+  // events to be processed by the client.
+  yield promiseEmptyRequestReceived;
+  yield promiseExecuteSoon();
+
+  // Check that this download has no progress report.
+  do_check_false(download.stopped);
+  do_check_false(download.hasProgress);
+  do_check_eq(download.currentBytes, 0);
+  do_check_eq(download.totalBytes, 0);
+
+  // Now allow the response to finish, and wait for the download to complete.
+  deferResponse.resolve();
+  yield download.whenSucceeded();
+
+  // Verify the state of the completed download.
+  do_check_true(download.stopped);
+  do_check_false(download.hasProgress);
+  do_check_eq(download.progress, 100);
+  do_check_eq(download.currentBytes, 0);
+  do_check_eq(download.totalBytes, 0);
+
+  do_check_eq(download.target.file.fileSize, 0);
+});
+
+/**
+ * Cancels a download and verifies that its state is reported correctly.
+ */
+add_task(function test_cancel_midway()
+{
+  let deferResponse = deferNextResponse();
+  let outPersist = {};
+  let download = yield promiseStartLegacyDownload(TEST_INTERRUPTIBLE_URI,
+                                                  outPersist);
+
+  try {
+    // Cancel the download after receiving the first part of the response.
+    let deferCancel = Promise.defer();
+    let onchange = function () {
+      if (!download.stopped && !download.canceled && download.progress == 50) {
+        deferCancel.resolve(download.cancel());
+
+        // The state change happens immediately after calling "cancel", but
+        // temporary files or part files may still exist at this point.
+        do_check_true(download.canceled);
+      }
+    };
+
+    // Register for the notification, but also call the function directly in
+    // case the download already reached the expected progress.
+    download.onchange = onchange;
+    onchange();
+
+    // Wait on the promise returned by the "cancel" method to ensure that the
+    // cancellation process finished and temporary files were removed.
+    yield deferCancel.promise;
+
+    // The nsIWebBrowserPersist instance should have been canceled now.
+    do_check_eq(outPersist.value.result, Cr.NS_ERROR_ABORT);
+
+    do_check_true(download.stopped);
+    do_check_true(download.canceled);
+    do_check_true(download.error === null);
+
+    do_check_false(download.target.file.exists());
+
+    // Progress properties are not reset by canceling.
+    do_check_eq(download.progress, 50);
+    do_check_eq(download.totalBytes, TEST_DATA_SHORT.length * 2);
+    do_check_eq(download.currentBytes, TEST_DATA_SHORT.length);
+  } finally {
+    deferResponse.resolve();
+  }
+});
+
+/**
+ * Ensures download error details are reported for legacy downloads.
+ */
+add_task(function test_error()
+{
+  let serverSocket = startFakeServer();
+  try {
+    let download = yield promiseStartLegacyDownload(TEST_FAKE_SOURCE_URI);
+
+    // We must check the download properties instead of calling the "start"
+    // method because the download has been started and may already be stopped.
+    let deferStopped = Promise.defer();
+    let onchange = function () {
+      if (download.stopped) {
+        deferStopped.resolve();
+      }
+    };
+    download.onchange = onchange;
+    onchange();
+    yield deferStopped.promise;
+
+    // Check the properties now that the 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();
+  }
+});

toolkit/components/jsdownloads/test/unit/xpcshell.ini

@@ -3,5 +3,6 @@ head = head.js
 tail = tail.js
 
 [test_DownloadCore.js]
+[test_DownloadLegacy.js]
 [test_DownloadList.js]
 [test_Downloads.js]