Backed out changeset 65bf6165dd79 (bug 899102)

This commit is contained in:
Ed Morley 2013-09-17 13:14:22 +01:00
parent 67d4689225
commit 8e511db649
5 changed files with 94 additions and 70 deletions

View File

@ -1901,8 +1901,14 @@ DownloadLegacySaver.prototype = {
return this.copySaver.cancel.apply(this.copySaver, arguments);
}
// Cancel the operation as soon as the object is connected.
// 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."));
},
/**

View File

@ -95,22 +95,6 @@ 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.
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;
}
}
});
}).then(null, Cu.reportError);
} else if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
(aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK)) {
@ -212,6 +196,14 @@ DownloadLegacyTransfer.prototype = {
contentType: contentType,
launcherPath: launcherPath
}).then(function DLT_I_onDownload(aDownload) {
// Now that the saver is available, hook up the cancellation handler.
aDownload.saver.deferCanceled.promise.then(() => {
// Only cancel if the object executing the download is still running.
if (!this._componentFailed) {
aCancelable.cancel(Cr.NS_ERROR_ABORT);
}
}).then(null, Cu.reportError);
// Legacy components keep partial data when they use a ".part" file.
if (aTempFile) {
aDownload.tryToKeepPartialData = true;

View File

@ -626,6 +626,12 @@ add_task(function test_cancel_midway_restart_tryToKeepPartialData()
let download = yield promiseStartDownload_tryToKeepPartialData();
yield download.cancel();
// This time-based solution is a workaround to avoid intermittent failures,
// and will be removed when bug 899102 is resolved.
if (gUseLegacySaver) {
yield promiseTimeout(250);
}
do_check_true(download.stopped);
do_check_true(download.hasPartialData);
@ -679,6 +685,12 @@ add_task(function test_cancel_midway_restart_removePartialData()
let download = yield promiseStartDownload_tryToKeepPartialData();
yield download.cancel();
// This time-based solution is a workaround to avoid intermittent failures,
// and will be removed when bug 899102 is resolved.
if (gUseLegacySaver) {
yield promiseTimeout(250);
}
do_check_true(download.hasPartialData);
yield promiseVerifyContents(download.target.partFilePath, TEST_DATA_SHORT);
@ -710,6 +722,12 @@ add_task(function test_cancel_midway_restart_tryToKeepPartialData_false()
let download = yield promiseStartDownload_tryToKeepPartialData();
yield download.cancel();
// This time-based solution is a workaround to avoid intermittent failures,
// and will be removed when bug 899102 is resolved.
if (gUseLegacySaver) {
yield promiseTimeout(250);
}
download.tryToKeepPartialData = false;
// The above property change does not affect existing partial data.
@ -732,6 +750,12 @@ add_task(function test_cancel_midway_restart_tryToKeepPartialData_false()
yield download.cancel();
// This time-based solution is a workaround to avoid intermittent failures,
// and will be removed when bug 899102 is resolved.
if (gUseLegacySaver) {
yield promiseTimeout(250);
}
// The ".part" file should be deleted now that the download is canceled.
do_check_false(download.hasPartialData);
do_check_false(yield OS.File.exists(download.target.partFilePath));
@ -938,6 +962,12 @@ add_task(function test_finalize_tryToKeepPartialData()
let download = yield promiseStartDownload_tryToKeepPartialData();
yield download.finalize();
// This time-based solution is a workaround to avoid intermittent failures,
// and will be removed when bug 899102 is resolved.
if (gUseLegacySaver) {
yield promiseTimeout(250);
}
do_check_true(download.hasPartialData);
do_check_true(yield OS.File.exists(download.target.partFilePath));
@ -948,6 +978,12 @@ add_task(function test_finalize_tryToKeepPartialData()
download = yield promiseStartDownload_tryToKeepPartialData();
yield download.finalize(true);
// This time-based solution is a workaround to avoid intermittent failures,
// and will be removed when bug 899102 is resolved.
if (gUseLegacySaver) {
yield promiseTimeout(250);
}
do_check_false(download.hasPartialData);
do_check_false(yield OS.File.exists(download.target.partFilePath));
});

View File

@ -1812,55 +1812,63 @@ NS_IMETHODIMP
nsExternalAppHandler::OnSaveComplete(nsIBackgroundFileSaver *aSaver,
nsresult aStatus)
{
if (!mCanceled) {
// Save the hash
(void)mSaver->GetSha256Hash(mHash);
// Free the reference that the saver keeps on us, even if we couldn't get
// the hash.
mSaver = nullptr;
if (NS_FAILED(aStatus)) {
nsAutoString path;
mTempFile->GetPath(path);
SendStatusChange(kWriteError, aStatus, nullptr, path);
if (!mCanceled)
Cancel(aStatus);
return NS_OK;
}
if (mCanceled)
return NS_OK;
// Save the hash
nsresult rv = mSaver->GetSha256Hash(mHash);
// Free the reference that the saver keeps on us, even if we couldn't get the
// hash.
mSaver = nullptr;
if (NS_FAILED(aStatus)) {
nsAutoString path;
mTempFile->GetPath(path);
SendStatusChange(kWriteError, aStatus, nullptr, path);
if (!mCanceled)
Cancel(aStatus);
return NS_OK;
}
// Notify the transfer object that we are done if the user has chosen an
// action. If the user hasn't chosen an action, the progress listener
// (nsITransfer) will be notified in CreateTransfer.
if (mTransfer) {
NotifyTransfer(aStatus);
rv = NotifyTransfer();
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
void nsExternalAppHandler::NotifyTransfer(nsresult aStatus)
nsresult nsExternalAppHandler::NotifyTransfer()
{
MOZ_ASSERT(NS_IsMainThread(), "Must notify on main thread");
MOZ_ASSERT(!mCanceled, "Can't notify if canceled or action "
"hasn't been chosen");
MOZ_ASSERT(mTransfer, "We must have an nsITransfer");
LOG(("Notifying progress listener"));
if (NS_SUCCEEDED(aStatus)) {
(void)mTransfer->SetSha256Hash(mHash);
(void)mTransfer->OnProgressChange64(nullptr, nullptr, mProgress,
mContentLength, mProgress, mContentLength);
}
nsresult rv = mTransfer->SetSha256Hash(mHash);
NS_ENSURE_SUCCESS(rv, rv);
(void)mTransfer->OnStateChange(nullptr, nullptr,
rv = mTransfer->OnProgressChange64(nullptr, nullptr, mProgress,
mContentLength, mProgress, mContentLength);
NS_ENSURE_SUCCESS(rv, rv);
rv = mTransfer->OnStateChange(nullptr, nullptr,
nsIWebProgressListener::STATE_STOP |
nsIWebProgressListener::STATE_IS_REQUEST |
nsIWebProgressListener::STATE_IS_NETWORK, aStatus);
nsIWebProgressListener::STATE_IS_NETWORK, NS_OK);
NS_ENSURE_SUCCESS(rv, rv);
// This nsITransfer object holds a reference to us (we are its observer), so
// we need to release the reference to break a reference cycle (and therefore
// to prevent leaking). We do this even if the previous calls failed.
// to prevent leaking)
mTransfer = nullptr;
return NS_OK;
}
NS_IMETHODIMP nsExternalAppHandler::GetMIMEInfo(nsIMIMEInfo ** aMIMEInfo)
@ -1958,7 +1966,7 @@ nsresult nsExternalAppHandler::CreateTransfer()
// processing the url. If that's the case then mStopRequestIssued will be
// true and OnSaveComplete has been called.
if (mStopRequestIssued && !mSaver && mTransfer) {
NotifyTransfer(NS_OK);
return NotifyTransfer();
}
return rv;
@ -2195,31 +2203,17 @@ NS_IMETHODIMP nsExternalAppHandler::LaunchWithApplication(nsIFile * aApplication
NS_IMETHODIMP nsExternalAppHandler::Cancel(nsresult aReason)
{
NS_ENSURE_ARG(NS_FAILED(aReason));
// XXX should not ignore the reason
if (mCanceled) {
return NS_OK;
}
mCanceled = true;
if (mSaver) {
// We are still writing to the target file. Give the saver a chance to
// close the target file, then notify the transfer object if necessary in
// the OnSaveComplete callback.
mSaver->Finish(aReason);
mSaver = nullptr;
} else {
if (mStopRequestIssued && mTempFile) {
// This branch can only happen when the user cancels the helper app dialog
// when the request has completed. The temp file has to be removed here,
// because mSaver has been released at that time with the temp file left.
(void)mTempFile->Remove(false);
}
// Notify the transfer object that the download has been canceled, if the
// user has already chosen an action and we didn't notify already.
if (mTransfer) {
NotifyTransfer(aReason);
}
} else if (mStopRequestIssued && mTempFile) {
// This branch can only happen when the user cancels the helper app dialog
// when the request has completed. The temp file has to be removed here,
// because mSaver has been released at that time with the temp file left.
(void)mTempFile->Remove(false);
}
// Break our reference cycle with the helper app dialog (set up in
@ -2231,6 +2225,7 @@ NS_IMETHODIMP nsExternalAppHandler::Cancel(nsresult aReason)
// Release the listener, to break the reference cycle with it (we are the
// observer of the listener).
mDialogProgressListener = nullptr;
mTransfer = nullptr;
return NS_OK;
}

View File

@ -373,14 +373,9 @@ protected:
void ProcessAnyRefreshTags();
/**
* Notify our nsITransfer object that we are done with the download. This is
* always called after the target file has been closed.
*
* @param aStatus
* NS_OK for success, or a failure code if the download failed.
* A partially downloaded file may still be available in this case.
* Notify our nsITransfer object that we are done with the download.
*/
void NotifyTransfer(nsresult aStatus);
nsresult NotifyTransfer();
/**
* Helper routine that searches a pref string for a given mime type