Bug 941726 - Prevent DownloadLegacy from keeping a reference to the network request after the download is stopped. r=enn
--- a/toolkit/components/jsdownloads/src/DownloadCore.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm
@@ -2038,38 +2038,48 @@ this.DownloadLegacySaver.prototype = {
// original one as the failure reason of the download. Note that on
// Windows we may get an access denied error instead of a no such file
// error if the file existed before, and was recently deleted.
if (!(e2 instanceof OS.File.Error &&
(e2.becauseNoSuchFile || e2.becauseAccessDenied))) {
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;
}
}.bind(this));
},
/**
* Implements "DownloadSaver.cancel".
*/
cancel: function DLS_cancel()
{
// We may be using a DownloadCopySaver to handle resuming.
if (this.copySaver) {
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();
+ }
},
/**
* Implements "DownloadSaver.removePartialData".
*/
removePartialData: function ()
{
// DownloadCopySaver and DownloadLeagcySaver use the same logic for removing
--- a/toolkit/components/jsdownloads/src/DownloadLegacy.js
+++ b/toolkit/components/jsdownloads/src/DownloadLegacy.js
@@ -110,17 +110,18 @@ DownloadLegacyTransfer.prototype = {
download.saver.onTransferStarted(
aRequest,
this._cancelable instanceof Ci.nsIHelperAppLauncher);
// 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) {
this._cancelable.cancel(Cr.NS_ERROR_ABORT);
if (this._cancelable instanceof Ci.nsIWebBrowserPersist) {
// This component will not send the STATE_STOP notification.
download.saver.onTransferFinished(aRequest, Cr.NS_ERROR_ABORT);
this._cancelable = null;
@@ -234,21 +235,18 @@ DownloadLegacyTransfer.prototype = {
contentType: contentType,
launcherPath: launcherPath
}).then(function DLT_I_onDownload(aDownload) {
// Legacy components keep partial data when they use a ".part" file.
if (aTempFile) {
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);
// Add the download to the list, allowing it to be seen and canceled.
return Downloads.getList(Downloads.ALL).then(list => list.add(aDownload));
}.bind(this)).then(null, Cu.reportError);
},