Bug 1354532 - Part 4 - Move the Places metadata cache used by the Downloads library views to the DownloadsCommon singleton. r?paolo draft
authorMike de Boer <mdeboer@mozilla.com>
Tue, 11 Jul 2017 12:24:05 +0200
changeset 606722 0f336a4287370ebf86eb258b98186a3fe755081a
parent 606721 ec27c5015577faf4b82a170cfd0df1885f2f14b5
child 606723 aa893d1ce09cc3b8127ef7df3060ed43a4797942
push id67791
push usermdeboer@mozilla.com
push dateTue, 11 Jul 2017 10:33:03 +0000
reviewerspaolo
bugs1354532
milestone56.0a1
Bug 1354532 - Part 4 - Move the Places metadata cache used by the Downloads library views to the DownloadsCommon singleton. r?paolo This makes sure that the cache doesn't get reset when the library window is closed or duplicated when the library window is opened. MozReview-Commit-ID: 3J6KjNOInN2
browser/components/downloads/DownloadsCommon.jsm
browser/components/downloads/DownloadsViewUI.jsm
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -86,16 +86,19 @@ const kDownloadsStringsRequiringFormatti
 const kDownloadsStringsRequiringPluralForm = {
   otherDownloads3: true
 };
 
 const kPartialDownloadSuffix = ".part";
 
 const kPrefBranch = Services.prefs.getBranch("browser.download.");
 
+const DESTINATION_FILE_URI_ANNO = "downloads/destinationFileURI";
+const DOWNLOAD_META_DATA_ANNO   = "downloads/metaData";
+
 var PrefObserver = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
                                          Ci.nsISupportsWeakReference]),
   getPref(name) {
     try {
       switch (typeof this.prefs[name]) {
         case "boolean":
           return kPrefBranch.getBoolPref(name);
@@ -191,16 +194,94 @@ this.DownloadsCommon = {
    * Indicates whether we should show visual notification on the indicator
    * when a download event is triggered.
    */
   get animateNotifications() {
     return PrefObserver.animateNotifications;
   },
 
   /**
+   * This cache exists in order to optimize the load of the Downloads View, when
+   * Places annotations for history downloads must be read. In fact, annotations
+   * are stored in a single table, and reading all of them at once is much more
+   * efficient than an individual query.
+   *
+   * When this property is first requested, it reads the annotations for all the
+   * history downloads and stores them indefinitely.
+   *
+   * The historical annotations are not expected to change for the duration of
+   * the session, except in the case where a session download is running for the
+   * same URI as a history download. To ensure we don't use stale data, URIs
+   * corresponding to session downloads are permanently removed from the cache.
+   * This is a very small mumber compared to history downloads.
+   *
+   * This property returns a Map from each download source URI found in Places
+   * annotations to 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
+   * properties may be missing from the object.
+   */
+  get cachedPlacesMetaData() {
+    if (!this._cachedPlacesMetaData) {
+      this._cachedPlacesMetaData = new Map();
+
+      // Read the metadata annotations first, but ignore invalid JSON.
+      for (let result of PlacesUtils.annotations.getAnnotationsWithName(
+                                                 DOWNLOAD_META_DATA_ANNO)) {
+        try {
+          this._cachedPlacesMetaData.set(result.uri.spec,
+                                          JSON.parse(result.annotationValue));
+        } catch (ex) {}
+      }
+
+      // Add the target file annotations to the metadata.
+      for (let result of PlacesUtils.annotations.getAnnotationsWithName(
+                                                 DESTINATION_FILE_URI_ANNO)) {
+        let metaData = this._cachedPlacesMetaData.get(result.uri.spec);
+        if (!metaData) {
+          metaData = {};
+          this._cachedPlacesMetaData.set(result.uri.spec, metaData);
+        }
+        metaData.targetFileSpec = result.annotationValue;
+      }
+    }
+
+    return this._cachedPlacesMetaData;
+  },
+
+  /**
+   * 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
+   * properties may be missing from the object.
+   */
+  getPlacesMetaDataFor(spec) {
+    let metaData = {};
+
+    try {
+      let uri = NetUtil.newURI(spec);
+      try {
+        metaData = JSON.parse(PlacesUtils.annotations.getPageAnnotation(
+                                          uri, DOWNLOAD_META_DATA_ANNO));
+      } catch (ex) {}
+      metaData.targetFileSpec = PlacesUtils.annotations.getPageAnnotation(
+                                            uri, DESTINATION_FILE_URI_ANNO);
+    } catch (ex) {}
+
+    return metaData;
+  },
+
+  /**
    * Get access to one of the DownloadsData or PrivateDownloadsData objects,
    * depending on the privacy status of the window in question.
    *
    * @param aWindow
    *        The browser window which owns the download button.
    */
   getData(aWindow) {
     if (PrivateBrowsingUtils.isContentWindowPrivate(aWindow)) {
--- a/browser/components/downloads/DownloadsViewUI.jsm
+++ b/browser/components/downloads/DownloadsViewUI.jsm
@@ -30,19 +30,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
                                   "resource:///modules/RecentWindow.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 
-const DESTINATION_FILE_URI_ANNO  = "downloads/destinationFileURI";
-const DOWNLOAD_META_DATA_ANNO    = "downloads/metaData";
-
 this.DownloadsViewUI = {
   /**
    * Returns true if the given string is the name of a command that can be
    * handled by the Downloads user interface, including standard commands.
    */
   isCommandName(name) {
     return name.startsWith("cmd_") || name.startsWith("downloadsCmd_");
   },
@@ -57,18 +54,16 @@ this.DownloadsViewUI.BaseDownloadsPlaces
     this.window = window;
 
     // Map download URLs to download element shells regardless of their type
     this._downloadElementsShellsForURI = new Map();
 
     // Map download data items to their element shells.
     this._viewItemsForDownloads = new WeakMap();
 
-    this.__cachedPlacesMetaData = null;
-
     // Points to the last session download element. We keep track of this
    // in order to keep all session downloads above past downloads.
     this._lastSessionDownloadElement = null;
 
     this._searchTerm = "";
 
     this._active = active;
 
@@ -82,94 +77,16 @@ this.DownloadsViewUI.BaseDownloadsPlaces
     DownloadsCommon.getIndicatorData(window).attention = DownloadsCommon.ATTENTION_NONE;
   }
 
   destructor() {
     this._downloadsData.removeView(this);
   }
 
   /**
-   * This cache exists in order to optimize the load of the Downloads View, when
-   * Places annotations for history downloads must be read. In fact, annotations
-   * are stored in a single table, and reading all of them at once is much more
-   * efficient than an individual query.
-   *
-   * When this property is first requested, it reads the annotations for all the
-   * history downloads and stores them indefinitely.
-   *
-   * The historical annotations are not expected to change for the duration of
-   * the session, except in the case where a session download is running for the
-   * same URI as a history download. To ensure we don't use stale data, URIs
-   * corresponding to session downloads are permanently removed from the cache.
-   * This is a very small mumber compared to history downloads.
-   *
-   * This property returns a Map from each download source URI found in Places
-   * annotations to 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
-   * properties may be missing from the object.
-   */
-  get _cachedPlacesMetaData() {
-    if (!this.__cachedPlacesMetaData) {
-      this.__cachedPlacesMetaData = new Map();
-
-      // Read the metadata annotations first, but ignore invalid JSON.
-      for (let result of PlacesUtils.annotations.getAnnotationsWithName(
-                                                 DOWNLOAD_META_DATA_ANNO)) {
-        try {
-          this.__cachedPlacesMetaData.set(result.uri.spec,
-                                          JSON.parse(result.annotationValue));
-        } catch (ex) {}
-      }
-
-      // Add the target file annotations to the metadata.
-      for (let result of PlacesUtils.annotations.getAnnotationsWithName(
-                                                 DESTINATION_FILE_URI_ANNO)) {
-        let metaData = this.__cachedPlacesMetaData.get(result.uri.spec);
-        if (!metaData) {
-          metaData = {};
-          this.__cachedPlacesMetaData.set(result.uri.spec, metaData);
-        }
-        metaData.targetFileSpec = result.annotationValue;
-      }
-    }
-
-    return this.__cachedPlacesMetaData;
-  }
-
-  /**
-   * 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
-   * properties may be missing from the object.
-   */
-  _getPlacesMetaDataFor(spec) {
-    let metaData = {};
-
-    try {
-      let uri = NetUtil.newURI(spec);
-      try {
-        metaData = JSON.parse(PlacesUtils.annotations.getPageAnnotation(
-                                          uri, DOWNLOAD_META_DATA_ANNO));
-      } catch (ex) {}
-      metaData.targetFileSpec = PlacesUtils.annotations.getPageAnnotation(
-                                            uri, DESTINATION_FILE_URI_ANNO);
-    } catch (ex) {}
-
-    return metaData;
-  }
-
-  /**
    * Given a data item for a session download, or a places node for a past
    * download, updates the view as necessary.
    *  1. If the given data is a places node, we check whether there are any
    *     elements for the same download url. If there are, then we just reset
    *     their places node. Otherwise we add a new download element.
    *  2. If the given data is a data item, we first check if there's a history
    *     download in the list that is not associated with a data item. If we
    *     found one, we use it for the data item as well and reposition it
@@ -205,17 +122,17 @@ this.DownloadsViewUI.BaseDownloadsPlaces
     // stale metadata around for the corresponding history download. This
     // prevents stale state from being used if the view is rebuilt.
     //
     // Note that we will eagerly load the data in the cache at this point, even
     // if we have seen no history download. The case where no history download
     // will appear at all is rare enough in normal usage, so we can apply this
     // simpler solution rather than keeping a list of cache items to ignore.
     if (sessionDownload) {
-      this._cachedPlacesMetaData.delete(sessionDownload.source.url);
+      DownloadsCommon.cachedPlacesMetaData.delete(sessionDownload.source.url);
     }
 
     let newOrUpdatedShell = null;
 
     // Trivial: if there are no shells for this download URI, we always
     // need to create one.
     let shouldCreateShell = shellsForURI.size == 0;
 
@@ -251,18 +168,18 @@ this.DownloadsViewUI.BaseDownloadsPlaces
     }
 
     if (shouldCreateShell) {
       // If we are adding a new history download here, it means there is no
       // associated session download, thus we must read the Places metadata,
       // because it will not be obscured by the session download.
       let historyDownload = null;
       if (aPlacesNode) {
-        let metaData = this._cachedPlacesMetaData.get(aPlacesNode.uri) ||
-                       this._getPlacesMetaDataFor(aPlacesNode.uri);
+        let metaData = DownloadsCommon.cachedPlacesMetaData.get(aPlacesNode.uri) ||
+                       DownloadsCommon.getPlacesMetaDataFor(aPlacesNode.uri);
         historyDownload = new DownloadsViewUI.HistoryDownload(aPlacesNode);
         historyDownload.updateFromMetaData(metaData);
       }
       let shell = new DownloadsViewUI.HistoryDownloadElementShell(sessionDownload,
         historyDownload, this.window, element);
       shell.element._placesNode = aPlacesNode;
       newOrUpdatedShell = shell;
       shellsForURI.add(shell);
@@ -382,17 +299,17 @@ this.DownloadsViewUI.BaseDownloadsPlaces
       }
     } else {
       // We have one download element shell containing both a session download
       // and a history download, and we are now removing the session download.
       // Previously, we did not use the Places metadata because it was obscured
       // by the session download. Since this is no longer the case, we have to
       // read the latest metadata before removing the session download.
       let url = shell.historyDownload.source.url;
-      let metaData = this._getPlacesMetaDataFor(url);
+      let metaData = DownloadsCommon.getPlacesMetaDataFor(url);
       shell.historyDownload.updateFromMetaData(metaData);
       shell.sessionDownload = null;
       // Move it below the session-download items;
       if (this._lastSessionDownloadElement == shell.element) {
         this._lastSessionDownloadElement = shell.element.previousSibling;
       } else {
         let before = null;
         if (this._lastSessionDownloadElement) {