Bug 1305563 - Correctly forward change sources when updating keywords. r?mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Tue, 10 Oct 2017 10:37:00 -0700
changeset 680372 f4cc89ccd2b28b42962647ff8cb8ee015c1fc2c0
parent 680371 c702d82aabd0679870ad3132b9327fb303cef60d
child 680373 70b3df565bd478ac0a6d7ac4ae9437e059daf0c0
push id84483
push userbmo:kit@mozilla.com
push dateFri, 13 Oct 2017 22:55:10 +0000
reviewersmak
bugs1305563
milestone58.0a1
Bug 1305563 - Correctly forward change sources when updating keywords. r?mak This patch fixes the keywords cache observer to forward the source for URL changes. Previously, Sync used the public `PlacesUtils.keywords` API, so this wasn't an issue. However, the new Sync bookmark buffer writes to `moz_keywords` directly, then fires `onItemChanged` observer notifications to update the cache. MozReview-Commit-ID: BMXvgTql9qb
toolkit/components/places/PlacesUtils.jsm
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2314,75 +2314,83 @@ XPCOMUtils.defineLazyGetter(this, "gKeyw
       let observer = {
         QueryInterface: XPCOMUtils.generateQI(Ci.nsINavBookmarkObserver),
         onBeginUpdateBatch() {},
         onEndUpdateBatch() {},
         onItemAdded() {},
         onItemVisited() {},
         onItemMoved() {},
 
-        onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid) {
+        onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid,
+                      source) {
           if (itemType != PlacesUtils.bookmarks.TYPE_BOOKMARK)
             return;
 
           let keywords = keywordsForHref(uri.spec);
           // This uri has no keywords associated, so there's nothing to do.
           if (keywords.length == 0)
             return;
 
           (async function() {
             // If the uri is not bookmarked anymore, we can remove this keyword.
             let bookmark = await PlacesUtils.bookmarks.fetch({ url: uri });
             if (!bookmark) {
               for (let keyword of keywords) {
-                await PlacesUtils.keywords.remove(keyword);
+                await PlacesUtils.keywords.remove({ keyword, source });
               }
             }
           })().catch(Cu.reportError);
         },
 
         onItemChanged(id, prop, isAnno, val, lastMod, itemType, parentId, guid,
-                      parentGuid, oldVal) {
+                      parentGuid, oldVal, source) {
           if (gIgnoreKeywordNotifications) {
             return;
           }
 
           if (prop == "keyword") {
             this._onKeywordChanged(guid, val, oldVal);
           } else if (prop == "uri") {
-            this._onUrlChanged(guid, val, oldVal).catch(Cu.reportError);
+            this._onUrlChanged(guid, val, oldVal, source).catch(Cu.reportError);
           }
         },
 
         _onKeywordChanged(guid, keyword, href) {
           if (keyword.length == 0) {
             // We are removing a keyword.
             let keywords = keywordsForHref(href)
             for (let kw of keywords) {
               cache.delete(kw);
             }
           } else {
             // We are adding a new keyword.
             cache.set(keyword, { keyword, url: new URL(href) });
           }
         },
 
-        async _onUrlChanged(guid, url, oldUrl) {
+        async _onUrlChanged(guid, url, oldUrl, source) {
           // Check if the old url is associated with keywords.
           let entries = [];
           await PlacesUtils.keywords.fetch({ url: oldUrl }, e => entries.push(e));
           if (entries.length == 0) {
             return;
           }
 
           // Move the keywords to the new url.
           for (let entry of entries) {
-            await PlacesUtils.keywords.remove(entry.keyword);
-            entry.url = new URL(url);
-            await PlacesUtils.keywords.insert(entry);
+            await PlacesUtils.keywords.remove({
+              keyword: entry.keyword,
+              source,
+            });
+            await PlacesUtils.keywords.insert({
+              keyword: entry.keyword,
+              url,
+              postData: entry.postData,
+              source,
+            });
           }
         },
       };
 
       PlacesUtils.bookmarks.addObserver(observer);
       PlacesUtils.registerShutdownFunction(() => {
         PlacesUtils.bookmarks.removeObserver(observer);
       });