Bug 1468980 - Change downloads code to save downloads via the new async history API. r?mak
MozReview-Commit-ID: BeKPtVH43RF
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -744,17 +744,17 @@ DownloadsDataCtor.prototype = {
if (download.succeeded ||
(download.canceled && !download.hasPartialData) ||
download.error) {
// Store the end time that may be displayed by the views.
download.endTime = Date.now();
// This state transition code should actually be located in a Downloads
// API module (bug 941009).
- DownloadHistory.updateMetaData(download);
+ DownloadHistory.updateMetaData(download).catch(Cu.reportError);
}
if (download.succeeded ||
(download.error && download.error.becauseBlocked)) {
this._notifyDownloadEvent("finish");
}
}
--- a/toolkit/components/downloads/DownloadCore.jsm
+++ b/toolkit/components/downloads/DownloadCore.jsm
@@ -18,32 +18,27 @@ var EXPORTED_SYMBOLS = [
"DownloadCopySaver",
"DownloadLegacySaver",
"DownloadPDFSaver",
];
ChromeUtils.import("resource://gre/modules/Integration.jsm");
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "FileUtils",
- "resource://gre/modules/FileUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "NetUtil",
- "resource://gre/modules/NetUtil.jsm");
-ChromeUtils.defineModuleGetter(this, "OS",
- "resource://gre/modules/osfile.jsm");
-ChromeUtils.defineModuleGetter(this, "PromiseUtils",
- "resource://gre/modules/PromiseUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "Services",
- "resource://gre/modules/Services.jsm");
-ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
- "resource://gre/modules/PrivateBrowsingUtils.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+ AppConstants: "resource://gre/modules/AppConstants.jsm",
+ DownloadHistory: "resource://gre/modules/DownloadHistory.jsm",
+ FileUtils: "resource://gre/modules/FileUtils.jsm",
+ NetUtil: "resource://gre/modules/NetUtil.jsm",
+ OS: "resource://gre/modules/osfile.jsm",
+ PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
+ PromiseUtils: "resource://gre/modules/PromiseUtils.jsm",
+ Services: "resource://gre/modules/Services.jsm",
+});
-XPCOMUtils.defineLazyServiceGetter(this, "gDownloadHistory",
- "@mozilla.org/browser/download-history;1",
- Ci.nsIDownloadHistory);
XPCOMUtils.defineLazyServiceGetter(this, "gExternalAppLauncher",
"@mozilla.org/uriloader/external-helper-app-service;1",
Ci.nsPIExternalAppLauncher);
XPCOMUtils.defineLazyServiceGetter(this, "gExternalHelperAppService",
"@mozilla.org/uriloader/external-helper-app-service;1",
Ci.nsIExternalHelperAppService);
XPCOMUtils.defineLazyServiceGetter(this, "gPrintSettingsService",
"@mozilla.org/gfx/printsettings-service;1",
@@ -1713,44 +1708,18 @@ this.DownloadSaver.prototype = {
async removeData() {},
/**
* 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() {
- 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;
-
- if ("@mozilla.org/browser/download-history;1" in Cc) {
- try {
- gDownloadHistory.addDownload(sourceUri, referrerUri, startPRTime,
- targetUri);
- } catch (ex) {
- if (!(ex instanceof Components.Exception) ||
- ex.result != Cr.NS_ERROR_NOT_AVAILABLE) {
- throw ex;
- }
- //
- // Under normal operation the download history service may not
- // be available. We don't want all downloads that are public to fail
- // when this happens so we'll ignore this error and this error only!
- //
- }
+ if (AppConstants.MOZ_PLACES) {
+ DownloadHistory.addDownloadToHistory(this.download).catch(Cu.reportError);
}
},
/**
* Returns a static representation of the current object state.
*
* @return A JavaScript object that can be serialized to JSON.
*/
--- a/toolkit/components/downloads/DownloadHistory.jsm
+++ b/toolkit/components/downloads/DownloadHistory.jsm
@@ -13,25 +13,25 @@
"use strict";
var EXPORTED_SYMBOLS = [
"DownloadHistory",
];
ChromeUtils.import("resource://gre/modules/DownloadList.jsm");
-ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "Downloads",
- "resource://gre/modules/Downloads.jsm");
-ChromeUtils.defineModuleGetter(this, "OS",
- "resource://gre/modules/osfile.jsm");
-ChromeUtils.defineModuleGetter(this, "PlacesUtils",
- "resource://gre/modules/PlacesUtils.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+ Downloads: "resource://gre/modules/Downloads.jsm",
+ FileUtils: "resource://gre/modules/FileUtils.jsm",
+ OS: "resource://gre/modules/osfile.jsm",
+ PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
+ Services: "resource://gre/modules/Services.jsm",
+});
// Places query used to retrieve all history downloads for the related list.
const HISTORY_PLACES_QUERY =
"place:transition=" + Ci.nsINavHistoryService.TRANSITION_DOWNLOAD +
"&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_DESCENDING;
const DESTINATIONFILEURI_ANNO = "downloads/destinationFileURI";
const METADATA_ANNO = "downloads/metaData";
@@ -81,28 +81,65 @@ var DownloadHistory = {
/**
* This object is populated with one key for each type of download list that
* can be returned by the getList method. The values are promises that resolve
* to DownloadHistoryList objects.
*/
_listPromises: {},
+ async addDownloadToHistory(download) {
+ if (download.source.isPrivate ||
+ !PlacesUtils.history.canAddURI(PlacesUtils.toURI(download.source.url))) {
+ return;
+ }
+
+ let targetFile = new FileUtils.File(download.target.path);
+ let targetUri = Services.io.newFileURI(targetFile);
+
+ let originalPageInfo = await PlacesUtils.history.fetch(download.source.url);
+
+ let pageInfo = await PlacesUtils.history.insert({
+ url: download.source.url,
+ // In case we are downloading a file that does not correspond to a web
+ // page for which the title is present, we populate the otherwise empty
+ // history title with the name of the destination file, to allow it to be
+ // visible and searchable in history results.
+ title: (originalPageInfo && originalPageInfo.title) || targetFile.leafName,
+ visits: [{
+ // The start time is always available when we reach this point.
+ date: download.startTime,
+ transition: PlacesUtils.history.TRANSITIONS.DOWNLOAD,
+ referrer: download.source.referrer,
+ }]
+ });
+
+ await PlacesUtils.history.update({
+ annotations: new Map([["downloads/destinationFileURI", targetUri.spec]]),
+ // XXX Bug 1479445: We shouldn't have to supply both guid and url here,
+ // but currently we do.
+ guid: pageInfo.guid,
+ url: pageInfo.url,
+ });
+
+ await this._updateHistoryListData(download.source.url);
+ },
+
/**
* Stores new detailed metadata for the given download in history. This is
* normally called after a download finishes, fails, or is canceled.
*
* Failed or canceled downloads with partial data are not stored as paused,
* because the information from the session download is required for resuming.
*
* @param download
* Download object whose metadata should be updated. If the object
* represents a private download, the call has no effect.
*/
- updateMetaData(download) {
+ async updateMetaData(download) {
if (download.source.isPrivate || !download.stopped) {
return;
}
let state = METADATA_STATE_CANCELED;
if (download.succeeded) {
state = METADATA_STATE_FINISHED;
} else if (download.error) {
@@ -122,26 +159,34 @@ var DownloadHistory = {
// The verdict may still be present even if the download succeeded.
if (download.error && download.error.reputationCheckVerdict) {
metaData.reputationCheckVerdict =
download.error.reputationCheckVerdict;
}
try {
- PlacesUtils.annotations.setPageAnnotation(
- Services.io.newURI(download.source.url),
- METADATA_ANNO,
- JSON.stringify(metaData), 0,
- PlacesUtils.annotations.EXPIRE_NEVER);
+ await PlacesUtils.history.update({
+ annotations: new Map([[METADATA_ANNO, JSON.stringify(metaData)]]),
+ url: download.source.url,
+ });
+
+ await this._updateHistoryListData(download.source.url);
} catch (ex) {
Cu.reportError(ex);
}
},
+ async _updateHistoryListData(sourceUrl) {
+ for (let key of Object.getOwnPropertyNames(this._listPromises)) {
+ let downloadHistoryList = await this._listPromises[key];
+ downloadHistoryList.updateForMetadataChange(sourceUrl);
+ }
+ },
+
/**
* Reads current metadata from Places annotations for the specified URI, and
* returns an object with the format:
*
* { targetFileSpec, state, endTime, fileSize, ... }
*
* The targetFileSpec property is the value of "downloads/destinationFileURI",
* while the other properties are taken from "downloads/metaData". Any of the
@@ -443,31 +488,52 @@ this.DownloadHistoryList.prototype = {
return this._result;
},
set result(result) {
if (this._result == result) {
return;
}
if (this._result) {
- PlacesUtils.annotations.removeObserver(this);
this._result.removeObserver(this);
this._result.root.containerOpen = false;
}
this._result = result;
if (this._result) {
this._result.root.containerOpen = true;
- PlacesUtils.annotations.addObserver(this);
}
},
_result: null,
/**
+ * Updates the download history item when the meta data or destination file
+ * changes.
+ *
+ * @param {String} sourceUrl The sourceUrl which was updated.
+ */
+ updateForMetadataChange(sourceUrl) {
+ let slotsForUrl = this._slotsForUrl.get(sourceUrl);
+ if (!slotsForUrl) {
+ return;
+ }
+
+ for (let slot of slotsForUrl) {
+ if (slot.sessionDownload) {
+ // The visible data doesn't change, so we don't have to notify views.
+ return;
+ }
+ slot.historyDownload.updateFromMetaData(
+ DownloadHistory.getPlacesMetaDataFor(sourceUrl));
+ this._notifyAllViews("onDownloadChanged", slot.download);
+ }
+ },
+
+ /**
* Index of the first slot that contains a session download. This is equal to
* the length of the list when there are no session downloads.
*/
_firstSessionSlotIndex: 0,
_insertSlot({ slot, index, slotsForUrl }) {
// Add the slot to the ordered array.
this._slots.splice(index, 0, slot);
@@ -604,45 +670,16 @@ this.DownloadHistoryList.prototype = {
nodeLastModifiedChanged() {},
nodeHistoryDetailsChanged() {},
nodeTagsChanged() {},
sortingChanged() {},
nodeMoved() {},
nodeURIChanged() {},
batching() {},
- // nsIAnnotationObserver
- onPageAnnotationSet(page, name) {
- // Annotations can only be added after a history node has been added, so we
- // have to listen for changes to nodes we already added to the list.
- if (name != DESTINATIONFILEURI_ANNO && name != METADATA_ANNO) {
- return;
- }
-
- let slotsForUrl = this._slotsForUrl.get(page.spec);
- if (!slotsForUrl) {
- return;
- }
-
- for (let slot of slotsForUrl) {
- if (slot.sessionDownload) {
- // The visible data doesn't change, so we don't have to notify views.
- return;
- }
- slot.historyDownload.updateFromMetaData(
- DownloadHistory.getPlacesMetaDataFor(page.spec));
- this._notifyAllViews("onDownloadChanged", slot.download);
- }
- },
-
- // nsIAnnotationObserver
- onItemAnnotationSet() {},
- onPageAnnotationRemoved() {},
- onItemAnnotationRemoved() {},
-
// DownloadList callback
onDownloadAdded(download) {
let url = download.source.url;
let slotsForUrl = this._slotsForUrl.get(url) || new Set();
// When a session download is attached to a slot, we ensure not to keep
// stale metadata around for the corresponding history download. This
// prevents stale state from being used if the view is rebuilt.
--- a/toolkit/components/downloads/test/unit/common_test_Download.js
+++ b/toolkit/components/downloads/test/unit/common_test_Download.js
@@ -2400,23 +2400,26 @@ add_task(async function test_history() {
await PlacesUtils.history.clear();
let promiseVisit = promiseWaitForVisit(sourceUrl);
let promiseAnnotation = waitForAnnotation(sourceUrl, "downloads/destinationFileURI");
// Start a download that is not allowed to finish yet.
let download = await promiseStartDownload(sourceUrl);
// The history and annotation notifications should be received before the download completes.
- let [time, transitionType] = await promiseVisit;
+ let [time, transitionType, lastKnownTitle] = await promiseVisit;
await promiseAnnotation;
+ let expectedFile = new FileUtils.File(download.target.path);
+
Assert.equal(time, download.startTime.getTime());
Assert.equal(transitionType, Ci.nsINavHistoryService.TRANSITION_DOWNLOAD);
-
- let expectedFileURI = Services.io.newFileURI(new FileUtils.File(download.target.path));
+ Assert.equal(lastKnownTitle, expectedFile.leafName);
+
+ let expectedFileURI = Services.io.newFileURI(expectedFile);
let destFileURI = PlacesUtils.annotations.getPageAnnotation(
Services.io.newURI(sourceUrl),
"downloads/destinationFileURI");
Assert.equal(destFileURI, expectedFileURI.spec,
"Should have saved the correct download target annotation.");
// Restart and complete the download after clearing history.
await PlacesUtils.history.clear();
--- a/toolkit/components/downloads/test/unit/head.js
+++ b/toolkit/components/downloads/test/unit/head.js
@@ -31,16 +31,18 @@ ChromeUtils.defineModuleGetter(this, "Pr
ChromeUtils.defineModuleGetter(this, "Services",
"resource://gre/modules/Services.jsm");
ChromeUtils.defineModuleGetter(this, "OS",
"resource://gre/modules/osfile.jsm");
ChromeUtils.defineModuleGetter(this, "FileTestUtils",
"resource://testing-common/FileTestUtils.jsm");
ChromeUtils.defineModuleGetter(this, "MockRegistrar",
"resource://testing-common/MockRegistrar.jsm");
+ChromeUtils.defineModuleGetter(this, "TestUtils",
+ "resource://testing-common/TestUtils.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "gExternalHelperAppService",
"@mozilla.org/uriloader/external-helper-app-service;1",
Ci.nsIExternalHelperAppService);
/* global DownloadIntegration */
Integration.downloads.defineModuleGetter(this, "DownloadIntegration",
"resource://gre/modules/DownloadIntegration.jsm");
@@ -147,17 +149,17 @@ function promiseTimeout(aTime) {
function promiseWaitForVisit(aUrl) {
return new Promise(resolve => {
function listener(aEvents) {
Assert.equal(aEvents.length, 1);
let event = aEvents[0];
Assert.equal(event.type, "page-visited");
if (event.url == aUrl) {
PlacesObservers.removeListener(["page-visited"], listener);
- resolve([event.visitTime, event.transitionType]);
+ resolve([event.visitTime, event.transitionType, event.lastKnownTitle]);
}
}
PlacesObservers.addListener(["page-visited"], listener);
});
}
/**
* Creates a new Download object, setting a temporary file as the target.
@@ -580,31 +582,20 @@ function isValidDate(aDate) {
return aDate && aDate.getTime && !isNaN(aDate.getTime());
}
/**
* Waits for the download annotations to be set for the given page, required
* because the addDownload method will add these to the database asynchronously.
*/
function waitForAnnotation(sourceUriSpec, annotationName) {
- let sourceUri = Services.io.newURI(sourceUriSpec);
- return new Promise(resolve => {
- PlacesUtils.annotations.addObserver({
- onPageAnnotationSet(page, name) {
- if (!page.equals(sourceUri) || name != annotationName) {
- return;
- }
- PlacesUtils.annotations.removeObserver(this);
- resolve();
- },
- onItemAnnotationSet() {},
- onPageAnnotationRemoved() {},
- onItemAnnotationRemoved() {},
- });
- });
+ return TestUtils.waitForCondition(async () => {
+ let pageInfo = await PlacesUtils.history.fetch(sourceUriSpec, {includeAnnotations: true});
+ return pageInfo.annotations.has(annotationName);
+ }, `Should have found annotation ${annotationName} for ${sourceUriSpec}`);
}
/**
* Position of the first byte served by the "interruptible_resumable.txt"
* handler during the most recent response.
*/
var gMostRecentFirstBytePos;