Bug 1412143 - Add `mozIAsyncLivemarks.invalidateCachedLivemarks`. r=mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Fri, 08 Sep 2017 15:27:32 -0700
changeset 699781 5b230d59b061ab12cd63d3f10d43487e03929625
parent 699771 304581a867a15234b05634fb750e38ba867f975e
child 740727 7398d712bc572d4676df5df0296bdedc71c45e84
push id89680
push userbmo:kit@mozilla.com
push dateFri, 17 Nov 2017 19:10:36 +0000
reviewersmak
bugs1412143
milestone59.0a1
Bug 1412143 - Add `mozIAsyncLivemarks.invalidateCachedLivemarks`. r=mak This patch uses a promise queue to serialize reads and writes to the livemarks cache. MozReview-Commit-ID: 8R7N6ORxrtV
toolkit/components/places/mozIAsyncLivemarks.idl
toolkit/components/places/nsLivemarkService.js
toolkit/components/places/tests/unit/test_async_transactions.js
toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js
--- a/toolkit/components/places/mozIAsyncLivemarks.idl
+++ b/toolkit/components/places/mozIAsyncLivemarks.idl
@@ -57,16 +57,18 @@ interface mozIAsyncLivemarks : nsISuppor
    *
    * @param [optional]aForceUpdate
    *        If set to true forces a reload even if contents are still valid.
    *
    * @note The update process is asynchronous, observers registered through
    *       registerForUpdates will be notified of updated contents.
    */
   void reloadLivemarks([optional]in boolean aForceUpdate);
+
+  jsval invalidateCachedLivemarks();
 };
 
 [scriptable, uuid(3a3c5e8f-ec4a-4086-ae0a-d16420d30c9f)]
 interface mozILivemarkInfo : nsISupports
 {
   /**
    * Id of the bookmarks folder representing this livemark.
    *
--- a/toolkit/components/places/nsLivemarkService.js
+++ b/toolkit/components/places/nsLivemarkService.js
@@ -46,44 +46,16 @@ XPCOMUtils.defineLazyGetter(this, "CACHE
           FROM moz_bookmarks b
           JOIN moz_bookmarks p ON b.parent = p.id
           JOIN moz_items_annos a ON a.item_id = b.id
           JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
           WHERE b.type = :folder_type
             AND n.name = :feedURI_anno`;
 });
 
-XPCOMUtils.defineLazyGetter(this, "gLivemarksCachePromised",
-  async function() {
-    let livemarksMap = new Map();
-    let conn = await PlacesUtils.promiseDBConnection();
-    let rows = await conn.executeCached(CACHE_SQL,
-      { folder_type: Ci.nsINavBookmarksService.TYPE_FOLDER,
-        feedURI_anno: PlacesUtils.LMANNO_FEEDURI,
-        siteURI_anno: PlacesUtils.LMANNO_SITEURI });
-    for (let row of rows) {
-      let siteURI = row.getResultByName("siteURI");
-      let livemark = new Livemark({
-        id: row.getResultByName("id"),
-        guid: row.getResultByName("guid"),
-        title: row.getResultByName("title"),
-        parentId: row.getResultByName("parentId"),
-        parentGuid: row.getResultByName("parentGuid"),
-        index: row.getResultByName("index"),
-        dateAdded: row.getResultByName("dateAdded"),
-        lastModified: row.getResultByName("lastModified"),
-        feedURI: NetUtil.newURI(row.getResultByName("feedURI")),
-        siteURI: siteURI ? NetUtil.newURI(siteURI) : null
-      });
-      livemarksMap.set(livemark.guid, livemark);
-    }
-    return livemarksMap;
-  }
-);
-
 /**
  * Convert a Date object to a PRTime (microseconds).
  *
  * @param date
  *        the Date object to convert.
  * @return microseconds from the epoch.
  */
 function toPRTime(date) {
@@ -104,21 +76,53 @@ function toDate(time) {
 // LivemarkService
 
 function LivemarkService() {
   // Cleanup on shutdown.
   Services.obs.addObserver(this, PlacesUtils.TOPIC_SHUTDOWN, true);
 
   // Observe bookmarks but don't init the service just for that.
   PlacesUtils.bookmarks.addObserver(this, true);
+
+  this._livemarksMap = null;
+  this._promiseLivemarksMapReady = Promise.resolve();
 }
 
 LivemarkService.prototype = {
-  // This is just an helper for code readability.
-  _promiseLivemarksMap: () => gLivemarksCachePromised,
+  _withLivemarksMap(func) {
+    let promise = this._promiseLivemarksMapReady.then(async () => {
+      if (!this._livemarksMap) {
+        this._livemarksMap = new Map();
+        let conn = await PlacesUtils.promiseDBConnection();
+        let rows = await conn.executeCached(CACHE_SQL,
+          { folder_type: Ci.nsINavBookmarksService.TYPE_FOLDER,
+            feedURI_anno: PlacesUtils.LMANNO_FEEDURI,
+            siteURI_anno: PlacesUtils.LMANNO_SITEURI });
+        for (let row of rows) {
+          let siteURI = row.getResultByName("siteURI");
+          let livemark = new Livemark({
+            id: row.getResultByName("id"),
+            guid: row.getResultByName("guid"),
+            title: row.getResultByName("title"),
+            parentId: row.getResultByName("parentId"),
+            parentGuid: row.getResultByName("parentGuid"),
+            index: row.getResultByName("index"),
+            dateAdded: row.getResultByName("dateAdded"),
+            lastModified: row.getResultByName("lastModified"),
+            feedURI: NetUtil.newURI(row.getResultByName("feedURI")),
+            siteURI: siteURI ? NetUtil.newURI(siteURI) : null
+          });
+          this._livemarksMap.set(livemark.guid, livemark);
+        }
+      }
+      return func(this._livemarksMap);
+    });
+    this._promiseLivemarksMapReady = promise.catch(Cu.reportError);
+    return promise;
+  },
 
   _reloading: false,
   _startReloadTimer(livemarksMap, forceUpdate, reloaded) {
     if (this._reloadTimer) {
       this._reloadTimer.cancel();
     } else {
       this._reloadTimer = Cc["@mozilla.org/timer;1"]
                             .createInstance(Ci.nsITimer);
@@ -132,35 +136,28 @@ LivemarkService.prototype = {
           reloaded.add(guid);
           livemark.reload(forceUpdate);
           this._startReloadTimer(livemarksMap, forceUpdate, reloaded);
           return;
         }
       }
       // All livemarks have been reloaded.
       this._reloading = false;
+      this._forceUpdate = false;
     }, RELOAD_DELAY_MS, Ci.nsITimer.TYPE_ONE_SHOT);
   },
 
   // nsIObserver
 
   observe(aSubject, aTopic, aData) {
     if (aTopic == PlacesUtils.TOPIC_SHUTDOWN) {
-      if (this._reloadTimer) {
-        this._reloading = false;
-        this._reloadTimer.cancel();
-        delete this._reloadTimer;
-      }
-
-      // Stop any ongoing network fetch.
-      this._promiseLivemarksMap().then(livemarksMap => {
-        for (let livemark of livemarksMap.values()) {
-          livemark.terminate();
-        }
-      });
+      this._invalidateCachedLivemarks({
+        // No need to restart the reload timer on shutdown.
+        keepReloading: false,
+      }).catch(Cu.reportError);
     }
   },
 
   // mozIAsyncLivemarks
 
   addLivemark(aLivemarkInfo) {
     if (!aLivemarkInfo) {
       throw new Components.Exception("Invalid arguments", Cr.NS_ERROR_INVALID_ARG);
@@ -174,22 +171,20 @@ LivemarkService.prototype = {
         (hasParentGuid && !/^[a-zA-Z0-9\-_]{12}$/.test(aLivemarkInfo.parentGuid)) ||
         (hasIndex && aLivemarkInfo.index < Ci.nsINavBookmarksService.DEFAULT_INDEX) ||
         !(aLivemarkInfo.feedURI instanceof Ci.nsIURI) ||
         (aLivemarkInfo.siteURI && !(aLivemarkInfo.siteURI instanceof Ci.nsIURI)) ||
         (aLivemarkInfo.guid && !/^[a-zA-Z0-9\-_]{12}$/.test(aLivemarkInfo.guid))) {
       throw new Components.Exception("Invalid arguments", Cr.NS_ERROR_INVALID_ARG);
     }
 
-    return (async () => {
+    return this._withLivemarksMap(async livemarksMap => {
       if (!aLivemarkInfo.parentGuid)
         aLivemarkInfo.parentGuid = await PlacesUtils.promiseItemGuid(aLivemarkInfo.parentId);
 
-      let livemarksMap = await this._promiseLivemarksMap();
-
       // Disallow adding a livemark inside another livemark.
       if (livemarksMap.has(aLivemarkInfo.parentGuid)) {
         throw new Components.Exception("Cannot create a livemark inside a livemark", Cr.NS_ERROR_INVALID_ARG);
       }
 
       // Create a new livemark.
       let folder = await PlacesUtils.bookmarks.insert({
         type: PlacesUtils.bookmarks.TYPE_FOLDER,
@@ -227,56 +222,55 @@ LivemarkService.prototype = {
                                              lastModified: toDate(aLivemarkInfo.lastModified),
                                              source: aLivemarkInfo.source });
         livemark.lastModified = aLivemarkInfo.lastModified;
       }
 
       livemarksMap.set(folder.guid, livemark);
 
       return livemark;
-    })();
+    });
   },
 
   removeLivemark(aLivemarkInfo) {
     if (!aLivemarkInfo) {
       throw new Components.Exception("Invalid arguments", Cr.NS_ERROR_INVALID_ARG);
     }
     // Accept either a guid or an id.
     let hasGuid = "guid" in aLivemarkInfo;
     let hasId = "id" in aLivemarkInfo;
     if ((hasGuid && !/^[a-zA-Z0-9\-_]{12}$/.test(aLivemarkInfo.guid)) ||
         (hasId && aLivemarkInfo.id < 1) ||
         (!hasId && !hasGuid)) {
       throw new Components.Exception("Invalid arguments", Cr.NS_ERROR_INVALID_ARG);
     }
 
-    return (async () => {
+    return this._withLivemarksMap(async livemarksMap => {
       if (!aLivemarkInfo.guid)
         aLivemarkInfo.guid = await PlacesUtils.promiseItemGuid(aLivemarkInfo.id);
 
-      let livemarksMap = await this._promiseLivemarksMap();
       if (!livemarksMap.has(aLivemarkInfo.guid))
         throw new Components.Exception("Invalid livemark", Cr.NS_ERROR_INVALID_ARG);
 
       await PlacesUtils.bookmarks.remove(aLivemarkInfo.guid,
                                          { source: aLivemarkInfo.source });
-    })();
+    });
   },
 
   reloadLivemarks(aForceUpdate) {
     // Check if there's a currently running reload, to save some useless work.
     let notWorthRestarting =
       this._forceUpdate || // We're already forceUpdating.
       !aForceUpdate; // The caller didn't request a forced update.
     if (this._reloading && notWorthRestarting) {
       // Ignore this call.
       return;
     }
 
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       this._forceUpdate = !!aForceUpdate;
       // Livemarks reloads happen on a timer for performance reasons.
       this._startReloadTimer(livemarksMap, this._forceUpdate, new Set());
     });
   },
 
   getLivemark(aLivemarkInfo) {
     if (!aLivemarkInfo) {
@@ -286,71 +280,110 @@ LivemarkService.prototype = {
     let hasGuid = "guid" in aLivemarkInfo;
     let hasId = "id" in aLivemarkInfo;
     if ((hasGuid && !/^[a-zA-Z0-9\-_]{12}$/.test(aLivemarkInfo.guid)) ||
         (hasId && aLivemarkInfo.id < 1) ||
         (!hasId && !hasGuid)) {
       throw new Components.Exception("Invalid arguments", Cr.NS_ERROR_INVALID_ARG);
     }
 
-    return (async () => {
+    return this._withLivemarksMap(async livemarksMap => {
       if (!aLivemarkInfo.guid)
         aLivemarkInfo.guid = await PlacesUtils.promiseItemGuid(aLivemarkInfo.id);
 
-      let livemarksMap = await this._promiseLivemarksMap();
       if (!livemarksMap.has(aLivemarkInfo.guid))
         throw new Components.Exception("Invalid livemark", Cr.NS_ERROR_INVALID_ARG);
 
       return livemarksMap.get(aLivemarkInfo.guid);
-    })();
+    });
+  },
+
+  _invalidateCachedLivemarks({ keepReloading = true } = {}) {
+    // Cancel pending reloads, since any livemarks we're currently reloading
+    // might no longer be valid.
+    let wasReloading = this._reloading;
+    this._reloading = false;
+
+    let wasForceUpdating = this._forceUpdate;
+    this._forceUpdate = false;
+
+    if (this._reloadTimer) {
+      this._reloadTimer.cancel();
+    }
+
+    // Clear out the livemarks cache.
+    let promise = this._promiseLivemarksMapReady.then(() => {
+      let livemarksMap = this._livemarksMap;
+      this._livemarksMap = null;
+      if (livemarksMap) {
+        // Stop any ongoing network fetch.
+        for (let livemark of livemarksMap.values()) {
+          livemark.terminate();
+        }
+      }
+    });
+    this._promiseLivemarksMapReady = promise.catch(Cu.reportError);
+
+    // Restart the timer if we were reloading before invalidating.
+    if (keepReloading) {
+      if (wasReloading) {
+        this.reloadLivemarks(wasForceUpdating);
+      }
+    } else {
+      delete this._reloadTimer;
+    }
+  },
+
+  invalidateCachedLivemarks() {
+    return this._invalidateCachedLivemarks();
   },
 
   // nsINavBookmarkObserver
 
   onBeginUpdateBatch() {},
   onEndUpdateBatch() {},
   onItemVisited() {},
   onItemAdded() {},
 
   onItemChanged(id, property, isAnno, value, lastModified, itemType, parentId,
                 guid, parentGuid) {
     if (itemType != Ci.nsINavBookmarksService.TYPE_FOLDER)
       return;
 
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       if (livemarksMap.has(guid)) {
         let livemark = livemarksMap.get(guid);
         if (property == "title") {
           livemark.title = value;
         }
         livemark.lastModified = lastModified;
       }
     });
   },
 
   onItemMoved(id, parentId, oldIndex, newParentId, newIndex, itemType, guid,
               oldParentGuid, newParentGuid) {
     if (itemType != Ci.nsINavBookmarksService.TYPE_FOLDER)
       return;
 
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       if (livemarksMap.has(guid)) {
         let livemark = livemarksMap.get(guid);
         livemark.parentId = newParentId;
         livemark.parentGuid = newParentGuid;
         livemark.index = newIndex;
       }
     });
   },
 
   onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid) {
     if (itemType != Ci.nsINavBookmarksService.TYPE_FOLDER)
       return;
 
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       if (livemarksMap.has(guid)) {
         let livemark = livemarksMap.get(guid);
         livemark.terminate();
         livemarksMap.delete(guid);
       }
     });
   },
 
@@ -359,33 +392,33 @@ LivemarkService.prototype = {
 
   // nsINavHistoryObserver
 
   onPageChanged() {},
   onTitleChanged() {},
   onDeleteVisits() {},
 
   onClearHistory() {
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       for (let livemark of livemarksMap.values()) {
         livemark.updateURIVisitedStatus(null, false);
       }
     });
   },
 
   onDeleteURI(aURI) {
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       for (let livemark of livemarksMap.values()) {
         livemark.updateURIVisitedStatus(aURI, false);
       }
     });
   },
 
   onVisit(aURI) {
-    this._promiseLivemarksMap().then(livemarksMap => {
+    this._withLivemarksMap(livemarksMap => {
       for (let livemark of livemarksMap.values()) {
         livemark.updateURIVisitedStatus(aURI, true);
       }
     });
   },
 
   // nsISupports
 
@@ -454,17 +487,17 @@ Livemark.prototype = {
     return this._status;
   },
 
   writeFeedURI(aFeedURI, aSource) {
     PlacesUtils.annotations
                .setItemAnnotation(this.id, PlacesUtils.LMANNO_FEEDURI,
                                   aFeedURI.spec,
                                   0, PlacesUtils.annotations.EXPIRE_NEVER,
-                                  aSource);
+                                  aSource, true);
     this.feedURI = aFeedURI;
   },
 
   writeSiteURI(aSiteURI, aSource) {
     if (!aSiteURI) {
       PlacesUtils.annotations.removeItemAnnotation(this.id,
                                                    PlacesUtils.LMANNO_SITEURI,
                                                    aSource);
@@ -481,17 +514,17 @@ Livemark.prototype = {
     } catch (ex) {
       return;
     }
 
     PlacesUtils.annotations
                .setItemAnnotation(this.id, PlacesUtils.LMANNO_SITEURI,
                                   aSiteURI.spec,
                                   0, PlacesUtils.annotations.EXPIRE_NEVER,
-                                  aSource);
+                                  aSource, true);
     this.siteURI = aSiteURI;
   },
 
   /**
    * Tries to updates the livemark if needed.
    * The update process is asynchronous.
    *
    * @param [optional] aForceUpdate
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -329,17 +329,18 @@ async function ensureEqualBookmarksTrees
 
   for (let property of Object.keys(aOriginal)) {
     if (property == "children") {
       Assert.equal(aOriginal.children.length, aNew.children.length);
       for (let i = 0; i < aOriginal.children.length; i++) {
         await ensureEqualBookmarksTrees(aOriginal.children[i],
                                         aNew.children[i],
                                         false,
-                                        true);
+                                        true,
+                                        aIgnoreAllDates);
       }
     } else if (property == "guid") {
       // guid shouldn't be copied if the item was not restored.
       Assert.notEqual(aOriginal.guid, aNew.guid);
     } else if (property == "dateAdded") {
       // dateAdded shouldn't be copied if the item was not restored.
       Assert.ok(is_time_ordered(aOriginal.dateAdded, aNew.dateAdded));
     } else if (property == "lastModified") {
--- a/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js
+++ b/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js
@@ -184,17 +184,17 @@ add_task(async function test_addLivemark
   do_check_true(livemark.id > 0);
   do_check_valid_places_guid(livemark.guid);
   do_check_eq(livemark.title, "test");
   do_check_eq(livemark.parentId, PlacesUtils.unfiledBookmarksFolderId);
   do_check_eq(livemark.parentGuid, PlacesUtils.bookmarks.unfiledGuid);
   do_check_true(livemark.feedURI.equals(FEED_URI));
   do_check_eq(livemark.siteURI, null);
   do_check_true(livemark.lastModified > 0);
-  do_check_true(is_time_ordered(livemark.dateAdded, livemark.lastModified));
+  do_check_eq(livemark.dateAdded, livemark.lastModified);
 
   let bookmark = await PlacesUtils.bookmarks.fetch(livemark.guid);
   do_check_eq(livemark.index, bookmark.index);
   do_check_eq(livemark.dateAdded, bookmark.dateAdded * 1000);
 });
 
 add_task(async function test_addLivemark_succeeds() {
   let livemark = await PlacesUtils.livemarks.addLivemark(