From 73a3374dc8d99149a8674fc8788269db8f788816 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Mon, 9 Sep 2013 16:27:49 -0700 Subject: [PATCH] Backed out changeset 493dd25e60c2 (bug 908240) --- .../jsdownloads/src/DownloadCore.jsm | 60 +------- .../jsdownloads/src/DownloadLegacy.js | 21 +-- .../test/unit/common_test_Download.js | 57 -------- .../components/jsdownloads/test/unit/head.js | 129 +++++------------- .../test/unit/test_DownloadList.js | 86 ++---------- 5 files changed, 53 insertions(+), 300 deletions(-) diff --git a/toolkit/components/jsdownloads/src/DownloadCore.jsm b/toolkit/components/jsdownloads/src/DownloadCore.jsm index 572b0fc3f15..7a1280fe4b4 100644 --- a/toolkit/components/jsdownloads/src/DownloadCore.jsm +++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm @@ -67,9 +67,6 @@ 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); @@ -1208,30 +1205,6 @@ 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. * @@ -1293,11 +1266,6 @@ 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 @@ -1320,16 +1288,6 @@ 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 @@ -1675,14 +1633,8 @@ 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, aAlreadyAddedToHistory) + onTransferStarted: function (aRequest) { // Store the entity ID to use for resuming if required. if (this.download.tryToKeepPartialData && @@ -1693,15 +1645,6 @@ 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(); - } }, /** @@ -1759,7 +1702,6 @@ 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 b9c58fd549e..4326c02c08c 100644 --- a/toolkit/components/jsdownloads/src/DownloadLegacy.js +++ b/toolkit/components/jsdownloads/src/DownloadLegacy.js @@ -91,21 +91,16 @@ 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(download => { - download.saver.onTransferStarted( - aRequest, - this._cancelable instanceof Ci.nsIHelperAppLauncher); + this._deferDownload.promise.then(function (aDownload) { + aDownload.saver.onTransferStarted(aRequest); }).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(download => { - download.saver.onTransferFinished(aRequest, aStatus); + this._deferDownload.promise.then(function DLT_OSC_onDownload(aDownload) { + aDownload.saver.onTransferFinished(aRequest, aStatus); }).then(null, Cu.reportError); - - // Release the reference to the component executing the download. - this._cancelable = null; } }, @@ -169,8 +164,6 @@ 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) { @@ -240,12 +233,6 @@ 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 9c24fde7dc2..0adebf1e9e1 100644 --- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js +++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js @@ -1676,60 +1676,3 @@ 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 f5c95821571..a69a82c2786 100644 --- a/toolkit/components/jsdownloads/test/unit/head.js +++ b/toolkit/components/jsdownloads/test/unit/head.js @@ -169,101 +169,6 @@ 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. * @@ -536,6 +441,40 @@ 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 5d201290792..209808112c3 100644 --- a/toolkit/components/jsdownloads/test/unit/test_DownloadList.js +++ b/toolkit/components/jsdownloads/test/unit/test_DownloadList.js @@ -9,62 +9,6 @@ "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 @@ -261,8 +205,6 @@ add_task(function test_notifications_this() */ add_task(function test_history_expiration() { - mustInterruptResponses(); - function cleanup() { Services.prefs.clearUserPref("places.history.expiration.max_pages"); } @@ -271,9 +213,15 @@ 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; @@ -286,27 +234,18 @@ add_task(function test_history_expiration() }; list.addView(downloadView); - // Work with one finished download and one canceled download. + // Start download one. 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(); @@ -317,6 +256,10 @@ 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(); @@ -337,9 +280,8 @@ add_task(function test_history_clear() yield downloadOne.start(); yield downloadTwo.start(); - yield promiseClearHistory(); + PlacesUtils.history.removeAllPages(); - // Wait for the removal notifications that may still be pending. yield deferred.promise; });