Bug 851461 - Make the JavaScript API for downloads available in parallel to nsIDownloadManager. r=mak

browser/components/downloads/src/BrowserDownloads.manifest

@@ -1,4 +1,4 @@
 component {49507fe5-2cee-4824-b6a3-e999150ce9b8} DownloadsStartup.js
 contract @mozilla.org/browser/downloadsstartup;1 {49507fe5-2cee-4824-b6a3-e999150ce9b8}
-category app-startup DownloadsStartup service,@mozilla.org/browser/downloadsstartup;1
+category profile-after-change DownloadsStartup @mozilla.org/browser/downloadsstartup;1
 component {4d99321e-d156-455b-81f7-e7aa2308134f} DownloadsUI.js

browser/components/downloads/src/DownloadsStartup.js

@@ -53,6 +53,16 @@ const kDownloadsUICid = Components.ID("{4d99321e-d156-455b-81f7-e7aa2308134f}");
  */
 const kDownloadsUIContractId = "@mozilla.org/download-manager-ui;1";
 
+/**
+ * CID of the JavaScript implementation of nsITransfer.
+ */
+const kTransferCid = Components.ID("{1b4c85df-cbdd-4bb6-b04e-613caece083c}");
+
+/**
+ * Contract ID of the service implementing nsITransfer.
+ */
+const kTransferContractId = "@mozilla.org/transfer;1";
+
 ////////////////////////////////////////////////////////////////////////////////
 //// DownloadsStartup
 
@@ -75,7 +85,7 @@ DownloadsStartup.prototype = {
   observe: function DS_observe(aSubject, aTopic, aData)
   {
     switch (aTopic) {
-      case "app-startup":
+      case "profile-after-change":
         kObservedTopics.forEach(
           function (topic) Services.obs.addObserver(this, topic, true),
           this);
@@ -86,6 +96,24 @@ DownloadsStartup.prototype = {
         Components.manager.QueryInterface(Ci.nsIComponentRegistrar)
                           .registerFactory(kDownloadsUICid, "",
                                            kDownloadsUIContractId, null);
+
+        // If the integration preference is enabled, override Toolkit's
+        // nsITransfer implementation with the one from the JavaScript API for
+        // downloads.  This should be used only by developers while testing new
+        // code that uses the JavaScript API, and will eventually be removed
+        // when nsIDownloadManager will not be available anymore (bug 851471).
+        let useJSTransfer = false;
+        try {
+          useJSTransfer =
+            Services.prefs.getBoolPref("browser.download.useJSTransfer");
+        } catch (ex) {
+          // This is a hidden preference that does not exist by default.
+        }
+        if (useJSTransfer) {
+          Components.manager.QueryInterface(Ci.nsIComponentRegistrar)
+                            .registerFactory(kTransferCid, "",
+                                             kTransferContractId, null);
+        }
         break;
 
       case "sessionstore-windows-restored":

browser/installer/package-manifest.in

@@ -355,10 +355,8 @@
 @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

configure.in

@@ -8630,12 +8630,6 @@ AC_SUBST(MOZ_POST_DSO_LIB_COMMAND)
 AC_SUBST(MOZ_POST_PROGRAM_COMMAND)
 AC_SUBST(MOZ_LINKER_EXTRACT)
 
-AC_SUBST(MOZ_JSDOWNLOADS)
-
-if test -n "$MOZ_JSDOWNLOADS" ; then
-    AC_DEFINE(MOZ_JSDOWNLOADS)
-fi
-
 dnl ========================================================
 dnl = Mac bundle name prefix
 dnl ========================================================

toolkit/components/build/nsToolkitCompsModule.cpp

@@ -111,9 +111,7 @@ 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
@@ -138,9 +136,7 @@ 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/Downloads.manifest

@@ -1,2 +1,8 @@
 component {1b4c85df-cbdd-4bb6-b04e-613caece083c} DownloadLegacy.js
-contract @mozilla.org/transfer;1 {1b4c85df-cbdd-4bb6-b04e-613caece083c}
+
+# The following contract definition is commented out because the same contract
+# is also implemented in "toolkit/components/downloads".  To use the component
+# in this folder experimentally, the contract must be registered manually.  When
+# the other folder is not included in builds anymore (bug 851471), we'll be able
+# to define the contract implementation in this manifest.
+# contract @mozilla.org/transfer;1 {1b4c85df-cbdd-4bb6-b04e-613caece083c}

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

@@ -19,6 +19,8 @@ const Cr = Components.results;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "DownloadPaths",
+                                  "resource://gre/modules/DownloadPaths.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Downloads",
                                   "resource://gre/modules/Downloads.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
@@ -71,26 +73,42 @@ function run_test()
 ////////////////////////////////////////////////////////////////////////////////
 //// Support functions
 
+// While the previous test file should have deleted all the temporary files it
+// used, on Windows these might still be pending deletion on the physical file
+// system.  Thus, start from a new base number every time, to make a collision
+// with a file that is still pending deletion highly unlikely.
+let gFileCounter = Math.floor(Math.random() * 1000000);
+
 /**
- * Returns a reference to a temporary file.  The file is deleted if it already
- * exists.  If the file is then created by the test suite, it will be removed
- * when tests in this file finish.
+ * Returns a reference to a temporary file, that is guaranteed not to exist, and
+ * to have never been created before.
+ *
+ * @param aLeafName
+ *        Suggested leaf name for the file to be created.
+ *
+ * @return nsIFile pointing to a non-existent file in a temporary directory.
+ *
+ * @note It is not enough to delete the file if it exists, or to delete the file
+ *       after calling nsIFile.createUnique, because on Windows the delete
+ *       operation in the file system may still be pending, preventing a new
+ *       file with the same name to be created.
  */
 function getTempFile(aLeafName)
 {
-  let file = FileUtils.getFile("TmpD", [aLeafName]);
-  function GTF_removeFile()
-  {
+  // Prepend a serial number to the extension in the suggested leaf name.
+  let [base, ext] = DownloadPaths.splitBaseNameAndExtension(aLeafName);
+  let leafName = base + "-" + gFileCounter + ext;
+  gFileCounter++;
+
+  // Get a file reference under the temporary directory for this test file.
+  let file = FileUtils.getFile("TmpD", [leafName]);
+  do_check_false(file.exists());
+
+  do_register_cleanup(function () {
     if (file.exists()) {
       file.remove(false);
     }
-  }
-
-  // Remove the file in case a previous test created it.
-  GTF_removeFile();
-
-  // Remove the file at the end of the test suite.
-  do_register_cleanup(GTF_removeFile);
+  });
 
   return file;
 }
@@ -110,8 +128,7 @@ function promiseExecuteSoon()
 }
 
 /**
- * Creates a new Download object, using TEST_TARGET_FILE_NAME as the target.
- * The target is deleted by getTempFile when this function is called.
+ * Creates a new Download object, setting a temporary file as the target.
  *
  * @param aSourceURI
  *        The nsIURI for the download source, or null to use TEST_SOURCE_URI.

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

@@ -302,7 +302,9 @@ add_task(function test_download_cancel_immediately()
   // been made, and the internal HTTP handler might be waiting to process it.
   // Thus, we process any pending events now, to avoid that the request is
   // processed during the tests that follow, interfering with them.
-  yield promiseExecuteSoon();
+  for (let i = 0; i < 5; i++) {
+    yield promiseExecuteSoon();
+  }
 });
 
 /**
@@ -389,7 +391,9 @@ add_task(function test_download_cancel_immediately_restart_immediately()
   // been made, and the internal HTTP handler might be waiting to process it.
   // Thus, we process any pending events now, to avoid that the request is
   // processed during the tests that follow, interfering with them.
-  yield promiseExecuteSoon();
+  for (let i = 0; i < 5; i++) {
+    yield promiseExecuteSoon();
+  }
 
   // Ensure the next request is now allowed to complete, regardless of whether
   // the canceled request was received by the server or not.
@@ -606,19 +610,26 @@ add_task(function test_download_error_target()
 
   // 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.
-  }
+    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.stopped);
-  do_check_false(download.canceled);
-  do_check_true(download.error !== null);
-  do_check_true(download.error.becauseTargetFailed);
-  do_check_false(download.error.becauseSourceFailed);
+    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);
+  } finally {
+    // Restore the default permissions to allow deleting the file on Windows.
+    if (download.target.file.exists()) {
+      download.target.file.permissions = FileUtils.PERMS_FILE;
+      download.target.file.remove(false);
+    }
+  }
 });
 
 /**
@@ -638,10 +649,18 @@ add_task(function test_download_error_restart()
     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.
-  }
+  } finally {
+    // Restore the default permissions to allow deleting the file on Windows.
+    if (download.target.file.exists()) {
+      download.target.file.permissions = FileUtils.PERMS_FILE;
 
-  if (download.target.file.exists()) {
-    download.target.file.remove(false);
+      // Also for Windows, rename the file before deleting.  This makes the
+      // current file name available immediately for a new file, while deleting
+      // in place prevents creation of a file with the same name for some time.
+      let fileToRemove = download.target.file.clone();
+      fileToRemove.moveTo(null, fileToRemove.leafName + ".delete.tmp");
+      fileToRemove.remove(false);
+    }
   }
 
   // Restart the download and wait for completion.

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

@@ -33,7 +33,14 @@ function promiseStartLegacyDownload(aSourceURI, aOutPersist) {
 
   let persist = Cc["@mozilla.org/embedding/browser/nsWebBrowserPersist;1"]
                   .createInstance(Ci.nsIWebBrowserPersist);
-  let transfer = Cc["@mozilla.org/transfer;1"].createInstance(Ci.nsITransfer);
+
+  // We must create the nsITransfer implementation using its class ID because
+  // the "@mozilla.org/transfer;1" contract is currently implemented in
+  // "toolkit/components/downloads".  When the other folder is not included in
+  // builds anymore (bug 851471), we'll be able to use the contract ID.
+  let transfer =
+      Components.classesByID["{1b4c85df-cbdd-4bb6-b04e-613caece083c}"]
+                .createInstance(Ci.nsITransfer);
 
   if (aOutPersist) {
     aOutPersist.value = persist;
@@ -78,18 +85,20 @@ function promiseStartLegacyDownload(aSourceURI, aOutPersist) {
  */
 add_task(function test_basic()
 {
-  let targetFile = getTempFile(TEST_TARGET_FILE_NAME);
+  let tempDirectory = FileUtils.getDir("TmpD", []);
 
   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));
+  do_check_true(download.target.file.parent.equals(tempDirectory));
 
-  // The download is already started, just wait for completion.
-  yield download.whenSucceeded();
+  // The download is already started, wait for completion and report any errors.
+  if (!download.stopped) {
+    yield download.start();
+  }
 
-  yield promiseVerifyContents(targetFile, TEST_DATA_SHORT);
+  yield promiseVerifyContents(download.target.file, TEST_DATA_SHORT);
 });
 
 /**
@@ -99,8 +108,10 @@ add_task(function test_final_state()
 {
   let download = yield promiseStartLegacyDownload();
 
-  // The download is already started, just wait for completion.
-  yield download.whenSucceeded();
+  // The download is already started, wait for completion and report any errors.
+  if (!download.stopped) {
+    yield download.start();
+  }
 
   do_check_true(download.stopped);
   do_check_true(download.succeeded);
@@ -134,8 +145,10 @@ add_task(function test_intermediate_progress()
   download.onchange = onchange;
   onchange();
 
-  // The download is already started, just wait for completion.
-  yield download.whenSucceeded();
+  // The download is already started, wait for completion and report any errors.
+  if (!download.stopped) {
+    yield download.start();
+  }
 
   do_check_true(download.stopped);
   do_check_eq(download.progress, 100);
@@ -151,7 +164,10 @@ add_task(function test_empty_progress()
 {
   let download = yield promiseStartLegacyDownload(TEST_EMPTY_URI);
 
-  yield download.whenSucceeded();
+  // The download is already started, wait for completion and report any errors.
+  if (!download.stopped) {
+    yield download.start();
+  }
 
   do_check_true(download.stopped);
   do_check_true(download.hasProgress);
@@ -184,9 +200,12 @@ add_task(function test_empty_noprogress()
   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.
+  // Now allow the response to finish, and wait for the download to complete,
+  // while reporting any errors that may occur.
   deferResponse.resolve();
-  yield download.whenSucceeded();
+  if (!download.stopped) {
+    yield download.start();
+  }
 
   // Verify the state of the completed download.
   do_check_true(download.stopped);

toolkit/components/moz.build

@@ -21,6 +21,7 @@ PARALLEL_DIRS += [
     'filepicker',
     'find',
     'intl',
+    'jsdownloads',
     'mediasniffer',
     'microformats',
     'osfile',
@@ -51,9 +52,6 @@ if CONFIG['MOZ_FEEDS']:
 if CONFIG['MOZ_HELP_VIEWER']:
     PARALLEL_DIRS += ['help']
 
-if CONFIG['MOZ_JSDOWNLOADS']:
-    PARALLEL_DIRS += ['jsdownloads']
-
 if CONFIG['NS_PRINTING']:
     PARALLEL_DIRS += ['printing']