Bug 1511700 - Use the new notification system (PlacesObserver) for bookmark removed notifications. r=Standard8,mak

Phasing out the old notification system for OnItemRemoved events.

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

--HG--
extra : moz-landing-system : lando

browser/base/content/browser-places.js

@@ -1602,7 +1602,7 @@ var BookmarkingUI = {
     if (this._hasBookmarksObserver) {
       PlacesUtils.bookmarks.removeObserver(this);
       PlacesUtils.observers.removeListener(
-        ["bookmark-added"],
+        ["bookmark-added", "bookmark-removed"],
         this.handlePlacesEvents
       );
     }
@@ -1653,7 +1653,7 @@ var BookmarkingUI = {
             PlacesUtils.bookmarks.addObserver(this);
             this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
             PlacesUtils.observers.addListener(
-              ["bookmark-added"],
+              ["bookmark-added", "bookmark-removed"],
               this.handlePlacesEvents
             );
             this._hasBookmarksObserver = true;
@@ -1946,30 +1946,32 @@ var BookmarkingUI = {
   },
 
   handlePlacesEvents(aEvents) {
+    for (let ev of aEvents) {
+      switch (ev.type) {
+        case "bookmark-added":
           // Only need to update the UI if it wasn't marked as starred before:
           if (this._itemGuids.size == 0) {
-      for (let { url, guid } of aEvents) {
+            if (ev.url && ev.url == this._uri.spec) {
-        if (url && url == this._uri.spec) {
               // If a new bookmark has been added to the tracked uri, register it.
-          if (!this._itemGuids.has(guid)) {
+              if (!this._itemGuids.has(ev.guid)) {
-            this._itemGuids.add(guid);
+                this._itemGuids.add(ev.guid);
                 this._updateStar();
               }
             }
           }
-    }
+          break;
-  },
+        case "bookmark-removed":
-
-  // nsINavBookmarkObserver
-  onItemRemoved(aItemId, aParentId, aIndex, aItemType, aURI, aGuid) {
           // If one of the tracked bookmarks has been removed, unregister it.
-    if (this._itemGuids.has(aGuid)) {
+          if (this._itemGuids.has(ev.guid)) {
-      this._itemGuids.delete(aGuid);
+            this._itemGuids.delete(ev.guid);
             // Only need to update the UI if the page is no longer starred
             if (this._itemGuids.size == 0) {
               this._updateStar();
             }
           }
+          break;
+      }
+    }
   },
 
   onItemChanged(

browser/base/content/test/general/browser_bug581253.js

@@ -55,8 +55,9 @@ add_task(async function test_remove_bookmark_with_tag_via_edit_bookmark() {
   );
 
   let removeNotification = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (id, parentId, index, type, itemUrl) => testURL == unescape(itemUrl.spec)
+    events => events.some(event => unescape(event.url) == testURL),
+    "places"
   );
 
   let removeButton = document.getElementById("editBookmarkPanelRemoveButton");

browser/base/content/test/pageActions/browser_page_action_menu.js

@@ -97,8 +97,9 @@ add_task(async function bookmark() {
     });
 
     let onItemRemovedPromise = PlacesTestUtils.waitForNotification(
-      "onItemRemoved",
+      "bookmark-removed",
-      (id, parentId, index, type, itemUrl) => url == itemUrl.spec
+      events => events.some(event => event.url == url),
+      "places"
     );
 
     // Click the remove-bookmark button in the panel.

browser/components/enterprisepolicies/tests/browser/browser_policy_bookmarks.js

@@ -84,23 +84,32 @@ async function promiseAllChangesMade({ itemsToAdd, itemsToRemove }) {
   return new Promise(resolve => {
     let listener = events => {
       is(events.length, 1, "Should only have 1 event.");
+      switch (events[0].type) {
+        case "bookmark-added":
           itemsToAdd--;
           if (itemsToAdd == 0 && itemsToRemove == 0) {
             PlacesUtils.bookmarks.removeObserver(bmObserver);
-        PlacesUtils.observers.removeListener(["bookmark-added"], listener);
+            PlacesUtils.observers.removeListener(
+              ["bookmark-added", "bookmark-removed"],
+              listener
+            );
             resolve();
           }
-    };
+          break;
-    let bmObserver = {
+        case "bookmark-removed":
-      onItemRemoved() {
           itemsToRemove--;
           if (itemsToAdd == 0 && itemsToRemove == 0) {
             PlacesUtils.bookmarks.removeObserver(bmObserver);
-          PlacesUtils.observers.removeListener(["bookmark-added"], listener);
+            PlacesUtils.observers.removeListener(
+              ["bookmark-added", "bookmark-removed"],
+              listener
+            );
             resolve();
           }
-      },
+          break;
-
+      }
+    };
+    let bmObserver = {
       onBeginUpdateBatch() {},
       onEndUpdateBatch() {},
       onItemChanged() {},
@@ -108,7 +117,10 @@ async function promiseAllChangesMade({ itemsToAdd, itemsToRemove }) {
       onItemMoved() {},
     };
     PlacesUtils.bookmarks.addObserver(bmObserver);
-    PlacesUtils.observers.addListener(["bookmark-added"], listener);
+    PlacesUtils.observers.addListener(
+      ["bookmark-added", "bookmark-removed"],
+      listener
+    );
   });
 }
 

browser/components/extensions/parent/ext-bookmarks.js

@@ -122,7 +122,6 @@ let observer = new (class extends EventEmitter {
     super();
 
     this.skipTags = true;
-    this.skipDescendantsOnItemRemoval = true;
 
     this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
   }
@@ -132,6 +131,8 @@ let observer = new (class extends EventEmitter {
 
   handlePlacesEvents(events) {
     for (let event of events) {
+      switch (event.type) {
+        case "bookmark-added":
           if (event.isTagging) {
             continue;
           }
@@ -150,6 +151,24 @@ let observer = new (class extends EventEmitter {
           }
 
           this.emit("created", bookmark);
+          break;
+        case "bookmark-removed":
+          if (event.isTagging || event.isDescendantRemoval) {
+            continue;
+          }
+          let node = {
+            id: event.guid,
+            parentId: event.parentGuid,
+            index: event.index,
+            type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(event.itemType),
+            url: getUrl(event.itemType, event.url),
+          };
+
+          this.emit("removed", {
+            guid: event.guid,
+            info: { parentId: event.parentGuid, index: event.index, node },
+          });
+      }
     }
   }
 
@@ -176,18 +195,6 @@ let observer = new (class extends EventEmitter {
     this.emit("moved", { guid, info });
   }
 
-  onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid, source) {
-    let node = {
-      id: guid,
-      parentId: parentGuid,
-      index,
-      type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(itemType),
-      url: getUrl(itemType, uri && uri.spec),
-    };
-
-    this.emit("removed", { guid, info: { parentId: parentGuid, index, node } });
-  }
-
   onItemChanged(
     id,
     prop,
@@ -220,7 +227,7 @@ const decrementListeners = () => {
   if (!listenerCount) {
     PlacesUtils.bookmarks.removeObserver(observer);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       observer.handlePlacesEvents
     );
   }
@@ -231,7 +238,7 @@ const incrementListeners = () => {
   if (listenerCount == 1) {
     PlacesUtils.bookmarks.addObserver(observer);
     PlacesUtils.observers.addListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       observer.handlePlacesEvents
     );
   }

browser/components/newtab/lib/PlacesFeed.jsm

@@ -95,34 +95,6 @@ class BookmarksObserver extends Observer {
     this.skipTags = true;
   }
 
-  /**
-   * onItemRemoved - Called when a bookmark is removed
-   *
-   * @param  {str} id
-   * @param  {str} folderId
-   * @param  {int} index
-   * @param  {int} type       Indicates if the bookmark is an actual bookmark,
-   *                          a folder, or a separator.
-   * @param  {str} uri
-   * @param  {str} guid      The unique id of the bookmark
-   */
-  // eslint-disable-next-line max-params
-  onItemRemoved(id, folderId, index, type, uri, guid, parentGuid, source) {
-    if (
-      type === PlacesUtils.bookmarks.TYPE_BOOKMARK &&
-      source !== PlacesUtils.bookmarks.SOURCES.IMPORT &&
-      source !== PlacesUtils.bookmarks.SOURCES.RESTORE &&
-      source !== PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP &&
-      source !== PlacesUtils.bookmarks.SOURCES.SYNC
-    ) {
-      this.dispatch({ type: at.PLACES_LINKS_CHANGED });
-      this.dispatch({
-        type: at.PLACES_BOOKMARK_REMOVED,
-        data: { url: uri.spec, bookmarkGuid: guid },
-      });
-    }
-  }
-
   // Empty functions to make xpconnect happy
   onBeginUpdateBatch() {}
 
@@ -155,7 +127,10 @@ class PlacesObserver extends Observer {
       title,
       url,
       isTagging,
+      type,
     } of events) {
+      switch (type) {
+        case "bookmark-added":
           // Skips items that are not bookmarks (like folders), about:* pages or
           // default bookmarks, added when the profile is created.
           if (
@@ -180,6 +155,24 @@ class PlacesObserver extends Observer {
               url,
             },
           });
+          break;
+        case "bookmark-removed":
+          if (
+            isTagging ||
+            (itemType === PlacesUtils.bookmarks.TYPE_BOOKMARK &&
+              source !== PlacesUtils.bookmarks.SOURCES.IMPORT &&
+              source !== PlacesUtils.bookmarks.SOURCES.RESTORE &&
+              source !== PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP &&
+              source !== PlacesUtils.bookmarks.SOURCES.SYNC)
+          ) {
+            this.dispatch({ type: at.PLACES_LINKS_CHANGED });
+            this.dispatch({
+              type: at.PLACES_BOOKMARK_REMOVED,
+              data: { url, bookmarkGuid: guid },
+            });
+          }
+          break;
+      }
     }
   }
 }
@@ -202,7 +195,7 @@ class PlacesFeed {
       .getService(Ci.nsINavBookmarksService)
       .addObserver(this.bookmarksObserver, true);
     PlacesUtils.observers.addListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       this.placesObserver.handlePlacesEvent
     );
 
@@ -246,7 +239,7 @@ class PlacesFeed {
     PlacesUtils.history.removeObserver(this.historyObserver);
     PlacesUtils.bookmarks.removeObserver(this.bookmarksObserver);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       this.placesObserver.handlePlacesEvent
     );
     Services.obs.removeObserver(this, LINK_BLOCKED_EVENT);

browser/components/newtab/test/unit/lib/PlacesFeed.test.js

@@ -111,7 +111,7 @@ describe("PlacesFeed", () => {
       );
       assert.calledWith(
         global.PlacesUtils.observers.addListener,
-        ["bookmark-added"],
+        ["bookmark-added", "bookmark-removed"],
         feed.placesObserver.handlePlacesEvent
       );
       assert.calledWith(global.Services.obs.addObserver, feed, BLOCKED_EVENT);
@@ -133,7 +133,7 @@ describe("PlacesFeed", () => {
       );
       assert.calledWith(
         global.PlacesUtils.observers.removeListener,
-        ["bookmark-added"],
+        ["bookmark-added", "bookmark-removed"],
         feed.placesObserver.handlePlacesEvent
       );
       assert.calledWith(
@@ -685,6 +685,7 @@ describe("PlacesFeed", () => {
           title: FAKE_BOOKMARK.bookmarkTitle,
           url: "https://www.foo.com",
           isTagging: false,
+          type: "bookmark-added",
         },
       ];
       await feed.placesObserver.handlePlacesEvent(args);
@@ -699,19 +700,22 @@ describe("PlacesFeed", () => {
     });
     it("should only dispatch 1 PLACES_LINKS_CHANGED action if many onItemRemoved notifications happened at once", async () => {
       const args = [
-        null,
+        {
-        null,
+          id: null,
-        null,
+          parentId: null,
-        TYPE_BOOKMARK,
+          index: null,
-        { spec: "foo.com" },
+          itemType: TYPE_BOOKMARK,
-        "123foo",
+          url: "foo.com",
-        "",
+          guid: "123foo",
-        SOURCES.DEFAULT,
+          parentGuid: "",
+          source: SOURCES.DEFAULT,
+          type: "bookmark-removed",
+        },
       ];
-      await feed.bookmarksObserver.onItemRemoved(...args);
+      await feed.placesObserver.handlePlacesEvent(args);
-      await feed.bookmarksObserver.onItemRemoved(...args);
+      await feed.placesObserver.handlePlacesEvent(args);
-      await feed.bookmarksObserver.onItemRemoved(...args);
+      await feed.placesObserver.handlePlacesEvent(args);
-      await feed.bookmarksObserver.onItemRemoved(...args);
+      await feed.placesObserver.handlePlacesEvent(args);
 
       assert.calledOnce(
         feed.store.dispatch.withArgs(
@@ -733,13 +737,13 @@ describe("PlacesFeed", () => {
   });
 
   describe("PlacesObserver", () => {
-    describe("#bookmark-added", () => {
     let dispatch;
     let observer;
     beforeEach(() => {
       dispatch = sandbox.spy();
       observer = new PlacesObserver(dispatch);
     });
+    describe("#bookmark-added", () => {
       it("should dispatch a PLACES_BOOKMARK_ADDED action with the bookmark data - http", async () => {
         const args = [
           {
@@ -750,6 +754,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "http://www.foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -774,6 +779,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "https://www.foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -798,6 +804,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -814,6 +821,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -830,6 +838,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -846,6 +855,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -862,6 +872,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -878,6 +889,7 @@ describe("PlacesFeed", () => {
             title: FAKE_BOOKMARK.bookmarkTitle,
             url: "https://www.foo.com",
             isTagging: false,
+            type: "bookmark-added",
           },
         ];
         await observer.handlePlacesEvent(args);
@@ -885,6 +897,117 @@ describe("PlacesFeed", () => {
         assert.notCalled(dispatch);
       });
     });
+    describe("#bookmark-removed", () => {
+      it("should ignore events that are not of TYPE_BOOKMARK", async () => {
+        const args = [
+          {
+            id: null,
+            parentId: null,
+            index: null,
+            itemType: "nottypebookmark",
+            url: null,
+            guid: "123foo",
+            parentGuid: "",
+            source: SOURCES.DEFAULT,
+            type: "bookmark-removed",
+          },
+        ];
+        await observer.handlePlacesEvent(args);
+        assert.notCalled(dispatch);
+      });
+      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has SYNC source", async () => {
+        const args = [
+          {
+            id: null,
+            parentId: null,
+            index: null,
+            itemType: TYPE_BOOKMARK,
+            url: "foo.com",
+            guid: "123foo",
+            parentGuid: "",
+            source: SOURCES.SYNC,
+            type: "bookmark-removed",
+          },
+        ];
+        await observer.handlePlacesEvent(args);
+
+        assert.notCalled(dispatch);
+      });
+      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has IMPORT source", async () => {
+        const args = [
+          {
+            id: null,
+            parentId: null,
+            index: null,
+            itemType: TYPE_BOOKMARK,
+            url: "foo.com",
+            guid: "123foo",
+            parentGuid: "",
+            source: SOURCES.IMPORT,
+            type: "bookmark-removed",
+          },
+        ];
+        await observer.handlePlacesEvent(args);
+
+        assert.notCalled(dispatch);
+      });
+      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE source", async () => {
+        const args = [
+          {
+            id: null,
+            parentId: null,
+            index: null,
+            itemType: TYPE_BOOKMARK,
+            url: "foo.com",
+            guid: "123foo",
+            parentGuid: "",
+            source: SOURCES.RESTORE,
+            type: "bookmark-removed",
+          },
+        ];
+        await observer.handlePlacesEvent(args);
+
+        assert.notCalled(dispatch);
+      });
+      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE_ON_STARTUP source", async () => {
+        const args = [
+          {
+            id: null,
+            parentId: null,
+            index: null,
+            itemType: TYPE_BOOKMARK,
+            url: "foo.com",
+            guid: "123foo",
+            parentGuid: "",
+            source: SOURCES.RESTORE_ON_STARTUP,
+            type: "bookmark-removed",
+          },
+        ];
+        await observer.handlePlacesEvent(args);
+
+        assert.notCalled(dispatch);
+      });
+      it("should dispatch a PLACES_BOOKMARK_REMOVED action with the right URL and bookmarkGuid", async () => {
+        const args = [
+          {
+            id: null,
+            parentId: null,
+            index: null,
+            itemType: TYPE_BOOKMARK,
+            url: "foo.com",
+            guid: "123foo",
+            parentGuid: "",
+            source: SOURCES.DEFAULT,
+            type: "bookmark-removed",
+          },
+        ];
+        await observer.handlePlacesEvent(args);
+        assert.calledWith(dispatch, {
+          type: at.PLACES_BOOKMARK_REMOVED,
+          data: { bookmarkGuid: "123foo", url: "foo.com" },
+        });
+      });
+    });
   });
 
   describe("BookmarksObserver", () => {
@@ -897,97 +1020,6 @@ describe("PlacesFeed", () => {
     it("should have a QueryInterface property", () => {
       assert.property(observer, "QueryInterface");
     });
-    describe("#onItemRemoved", () => {
-      it("should ignore events that are not of TYPE_BOOKMARK", async () => {
-        await observer.onItemRemoved(
-          null,
-          null,
-          null,
-          "nottypebookmark",
-          null,
-          "123foo",
-          "",
-          SOURCES.DEFAULT
-        );
-        assert.notCalled(dispatch);
-      });
-      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has SYNC source", async () => {
-        const args = [
-          null,
-          null,
-          null,
-          TYPE_BOOKMARK,
-          { spec: "foo.com" },
-          "123foo",
-          "",
-          SOURCES.SYNC,
-        ];
-        await observer.onItemRemoved(...args);
-
-        assert.notCalled(dispatch);
-      });
-      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has IMPORT source", async () => {
-        const args = [
-          null,
-          null,
-          null,
-          TYPE_BOOKMARK,
-          { spec: "foo.com" },
-          "123foo",
-          "",
-          SOURCES.IMPORT,
-        ];
-        await observer.onItemRemoved(...args);
-
-        assert.notCalled(dispatch);
-      });
-      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE source", async () => {
-        const args = [
-          null,
-          null,
-          null,
-          TYPE_BOOKMARK,
-          { spec: "foo.com" },
-          "123foo",
-          "",
-          SOURCES.RESTORE,
-        ];
-        await observer.onItemRemoved(...args);
-
-        assert.notCalled(dispatch);
-      });
-      it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE_ON_STARTUP source", async () => {
-        const args = [
-          null,
-          null,
-          null,
-          TYPE_BOOKMARK,
-          { spec: "foo.com" },
-          "123foo",
-          "",
-          SOURCES.RESTORE_ON_STARTUP,
-        ];
-        await observer.onItemRemoved(...args);
-
-        assert.notCalled(dispatch);
-      });
-      it("should dispatch a PLACES_BOOKMARK_REMOVED action with the right URL and bookmarkGuid", () => {
-        observer.onItemRemoved(
-          null,
-          null,
-          null,
-          TYPE_BOOKMARK,
-          { spec: "foo.com" },
-          "123foo",
-          "",
-          SOURCES.DEFAULT
-        );
-        assert.calledWith(dispatch, {
-          type: at.PLACES_BOOKMARK_REMOVED,
-          data: { bookmarkGuid: "123foo", url: "foo.com" },
-        });
-      });
-    });
     describe("Other empty methods (to keep code coverage happy)", () => {
       it("should have a various empty functions for xpconnect happiness", () => {
         observer.onBeginUpdateBatch();

browser/components/places/content/editBookmark.js

@@ -1240,7 +1240,6 @@ var gEditItemOverlay = {
     });
   },
 
-  onItemRemoved() {},
   onBeginUpdateBatch() {},
   onEndUpdateBatch() {},
   onItemVisited() {},

browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js

@@ -6,8 +6,9 @@ const TEST_URL =
 function closeHandler(dialogWin) {
   let savedItemId = dialogWin.gEditItemOverlay.itemId;
   return PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    itemId => itemId === savedItemId
+    events => events.some(event => event.id === savedItemId),
+    "places"
   );
 }
 

browser/components/places/tests/browser/browser_bookmarkProperties_bookmarkAllTabs.js

@@ -42,8 +42,9 @@ add_task(async function() {
       let savedItemId = dialog.gEditItemOverlay.itemId;
       Assert.greater(savedItemId, 0, "Found the itemId");
       return PlacesTestUtils.waitForNotification(
-        "onItemRemoved",
+        "bookmark-removed",
-        id => id === savedItemId
+        events => events.some(event => event.id === savedItemId),
+        "places"
       );
     }
   );

browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js

@@ -67,9 +67,22 @@ add_task(async function() {
         }
       );
 
+      let promiseTagRemoveNotification = PlacesTestUtils.waitForNotification(
+        "bookmark-removed",
+        events =>
+          events.some(
+            event => event.parentGuid == PlacesUtils.bookmarks.tagsGuid
+          ),
+        "places"
+      );
       fillBookmarkTextField("editBMPanel_namePicker", "tag2", dialogWin);
 
       await promiseTagChangeNotification;
+      await promiseTagRemoveNotification;
+
+      // Although we have received the expected notifications, we need
+      // to let everything resolve to ensure the UI is updated.
+      await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
 
       Assert.equal(
         namepicker.value,

browser/components/places/tests/browser/browser_bookmark_popup.js

@@ -126,13 +126,17 @@ async function test_bookmarks_popup({
         );
       }
 
-      let onItemRemovedPromise = Promise.resolve();
+      let bookmarkRemovedPromise = Promise.resolve();
       if (isBookmarkRemoved) {
-        onItemRemovedPromise = PlacesTestUtils.waitForNotification(
+        bookmarkRemovedPromise = PlacesTestUtils.waitForNotification(
-          "onItemRemoved",
+          "bookmark-removed",
-          (id, parentId, index, type, uri, guid, parentGuid) =>
+          events =>
-            parentGuid == PlacesUtils.bookmarks.unfiledGuid &&
+            events.some(
-            TEST_URL == uri.spec
+              event =>
+                event.parentGuid == PlacesUtils.bookmarks.unfiledGuid &&
+                TEST_URL == event.url
+            ),
+          "places"
         );
       }
 
@@ -140,7 +144,7 @@ async function test_bookmarks_popup({
       if (popupHideFn) {
         await popupHideFn();
       }
-      await Promise.all([hiddenPromise, onItemRemovedPromise]);
+      await Promise.all([hiddenPromise, bookmarkRemovedPromise]);
 
       Assert.equal(
         bookmarkStar.hasAttribute("starred"),

browser/components/places/tests/browser/browser_bookmarksProperties.js

@@ -401,9 +401,9 @@ gTests.push({
 
         self._cleanShutdown = true;
         self._removeObserver = PlacesTestUtils.waitForNotification(
-          "onItemRemoved",
+          "bookmark-removed",
-          (itemId, parentId, index, type, uri, guid) =>
+          events => events.some(eve => eve.guid == self._bookmarkGuid),
-            guid == self._bookmarkGuid
+          "places"
         );
 
         self.window.document

browser/components/places/tests/browser/browser_bug631374_tags_selector_scroll.js

@@ -119,6 +119,9 @@ add_task(async function() {
   // Remove the second bookmark, then nuke some of the tags.
   await PlacesUtils.bookmarks.remove(bm2);
 
+  // Allow the tag updates to complete
+  await PlacesTestUtils.promiseAsyncUpdates();
+
   // Doing this backwords tests more interesting paths.
   for (let i = tags.length - 1; i >= 0; i -= 2) {
     tagsSelector.selectedIndex = i;

browser/components/places/tests/browser/browser_editBookmark_keywords.js

@@ -8,7 +8,6 @@ add_task(async function() {
       PlacesUtils.bookmarks.addObserver({
         onBeginUpdateBatch() {},
         onEndUpdateBatch() {},
-        onItemRemoved() {},
         onItemVisited() {},
         onItemMoved() {},
         onItemChanged(id, property, isAnno, value) {

browser/components/places/tests/browser/browser_library_commands.js

@@ -177,8 +177,9 @@ add_task(async function test_query_on_toolbar() {
 
   // Execute the delete command and check bookmark has been removed.
   let promiseItemRemoved = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (...args) => query.guid == args[5]
+    events => events.some(event => query.guid == event.guid),
+    "places"
   );
   PO._places.controller.doCommand("cmd_delete");
   await promiseItemRemoved;

browser/components/places/tests/browser/browser_library_delete.js

@@ -69,9 +69,9 @@ add_task(async function test_create_and_remove_bookmarks() {
     "Delete command is enabled"
   );
   let promiseItemRemovedNotification = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (itemId, parentId, index, type, uri, guid) =>
+    events => events.some(event => event.guid == folderNode.bookmarkGuid),
-      guid == folderNode.bookmarkGuid
+    "places"
   );
 
   // Execute the delete command and check bookmark has been removed.

browser/components/places/tests/browser/browser_remove_bookmarks.js

@@ -54,8 +54,9 @@ add_task(async function test_remove_bookmark_from_toolbar() {
   let contextMenuDeleteItem = document.getElementById("placesContext_delete");
 
   let removePromise = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (itemId, parentId, index, type, uri, guid) => uri.spec == TEST_URL
+    events => events.some(event => event.url == TEST_URL),
+    "places"
   );
 
   EventUtils.synthesizeMouseAtCenter(contextMenuDeleteItem, {});
@@ -138,8 +139,9 @@ add_task(async function test_remove_bookmark_from_library() {
   );
 
   let removePromise = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (itemId, parentId, index, type, uri, guid) => uri.spec == uris[0]
+    events => events.some(event => event.url == uris[0]),
+    "places"
   );
   EventUtils.synthesizeMouseAtCenter(contextMenuDeleteItem, {}, library);
 

browser/components/places/tests/browser/browser_views_liveupdate.js

@@ -116,7 +116,7 @@ add_task(async function test() {
     );
     PlacesUtils.bookmarks.addObserver(bookmarksObserver);
     PlacesUtils.observers.addListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       bookmarksObserver.handlePlacesEvents
     );
     var addedBookmarks = [];
@@ -152,7 +152,7 @@ add_task(async function test() {
     // Remove observers.
     PlacesUtils.bookmarks.removeObserver(bookmarksObserver);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       bookmarksObserver.handlePlacesEvents
     );
   });
@@ -173,13 +173,21 @@ var bookmarksObserver = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsINavBookmarkObserver]),
 
   handlePlacesEvents(events) {
-    for (let { parentGuid, guid, index } of events) {
+    for (let { type, parentGuid, guid, index } of events) {
-      this._notifications.push(["assertItemAdded", parentGuid, guid, index]);
+      switch (type) {
-    }
+        case "bookmark-added":
-  },
+          this._notifications.push([
-
+            "assertItemAdded",
-  onItemRemoved(itemId, folderId, index, itemType, uri, guid, parentGuid) {
+            parentGuid,
+            guid,
+            index,
+          ]);
+          break;
+        case "bookmark-removed":
           this._notifications.push(["assertItemRemoved", parentGuid, guid]);
+          break;
+      }
+    }
   },
 
   onItemMoved(

dom/base/PlacesBookmarkRemoved.h

@@ -0,0 +1,57 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_dom_PlacesBookmarkRemoved_h
+#define mozilla_dom_PlacesBookmarkRemoved_h
+
+#include "mozilla/dom/PlacesBookmark.h"
+
+namespace mozilla {
+namespace dom {
+class PlacesBookmarkRemoved final : public PlacesBookmark {
+ public:
+  explicit PlacesBookmarkRemoved()
+      : PlacesBookmark(PlacesEventType::Bookmark_removed) {}
+
+  static already_AddRefed<PlacesBookmarkRemoved> Constructor(
+      const GlobalObject& aGlobal, const PlacesBookmarkRemovedInit& aInitDict) {
+    RefPtr<PlacesBookmarkRemoved> event = new PlacesBookmarkRemoved();
+    event->mItemType = aInitDict.mItemType;
+    event->mId = aInitDict.mId;
+    event->mParentId = aInitDict.mParentId;
+    event->mIndex = aInitDict.mIndex;
+    event->mUrl = aInitDict.mUrl;
+    event->mGuid = aInitDict.mGuid;
+    event->mParentGuid = aInitDict.mParentGuid;
+    event->mSource = aInitDict.mSource;
+    event->mIsTagging = aInitDict.mIsTagging;
+    event->mIsDescendantRemoval = aInitDict.mIsDescendantRemoval;
+    return event.forget();
+  }
+
+  JSObject* WrapObject(JSContext* aCx,
+                       JS::Handle<JSObject*> aGivenProto) override {
+    return PlacesBookmarkRemoved_Binding::Wrap(aCx, this, aGivenProto);
+  }
+
+  const PlacesBookmarkRemoved* AsPlacesBookmarkRemoved() const override {
+    return this;
+  }
+
+  int32_t Index() { return mIndex; }
+  bool IsDescendantRemoval() { return mIsDescendantRemoval; }
+
+  int32_t mIndex;
+  bool mIsDescendantRemoval;
+
+ private:
+  ~PlacesBookmarkRemoved() = default;
+};
+
+}  // namespace dom
+}  // namespace mozilla
+
+#endif

dom/base/PlacesEvent.h

@@ -37,6 +37,9 @@ class PlacesEvent : public nsWrapperCache {
   virtual const PlacesBookmarkAddition* AsPlacesBookmarkAddition() const {
     return nullptr;
   }
+  virtual const PlacesBookmarkRemoved* AsPlacesBookmarkRemoved() const {
+    return nullptr;
+  }
 
  protected:
   virtual ~PlacesEvent() = default;

dom/base/moz.build

@@ -211,6 +211,7 @@ EXPORTS.mozilla.dom += [
     'ParentProcessMessageManager.h',
     'PlacesBookmark.h',
     'PlacesBookmarkAddition.h',
+    'PlacesBookmarkRemoved.h',
     'PlacesEvent.h',
     'PlacesObservers.h',
     'PlacesVisit.h',

dom/chrome-webidl/PlacesEvent.webidl

@@ -10,6 +10,11 @@ enum PlacesEventType {
    * (or a bookmark folder/separator) is created.
    */
   "bookmark-added",
+  /**
+   * data: PlacesBookmarkRemoved. Fired whenever a bookmark
+   * (or a bookmark folder/separator) is created.
+   */
+  "bookmark-removed",
 };
 
 [ChromeOnly, Exposed=Window]
@@ -154,3 +159,30 @@ interface PlacesBookmarkAddition : PlacesBookmark {
    */
   readonly attribute unsigned long long dateAdded;
 };
+
+dictionary PlacesBookmarkRemovedInit {
+  required long long id;
+  required long long parentId;
+  required unsigned short itemType;
+  required DOMString url;
+  required ByteString guid;
+  required ByteString parentGuid;
+  required unsigned short source;
+  required long index;
+  required boolean isTagging;
+  boolean isDescendantRemoval = false;
+};
+
+[ChromeOnly, Exposed=Window]
+interface PlacesBookmarkRemoved : PlacesBookmark {
+  constructor(PlacesBookmarkRemovedInit initDict);
+  /**
+   * The item's index in the folder.
+   */
+  readonly attribute long index;
+
+  /**
+   * The item is a descendant of an item whose notification has been sent out.
+   */
+  readonly attribute boolean isDescendantRemoval;
+};

services/sync/modules/engines/bookmarks.js

@@ -1400,7 +1400,10 @@ BookmarksTracker.prototype = {
     this._placesListener = new PlacesWeakCallbackWrapper(
       this.handlePlacesEvents.bind(this)
     );
-    PlacesUtils.observers.addListener(["bookmark-added"], this._placesListener);
+    PlacesUtils.observers.addListener(
+      ["bookmark-added", "bookmark-removed"],
+      this._placesListener
+    );
     Svc.Obs.add("bookmarks-restore-begin", this);
     Svc.Obs.add("bookmarks-restore-success", this);
     Svc.Obs.add("bookmarks-restore-failed", this);
@@ -1409,7 +1412,7 @@ BookmarksTracker.prototype = {
   onStop() {
     PlacesUtils.bookmarks.removeObserver(this);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       this._placesListener
     );
     Svc.Obs.remove("bookmarks-restore-begin", this);
@@ -1483,22 +1486,25 @@ BookmarksTracker.prototype = {
 
   handlePlacesEvents(events) {
     for (let event of events) {
+      switch (event.type) {
+        case "bookmark-added":
           if (IGNORED_SOURCES.includes(event.source)) {
             continue;
           }
 
           this._log.trace("'bookmark-added': " + event.id);
           this._upScore();
-    }
+          break;
-  },
+        case "bookmark-removed":
-
+          if (IGNORED_SOURCES.includes(event.source)) {
-  onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid, source) {
+            continue;
-    if (IGNORED_SOURCES.includes(source)) {
-      return;
           }
 
-    this._log.trace("onItemRemoved: " + itemId);
+          this._log.trace("'bookmark-removed': " + event.id);
           this._upScore();
+          break;
+      }
+    }
   },
 
   // This method is oddly structured, but the idea is to return as quickly as

toolkit/components/places/Bookmarks.jsm

@@ -1287,26 +1287,24 @@ var Bookmarks = Object.freeze({
       // Notify onItemRemoved to listeners.
       for (let item of removeItems) {
         let observers = PlacesUtils.bookmarks.getObservers();
-        let uri = item.hasOwnProperty("url")
-          ? PlacesUtils.toURI(item.url)
-          : null;
         let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
-        notify(
+        let url = "";
-          observers,
+        if (item.type == Bookmarks.TYPE_BOOKMARK) {
-          "onItemRemoved",
+          url = item.hasOwnProperty("url") ? item.url.href : null;
-          [
+        }
-            item._id,
+        let notification = new PlacesBookmarkRemoved({
-            item._parentId,
+          id: item._id,
-            item.index,
+          url,
-            item.type,
+          itemType: item.type,
-            uri,
+          parentId: item._parentId,
-            item.guid,
+          index: item.index,
-            item.parentGuid,
+          guid: item.guid,
-            options.source,
+          parentGuid: item.parentGuid,
-          ],
+          source: options.source,
-          { isTagging: isUntagging }
+          isTagging: isUntagging,
-        );
+          isDescendantRemoval: false,
-
+        });
+        PlacesObservers.notifyListeners([notification]);
         if (isUntagging) {
           for (let entry of await fetchBookmarksByURL(item, {
             concurrent: true,
@@ -1823,7 +1821,6 @@ function notify(observers, notification, args = [], information = {}) {
 
     if (
       information.isDescendantRemoval &&
-      observer.skipDescendantsOnItemRemoval &&
       !PlacesUtils.bookmarks.userContentRoots.includes(information.parentGuid)
     ) {
       continue;
@@ -3217,30 +3214,28 @@ var removeFoldersContents = async function(db, folderGuids, options) {
   // Notify listeners in reverse order to serve children before parents.
   let { source = Bookmarks.SOURCES.DEFAULT } = options;
   let observers = PlacesUtils.bookmarks.getObservers();
+  let notification = [];
   for (let item of itemsRemoved.reverse()) {
-    let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null;
-    notify(
-      observers,
-      "onItemRemoved",
-      [
-        item._id,
-        item._parentId,
-        item.index,
-        item.type,
-        uri,
-        item.guid,
-        item.parentGuid,
-        source,
-      ],
-      // Notify observers that this item is being
-      // removed as a descendent.
-      {
-        isDescendantRemoval: true,
-        parentGuid: item.parentGuid,
-      }
-    );
-
     let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
+    let url = "";
+    if (item.type == Bookmarks.TYPE_BOOKMARK) {
+      url = item.hasOwnProperty("url") ? item.url.href : null;
+    }
+    notification = new PlacesBookmarkRemoved({
+      id: item._id,
+      url,
+      parentId: item._parentId,
+      index: item.index,
+      itemType: item.type,
+      guid: item.guid,
+      parentGuid: item.parentGuid,
+      source,
+      isTagging: isUntagging,
+      isDescendantRemoval: !PlacesUtils.bookmarks.userContentRoots.includes(
+        item.parentGuid
+      ),
+    });
+    PlacesObservers.notifyListeners([notification]);
     if (isUntagging) {
       for (let entry of await fetchBookmarksByURL(item, true)) {
         notify(observers, "onItemChanged", [

toolkit/components/places/PlacesUtils.jsm

@@ -2925,7 +2925,9 @@ var GuidHelper = {
   },
 
   ensureObservingRemovedItems() {
-    if (!("observer" in this)) {
+    if (this.addListeners) {
+      return;
+    }
     /**
      * This observers serves two purposes:
      * (1) Invalidate cached id<->GUID paris on when items are removed.
@@ -2935,39 +2937,30 @@ var GuidHelper = {
      */
     let listener = events => {
       for (let event of events) {
+        switch (event.type) {
+          case "bookmark-added":
             this.updateCache(event.id, event.guid);
             this.updateCache(event.parentId, event.parentGuid);
+            break;
+          case "bookmark-removed":
+            this.guidsForIds.delete(event.id);
+            this.idsForGuids.delete(event.guid);
+            this.updateCache(event.parentId, event.parentGuid);
+            break;
+        }
       }
     };
-      this.observer = {
-        onItemRemoved: (
-          aItemId,
-          aParentId,
-          aIndex,
-          aItemTyep,
-          aURI,
-          aGuid,
-          aParentGuid
-        ) => {
-          this.guidsForIds.delete(aItemId);
-          this.idsForGuids.delete(aGuid);
-          this.updateCache(aParentId, aParentGuid);
-        },
 
-        QueryInterface: ChromeUtils.generateQI([Ci.nsINavBookmarkObserver]),
+    this.addListeners = true;
-
+    PlacesUtils.observers.addListener(
-        onBeginUpdateBatch() {},
+      ["bookmark-added", "bookmark-removed"],
-        onEndUpdateBatch() {},
+      listener
-        onItemChanged() {},
+    );
-        onItemVisited() {},
-        onItemMoved() {},
-      };
-      PlacesUtils.bookmarks.addObserver(this.observer);
-      PlacesUtils.observers.addListener(["bookmark-added"], listener);
     PlacesUtils.registerShutdownFunction(() => {
-        PlacesUtils.bookmarks.removeObserver(this.observer);
+      PlacesUtils.observers.removeListener(
-        PlacesUtils.observers.removeListener(["bookmark-added"], listener);
+        ["bookmark-added", "bookmark-removed"],
+        listener
+      );
     });
-    }
   },
 };

toolkit/components/places/SyncedBookmarksMirror.jsm

@@ -51,7 +51,6 @@
 
 var EXPORTED_SYMBOLS = ["SyncedBookmarksMirror"];
 
-const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
@@ -2077,7 +2076,6 @@ class BookmarkObserverRecorder {
     this.notifyInStableOrder = notifyInStableOrder;
     this.signal = signal;
     this.placesEvents = [];
-    this.itemRemovedNotifications = [];
     this.guidChangedArgs = [];
     this.itemMovedArgs = [];
     this.itemChangedArgs = [];
@@ -2388,20 +2386,20 @@ class BookmarkObserverRecorder {
   }
 
   noteItemRemoved(info) {
-    let uri = info.urlHref ? Services.io.newURI(info.urlHref) : null;
+    this.placesEvents.push(
-    this.itemRemovedNotifications.push({
+      new PlacesBookmarkRemoved({
+        id: info.id,
+        parentId: info.parentId,
+        index: info.position,
+        url: info.urlHref || "",
+        guid: info.guid,
+        parentGuid: info.parentGuid,
+        source: PlacesUtils.bookmarks.SOURCES.SYNC,
+        itemType: info.type,
         isTagging: info.isUntagging,
-      args: [
+        isDescendantRemoval: false,
-        info.id,
+      })
-        info.parentId,
+    );
-        info.position,
-        info.type,
-        uri,
-        info.guid,
-        info.parentGuid,
-        PlacesUtils.bookmarks.SOURCES.SYNC,
-      ],
-    });
   }
 
   async notifyBookmarkObservers() {
@@ -2410,18 +2408,6 @@ class BookmarkObserverRecorder {
     for (let observer of observers) {
       this.notifyObserver(observer, "onBeginUpdateBatch");
     }
-    await Async.yieldingForEach(
-      this.itemRemovedNotifications,
-      info => {
-        if (this.signal.aborted) {
-          throw new SyncedBookmarksMirror.InterruptedError(
-            "Interrupted while notifying observers for removed items"
-          );
-        }
-        this.notifyObserversWithInfo(observers, "onItemRemoved", info);
-      },
-      yieldState
-    );
     await Async.yieldingForEach(
       this.guidChangedArgs,
       args => {

toolkit/components/places/TaggingService.jsm

@@ -22,7 +22,7 @@ function TaggingService() {
   // Observe bookmarks changes.
   PlacesUtils.bookmarks.addObserver(this);
   PlacesUtils.observers.addListener(
-    ["bookmark-added"],
+    ["bookmark-added", "bookmark-removed"],
     this.handlePlacesEvents
   );
 
@@ -104,7 +104,6 @@ TaggingService.prototype = {
     }
     return -1;
   },
-
   /**
    * Makes a proper array of tag objects like  { id: number, name: string }.
    *
@@ -328,7 +327,7 @@ TaggingService.prototype = {
     if (aTopic == TOPIC_SHUTDOWN) {
       PlacesUtils.bookmarks.removeObserver(this);
       PlacesUtils.observers.removeListener(
-        ["bookmark-added"],
+        ["bookmark-added", "bookmark-removed"],
         this.handlePlacesEvents
       );
       Services.obs.removeObserver(this, TOPIC_SHUTDOWN);
@@ -347,7 +346,7 @@ TaggingService.prototype = {
    * @returns an array of item ids
    */
   _getTaggedItemIdsIfUnbookmarkedURI: function TS__getTaggedItemIdsIfUnbookmarkedURI(
-    aURI
+    url
   ) {
     var itemIds = [];
     var isBookmarked = false;
@@ -360,7 +359,7 @@ TaggingService.prototype = {
        FROM moz_bookmarks
        WHERE fk = (SELECT id FROM moz_places WHERE url_hash = hash(:page_url) AND url = :page_url)`
     );
-    stmt.params.page_url = aURI.spec;
+    stmt.params.page_url = url;
     try {
       while (stmt.executeStep() && !isBookmarked) {
         if (this._tagFolders[stmt.row.parent]) {
@@ -380,6 +379,8 @@ TaggingService.prototype = {
 
   handlePlacesEvents(events) {
     for (let event of events) {
+      switch (event.type) {
+        case "bookmark-added":
           if (
             !event.isTagging ||
             event.itemType != PlacesUtils.bookmarks.TYPE_FOLDER
@@ -388,37 +389,36 @@ TaggingService.prototype = {
           }
 
           this._tagFolders[event.id] = event.title;
-    }
+          break;
-  },
+        case "bookmark-removed":
-
-  // nsINavBookmarkObserver
-  onItemRemoved: function TS_onItemRemoved(
-    aItemId,
-    aFolderId,
-    aIndex,
-    aItemType,
-    aURI,
-    aGuid,
-    aParentGuid,
-    aSource
-  ) {
           // Item is a tag folder.
-    if (aFolderId == PlacesUtils.tagsFolderId && this._tagFolders[aItemId]) {
+          if (
-      delete this._tagFolders[aItemId];
+            event.parentId == PlacesUtils.tagsFolderId &&
-    } else if (aURI && !this._tagFolders[aFolderId]) {
+            this._tagFolders[event.id]
+          ) {
+            delete this._tagFolders[event.id];
+            break;
+          }
+
+          Services.tm.dispatchToMainThread(() => {
+            if (event.url && !this._tagFolders[event.parentId]) {
               // Item is a bookmark that was removed from a non-tag folder.
               // If the only bookmark items now associated with the bookmark's URI are
               // contained in tag folders, the URI is no longer properly bookmarked, so
               // untag it.
-      let itemIds = this._getTaggedItemIdsIfUnbookmarkedURI(aURI);
+              let itemIds = this._getTaggedItemIdsIfUnbookmarkedURI(event.url);
               for (let i = 0; i < itemIds.length; i++) {
                 try {
-          PlacesUtils.bookmarks.removeItem(itemIds[i], aSource);
+                  PlacesUtils.bookmarks.removeItem(itemIds[i], event.source);
                 } catch (ex) {}
               }
-    } else if (aURI && this._tagFolders[aFolderId]) {
+            } else if (event.url && this._tagFolders[event.parentId]) {
               // Item is a tag entry.  If this was the last entry for this tag, remove it.
-      this._removeTagIfEmpty(aFolderId, aSource);
+              this._removeTagIfEmpty(event.parentId, event.source);
+            }
+          });
+          break;
+      }
     }
   },
 

toolkit/components/places/nsINavBookmarksService.idl

@@ -21,12 +21,6 @@ interface nsINavBookmarkObserver : nsISupports
    */
   readonly attribute boolean skipTags;
 
-  /*
-   * This observer should not be called for descendants when the parent is removed.
-   * For example when revmoing a folder containing bookmarks.
-   */
-  readonly attribute boolean skipDescendantsOnItemRemoval;
-
   /**
    * Notifies that a batch transaction has started.
    * Other notifications will be sent during the batch, but the observer is
@@ -42,40 +36,6 @@ interface nsINavBookmarkObserver : nsISupports
    */
   void onEndUpdateBatch();
 
-  /**
-   * Notifies that an item was removed.  Called after the actual remove took
-   * place.
-   * When an item is removed, all the items following it in the same folder
-   * will have their index shifted down, but no additional notifications will
-   * be sent.
-   *
-   * @param aItemId
-   *        The id of the item that was removed.
-   * @param aParentId
-   *        The id of the folder from which the item was removed.
-   * @param aIndex
-   *        The bookmark's index in the folder.
-   * @param aItemType
-   *        The type of the item to be removed (see TYPE_* constants below).
-   * @param aURI
-   *        The URI of the added item if it was TYPE_BOOKMARK, null otherwise.
-   * @param aGuid
-   *        The unique ID associated with the item.
-   * @param aParentGuid
-   *        The unique ID associated with the item's parent.
-   * @param aSource
-   *        A change source constant from nsINavBookmarksService::SOURCE_*,
-   *        passed to the method that notifies the observer.
-   */
-  void onItemRemoved(in long long aItemId,
-                     in long long aParentId,
-                     in long aIndex,
-                     in unsigned short aItemType,
-                     in nsIURI aURI,
-                     in ACString aGuid,
-                     in ACString aParentGuid,
-                     in unsigned short aSource);
-
   /**
    * Notifies that an item's information has changed.  This will be called
    * whenever any attributes like "title" are changed.
@@ -359,6 +319,7 @@ interface nsINavBookmarksService : nsISupports
    *         The change source, forwarded to all bookmark observers. Defaults
    *         to SOURCE_DEFAULT.
    */
+  [can_run_script]
   void removeItem(in long long aItemId, [optional] in unsigned short aSource);
 
   /**

toolkit/components/places/nsNavBookmarks.cpp

@@ -18,6 +18,7 @@
 #include "mozilla/Preferences.h"
 #include "mozilla/storage.h"
 #include "mozilla/dom/PlacesBookmarkAddition.h"
+#include "mozilla/dom/PlacesBookmarkRemoved.h"
 #include "mozilla/dom/PlacesObservers.h"
 #include "mozilla/dom/PlacesVisit.h"
 
@@ -59,11 +60,6 @@ bool SkipTags(nsCOMPtr<nsINavBookmarkObserver> obs) {
   (void)obs->GetSkipTags(&skipTags);
   return skipTags;
 }
-bool SkipDescendants(nsCOMPtr<nsINavBookmarkObserver> obs) {
-  bool skipDescendantsOnItemRemoval = false;
-  (void)obs->GetSkipDescendantsOnItemRemoval(&skipDescendantsOnItemRemoval);
-  return skipDescendantsOnItemRemoval;
-}
 
 template <typename Method, typename DataType>
 class AsyncGetBookmarksForURI : public AsyncStatementCallback {
@@ -643,14 +639,27 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource) {
     NS_WARNING_ASSERTION(uri, "Invalid URI in RemoveItem");
   }
 
-  NOTIFY_BOOKMARKS_OBSERVERS(
+  if (mCanNotify) {
-      mCanNotify, mObservers,
+    Sequence<OwningNonNull<PlacesEvent>> events;
-      SKIP_TAGS(bookmark.parentId == tagsRootId ||
+    RefPtr<PlacesBookmarkRemoved> bookmarkRef = new PlacesBookmarkRemoved();
-                bookmark.grandParentId == tagsRootId),
+    bookmarkRef->mItemType = bookmark.type;
-      OnItemRemoved(bookmark.id, bookmark.parentId, bookmark.position,
+    bookmarkRef->mId = bookmark.id;
-                    bookmark.type, uri, bookmark.guid, bookmark.parentGuid,
+    bookmarkRef->mParentId = bookmark.parentId;
-                    aSource));
+    bookmarkRef->mIndex = bookmark.position;
+    if (bookmark.type == TYPE_BOOKMARK) {
+      bookmarkRef->mUrl.Assign(NS_ConvertUTF8toUTF16(bookmark.url));
+    }
+    bookmarkRef->mGuid.Assign(bookmark.guid);
+    bookmarkRef->mParentGuid.Assign(bookmark.parentGuid);
+    bookmarkRef->mSource = aSource;
+    bookmarkRef->mIsTagging =
+        bookmark.parentId == tagsRootId || bookmark.grandParentId == tagsRootId;
+    bookmarkRef->mIsDescendantRemoval = false;
+    bool success = !!events.AppendElement(bookmarkRef.forget(), fallible);
+    MOZ_RELEASE_ASSERT(success);
 
+    PlacesObservers::NotifyListeners(events);
+  }
   if (bookmark.type == TYPE_BOOKMARK && bookmark.grandParentId == tagsRootId &&
       uri) {
     // If the removed bookmark was child of a tag container, notify a tags
@@ -930,12 +939,24 @@ nsresult nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId,
       NS_WARNING_ASSERTION(uri, "Invalid URI in RemoveFolderChildren");
     }
 
-    NOTIFY_BOOKMARKS_OBSERVERS(
+    if (mCanNotify) {
-        mCanNotify, mObservers,
+      Sequence<OwningNonNull<PlacesEvent>> events;
-        ((child.grandParentId == tagsRootId) ? SkipTags : SkipDescendants),
+      RefPtr<PlacesBookmarkRemoved> bookmark = new PlacesBookmarkRemoved();
-        OnItemRemoved(child.id, child.parentId, child.position, child.type, uri,
+      bookmark->mItemType = TYPE_BOOKMARK;
-                      child.guid, child.parentGuid, aSource));
+      bookmark->mId = child.id;
+      bookmark->mParentId = child.parentId;
+      bookmark->mIndex = child.position;
+      bookmark->mUrl.Assign(NS_ConvertUTF8toUTF16(child.url));
+      bookmark->mGuid.Assign(child.guid);
+      bookmark->mParentGuid.Assign(child.parentGuid);
+      bookmark->mSource = aSource;
+      bookmark->mIsTagging = (child.grandParentId == tagsRootId);
+      bookmark->mIsDescendantRemoval = (child.grandParentId != tagsRootId);
+      bool success = !!events.AppendElement(bookmark.forget(), fallible);
+      MOZ_RELEASE_ASSERT(success);
 
+      PlacesObservers::NotifyListeners(events);
+    }
     if (child.type == TYPE_BOOKMARK && child.grandParentId == tagsRootId &&
         uri) {
       // If the removed bookmark was a child of a tag container, notify all

toolkit/components/places/nsNavBookmarks.h

@@ -302,6 +302,7 @@ class nsNavBookmarks final
                                int64_t aSyncChangeDelta, int64_t aItemId,
                                PRTime aValue);
 
+  MOZ_CAN_RUN_SCRIPT
   nsresult RemoveFolderChildren(int64_t aFolderId, uint16_t aSource);
 
   // Recursive method to build an array of folder's children

toolkit/components/places/nsNavHistoryResult.cpp

@@ -1973,13 +1973,6 @@ nsNavHistoryQueryResultNode::GetSkipTags(bool* aSkipTags) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNavHistoryQueryResultNode::GetSkipDescendantsOnItemRemoval(
-    bool* aSkipDescendantsOnItemRemoval) {
-  *aSkipDescendantsOnItemRemoval = false;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::OnBeginUpdateBatch() { return NS_OK; }
 
@@ -2451,14 +2444,15 @@ nsresult nsNavHistoryQueryResultNode::OnItemAdded(
   return NS_OK;
 }
 
-NS_IMETHODIMP
+nsresult nsNavHistoryQueryResultNode::OnItemRemoved(
-nsNavHistoryQueryResultNode::OnItemRemoved(int64_t aItemId, int64_t aParentId,
+    int64_t aItemId, int64_t aParentFolder, int32_t aIndex, uint16_t aItemType,
-                                           int32_t aIndex, uint16_t aItemType,
+    nsIURI* aURI, const nsACString& aGUID, const nsACString& aParentGUID,
-                                           nsIURI* aURI,
-                                           const nsACString& aGUID,
-                                           const nsACString& aParentGUID,
     uint16_t aSource) {
-  if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK &&
+  if ((aItemType == nsINavBookmarksService::TYPE_BOOKMARK ||
+       (aItemType == nsINavBookmarksService::TYPE_FOLDER &&
+        mOptions->ResultType() ==
+            nsINavHistoryQueryOptions::RESULTS_AS_TAGS_ROOT &&
+        aParentGUID.EqualsLiteral(TAGS_ROOT_GUID))) &&
       mLiveUpdate != QUERYUPDATE_SIMPLE && mLiveUpdate != QUERYUPDATE_TIME &&
       mLiveUpdate != QUERYUPDATE_MOBILEPREF) {
     nsresult rv = Refresh();
@@ -3036,13 +3030,6 @@ nsNavHistoryFolderResultNode::GetSkipTags(bool* aSkipTags) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNavHistoryFolderResultNode::GetSkipDescendantsOnItemRemoval(
-    bool* aSkipDescendantsOnItemRemoval) {
-  *aSkipDescendantsOnItemRemoval = false;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::OnBeginUpdateBatch() { return NS_OK; }
 
@@ -3167,8 +3154,7 @@ nsresult nsNavHistoryQueryResultNode::OnMobilePrefChanged(bool newValue) {
   return RemoveChildAt(existingIndex);
 }
 
-NS_IMETHODIMP
+nsresult nsNavHistoryFolderResultNode::OnItemRemoved(
-nsNavHistoryFolderResultNode::OnItemRemoved(
     int64_t aItemId, int64_t aParentFolder, int32_t aIndex, uint16_t aItemType,
     nsIURI* aURI, const nsACString& aGUID, const nsACString& aParentGUID,
     uint16_t aSource) {
@@ -3539,7 +3525,7 @@ nsNavHistoryResult::~nsNavHistoryResult() {
 }
 
 void nsNavHistoryResult::StopObserving() {
-  AutoTArray<PlacesEventType, 2> events;
+  AutoTArray<PlacesEventType, 3> events;
   if (mIsBookmarkFolderObserver || mIsAllBookmarksObserver) {
     nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
     if (bookmarks) {
@@ -3548,6 +3534,7 @@ void nsNavHistoryResult::StopObserving() {
       mIsAllBookmarksObserver = false;
     }
     events.AppendElement(PlacesEventType::Bookmark_added);
+    events.AppendElement(PlacesEventType::Bookmark_removed);
   }
   if (mIsMobilePrefObserver) {
     Preferences::UnregisterCallback(OnMobilePrefChangedCallback,
@@ -3594,8 +3581,9 @@ void nsNavHistoryResult::AddAllBookmarksObserver(
       return;
     }
     bookmarks->AddObserver(this, true);
-    AutoTArray<PlacesEventType, 1> events;
+    AutoTArray<PlacesEventType, 2> events;
     events.AppendElement(PlacesEventType::Bookmark_added);
+    events.AppendElement(PlacesEventType::Bookmark_removed);
     PlacesObservers::AddListener(events, this);
     mIsAllBookmarksObserver = true;
   }
@@ -3631,8 +3619,9 @@ void nsNavHistoryResult::AddBookmarkFolderObserver(
       return;
     }
     bookmarks->AddObserver(this, true);
-    AutoTArray<PlacesEventType, 1> events;
+    AutoTArray<PlacesEventType, 2> events;
     events.AppendElement(PlacesEventType::Bookmark_added);
+    events.AppendElement(PlacesEventType::Bookmark_removed);
     PlacesObservers::AddListener(events, this);
     mIsBookmarkFolderObserver = true;
   }
@@ -3823,13 +3812,6 @@ nsNavHistoryResult::GetSkipTags(bool* aSkipTags) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNavHistoryResult::GetSkipDescendantsOnItemRemoval(
-    bool* aSkipDescendantsOnItemRemoval) {
-  *aSkipDescendantsOnItemRemoval = true;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsNavHistoryResult::OnBeginUpdateBatch() {
   // Since we could be observing both history and bookmarks, it's possible both
@@ -3867,26 +3849,6 @@ nsNavHistoryResult::OnEndUpdateBatch() {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNavHistoryResult::OnItemRemoved(int64_t aItemId, int64_t aParentId,
-                                  int32_t aIndex, uint16_t aItemType,
-                                  nsIURI* aURI, const nsACString& aGUID,
-                                  const nsACString& aParentGUID,
-                                  uint16_t aSource) {
-  NS_ENSURE_ARG(aItemType != nsINavBookmarksService::TYPE_BOOKMARK || aURI);
-
-  ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
-      aParentId, OnItemRemoved(aItemId, aParentId, aIndex, aItemType, aURI,
-                               aGUID, aParentGUID, aSource));
-  ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemRemoved(aItemId, aParentId, aIndex,
-                                                  aItemType, aURI, aGUID,
-                                                  aParentGUID, aSource));
-  ENUMERATE_HISTORY_OBSERVERS(OnItemRemoved(aItemId, aParentId, aIndex,
-                                            aItemType, aURI, aGUID, aParentGUID,
-                                            aSource));
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsNavHistoryResult::OnItemChanged(
     int64_t aItemId, const nsACString& aProperty, bool aIsAnnotationProperty,
@@ -4113,6 +4075,31 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
                         item->mGuid, item->mParentGuid, item->mSource));
         break;
       }
+      case PlacesEventType::Bookmark_removed: {
+        const dom::PlacesBookmarkRemoved* item =
+            event->AsPlacesBookmarkRemoved();
+        if (NS_WARN_IF(!item)) {
+          continue;
+        }
+
+        nsCOMPtr<nsIURI> uri;
+
+        if (item->mIsDescendantRemoval) {
+          continue;
+        }
+        ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
+            item->mParentId,
+            OnItemRemoved(item->mId, item->mParentId, item->mIndex,
+                          item->mItemType, uri, item->mGuid, item->mParentGuid,
+                          item->mSource));
+        ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemRemoved(
+            item->mId, item->mParentId, item->mIndex, item->mItemType, uri,
+            item->mGuid, item->mParentGuid, item->mSource));
+        ENUMERATE_HISTORY_OBSERVERS(OnItemRemoved(
+            item->mId, item->mParentId, item->mIndex, item->mItemType, uri,
+            item->mGuid, item->mParentGuid, item->mSource));
+        break;
+      }
       default: {
         MOZ_ASSERT_UNREACHABLE(
             "Receive notification of a type not subscribed to.");

toolkit/components/places/nsNavHistoryResult.h

@@ -687,6 +687,12 @@ class nsNavHistoryQueryResultNode final
                        uint16_t aItemType, nsIURI* aURI, PRTime aDateAdded,
                        const nsACString& aGUID, const nsACString& aParentGUID,
                        uint16_t aSource);
+
+  nsresult OnItemRemoved(int64_t aItemId, int64_t aParentFolder, int32_t aIndex,
+                         uint16_t aItemType, nsIURI* aURI,
+                         const nsACString& aGUID, const nsACString& aParentGUID,
+                         uint16_t aSource);
+
   // The internal version has an output aAdded parameter, it is incremented by
   // query nodes when the visited uri belongs to them. If no such query exists,
   // the history result creates a new query node dynamically.
@@ -768,7 +774,10 @@ class nsNavHistoryFolderResultNode final
                        uint16_t aItemType, nsIURI* aURI, PRTime aDateAdded,
                        const nsACString& aGUID, const nsACString& aParentGUID,
                        uint16_t aSource);
-
+  nsresult OnItemRemoved(int64_t aItemId, int64_t aParentFolder, int32_t aIndex,
+                         uint16_t aItemType, nsIURI* aURI,
+                         const nsACString& aGUID, const nsACString& aParentGUID,
+                         uint16_t aSource);
   virtual void OnRemoving() override;
 
   // this indicates whether the folder contents are valid, they don't go away

toolkit/components/places/tests/PlacesTestUtils.jsm

@@ -417,7 +417,7 @@ var PlacesTestUtils = Object.freeze({
                 }
               };
             }
-            if (name == "skipTags" || name == "skipDescendantsOnItemRemoval") {
+            if (name == "skipTags") {
               return false;
             }
             return () => false;

toolkit/components/places/tests/bookmarks/head_bookmarks.js

@@ -15,14 +15,10 @@ var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 // Put any other stuff relative to this test folder below.
 
-function expectNotifications(skipDescendants, checkAllArgs) {
+function expectNotifications(checkAllArgs) {
   let notifications = [];
   let observer = new Proxy(NavBookmarkObserver, {
     get(target, name) {
-      if (name == "skipDescendantsOnItemRemoval") {
-        return skipDescendants;
-      }
-
       if (name == "check") {
         PlacesUtils.bookmarks.removeObserver(observer);
         return expectedNotifications =>
@@ -58,10 +54,16 @@ function expectNotifications(skipDescendants, checkAllArgs) {
   return observer;
 }
 
-function expectPlacesObserverNotifications(types, checkAllArgs) {
+function expectPlacesObserverNotifications(
+  types,
+  checkAllArgs = true,
+  skipDescendants = false
+) {
   let notifications = [];
   let listener = events => {
     for (let event of events) {
+      switch (event.type) {
+        case "bookmark-added":
           notifications.push({
             type: event.type,
             id: event.id,
@@ -76,6 +78,36 @@ function expectPlacesObserverNotifications(types, checkAllArgs) {
             source: event.source,
             isTagging: event.isTagging,
           });
+          break;
+        case "bookmark-removed":
+          if (
+            !(
+              skipDescendants &&
+              event.isDescendantRemoval &&
+              !PlacesUtils.bookmarks.userContentRoots.includes(event.parentGuid)
+            )
+          ) {
+            if (checkAllArgs) {
+              notifications.push({
+                type: event.type,
+                id: event.id,
+                itemType: event.itemType,
+                parentId: event.parentId,
+                index: event.index,
+                url: event.url || null,
+                guid: event.guid,
+                parentGuid: event.parentGuid,
+                source: event.source,
+                isTagging: event.isTagging,
+              });
+            } else {
+              notifications.push({
+                type: event.type,
+                guid: event.guid,
+              });
+            }
+          }
+      }
     }
   };
   PlacesUtils.observers.addListener(types, listener);

toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js

@@ -184,24 +184,27 @@ add_task(async function test_notifications() {
     ],
   });
 
-  let skipDescendantsObserver = expectNotifications(true);
+  let skipDescendantsObserver = expectPlacesObserverNotifications(
-  let receiveAllObserver = expectNotifications(false);
+    ["bookmark-removed"],
+    false,
+    true
+  );
+  let receiveAllObserver = expectPlacesObserverNotifications(
+    ["bookmark-removed"],
+    false
+  );
 
   await PlacesUtils.bookmarks.eraseEverything();
 
   let expectedNotifications = [
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: {
       guid: bms[1].guid,
     },
-    },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: {
       guid: bms[0].guid,
     },
-    },
   ];
 
   // If we're skipping descendents, we'll only be notified of the folder.
@@ -209,10 +212,8 @@ add_task(async function test_notifications() {
 
   // Note: Items of folders get notified first.
   expectedNotifications.unshift({
-    name: "onItemRemoved",
+    type: "bookmark-removed",
-    arguments: {
     guid: bms[2].guid,
-    },
   });
 
   receiveAllObserver.check(expectedNotifications);

toolkit/components/places/tests/bookmarks/test_bookmarks_moveToFolder.js

@@ -198,7 +198,7 @@ async function testMoveToFolder(details) {
 
   let observer;
   if (details.notifications) {
-    observer = expectNotifications(false, true);
+    observer = expectNotifications(true);
   }
 
   let movedItems = await PlacesUtils.bookmarks.moveToFolder(

toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js

@@ -437,21 +437,20 @@ add_task(async function remove_bookmark() {
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
   await PlacesUtils.bookmarks.remove(bm.guid);
   observer.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: itemId,
-        itemId,
       parentId,
-        bm.index,
+      index: bm.index,
-        bm.type,
+      url: bm.url,
-        bm.url,
+      guid: bm.guid,
-        bm.guid,
+      parentGuid: bm.parentGuid,
-        bm.parentGuid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      ],
+      isTagging: false,
     },
   ]);
 });
@@ -472,34 +471,32 @@ add_task(async function remove_multiple_bookmarks() {
   let itemId2 = await PlacesUtils.promiseItemId(bm2.guid);
   let parentId2 = await PlacesUtils.promiseItemId(bm2.parentGuid);
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
   await PlacesUtils.bookmarks.remove([bm1, bm2]);
   observer.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: itemId1,
-        itemId1,
+      parentId: parentId1,
-        parentId1,
+      index: bm1.index,
-        bm1.index,
+      url: bm1.url,
-        bm1.type,
+      guid: bm1.guid,
-        bm1.url,
+      parentGuid: bm1.parentGuid,
-        bm1.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        bm1.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: itemId2,
-        itemId2,
+      parentId: parentId2,
-        parentId2,
+      index: bm2.index - 1,
-        bm2.index - 1,
+      url: bm2.url,
-        bm2.type,
+      guid: bm2.guid,
-        bm2.url,
+      parentGuid: bm2.parentGuid,
-        bm2.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        bm2.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
   ]);
 });
@@ -512,21 +509,20 @@ add_task(async function remove_folder() {
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
   await PlacesUtils.bookmarks.remove(bm.guid);
   observer.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: itemId,
-        itemId,
       parentId,
-        bm.index,
+      index: bm.index,
-        bm.type,
+      url: null,
-        null,
+      guid: bm.guid,
-        bm.guid,
+      parentGuid: bm.parentGuid,
-        bm.parentGuid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-      ],
+      isTagging: false,
     },
   ]);
 });
@@ -553,23 +549,25 @@ add_task(async function remove_bookmark_tag_notification() {
   let tagId = await PlacesUtils.promiseItemId(tag.guid);
   let tagParentId = await PlacesUtils.promiseItemId(tag.parentGuid);
 
+  let placesObserver = expectPlacesObserverNotifications(["bookmark-removed"]);
   let observer = expectNotifications();
   await PlacesUtils.bookmarks.remove(tag.guid);
 
-  observer.check([
+  placesObserver.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: tagId,
-        tagId,
+      parentId: tagParentId,
-        tagParentId,
+      index: tag.index,
-        tag.index,
+      url: tag.url,
-        tag.type,
+      guid: tag.guid,
-        tag.url,
+      parentGuid: tag.parentGuid,
-        tag.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        tag.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: true,
-      ],
     },
+  ]);
+  observer.check([
     {
       name: "onItemChanged",
       arguments: [
@@ -617,61 +615,57 @@ add_task(async function remove_folder_notification() {
   });
   let bm2ItemId = await PlacesUtils.promiseItemId(bm2.guid);
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
   await PlacesUtils.bookmarks.remove(folder1.guid);
 
   observer.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: bm2ItemId,
-        bm2ItemId,
+      parentId: folder2Id,
-        folder2Id,
+      index: bm2.index,
-        bm2.index,
+      url: bm2.url,
-        bm2.type,
+      guid: bm2.guid,
-        bm2.url,
+      parentGuid: bm2.parentGuid,
-        bm2.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        bm2.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: folder2Id,
-        folder2Id,
+      parentId: folder1Id,
-        folder1Id,
+      index: folder2.index,
-        folder2.index,
+      url: null,
-        folder2.type,
+      guid: folder2.guid,
-        null,
+      parentGuid: folder2.parentGuid,
-        folder2.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        folder2.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: bmItemId,
-        bmItemId,
+      parentId: folder1Id,
-        folder1Id,
+      index: bm.index,
-        bm.index,
+      url: bm.url,
-        bm.type,
+      guid: bm.guid,
-        bm.url,
+      parentGuid: bm.parentGuid,
-        bm.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        bm.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: folder1Id,
-        folder1Id,
+      parentId: folder1ParentId,
-        folder1ParentId,
+      index: folder1.index,
-        folder1.index,
+      url: null,
-        folder1.type,
+      guid: folder1.guid,
-        null,
+      parentGuid: folder1.parentGuid,
-        folder1.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        folder1.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
   ]);
 });
@@ -718,75 +712,70 @@ add_task(async function eraseEverything_notification() {
   let menuBmId = await PlacesUtils.promiseItemId(menuBm.guid);
   let menuBmParentId = await PlacesUtils.promiseItemId(menuBm.parentGuid);
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
   await PlacesUtils.bookmarks.eraseEverything();
 
   // Bookmarks should always be notified before their parents.
   observer.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: itemId,
-        itemId,
       parentId,
-        bm.index,
+      index: bm.index,
-        bm.type,
+      url: bm.url,
-        bm.url,
+      guid: bm.guid,
-        bm.guid,
+      parentGuid: bm.parentGuid,
-        bm.parentGuid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      ],
+      isTagging: false,
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: folder2Id,
-        folder2Id,
+      parentId: folder2ParentId,
-        folder2ParentId,
+      index: folder2.index,
-        folder2.index,
+      url: null,
-        folder2.type,
+      guid: folder2.guid,
-        null,
+      parentGuid: folder2.parentGuid,
-        folder2.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        folder2.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: folder1Id,
-        folder1Id,
+      parentId: folder1ParentId,
-        folder1ParentId,
+      index: folder1.index,
-        folder1.index,
+      url: null,
-        folder1.type,
+      guid: folder1.guid,
-        null,
+      parentGuid: folder1.parentGuid,
-        folder1.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        folder1.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: menuBmId,
-        menuBmId,
+      parentId: menuBmParentId,
-        menuBmParentId,
+      index: menuBm.index,
-        menuBm.index,
+      url: menuBm.url,
-        menuBm.type,
+      guid: menuBm.guid,
-        menuBm.url,
+      parentGuid: menuBm.parentGuid,
-        menuBm.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        menuBm.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: toolbarBmId,
-        toolbarBmId,
+      parentId: toolbarBmParentId,
-        toolbarBmParentId,
+      index: toolbarBm.index,
-        toolbarBm.index,
+      url: toolbarBm.url,
-        toolbarBm.type,
+      guid: toolbarBm.guid,
-        toolbarBm.url,
+      parentGuid: toolbarBm.parentGuid,
-        toolbarBm.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        toolbarBm.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
   ]);
 });
@@ -820,49 +809,46 @@ add_task(async function eraseEverything_reparented_notification() {
   bm = await PlacesUtils.bookmarks.update(bm);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
-  let observer = expectNotifications();
+  let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
   await PlacesUtils.bookmarks.eraseEverything();
 
   // Bookmarks should always be notified before their parents.
   observer.check([
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: itemId,
-        itemId,
       parentId,
-        bm.index,
+      index: bm.index,
-        bm.type,
+      url: bm.url,
-        bm.url,
+      guid: bm.guid,
-        bm.guid,
+      parentGuid: bm.parentGuid,
-        bm.parentGuid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      ],
+      isTagging: false,
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: folder2Id,
-        folder2Id,
+      parentId: folder2ParentId,
-        folder2ParentId,
+      index: folder2.index,
-        folder2.index,
+      url: null,
-        folder2.type,
+      guid: folder2.guid,
-        null,
+      parentGuid: folder2.parentGuid,
-        folder2.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        folder2.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: [
+      id: folder1Id,
-        folder1Id,
+      parentId: folder1ParentId,
-        folder1ParentId,
+      index: folder1.index,
-        folder1.index,
+      url: null,
-        folder1.type,
+      guid: folder1.guid,
-        null,
+      parentGuid: folder1.parentGuid,
-        folder1.guid,
+      source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
-        folder1.parentGuid,
+      itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
-        Ci.nsINavBookmarksService.SOURCE_DEFAULT,
+      isTagging: false,
-      ],
     },
   ]);
 });

toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js

@@ -198,11 +198,14 @@ add_task(async function remove_multiple_bookmarks_complex() {
   let bmsToRemove = bms.slice(2, 4);
   let notifiedIndexes = [];
   let notificationPromise = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (itemId, parentId, index, itemType, uri, guid, parentGuid, source) => {
+    events => {
-      notifiedIndexes.push({ guid, index });
+      for (let event of events) {
-      return notifiedIndexes.length == bmsToRemove.length;
+        notifiedIndexes.push({ guid: event.guid, index: event.index });
       }
+      return notifiedIndexes.length == bmsToRemove.length;
+    },
+    "places"
   );
   await PlacesUtils.bookmarks.remove(bmsToRemove);
   await notificationPromise;
@@ -238,11 +241,14 @@ add_task(async function remove_multiple_bookmarks_complex() {
   bmsToRemove = [bms[1], bms[5], bms[6], bms[8]];
   notifiedIndexes = [];
   notificationPromise = PlacesTestUtils.waitForNotification(
-    "onItemRemoved",
+    "bookmark-removed",
-    (itemId, parentId, index, itemType, uri, guid, parentGuid, source) => {
+    events => {
-      notifiedIndexes.push({ guid, index });
+      for (let event of events) {
-      return notifiedIndexes.length == bmsToRemove.length;
+        notifiedIndexes.push({ guid: event.guid, index: event.index });
       }
+      return notifiedIndexes.length == bmsToRemove.length;
+    },
+    "places"
   );
   await PlacesUtils.bookmarks.remove(bmsToRemove);
   await notificationPromise;
@@ -344,8 +350,16 @@ add_task(async function test_contents_removed() {
     title: "",
   });
 
-  let skipDescendantsObserver = expectNotifications(true);
+  let skipDescendantsObserver = expectPlacesObserverNotifications(
-  let receiveAllObserver = expectNotifications(false);
+    ["bookmark-removed"],
+    false,
+    true
+  );
+  let receiveAllObserver = expectPlacesObserverNotifications(
+    ["bookmark-removed"],
+    false,
+    false
+  );
   let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0);
   await PlacesUtils.bookmarks.remove(folder1);
   Assert.strictEqual(await PlacesUtils.bookmarks.fetch(folder1.guid), null);
@@ -355,11 +369,9 @@ add_task(async function test_contents_removed() {
 
   let expectedNotifications = [
     {
-      name: "onItemRemoved",
+      type: "bookmark-removed",
-      arguments: {
       guid: folder1.guid,
     },
-    },
   ];
 
   // If we're skipping descendents, we'll only be notified of the folder.
@@ -367,12 +379,9 @@ add_task(async function test_contents_removed() {
 
   // Note: Items of folders get notified first.
   expectedNotifications.unshift({
-    name: "onItemRemoved",
+    type: "bookmark-removed",
-    arguments: {
     guid: bm1.guid,
-    },
   });
-
   // If we don't skip descendents, we'll be notified of the folder and the
   // bookmark.
   receiveAllObserver.check(expectedNotifications);

toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js

@@ -63,9 +63,6 @@ var gBookmarksObserver = {
   onEndUpdateBatch() {
     return this.validate("onEndUpdateBatch", arguments);
   },
-  onItemRemoved() {
-    return this.validate("onItemRemoved", arguments);
-  },
   onItemChanged() {
     return this.validate("onItemChanged", arguments);
   },
@@ -82,7 +79,6 @@ var gBookmarksObserver = {
 
 var gBookmarkSkipObserver = {
   skipTags: true,
-  skipDescendantsOnItemRemoval: true,
 
   expected: null,
   setup(expected) {
@@ -122,9 +118,6 @@ var gBookmarkSkipObserver = {
   onEndUpdateBatch() {
     return this.validate("onEndUpdateBatch", arguments);
   },
-  onItemRemoved() {
-    return this.validate("onItemRemoved", arguments);
-  },
   onItemChanged() {
     return this.validate("onItemChanged", arguments);
   },
@@ -152,11 +145,11 @@ add_task(async function setup() {
     gBookmarkSkipObserver
   );
   PlacesUtils.observers.addListener(
-    ["bookmark-added"],
+    ["bookmark-added", "bookmark-removed"],
     gBookmarksObserver.handlePlacesEvents
   );
   PlacesUtils.observers.addListener(
-    ["bookmark-added"],
+    ["bookmark-added", "bookmark-removed"],
     gBookmarkSkipObserver.handlePlacesEvents
   );
 });
@@ -429,42 +422,16 @@ add_task(async function onItemChanged_tags_bookmark() {
         ],
       },
       {
-        name: "onItemRemoved", // This is the tag.
+        eventType: "bookmark-removed", // This is the tag.
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
           },
-          { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
+          { name: "url", check: v => v == uri.spec },
-          {
-            name: "guid",
-            check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
-          },
-          {
-            name: "parentGuid",
-            check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
-          },
-          {
-            name: "source",
-            check: v =>
-              Object.values(PlacesUtils.bookmarks.SOURCES).includes(v),
-          },
-        ],
-      },
-      {
-        name: "onItemRemoved", // This is the tag folder.
-        args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
-          { name: "parentId", check: v => v === PlacesUtils.tagsFolderId },
-          { name: "index", check: v => v === 0 },
-          {
-            name: "itemType",
-            check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
-          },
-          { name: "uri", check: v => v === null },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -480,6 +447,7 @@ add_task(async function onItemChanged_tags_bookmark() {
           },
         ],
       },
+
       {
         name: "onItemChanged",
         args: [
@@ -509,6 +477,32 @@ add_task(async function onItemChanged_tags_bookmark() {
           },
         ],
       },
+      {
+        eventType: "bookmark-removed", // This is the tag folder.
+        args: [
+          { name: "id", check: v => typeof v == "number" && v > 0 },
+          { name: "parentId", check: v => v === PlacesUtils.tagsFolderId },
+          { name: "index", check: v => v === 0 },
+          {
+            name: "itemType",
+            check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
+          },
+          { name: "url", check: v => v === "" },
+          {
+            name: "guid",
+            check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
+          },
+          {
+            name: "parentGuid",
+            check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
+          },
+          {
+            name: "source",
+            check: v =>
+              Object.values(PlacesUtils.bookmarks.SOURCES).includes(v),
+          },
+        ],
+      },
     ]),
   ]);
   PlacesUtils.tagging.tagURI(uri, [TAG]);
@@ -646,26 +640,26 @@ add_task(async function onItemMoved_bookmark() {
   await promise;
 });
 
-add_task(async function onItemRemoved_bookmark() {
+add_task(async function bookmarkItemRemoved_bookmark() {
   let bm = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     index: 0,
   });
   let uri = Services.io.newURI(bm.url.href);
   let promise = Promise.all([
-    gBookmarkSkipObserver.setup(["onItemRemoved"]),
+    gBookmarkSkipObserver.setup(["bookmark-removed"]),
     gBookmarksObserver.setup([
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => v === gUnfiledFolderId },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
           },
-          { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
+          { name: "url", check: v => v === uri.spec },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -687,25 +681,25 @@ add_task(async function onItemRemoved_bookmark() {
   await promise;
 });
 
-add_task(async function onItemRemoved_separator() {
+add_task(async function bookmarkItemRemoved_separator() {
   let bm = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     index: 0,
   });
   let promise = Promise.all([
-    gBookmarkSkipObserver.setup(["onItemRemoved"]),
+    gBookmarkSkipObserver.setup(["bookmark-removed"]),
     gBookmarksObserver.setup([
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_SEPARATOR,
           },
-          { name: "uri", check: v => v === null },
+          { name: "url", check: v => v === "" },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -727,25 +721,25 @@ add_task(async function onItemRemoved_separator() {
   await promise;
 });
 
-add_task(async function onItemRemoved_folder() {
+add_task(async function bookmarkItemRemoved_folder() {
   let bm = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     index: 0,
   });
   let promise = Promise.all([
-    gBookmarkSkipObserver.setup(["onItemRemoved"]),
+    gBookmarkSkipObserver.setup(["bookmark-removed"]),
     gBookmarksObserver.setup([
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
           },
-          { name: "uri", check: v => v === null },
+          { name: "url", check: v => v === "" },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -767,7 +761,7 @@ add_task(async function onItemRemoved_folder() {
   await promise;
 });
 
-add_task(async function onItemRemoved_folder_recursive() {
+add_task(async function bookmarkItemRemoved_folder_recursive() {
   const title = "Folder 3";
   const BMTITLE = "Bookmark 1";
   let uri = Services.io.newURI("http://1.mozilla.org/");
@@ -777,7 +771,10 @@ add_task(async function onItemRemoved_folder_recursive() {
       "bookmark-added",
       "bookmark-added",
       "bookmark-added",
-      "onItemRemoved",
+      "bookmark-removed",
+      "bookmark-removed",
+      "bookmark-removed",
+      "bookmark-removed",
     ]),
     gBookmarksObserver.setup([
       {
@@ -893,16 +890,16 @@ add_task(async function onItemRemoved_folder_recursive() {
         ],
       },
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
           },
-          { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
+          { name: "url", check: v => v === uri.spec },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -919,16 +916,16 @@ add_task(async function onItemRemoved_folder_recursive() {
         ],
       },
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 1 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
           },
-          { name: "uri", check: v => v === null },
+          { name: "url", check: v => v === "" },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -945,16 +942,16 @@ add_task(async function onItemRemoved_folder_recursive() {
         ],
       },
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
           },
-          { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
+          { name: "url", check: v => v === uri.spec },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@@ -971,16 +968,16 @@ add_task(async function onItemRemoved_folder_recursive() {
         ],
       },
       {
-        name: "onItemRemoved",
+        eventType: "bookmark-removed",
         args: [
-          { name: "itemId", check: v => typeof v == "number" && v > 0 },
+          { name: "id", check: v => typeof v == "number" && v > 0 },
           { name: "parentId", check: v => typeof v == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           {
             name: "itemType",
             check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
           },
-          { name: "uri", check: v => v === null },
+          { name: "url", check: v => v === "" },
           {
             name: "guid",
             check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),

toolkit/components/places/tests/bookmarks/test_removeFolderTransaction_reinsert.js

@@ -31,7 +31,7 @@ add_task(async function test_removeFolderTransaction_reinsert() {
   let listener = events => {
     for (let event of events) {
       notifications.push([
-        "bookmark-added",
+        event.type,
         event.id,
         event.parentId,
         event.guid,
@@ -41,15 +41,18 @@ add_task(async function test_removeFolderTransaction_reinsert() {
   };
   let observer = {
     __proto__: NavBookmarkObserver.prototype,
-    onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid) {
-      notifications.push(["onItemRemoved", itemId, parentId, guid, parentGuid]);
-    },
   };
   PlacesUtils.bookmarks.addObserver(observer);
-  PlacesUtils.observers.addListener(["bookmark-added"], listener);
+  PlacesUtils.observers.addListener(
+    ["bookmark-added", "bookmark-removed"],
+    listener
+  );
   PlacesUtils.registerShutdownFunction(function() {
     PlacesUtils.bookmarks.removeObserver(observer);
-    PlacesUtils.observers.removeListener(["bookmark-added"], listener);
+    PlacesUtils.observers.removeListener(
+      ["bookmark-added", "bookmark-removed"],
+      listener
+    );
   });
 
   let transaction = PlacesTransactions.Remove({ guid: folder.guid });
@@ -62,10 +65,10 @@ add_task(async function test_removeFolderTransaction_reinsert() {
 
   checkNotifications(
     [
-      ["onItemRemoved", tbId, folderId, tb.guid, folder.guid],
+      ["bookmark-removed", tbId, folderId, tb.guid, folder.guid],
-      ["onItemRemoved", fxId, folderId, fx.guid, folder.guid],
+      ["bookmark-removed", fxId, folderId, fx.guid, folder.guid],
       [
-        "onItemRemoved",
+        "bookmark-removed",
         folderId,
         PlacesUtils.bookmarksMenuFolderId,
         folder.guid,
@@ -100,10 +103,10 @@ add_task(async function test_removeFolderTransaction_reinsert() {
 
   checkNotifications(
     [
-      ["onItemRemoved", tbId, folderId, tb.guid, folder.guid],
+      ["bookmark-removed", tbId, folderId, tb.guid, folder.guid],
-      ["onItemRemoved", fxId, folderId, fx.guid, folder.guid],
+      ["bookmark-removed", fxId, folderId, fx.guid, folder.guid],
       [
-        "onItemRemoved",
+        "bookmark-removed",
         folderId,
         PlacesUtils.bookmarksMenuFolderId,
         folder.guid,

toolkit/components/places/tests/legacy/test_bookmarks.js

@@ -12,6 +12,8 @@ var bookmarksObserver = {
   handlePlacesEvents(events) {
     Assert.equal(events.length, 1);
     let event = events[0];
+    switch (event.type) {
+      case "bookmark-added":
         bookmarksObserver._itemAddedId = event.id;
         bookmarksObserver._itemAddedParent = event.parentId;
         bookmarksObserver._itemAddedIndex = event.index;
@@ -32,6 +34,12 @@ var bookmarksObserver = {
         do_check_valid_places_guid(stmt.row.guid);
         Assert.equal(stmt.row.guid, event.guid);
         stmt.finalize();
+        break;
+      case "bookmark-removed":
+        bookmarksObserver._itemRemovedId = event.id;
+        bookmarksObserver._itemRemovedFolder = event.parentId;
+        bookmarksObserver._itemRemovedIndex = event.index;
+    }
   },
 
   onBeginUpdateBatch() {
@@ -40,11 +48,7 @@ var bookmarksObserver = {
   onEndUpdateBatch() {
     this._endUpdateBatch = true;
   },
-  onItemRemoved(id, folder, index, itemType) {
+
-    this._itemRemovedId = id;
-    this._itemRemovedFolder = folder;
-    this._itemRemovedIndex = index;
-  },
   onItemChanged(
     id,
     property,
@@ -84,7 +88,10 @@ var bmStartIndex = 0;
 
 add_task(async function test_bookmarks() {
   bs.addObserver(bookmarksObserver);
-  os.addListener(["bookmark-added"], bookmarksObserver.handlePlacesEvents);
+  os.addListener(
+    ["bookmark-added", "bookmark-removed"],
+    bookmarksObserver.handlePlacesEvents
+  );
 
   // test special folders
   Assert.ok(bs.placesRoot > 0);

toolkit/components/places/tests/queries/test_bookmarks.js

@@ -37,7 +37,12 @@ add_task(async function test_eraseEverything() {
   }
   Assert.equal(root.getChild(0).title, "title 1");
   Assert.equal(root.getChild(1).title, "title 2");
+
   await PlacesUtils.bookmarks.eraseEverything();
+
+  // Refetch the guid to refresh the data.
+  unfiled = PlacesUtils.getFolderContents(PlacesUtils.bookmarks.unfiledGuid)
+    .root;
   Assert.equal(unfiled.childCount, 0);
   unfiled.containerOpen = false;
 });

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

@@ -296,6 +296,8 @@ function BookmarkObserver({ ignoreDates = true, skipTags = false } = {}) {
 BookmarkObserver.prototype = {
   handlePlacesEvents(events) {
     for (let event of events) {
+      switch (event.type) {
+        case "bookmark-added": {
           if (this.skipTags && event.isTagging) {
             continue;
           }
@@ -314,26 +316,39 @@ BookmarkObserver.prototype = {
             params.dateAdded = event.dateAdded * 1000;
           }
           this.notifications.push({ name: "bookmark-added", params });
+          break;
+        }
+        case "bookmark-removed": {
+          if (this.skipTags && event.isTagging) {
+            continue;
+          }
+          // Since we are now skipping tags on the listener side we don't
+          // prevent unTagging notifications from going out. These events cause empty
+          // tags folders to be removed which creates another bookmark-removed notification
+          if (
+            this.skipTags &&
+            event.parentGuid == PlacesUtils.bookmarks.tagsGuid
+          ) {
+            continue;
+          }
+          let params = {
+            itemId: event.id,
+            parentId: event.parentId,
+            index: event.index,
+            type: event.itemType,
+            urlHref: event.url || null,
+            guid: event.guid,
+            parentGuid: event.parentGuid,
+            source: event.source,
+          };
+          this.notifications.push({ name: "bookmark-removed", params });
+          break;
+        }
+      }
     }
   },
   onBeginUpdateBatch() {},
   onEndUpdateBatch() {},
-  onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid, source) {
-    let urlHref = uri ? uri.spec : null;
-    this.notifications.push({
-      name: "onItemRemoved",
-      params: {
-        itemId,
-        parentId,
-        index,
-        type,
-        urlHref,
-        guid,
-        parentGuid,
-        source,
-      },
-    });
-  },
   onItemChanged(
     itemId,
     property,
@@ -401,7 +416,7 @@ BookmarkObserver.prototype = {
   check(expectedNotifications) {
     PlacesUtils.bookmarks.removeObserver(this);
     PlacesUtils.observers.removeListener(
-      ["bookmark-added"],
+      ["bookmark-added", "bookmark-removed"],
       this.handlePlacesEvents
     );
     if (!ObjectUtils.deepEqual(this.notifications, expectedNotifications)) {
@@ -419,7 +434,7 @@ function expectBookmarkChangeNotifications(options) {
   let observer = new BookmarkObserver(options);
   PlacesUtils.bookmarks.addObserver(observer);
   PlacesUtils.observers.addListener(
-    ["bookmark-added"],
+    ["bookmark-added", "bookmark-removed"],
     observer.handlePlacesEvents
   );
   return observer;

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

@@ -210,8 +210,17 @@ add_task(async function test_changes_deleted_bookmark() {
   );
   await PlacesTestUtils.markBookmarksAsSynced();
 
+  let wait = PlacesTestUtils.waitForNotification(
+    "bookmark-removed",
+    events =>
+      events.some(event => event.parentGuid == PlacesUtils.bookmarks.tagsGuid),
+    "places"
+  );
   await PlacesUtils.bookmarks.remove("mozBmk______");
 
+  await wait;
+  // Wait for everything to be finished
+  await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
   let controller = new AbortController();
   const wasMerged = await buf.merge(controller.signal);
   Assert.ok(wasMerged);

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

@@ -433,19 +433,6 @@ add_task(async function test_apply_then_revert() {
   ]);
 
   observer.check([
-    {
-      name: "onItemRemoved",
-      params: {
-        itemId: localIdForD,
-        parentId: PlacesUtils.bookmarksMenuFolderId,
-        index: 1,
-        type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-        urlHref: "http://example.com/d",
-        guid: "bookmarkDDDD",
-        parentGuid: PlacesUtils.bookmarks.menuGuid,
-        source: PlacesUtils.bookmarks.SOURCES.SYNC,
-      },
-    },
     {
       name: "onItemChanged",
       params: {
@@ -461,6 +448,19 @@ add_task(async function test_apply_then_revert() {
         source: PlacesUtils.bookmarks.SOURCES.SYNC,
       },
     },
+    {
+      name: "bookmark-removed",
+      params: {
+        itemId: localIdForD,
+        parentId: PlacesUtils.bookmarksMenuFolderId,
+        index: 1,
+        type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+        urlHref: "http://example.com/d",
+        guid: "bookmarkDDDD",
+        parentGuid: PlacesUtils.bookmarks.menuGuid,
+        source: PlacesUtils.bookmarks.SOURCES.SYNC,
+      },
+    },
     {
       name: "bookmark-added",
       params: {

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

@@ -1890,11 +1890,22 @@ add_task(async function test_tags() {
     "eleven",
     "twelve",
   ]);
+
+  let wait = PlacesTestUtils.waitForNotification(
+    "bookmark-removed",
+    events =>
+      events.some(event => event.parentGuid == PlacesUtils.bookmarks.tagsGuid),
+    "places"
+  );
+
   PlacesUtils.tagging.untagURI(
     Services.io.newURI("http://example.com/d"),
     null
   );
 
+  await wait;
+
+  await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
   info("Apply remote");
   let changesToUpload = await buf.apply();
   deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items");

toolkit/components/places/tests/unit/test_async_transactions.js

@@ -33,6 +33,8 @@ var observer = {
 
   handlePlacesEvents(events) {
     for (let event of events) {
+      switch (event.type) {
+        case "bookmark-added":
           // Ignore tag items.
           if (event.isTagging) {
             this.tagRelatedGuids.add(event.guid);
@@ -47,6 +49,18 @@ var observer = {
             title: event.title,
             url: event.url,
           });
+          break;
+        case "bookmark-removed":
+          if (this.tagRelatedGuids.has(event.guid)) {
+            return;
+          }
+
+          this.itemsRemoved.set(event.guid, {
+            parentGuid: event.parentGuid,
+            index: event.index,
+            itemType: event.itemType,
+          });
+      }
     }
   },
 
@@ -58,26 +72,6 @@ var observer = {
     this.endUpdateBatch = true;
   },
 
-  onItemRemoved(
-    aItemId,
-    aParentId,
-    aIndex,
-    aItemType,
-    aURI,
-    aGuid,
-    aParentGuid
-  ) {
-    if (this.tagRelatedGuids.has(aGuid)) {
-      return;
-    }
-
-    this.itemsRemoved.set(aGuid, {
-      parentGuid: aParentGuid,
-      index: aIndex,
-      itemType: aItemType,
-    });
-  },
-
   onItemChanged(
     aItemId,
     aProperty,
@@ -137,10 +131,16 @@ var bmStartIndex = 0;
 function run_test() {
   bmsvc.addObserver(observer);
   observer.handlePlacesEvents = observer.handlePlacesEvents.bind(observer);
-  obsvc.addListener(["bookmark-added"], observer.handlePlacesEvents);
+  obsvc.addListener(
+    ["bookmark-added", "bookmark-removed"],
+    observer.handlePlacesEvents
+  );
   registerCleanupFunction(function() {
     bmsvc.removeObserver(observer);
-    obsvc.removeListener(["bookmark-added"], observer.handlePlacesEvents);
+    obsvc.removeListener(
+      ["bookmark-added", "bookmark-removed"],
+      observer.handlePlacesEvents
+    );
   });
 
   run_next_test();
@@ -1241,7 +1241,34 @@ add_task(async function test_add_and_remove_bookmarks_with_additional_info() {
 
   observer.reset();
   await PT.redo();
+  // The tag containers are removed in async and take some time
+  let oldCountTag1 = 0;
+  let oldCountTag2 = 0;
+  let allTags = await bmsvc.fetchTags();
+  for (let i of allTags) {
+    if (i.name == TAG_1) {
+      oldCountTag1 = i.count;
+    }
+    if (i.name == TAG_2) {
+      oldCountTag2 = i.count;
+    }
+  }
+  await TestUtils.waitForCondition(async () => {
+    allTags = await bmsvc.fetchTags();
+    let newCountTag1 = 0;
+    let newCountTag2 = 0;
+    for (let i of allTags) {
+      if (i.name == TAG_1) {
+        newCountTag1 = i.count;
+      }
+      if (i.name == TAG_2) {
+        newCountTag2 = i.count;
+      }
+    }
+    return newCountTag1 == oldCountTag1 - 1 && newCountTag2 == oldCountTag2 - 1;
+  });
   await ensureItemsRemoved(b2_info);
+
   ensureTags([]);
 
   observer.reset();

toolkit/components/places/tests/unit/test_childlessTags.js

@@ -31,9 +31,21 @@ add_task(async function test_removing_tagged_bookmark_removes_tag() {
   let tags = ["foo", "bar"];
   tagssvc.tagURI(BOOKMARK_URI, tags);
   ensureTagsExist(tags);
+  let root = getTagRoot();
+  root.containerOpen = true;
+  let oldCount = root.childCount;
+  root.containerOpen = false;
 
   print("  Remove the bookmark.  The tags should no longer exist.");
+  let wait = TestUtils.waitForCondition(() => {
+    root = getTagRoot();
+    root.containerOpen = true;
+    let val = root.childCount == oldCount - 2;
+    root.containerOpen = false;
+    return val;
+  });
   await PlacesUtils.bookmarks.remove(bookmark.guid);
+  await wait;
   ensureTagsExist([]);
 });
 
@@ -58,12 +70,40 @@ add_task(
     tagssvc.tagURI(BOOKMARK_URI, tags);
     ensureTagsExist(tags);
 
+    // The tag containers are removed in async and take some time
+    let oldCountFoo = await tagCount("foo");
+    let oldCountBar = await tagCount("bar");
+
     print("  Remove the folder.  The tags should no longer exist.");
+
+    let wait = TestUtils.waitForCondition(async () => {
+      let newCountFoo = await tagCount("foo");
+      let newCountBar = await tagCount("bar");
+      return newCountFoo == oldCountFoo - 1 && newCountBar == oldCountBar - 1;
+    });
     await PlacesUtils.bookmarks.remove(bookmark.guid);
+    await wait;
     ensureTagsExist([]);
   }
 );
 
+async function tagCount(aTag) {
+  let allTags = await PlacesUtils.bookmarks.fetchTags();
+  for (let i of allTags) {
+    if (i.name == aTag) {
+      return i.count;
+    }
+  }
+  return 0;
+}
+
+function getTagRoot() {
+  var query = histsvc.getNewQuery();
+  var opts = histsvc.getNewQueryOptions();
+  opts.resultType = opts.RESULTS_AS_TAGS_ROOT;
+  var resultRoot = histsvc.executeQuery(query, opts).root;
+  return resultRoot;
+}
 /**
  * Runs a tag query and ensures that the tags returned are those and only those
  * in aTags.  aTags may be empty, in which case this function ensures that no

toolkit/components/places/tests/unit/test_onItemChanged_tags.js

@@ -36,13 +36,20 @@ add_task(async function run_test() {
         this._changedCount++;
       }
     },
-
+    handlePlacesEvents(events) {
-    onItemRemoved(aItemId, aParentId, aIndex, aItemType, aURI, aGuid) {
+      for (let event of events) {
-      if (aGuid == bookmark.guid) {
+        switch (event.type) {
-        PlacesUtils.bookmarks.removeObserver(this);
+          case "bookmark-removed":
+            if (event.guid == bookmark.guid) {
+              PlacesUtils.observers.removeListener(
+                ["bookmark-removed"],
+                this.handlePlacesEvents
+              );
               Assert.equal(this._changedCount, 2);
               promise.resolve();
             }
+        }
+      }
     },
 
     onBeginUpdateBatch() {},
@@ -51,6 +58,13 @@ add_task(async function run_test() {
     onItemMoved() {},
   };
   PlacesUtils.bookmarks.addObserver(bookmarksObserver);
+  bookmarksObserver.handlePlacesEvents = bookmarksObserver.handlePlacesEvents.bind(
+    bookmarksObserver
+  );
+  PlacesUtils.observers.addListener(
+    ["bookmark-removed"],
+    bookmarksObserver.handlePlacesEvents
+  );
 
   PlacesUtils.tagging.tagURI(uri, ["d"]);
   PlacesUtils.tagging.tagURI(uri, ["e"]);

toolkit/components/places/tests/unit/test_tagging.js

@@ -64,9 +64,16 @@ function run_test() {
   tagssvc.untagURI(uri1, ["tag 1"]);
   Assert.equal(tag1node.childCount, 1);
 
+  let wait = PlacesTestUtils.waitForNotification(
+    "bookmark-removed",
+    events => events.some(event => event.id == tagRoot.itemId),
+    "places"
+  );
   // removing the last uri from a tag should remove the tag-container
   tagssvc.untagURI(uri2, ["tag 1"]);
+  wait.then(() => {
     Assert.equal(tagRoot.childCount, 1);
+  });
 
   // cleanup
   tag1node.containerOpen = false;
@@ -116,8 +123,15 @@ function run_test() {
     do_throw("Passing a sparse array should not throw");
   }
   try {
+    wait = PlacesTestUtils.waitForNotification(
+      "bookmark-removed",
+      events => events.some(event => event.id == tagRoot.itemId),
+      "places"
+    );
     tagssvc.untagURI(uri1, [undefined, "tagSparse"]);
+    wait.then(() => {
       Assert.equal(tagRoot.childCount, curChildCount);
+    });
   } catch (ex) {
     do_throw("Passing a sparse array should not throw");
   }

tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js

@@ -420,6 +420,7 @@ module.exports = {
     Permissions: false,
     PlacesBookmark: false,
     PlacesBookmarkAddition: false,
+    PlacesBookmarkRemoved: false,
     PlacesEvent: false,
     PlacesObservers: false,
     PlacesVisit: false,