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 288746 74917ce213120fdc82773c9c336f69f252b6b74e
parent 288745 beac25d81d5627f6d3827f162c911706de03b947
child 288747 af2e137813b628c303c3faf9197aacab82fec0f1
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, ritu
bugs1192692
milestone42.0a2
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
@@ -2409,17 +2409,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)
@@ -2428,33 +2427,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]