Bug 941726 - Prevent DownloadLegacy from keeping a reference to the network request after the download is stopped. r=enn

toolkit/components/jsdownloads/src/DownloadCore.jsm

@@ -2053,10 +2053,16 @@ this.DownloadLegacySaver.prototype = {
             Cu.reportError(e2);
           }
         }
+        // In case the operation failed, ensure we stop downloading data.  Since
+        // we never re-enter this function, deferCanceled is always available.
+        this.deferCanceled.resolve();
         throw ex;
       } finally {
-        // We don't need the reference to the request anymore.
+        // We don't need the reference to the request anymore.  We must also set
+        // deferCanceled to null in order to free any indirect references it
+        // may hold to the request.
         this.request = null;
+        this.deferCanceled = null;
         // Allow the download to restart through a DownloadCopySaver.
         this.firstExecutionFinished = true;
       }
@@ -2073,8 +2079,12 @@ this.DownloadLegacySaver.prototype = {
       return this.copySaver.cancel.apply(this.copySaver, arguments);
     }
 
-    // Cancel the operation as soon as the object is connected.
-    this.deferCanceled.resolve();
+    // If the download hasn't stopped already, resolve deferCanceled so that the
+    // operation is canceled as soon as a cancellation handler is registered.
+    // Note that the handler might not have been registered yet.
+    if (this.deferCanceled) {
+      this.deferCanceled.resolve();
+    }
   },
 
   /**

toolkit/components/jsdownloads/src/DownloadLegacy.js

@@ -99,7 +99,8 @@ DownloadLegacyTransfer.prototype = {
         // To handle asynchronous cancellation properly, we should hook up the
         // handler only after we have been notified that the main request
         // started.  We will wait until the main request stopped before
-        // notifying that the download has been canceled.
+        // notifying that the download has been canceled.  Since the request has
+        // not completed yet, deferCanceled is guaranteed to be set.
         return download.saver.deferCanceled.promise.then(() => {
           // Only cancel if the object executing the download is still running.
           if (this._cancelable && !this._componentFailed) {
@@ -224,11 +225,8 @@ DownloadLegacyTransfer.prototype = {
         aDownload.tryToKeepPartialData = true;
       }
 
-      // Start the download before allowing it to be controlled.
-      aDownload.start().then(null, function () {
-        // In case the operation failed, ensure we stop downloading data.
-        aDownload.saver.deferCanceled.resolve();
-      });
+      // Start the download before allowing it to be controlled.  Ignore errors.
+      aDownload.start().then(null, () => {});
 
       // Start processing all the other events received through nsITransfer.
       this._deferDownload.resolve(aDownload);