Bug 1468980 - Change downloads code to save downloads via the new async history API. r=mak

MozReview-Commit-ID: BeKPtVH43RF

--HG--
extra : rebase_source : a12866b36a31e7036493451fcec3c3d0d929b079

browser/components/downloads/DownloadsCommon.jsm

@@ -749,7 +749,7 @@ DownloadsDataCtor.prototype = {
 
         // 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 ||

toolkit/components/downloads/DownloadCore.jsm

@@ -23,22 +23,17 @@ var EXPORTED_SYMBOLS = [
 ChromeUtils.import("resource://gre/modules/Integration.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
-ChromeUtils.defineModuleGetter(this, "FileUtils",
+XPCOMUtils.defineLazyModuleGetters(this, {
-                               "resource://gre/modules/FileUtils.jsm");
+  AppConstants: "resource://gre/modules/AppConstants.jsm",
-ChromeUtils.defineModuleGetter(this, "NetUtil",
+  DownloadHistory: "resource://gre/modules/DownloadHistory.jsm",
-                               "resource://gre/modules/NetUtil.jsm");
+  FileUtils: "resource://gre/modules/FileUtils.jsm",
-ChromeUtils.defineModuleGetter(this, "OS",
+  NetUtil: "resource://gre/modules/NetUtil.jsm",
-                               "resource://gre/modules/osfile.jsm");
+  OS: "resource://gre/modules/osfile.jsm",
-ChromeUtils.defineModuleGetter(this, "PromiseUtils",
+  PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
-                               "resource://gre/modules/PromiseUtils.jsm");
+  PromiseUtils: "resource://gre/modules/PromiseUtils.jsm",
-ChromeUtils.defineModuleGetter(this, "Services",
+  Services: "resource://gre/modules/Services.jsm",
-                               "resource://gre/modules/Services.jsm");
+});
-ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
-                               "resource://gre/modules/PrivateBrowsingUtils.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);
@@ -1718,34 +1713,8 @@ this.DownloadSaver.prototype = {
    * the download is private.
    */
   addToHistory() {
-    if (this.download.source.isPrivate) {
+    if (AppConstants.MOZ_PLACES) {
-      return;
+      DownloadHistory.addDownloadToHistory(this.download).catch(Cu.reportError);
-    }
-
-    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!
-        //
-      }
     }
   },
 

toolkit/components/downloads/DownloadHistory.jsm

@@ -18,15 +18,15 @@ var EXPORTED_SYMBOLS = [
 ];
 
 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",
+XPCOMUtils.defineLazyModuleGetters(this, {
-                               "resource://gre/modules/Downloads.jsm");
+  Downloads: "resource://gre/modules/Downloads.jsm",
-ChromeUtils.defineModuleGetter(this, "OS",
+  FileUtils: "resource://gre/modules/FileUtils.jsm",
-                               "resource://gre/modules/osfile.jsm");
+  OS: "resource://gre/modules/osfile.jsm",
-ChromeUtils.defineModuleGetter(this, "PlacesUtils",
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
-                               "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 =
@@ -86,6 +86,43 @@ var DownloadHistory = {
    */
   _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.
@@ -97,7 +134,7 @@ var DownloadHistory = {
    *        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;
     }
@@ -127,16 +164,24 @@ var DownloadHistory = {
     }
 
     try {
-      PlacesUtils.annotations.setPageAnnotation(
+      await PlacesUtils.history.update({
-                                 Services.io.newURI(download.source.url),
+        annotations: new Map([[METADATA_ANNO, JSON.stringify(metaData)]]),
-                                 METADATA_ANNO,
+        url: download.source.url,
-                                 JSON.stringify(metaData), 0,
+      });
-                                 PlacesUtils.annotations.EXPIRE_NEVER);
+
+      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:
@@ -448,7 +493,6 @@ this.DownloadHistoryList.prototype = {
     }
 
     if (this._result) {
-      PlacesUtils.annotations.removeObserver(this);
       this._result.removeObserver(this);
       this._result.root.containerOpen = false;
     }
@@ -457,11 +501,33 @@ this.DownloadHistoryList.prototype = {
 
     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.
@@ -609,35 +675,6 @@ this.DownloadHistoryList.prototype = {
   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;

toolkit/components/downloads/test/unit/common_test_Download.js

@@ -2405,13 +2405,16 @@ add_task(async function test_history() {
   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);
+  Assert.equal(lastKnownTitle, expectedFile.leafName);
 
-  let expectedFileURI = Services.io.newFileURI(new FileUtils.File(download.target.path));
+  let expectedFileURI = Services.io.newFileURI(expectedFile);
   let destFileURI = PlacesUtils.annotations.getPageAnnotation(
     Services.io.newURI(sourceUrl),
     "downloads/destinationFileURI");

toolkit/components/downloads/test/unit/head.js

@@ -36,6 +36,8 @@ 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",
@@ -152,7 +154,7 @@ function promiseWaitForVisit(aUrl) {
       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);
@@ -585,21 +587,10 @@ function isValidDate(aDate) {
  * because the addDownload method will add these to the database asynchronously.
  */
 function waitForAnnotation(sourceUriSpec, annotationName) {
-  let sourceUri = Services.io.newURI(sourceUriSpec);
+  return TestUtils.waitForCondition(async () => {
-  return new Promise(resolve => {
+    let pageInfo = await PlacesUtils.history.fetch(sourceUriSpec, {includeAnnotations: true});
-    PlacesUtils.annotations.addObserver({
+    return pageInfo.annotations.has(annotationName);
-      onPageAnnotationSet(page, name) {
+  }, `Should have found annotation ${annotationName} for ${sourceUriSpec}`);
-        if (!page.equals(sourceUri) || name != annotationName) {
-          return;
-        }
-        PlacesUtils.annotations.removeObserver(this);
-        resolve();
-      },
-      onItemAnnotationSet() {},
-      onPageAnnotationRemoved() {},
-      onItemAnnotationRemoved() {},
-    });
-  });
 }
 
 /**