Bug 908240 - Legacy downloads not executed by nsIHelperAppLauncher should be added to history. r=enn

This commit is contained in:
Paolo Amadini 2013-09-10 12:45:49 +02:00
parent f8f2853270
commit 73a63009ca
5 changed files with 300 additions and 53 deletions

View File

@ -67,6 +67,9 @@ XPCOMUtils.defineLazyModuleGetter(this, "Promise",
XPCOMUtils.defineLazyModuleGetter(this, "Task",
"resource://gre/modules/Task.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "gDownloadHistory",
"@mozilla.org/browser/download-history;1",
Ci.nsIDownloadHistory);
XPCOMUtils.defineLazyServiceGetter(this, "gExternalHelperAppService",
"@mozilla.org/uriloader/external-helper-app-service;1",
Ci.nsIExternalHelperAppService);
@ -1205,6 +1208,30 @@ DownloadSaver.prototype = {
return Promise.resolve();
},
/**
* This can be called by the saver implementation when the download is already
* started, to add it to the browsing history. This method has no effect if
* the download is private.
*/
addToHistory: function ()
{
if (this.download.source.isPrivate) {
return;
}
let sourceUri = NetUtil.newURI(this.download.source.url);
let referrer = this.download.source.referrer;
let referrerUri = referrer ? NetUtil.newURI(referrer) : null;
let targetUri = NetUtil.newURI(new FileUtils.File(
this.download.target.path));
// The start time is always available when we reach this point.
let startPRTime = this.download.startTime.getTime() * 1000;
gDownloadHistory.addDownload(sourceUri, referrerUri, startPRTime,
targetUri);
},
/**
* Returns a static representation of the current object state.
*
@ -1266,6 +1293,11 @@ DownloadCopySaver.prototype = {
*/
_canceled: false,
/**
* True if the associated download has already been added to browsing history.
*/
alreadyAddedToHistory: false,
/**
* String corresponding to the entityID property of the nsIResumableChannel
* used to execute the download, or null if the channel was not resumable or
@ -1288,6 +1320,16 @@ DownloadCopySaver.prototype = {
let keepPartialData = download.tryToKeepPartialData;
return Task.spawn(function task_DCS_execute() {
// Add the download to history the first time it is started in this
// session. If the download is restarted in a different session, a new
// history visit will be added. We do this just to avoid the complexity
// of serializing this state between sessions, since adding a new visit
// does not have any noticeable side effect.
if (!this.alreadyAddedToHistory) {
this.addToHistory();
this.alreadyAddedToHistory = true;
}
// To reduce the chance that other downloads reuse the same final target
// file name, we should create a placeholder as soon as possible, before
// starting the network request. The placeholder is also required in case
@ -1633,8 +1675,14 @@ DownloadLegacySaver.prototype = {
*
* @param aRequest
* nsIRequest associated to the status update.
* @param aAlreadyAddedToHistory
* Indicates that the nsIExternalHelperAppService component already
* added the download to the browsing history, unless it was started
* from a private browsing window. When this parameter is false, the
* download is added to the browsing history here. Private downloads
* are never added to history even if this parameter is false.
*/
onTransferStarted: function (aRequest)
onTransferStarted: function (aRequest, aAlreadyAddedToHistory)
{
// Store the entity ID to use for resuming if required.
if (this.download.tryToKeepPartialData &&
@ -1645,6 +1693,15 @@ DownloadLegacySaver.prototype = {
} catch (ex if ex instanceof Components.Exception &&
ex.result == Cr.NS_ERROR_NOT_RESUMABLE) { }
}
// For legacy downloads, we must update the referrer at this time.
if (aRequest instanceof Ci.nsIHttpChannel && aRequest.referrer) {
this.download.source.referrer = aRequest.referrer.spec;
}
if (!aAlreadyAddedToHistory) {
this.addToHistory();
}
},
/**
@ -1702,6 +1759,7 @@ DownloadLegacySaver.prototype = {
this.copySaver = new DownloadCopySaver();
this.copySaver.download = this.download;
this.copySaver.entityID = this.entityID;
this.copySaver.alreadyAddedToHistory = true;
}
return this.copySaver.execute.apply(this.copySaver, arguments);
}

View File

@ -91,16 +91,21 @@ DownloadLegacyTransfer.prototype = {
(aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK)) {
// The main request has just started. Wait for the associated Download
// object to be available before notifying.
this._deferDownload.promise.then(function (aDownload) {
aDownload.saver.onTransferStarted(aRequest);
this._deferDownload.promise.then(download => {
download.saver.onTransferStarted(
aRequest,
this._cancelable instanceof Ci.nsIHelperAppLauncher);
}).then(null, Cu.reportError);
} else if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
(aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK)) {
// The last file has been received, or the download failed. Wait for the
// associated Download object to be available before notifying.
this._deferDownload.promise.then(function DLT_OSC_onDownload(aDownload) {
aDownload.saver.onTransferFinished(aRequest, aStatus);
this._deferDownload.promise.then(download => {
download.saver.onTransferFinished(aRequest, aStatus);
}).then(null, Cu.reportError);
// Release the reference to the component executing the download.
this._cancelable = null;
}
},
@ -164,6 +169,8 @@ DownloadLegacyTransfer.prototype = {
init: function DLT_init(aSource, aTarget, aDisplayName, aMIMEInfo, aStartTime,
aTempFile, aCancelable, aIsPrivate)
{
this._cancelable = aCancelable;
let launchWhenSucceeded = false, contentType = null, launcherPath = null;
if (aMIMEInfo instanceof Ci.nsIMIMEInfo) {
@ -233,6 +240,12 @@ DownloadLegacyTransfer.prototype = {
*/
_deferDownload: null,
/**
* Reference to the component that is executing the download. This component
* allows cancellation through its nsICancelable interface.
*/
_cancelable: null,
/**
* Indicates that the component that executes the download has notified a
* failure condition. In this case, we should never use the component methods

View File

@ -1676,3 +1676,60 @@ add_task(function test_platform_integration()
yield promiseVerifyContents(download.target.path, TEST_DATA_SHORT);
}
});
/**
* Checks that downloads are added to browsing history when they start.
*/
add_task(function test_history()
{
mustInterruptResponses();
// We will wait for the visit to be notified during the download.
yield promiseClearHistory();
let promiseVisit = promiseWaitForVisit(httpUrl("interruptible.txt"));
// Start a download that is not allowed to finish yet.
let download = yield promiseStartDownload(httpUrl("interruptible.txt"));
// The history notifications should be received before the download completes.
let [time, transitionType] = yield promiseVisit;
do_check_eq(time, download.startTime.getTime() * 1000);
do_check_eq(transitionType, Ci.nsINavHistoryService.TRANSITION_DOWNLOAD);
// Restart and complete the download after clearing history.
yield promiseClearHistory();
download.cancel();
continueResponses();
yield download.start();
// The restart should not have added a new history visit.
do_check_false(yield promiseIsURIVisited(httpUrl("interruptible.txt")));
});
/**
* Checks that downloads started by nsIHelperAppService are added to the
* browsing history when they start.
*/
add_task(function test_history_tryToKeepPartialData()
{
// We will wait for the visit to be notified during the download.
yield promiseClearHistory();
let promiseVisit =
promiseWaitForVisit(httpUrl("interruptible_resumable.txt"));
// Start a download that is not allowed to finish yet.
let beforeStartTimeMs = Date.now();
let download = yield promiseStartDownload_tryToKeepPartialData();
// The history notifications should be received before the download completes.
let [time, transitionType] = yield promiseVisit;
do_check_eq(transitionType, Ci.nsINavHistoryService.TRANSITION_DOWNLOAD);
// The time set by nsIHelperAppService may be different than the start time in
// the download object, thus we only check that it is a meaningful time.
do_check_true(time >= beforeStartTimeMs * 1000);
// Complete the download before finishing the test.
continueResponses();
yield promiseDownloadStopped(download);
});

View File

@ -169,6 +169,101 @@ function promiseTimeout(aTime)
return deferred.promise;
}
/**
* Allows waiting for an observer notification once.
*
* @param aTopic
* Notification topic to observe.
*
* @return {Promise}
* @resolves The array [aSubject, aData] from the observed notification.
* @rejects Never.
*/
function promiseTopicObserved(aTopic)
{
let deferred = Promise.defer();
Services.obs.addObserver(
function PTO_observe(aSubject, aTopic, aData) {
Services.obs.removeObserver(PTO_observe, aTopic);
deferred.resolve([aSubject, aData]);
}, aTopic, false);
return deferred.promise;
}
/**
* Clears history asynchronously.
*
* @return {Promise}
* @resolves When history has been cleared.
* @rejects Never.
*/
function promiseClearHistory()
{
let promise = promiseTopicObserved(PlacesUtils.TOPIC_EXPIRATION_FINISHED);
do_execute_soon(function() PlacesUtils.bhistory.removeAllPages());
return promise;
}
/**
* Waits for a new history visit to be notified for the specified URI.
*
* @param aUrl
* String containing the URI that will be visited.
*
* @return {Promise}
* @resolves Array [aTime, aTransitionType] from nsINavHistoryObserver.onVisit.
* @rejects Never.
*/
function promiseWaitForVisit(aUrl)
{
let deferred = Promise.defer();
let uri = NetUtil.newURI(aUrl);
PlacesUtils.history.addObserver({
QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver]),
onBeginUpdateBatch: function () {},
onEndUpdateBatch: function () {},
onVisit: function (aURI, aVisitID, aTime, aSessionID, aReferringID,
aTransitionType, aGUID, aHidden) {
if (aURI.equals(uri)) {
PlacesUtils.history.removeObserver(this);
deferred.resolve([aTime, aTransitionType]);
}
},
onTitleChanged: function () {},
onDeleteURI: function () {},
onClearHistory: function () {},
onPageChanged: function () {},
onDeleteVisits: function () {},
}, false);
return deferred.promise;
}
/**
* Check browsing history to see whether the given URI has been visited.
*
* @param aUrl
* String containing the URI that will be visited.
*
* @return {Promise}
* @resolves Boolean indicating whether the URI has been visited.
* @rejects JavaScript exception.
*/
function promiseIsURIVisited(aUrl) {
let deferred = Promise.defer();
PlacesUtils.asyncHistory.isURIVisited(NetUtil.newURI(aUrl),
function (aURI, aIsVisited) {
deferred.resolve(aIsVisited);
});
return deferred.promise;
}
/**
* Creates a new Download object, setting a temporary file as the target.
*
@ -441,40 +536,6 @@ function promiseVerifyContents(aPath, aExpectedContents)
});
}
/**
* Adds entry for download.
*
* @param aSourceUrl
* String containing the URI for the download source, or null to use
* httpUrl("source.txt").
*
* @return {Promise}
* @rejects JavaScript exception.
*/
function promiseAddDownloadToHistory(aSourceUrl) {
let deferred = Promise.defer();
PlacesUtils.asyncHistory.updatePlaces(
{
uri: NetUtil.newURI(aSourceUrl || httpUrl("source.txt")),
visits: [{
transitionType: Ci.nsINavHistoryService.TRANSITION_DOWNLOAD,
visitDate: Date.now()
}]
},
{
handleError: function handleError(aResultCode, aPlaceInfo) {
let ex = new Components.Exception("Unexpected error in adding visits.",
aResultCode);
deferred.reject(ex);
},
handleResult: function () {},
handleCompletion: function handleCompletion() {
deferred.resolve();
}
});
return deferred.promise;
}
/**
* Starts a socket listener that closes each incoming connection.
*

View File

@ -9,6 +9,62 @@
"use strict";
////////////////////////////////////////////////////////////////////////////////
//// Globals
/**
* Returns a PRTime in the past usable to add expirable visits.
*
* @note Expiration ignores any visit added in the last 7 days, but it's
* better be safe against DST issues, by going back one day more.
*/
function getExpirablePRTime()
{
let dateObj = new Date();
// Normalize to midnight
dateObj.setHours(0);
dateObj.setMinutes(0);
dateObj.setSeconds(0);
dateObj.setMilliseconds(0);
dateObj = new Date(dateObj.getTime() - 8 * 86400000);
return dateObj.getTime() * 1000;
}
/**
* Adds an expirable history visit for a download.
*
* @param aSourceUrl
* String containing the URI for the download source, or null to use
* httpUrl("source.txt").
*
* @return {Promise}
* @rejects JavaScript exception.
*/
function promiseExpirableDownloadVisit(aSourceUrl)
{
let deferred = Promise.defer();
PlacesUtils.asyncHistory.updatePlaces(
{
uri: NetUtil.newURI(aSourceUrl || httpUrl("source.txt")),
visits: [{
transitionType: Ci.nsINavHistoryService.TRANSITION_DOWNLOAD,
visitDate: getExpirablePRTime(),
}]
},
{
handleError: function handleError(aResultCode, aPlaceInfo) {
let ex = new Components.Exception("Unexpected error in adding visits.",
aResultCode);
deferred.reject(ex);
},
handleResult: function () {},
handleCompletion: function handleCompletion() {
deferred.resolve();
}
});
return deferred.promise;
}
////////////////////////////////////////////////////////////////////////////////
//// Tests
@ -205,6 +261,8 @@ add_task(function test_notifications_this()
*/
add_task(function test_history_expiration()
{
mustInterruptResponses();
function cleanup() {
Services.prefs.clearUserPref("places.history.expiration.max_pages");
}
@ -213,15 +271,9 @@ add_task(function test_history_expiration()
// Set max pages to 0 to make the download expire.
Services.prefs.setIntPref("places.history.expiration.max_pages", 0);
// Add expirable visit for downloads.
yield promiseAddDownloadToHistory();
yield promiseAddDownloadToHistory(httpUrl("interruptible.txt"));
let list = yield promiseNewDownloadList();
let downloadOne = yield promiseNewDownload();
let downloadTwo = yield promiseNewDownload(httpUrl("interruptible.txt"));
list.add(downloadOne);
list.add(downloadTwo);
let deferred = Promise.defer();
let removeNotifications = 0;
@ -234,18 +286,27 @@ add_task(function test_history_expiration()
};
list.addView(downloadView);
// Start download one.
// Work with one finished download and one canceled download.
yield downloadOne.start();
// Start download two and then cancel it.
downloadTwo.start();
yield downloadTwo.cancel();
// We must replace the visits added while executing the downloads with visits
// that are older than 7 days, otherwise they will not be expired.
yield promiseClearHistory();
yield promiseExpirableDownloadVisit();
yield promiseExpirableDownloadVisit(httpUrl("interruptible.txt"));
// After clearing history, we can add the downloads to be removed to the list.
list.add(downloadOne);
list.add(downloadTwo);
// Force a history expiration.
let expire = Cc["@mozilla.org/places/expiration;1"]
.getService(Ci.nsIObserver);
expire.observe(null, "places-debug-start-expiration", -1);
// Wait for both downloads to be removed.
yield deferred.promise;
cleanup();
@ -256,10 +317,6 @@ add_task(function test_history_expiration()
*/
add_task(function test_history_clear()
{
// Add expirable visit for downloads.
yield promiseAddDownloadToHistory();
yield promiseAddDownloadToHistory();
let list = yield promiseNewDownloadList();
let downloadOne = yield promiseNewDownload();
let downloadTwo = yield promiseNewDownload();
@ -280,8 +337,9 @@ add_task(function test_history_clear()
yield downloadOne.start();
yield downloadTwo.start();
PlacesUtils.history.removeAllPages();
yield promiseClearHistory();
// Wait for the removal notifications that may still be pending.
yield deferred.promise;
});