Bug 1192692 - ensure PlacesUtils.promiseBookmarksTree() correctly primes the guid cache. r=mak
--- 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]