This is only used by tests https://searchfox.org/mozilla-central/search?q=setKeywordForBookmark&path= Along with it, we can remove the bookmarks observer from gKeywordsCachePromise, and we can stop initializing the cache with PlacesUtils.keywords (https://searchfox.org/mozilla-central/rev/c56f656febb1aa58c72b218699e24a76425bd284/toolkit/components/places/PlacesUtils.jsm#1925), that should be a perf win.

Unfortunately we can't remove the observer, because the UI still links keywords to bookmarks :(

This is not ready for 2 reasons: 1. Bug 1313188, the new API doesn't handle the case properly 2. test_bookmark_value_changes.js fails, that's strange and I'm not sure why. Kit, any idea why with these changes (that are pretty much just removing an unused old API) test_bookmark_value_changes.js starts failing on the "fb" keyword?

I'm sorry, the keyword is "tb".

Comment on attachment 8947465 [details] Bug 1434607 - Remove synchronous Bookmarks::setKeywordForBookmark. https://reviewboard.mozilla.org/r/217164/#review223010 ::: toolkit/components/places/PlacesUtils.jsm (Diff revision 1) > parentGuid, oldVal, source) { > - if (gIgnoreKeywordNotifications) { > + if (prop == "uri") { > - return; > - } > - > - if (prop == "keyword") { The mirror still passes the `"keyword"` prop to update the keywords cache (https://searchfox.org/mozilla-central/rev/f41017f5cd7b5f3835d282fbe011d2899574fc2b/toolkit/components/places/SyncedBookmarksMirror.jsm#3997-4005), which is why I think it's failing now. Would it make sense to add a `PlacesUtils.keywords.updateCachedKeyword` method that the mirror can call to update the cache? Alternatively, could we just invalidate the entire keyword cache, as we do for livemarks? If we did that, we could remove keyword change tracking from the mirror entirely (https://searchfox.org/mozilla-central/rev/f41017f5cd7b5f3835d282fbe011d2899574fc2b/toolkit/components/places/SyncedBookmarksMirror.jsm#1329-1353), and avoid issues with the cache becoming out of sync with the database...but might be too expensive. WDYT?

Ah right, because the mirror doesn't go through the API, so it doesn't know about any caches. Yes, I think we could invalidate the keywords cache completely through an API.

(In reply to Marco Bonardo [::mak] from comment #7) > Yes, I think we could invalidate the keywords cache completely through an > API. Cool. I'm happy to put together a patch tomorrow, unless you'd like to. :-)

I have quite some reviews in my queue, not sure I could do that today regardless.

So, this really depends on Bug 1435354, because otherwise test_bookmark_value_changes.js fails.

Comment on attachment 8947465 [details] Bug 1434607 - Remove synchronous Bookmarks::setKeywordForBookmark. https://reviewboard.mozilla.org/r/217164/#review228584 Looks good, nice cleanup in PlacesUtils.jsm

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/21d50f9ec4f9 Remove synchronous Bookmarks::setKeywordForBookmark. r=standard8