Bug 1394801 - Ensure livemarks are removed from the PlacesUIUtils livemark cache with async PlacesTransactions turned on. r?mak draft
authorMark Banner <standard8@mozilla.com>
Tue, 29 Aug 2017 17:06:34 +0100
changeset 655910 c359c35a810026563115d6bf166b6b718f028517
parent 655385 db7f19e26e571ae1dd309f5d2f387b06ba670c30
child 728944 2d7d304a6668c28a5adfd59ede05f6a8e8017329
push id76993
push userbmo:standard8@mozilla.com
push dateWed, 30 Aug 2017 14:01:34 +0000
reviewersmak
bugs1394801
milestone57.0a1
Bug 1394801 - Ensure livemarks are removed from the PlacesUIUtils livemark cache with async PlacesTransactions turned on. r?mak MozReview-Commit-ID: B6ubzDhReCE
browser/components/places/PlacesUIUtils.jsm
browser/components/places/tests/unit/test_PUIU_livemarksCache.js
browser/components/places/tests/unit/xpcshell.ini
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -49,35 +49,47 @@ function IsLivemark(aItemId) {
   let self = IsLivemark;
   if (!("ids" in self)) {
     const LIVEMARK_ANNO = PlacesUtils.LMANNO_FEEDURI;
 
     let idsVec = PlacesUtils.annotations.getItemsWithAnnotation(LIVEMARK_ANNO);
     self.ids = new Set(idsVec);
 
     let obs = Object.freeze({
-      QueryInterface: XPCOMUtils.generateQI(Ci.nsIAnnotationObserver),
+      QueryInterface: XPCOMUtils.generateQI([Ci.nsINavBookmarksObserver]),
+
+      // Ci.nsINavBookmarkObserver items.
 
-      onItemAnnotationSet(itemId, annoName) {
-        if (annoName == LIVEMARK_ANNO)
+      onItemChanged(itemId, property, isAnnoProperty, newValue, lastModified,
+                    itemType, parentId, guid) {
+        if (isAnnoProperty && property == LIVEMARK_ANNO) {
           self.ids.add(itemId);
+        }
       },
 
-      onItemAnnotationRemoved(itemId, annoName) {
-        // If annoName is set to an empty string, the item is gone.
-        if (annoName == LIVEMARK_ANNO || annoName == "")
-          self.ids.delete(itemId);
+      onItemRemoved(itemId) {
+        // Since the bookmark is removed, we know we can remove any references
+        // to it from the cache.
+        self.ids.delete(itemId);
       },
 
+      onItemAdded() {},
+      onBeginUpdateBatch() {},
+      onEndUpdateBatch() {},
+      onItemVisited() {},
+      onItemMoved() {},
       onPageAnnotationSet() { },
       onPageAnnotationRemoved() { },
+      skipDescendantsOnItemRemoval: false,
+      skipTags: false,
     });
-    PlacesUtils.annotations.addObserver(obs);
+
+    PlacesUtils.bookmarks.addObserver(obs);
     PlacesUtils.registerShutdownFunction(() => {
-      PlacesUtils.annotations.removeObserver(obs);
+      PlacesUtils.bookmarks.removeObserver(obs);
     });
   }
   return self.ids.has(aItemId);
 }
 
 let InternalFaviconLoader = {
   /**
    * This gets called for every inner window that is destroyed.
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/unit/test_PUIU_livemarksCache.js
@@ -0,0 +1,40 @@
+"use strict";
+
+let {IsLivemark} = Cu.import("resource:///modules/PlacesUIUtils.jsm", {});
+
+add_task(function test_livemark_cache_builtin_folder() {
+  // This test checks a basic livemark, and also initializes the observer for
+  // updates to the bookmarks.
+  Assert.ok(!IsLivemark(PlacesUtils.unfiledBookmarksFolderId),
+    "unfiled bookmarks should not be seen as a livemark");
+});
+
+add_task(async function test_livemark_add_and_remove_items() {
+  let bookmark = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    title: "Grandsire",
+    url: "http://example.com",
+  });
+
+  let bookmarkId = await PlacesUtils.promiseItemId(bookmark.guid);
+
+  Assert.ok(!IsLivemark(bookmarkId),
+    "a simple bookmark should not be seen as a livemark");
+
+  let livemark = await PlacesUtils.livemarks.addLivemark({
+    title: "Stedman",
+    feedURI: Services.io.newURI("http://livemark.com/"),
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+  });
+
+  let livemarkId = await PlacesUtils.promiseItemId(livemark.guid);
+
+  Assert.ok(IsLivemark(livemarkId),
+    "a livemark should be reported as a livemark");
+
+  // Now remove the livemark.
+  await PlacesUtils.livemarks.removeLivemark(livemark);
+
+  Assert.ok(!IsLivemark(livemarkId),
+    "the livemark should have been removed from the cache");
+});
--- a/browser/components/places/tests/unit/xpcshell.ini
+++ b/browser/components/places/tests/unit/xpcshell.ini
@@ -16,9 +16,10 @@ support-files =
 [test_browserGlue_distribution.js]
 [test_browserGlue_migrate.js]
 [test_browserGlue_prefs.js]
 [test_browserGlue_restore.js]
 [test_browserGlue_smartBookmarks.js]
 [test_browserGlue_urlbar_defaultbehavior_migration.js]
 [test_clearHistory_shutdown.js]
 [test_leftpane_corruption_handling.js]
+[test_PUIU_livemarksCache.js]
 [test_PUIU_makeTransaction.js]