Bug 1678626: Apply bookmark-guid-changed event. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D125771

services/sync/modules/engines/bookmarks.js

@@ -883,6 +883,7 @@ BookmarksTracker.prototype = {
         case "bookmark-added":
         case "bookmark-removed":
         case "bookmark-moved":
+        case "bookmark-guid-changed":
         case "bookmark-title-changed":
         case "bookmark-url-changed":
           if (IGNORED_SOURCES.includes(event.source)) {

toolkit/components/places/SyncedBookmarksMirror.jsm

@@ -2060,8 +2060,6 @@ class BookmarkObserverRecorder {
     this.notifyInStableOrder = notifyInStableOrder;
     this.signal = signal;
     this.placesEvents = [];
-    this.guidChangedArgs = [];
-    this.itemChangedArgs = [];
     this.shouldInvalidateKeywords = false;
   }
 
@@ -2075,7 +2073,7 @@ class BookmarkObserverRecorder {
     if (this.shouldInvalidateKeywords) {
       await PlacesUtils.keywords.invalidateCachedKeywords();
     }
-    await this.notifyBookmarkObservers();
+    this.notifyBookmarkObservers();
     if (this.signal.aborted) {
       throw new SyncedBookmarksMirror.InterruptedError(
         "Interrupted before recalculating frecencies for new URLs"
@@ -2132,8 +2130,7 @@ class BookmarkObserverRecorder {
     MirrorLog.trace("Recording observer notifications for changed GUIDs");
     await this.db.execute(
       `SELECT b.id, b.lastModified, b.type, b.guid AS newGuid,
-              c.oldGuid, p.id AS parentId, p.guid AS parentGuid,
-              gp.guid AS grandParentGuid
+              p.guid AS parentGuid, gp.guid AS grandParentGuid
        FROM guidsChanged c
        JOIN moz_bookmarks b ON b.id = c.itemId
        JOIN moz_bookmarks p ON p.id = b.parent
@@ -2150,8 +2147,6 @@ class BookmarkObserverRecorder {
           lastModified: row.getResultByName("lastModified"),
           type: row.getResultByName("type"),
           newGuid: row.getResultByName("newGuid"),
-          oldGuid: row.getResultByName("oldGuid"),
-          parentId: row.getResultByName("parentId"),
           parentGuid: row.getResultByName("parentGuid"),
           grandParentGuid: row.getResultByName("grandParentGuid"),
         };
@@ -2314,21 +2309,6 @@ class BookmarkObserverRecorder {
 
   noteGuidChanged(info) {
     PlacesUtils.invalidateCachedGuidFor(info.id);
-
-    this.guidChangedArgs.push([
-      info.id,
-      "guid",
-      /* isAnnotationProperty */ false,
-      info.newGuid,
-      info.lastModified,
-      info.type,
-      info.parentId,
-      info.newGuid,
-      info.parentGuid,
-      info.oldGuid,
-      PlacesUtils.bookmarks.SOURCES.SYNC,
-    ]);
-
     this.placesEvents.push(
       new PlacesBookmarkGuid({
         id: info.id,
@@ -2417,68 +2397,15 @@ class BookmarkObserverRecorder {
     );
   }
 
-  async notifyBookmarkObservers() {
+  notifyBookmarkObservers() {
     MirrorLog.trace("Notifying bookmark observers");
-    let observers = PlacesUtils.bookmarks.getObservers();
-    await Async.yieldingForEach(
-      this.guidChangedArgs,
-      args => {
-        if (this.signal.aborted) {
-          throw new SyncedBookmarksMirror.InterruptedError(
-            "Interrupted while notifying observers for changed GUIDs"
-          );
-        }
-        this.notifyObserversWithInfo(observers, "onItemChanged", {
-          isTagging: false,
-          args,
-        });
-      },
-      yieldState
-    );
-    if (this.signal.aborted) {
-      throw new SyncedBookmarksMirror.InterruptedError(
-        "Interrupted before notifying observers for new items"
-      );
-    }
 
     if (this.placesEvents.length) {
       PlacesObservers.notifyListeners(this.placesEvents);
     }
 
-    await Async.yieldingForEach(
-      this.itemChangedArgs,
-      args => {
-        if (this.signal.aborted) {
-          throw new SyncedBookmarksMirror.InterruptedError(
-            "Interrupted before notifying observers for changed items"
-          );
-        }
-        this.notifyObserversWithInfo(observers, "onItemChanged", {
-          isTagging: false,
-          args,
-        });
-      },
-      yieldState
-    );
     MirrorLog.trace("Notified bookmark observers");
   }
-
-  notifyObserversWithInfo(observers, name, info) {
-    for (let observer of observers) {
-      if (info.isTagging && observer.skipTags) {
-        return;
-      }
-      this.notifyObserver(observer, name, info.args);
-    }
-  }
-
-  notifyObserver(observer, notification, args = []) {
-    try {
-      observer[notification](...args);
-    } catch (ex) {
-      MirrorLog.warn("Error notifying observer", ex);
-    }
-  }
 }
 
 /**

toolkit/components/places/tests/sync/head_sync.js

@@ -360,6 +360,19 @@ BookmarkObserver.prototype = {
           this.notifications.push({ name: "bookmark-moved", params });
           break;
         }
+        case "bookmark-guid-changed": {
+          const params = {
+            itemId: event.id,
+            type: event.itemType,
+            urlHref: event.url,
+            guid: event.guid,
+            parentGuid: event.parentGuid,
+            source: event.source,
+            isTagging: event.isTagging,
+          };
+          this.notifications.push({ name: "bookmark-guid-changed", params });
+          break;
+        }
         case "bookmark-title-changed": {
           const params = {
             itemId: event.id,
@@ -426,6 +439,7 @@ BookmarkObserver.prototype = {
         "bookmark-added",
         "bookmark-removed",
         "bookmark-moved",
+        "bookmark-guid-changed",
         "bookmark-title-changed",
         "bookmark-url-changed",
       ],
@@ -450,6 +464,7 @@ function expectBookmarkChangeNotifications(options) {
       "bookmark-added",
       "bookmark-removed",
       "bookmark-moved",
+      "bookmark-guid-changed",
       "bookmark-title-changed",
       "bookmark-url-changed",
     ],

toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js

@@ -433,21 +433,6 @@ add_task(async function test_apply_then_revert() {
   ]);
 
   observer.check([
-    {
-      name: "onItemChanged",
-      params: {
-        itemId: localItemIds.get("bookmarkEEEE"),
-        property: "guid",
-        isAnnoProperty: false,
-        newValue: "bookmarkEEEE",
-        type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        parentId: PlacesUtils.bookmarksMenuFolderId,
-        guid: "bookmarkEEEE",
-        parentGuid: PlacesUtils.bookmarks.menuGuid,
-        oldValue: "bookmarkEEE1",
-        source: PlacesUtils.bookmarks.SOURCES.SYNC,
-      },
-    },
     {
       name: "bookmark-removed",
       params: {
@@ -461,6 +446,18 @@ add_task(async function test_apply_then_revert() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
       },
     },
+    {
+      name: "bookmark-guid-changed",
+      params: {
+        itemId: localItemIds.get("bookmarkEEEE"),
+        type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+        urlHref: "",
+        guid: "bookmarkEEEE",
+        parentGuid: PlacesUtils.bookmarks.menuGuid,
+        source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        isTagging: false,
+      },
+    },
     {
       name: "bookmark-added",
       params: {