diff --git a/toolkit/components/jsdownloads/src/DownloadCore.jsm b/toolkit/components/jsdownloads/src/DownloadCore.jsm index 7a1280fe4b4..572b0fc3f15 100644 --- a/toolkit/components/jsdownloads/src/DownloadCore.jsm +++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm @@ -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); } diff --git a/toolkit/components/jsdownloads/src/DownloadLegacy.js b/toolkit/components/jsdownloads/src/DownloadLegacy.js index 4326c02c08c..b9c58fd549e 100644 --- a/toolkit/components/jsdownloads/src/DownloadLegacy.js +++ b/toolkit/components/jsdownloads/src/DownloadLegacy.js @@ -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 diff --git a/toolkit/components/jsdownloads/test/unit/common_test_Download.js b/toolkit/components/jsdownloads/test/unit/common_test_Download.js index 0adebf1e9e1..9c24fde7dc2 100644 --- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js +++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js @@ -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); +}); diff --git a/toolkit/components/jsdownloads/test/unit/head.js b/toolkit/components/jsdownloads/test/unit/head.js index a69a82c2786..f5c95821571 100644 --- a/toolkit/components/jsdownloads/test/unit/head.js +++ b/toolkit/components/jsdownloads/test/unit/head.js @@ -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. * diff --git a/toolkit/components/jsdownloads/test/unit/test_DownloadList.js b/toolkit/components/jsdownloads/test/unit/test_DownloadList.js index 209808112c3..5d201290792 100644 --- a/toolkit/components/jsdownloads/test/unit/test_DownloadList.js +++ b/toolkit/components/jsdownloads/test/unit/test_DownloadList.js @@ -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; });