Bug 1192692 - Ensure PlacesUtils.promiseBookmarksTree() correctly primes the guid cache. r=mak, a=ritu
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 13 Aug 2015 17:52:28 +1000
changeset 291451 06b3fd70e4dce37c41baf3237c889a7c1013c9af
parent 291450 178388fe6e61e0f101a84c122c3629b46b16f0ef
child 291452 e259ef0a5197020730d27e921853c7bc62df5a53
push id5246
push usermozilla@noorenberghe.ca
push dateWed, 09 Sep 2015 21:17:14 +0000
reviewersmak, ritu
bugs1192692
milestone41.0
Bug 1192692 - Ensure PlacesUtils.promiseBookmarksTree() correctly primes the guid cache. r=mak, a=ritu
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/bookmarks/test_bookmarkstree_cache.js
toolkit/components/places/tests/bookmarks/xpcshell.ini
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2408,17 +2408,16 @@ let GuidHelper = {
     let conn = yield PlacesUtils.promiseDBConnection();
 
     let rows = yield conn.executeCached(
       "SELECT b.id, b.guid from moz_bookmarks b WHERE b.guid = :guid LIMIT 1",
       { guid: aGuid });
     if (rows.length == 0)
       throw new Error("no item found for the given GUID");
 
-    this.ensureObservingRemovedItems();
     let itemId = rows[0].getResultByName("id");
     this.updateCache(itemId, aGuid);
     return itemId;
   }),
 
   getItemGuid: Task.async(function* (aItemId) {
     let cached = this.guidsForIds.get(aItemId);
     if (cached !== undefined)
@@ -2427,33 +2426,33 @@ let GuidHelper = {
     let conn = yield PlacesUtils.promiseDBConnection();
 
     let rows = yield conn.executeCached(
       "SELECT b.id, b.guid from moz_bookmarks b WHERE b.id = :id LIMIT 1",
       { id: aItemId });
     if (rows.length == 0)
       throw new Error("no item found for the given itemId");
 
-    this.ensureObservingRemovedItems();
     let guid = rows[0].getResultByName("guid");
     this.updateCache(aItemId, guid);
     return guid;
   }),
 
   /**
    * Updates the cache.
    *
    * @note This is the only place where the cache should be populated,
    *       invalidation relies on both Maps being populated at the same time.
    */
   updateCache(aItemId, aGuid) {
     if (typeof(aItemId) != "number" || aItemId <= 0)
       throw new Error("Trying to update the GUIDs cache with an invalid itemId");
     if (typeof(aGuid) != "string" || !/^[a-zA-Z0-9\-_]{12}$/.test(aGuid))
       throw new Error("Trying to update the GUIDs cache with an invalid GUID");
+    this.ensureObservingRemovedItems();
     this.guidsForIds.set(aItemId, aGuid);
     this.idsForGuids.set(aGuid, aItemId);
   },
 
   invalidateCacheForItemId(aItemId) {
     let guid = this.guidsForIds.get(aItemId);
     this.guidsForIds.delete(aItemId);
     this.idsForGuids.delete(guid);
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarkstree_cache.js
@@ -0,0 +1,18 @@
+
+// Bug 1192692 - promiseBookmarksTree caches items without adding observers to
+// invalidate the cache.
+add_task(function* boookmarks_tree_cache() {
+  // Note that for this test to be effective, it needs to use the "old" sync
+  // bookmarks methods - using, eg, PlacesUtils.bookmarks.insert() doesn't
+  // demonstrate the problem as it indirectly arranges for the observers to
+  // be added.
+  let id = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                                uri("http://example.com"),
+                                                PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                                "A title");
+  yield PlacesUtils.promiseBookmarksTree();
+
+  PlacesUtils.bookmarks.removeItem(id);
+
+  yield Assert.rejects(PlacesUtils.promiseItemGuid(id));
+});
--- a/toolkit/components/places/tests/bookmarks/xpcshell.ini
+++ b/toolkit/components/places/tests/bookmarks/xpcshell.ini
@@ -24,16 +24,17 @@ skip-if = toolkit == 'android' || toolki
 [test_818584-discard-duplicate-backups.js]
 [test_818587_compress-bookmarks-backups.js]
 [test_818593-store-backup-metadata.js]
 [test_992901-backup-unsorted-hierarchy.js]
 [test_997030-bookmarks-html-encode.js]
 [test_1129529.js]
 [test_async_observers.js]
 [test_bmindex.js]
+[test_bookmarkstree_cache.js]
 [test_bookmarks.js]
 [test_bookmarks_eraseEverything.js]
 [test_bookmarks_fetch.js]
 [test_bookmarks_insert.js]
 [test_bookmarks_notifications.js]
 [test_bookmarks_remove.js]
 [test_bookmarks_reorder.js]
 [test_bookmarks_update.js]