Bug 1511062: Remove old/new parent id from onItemMoved in nsINavBookmarkObserver. r=mak

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

browser/base/content/browser-places.js

@@ -2272,10 +2272,8 @@ var BookmarkingUI = {
 
   onItemMoved(
     aItemId,
-    aProperty,
+    aOldIndex,
-    aIsAnnotationProperty,
+    aNewIndex,
-    aNewValue,
-    aLastModified,
     aItemType,
     aGuid,
     oldParentGuid,

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

@@ -171,9 +171,7 @@ let observer = new (class extends EventEmitter {
 
   onItemMoved(
     id,
-    oldParentId,
     oldIndex,
-    newParentId,
     newIndex,
     itemType,
     guid,

browser/components/places/PlacesUIUtils.jsm

@@ -1191,10 +1191,8 @@ var PlacesUIUtils = {
       onItemChanged() {},
       onItemMoved(
         aItemId,
-        aProperty,
+        aOldIndex,
-        aIsAnnotationProperty,
+        aNewIndex,
-        aNewValue,
-        aLastModified,
         aItemType,
         aGuid,
         oldParentGuid,

browser/components/places/content/editBookmark.js

@@ -1277,9 +1277,7 @@ var gEditItemOverlay = {
 
   onItemMoved(
     id,
-    oldParentId,
     oldIndex,
-    newParentId,
     newIndex,
     type,
     guid,

browser/components/places/tests/browser/browser_autoshow_bookmarks_toolbar.js

@@ -166,8 +166,8 @@ add_task(async function test_move_existing_to_toolbar() {
       let menuList = win.document.getElementById("editBMPanel_folderMenuList");
       let itemMovedPromise = PlacesTestUtils.waitForNotification(
         "onItemMoved",
-        (id, oldParentId, oldIndex, newParentId, newIndex, type, guid) =>
+        (id, oldIndex, newIndex, type, guid, oldParentGuid, newParentGuid) =>
-          newParentId == PlacesUtils.toolbarFolderId && guid == bm.guid
+          newParentGuid == PlacesUtils.bookmarks.toolbarGuid && guid == bm.guid
       );
       let promisePopup = BrowserTestUtils.waitForEvent(
         menuList.menupopup,

browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js

@@ -19,17 +19,8 @@ async function openPopupAndSelectFolder(guid, newBookmark = false) {
   if (!newBookmark) {
     notificationPromise = PlacesTestUtils.waitForNotification(
       "onItemMoved",
-      (
+      (id, oldIndex, newIndex, type, itemGuid, oldParentGuid, newParentGuid) =>
-        id,
+        guid == newParentGuid
-        oldParentId,
-        oldIndex,
-        newParentId,
-        newIndex,
-        type,
-        itemGuid,
-        oldParentGuid,
-        newParentGuid
-      ) => guid == newParentGuid
     );
   }
 

browser/components/places/tests/browser/browser_controller_onDrop.js

@@ -109,17 +109,8 @@ async function run_drag_test(startBookmarkIndex, insertionIndex) {
 add_task(async function test_simple_move_down() {
   let moveNotification = PlacesTestUtils.waitForNotification(
     "onItemMoved",
-    (
+    (id, oldIndex, newIndex, itemType, guid, oldParentGuid, newParentGuid) =>
-      id,
+      guid == bookmarks[0].guid && oldIndex == 0 && newIndex == 1
-      oldParentId,
-      oldIndex,
-      newParentId,
-      newIndex,
-      itemType,
-      guid,
-      oldParentGuid,
-      newParentGuid
-    ) => guid == bookmarks[0].guid && oldIndex == 0 && newIndex == 1
   );
 
   await run_drag_test(0, 2);

browser/components/places/tests/browser/browser_default_bookmark_location.js

@@ -177,8 +177,8 @@ add_task(async function test_change_location_panel() {
   // Make sure we wait for the move to complete.
   let itemMovedPromise = PlacesTestUtils.waitForNotification(
     "onItemMoved",
-    (id, oldParentId, oldIndex, newParentId, newIndex, type, guid) =>
+    (id, oldIndex, newIndex, type, guid, oldParentGuid, newParentGuid) =>
-      newParentId == PlacesUtils.bookmarksMenuFolderId && guid == itemGuid
+      newParentGuid == PlacesUtils.bookmarks.menuGuid && guid == itemGuid
   );
 
   // Wait for the pref to change

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

@@ -192,9 +192,7 @@ var bookmarksObserver = {
 
   onItemMoved(
     itemId,
-    oldFolderId,
     oldIndex,
-    newFolderId,
     newIndex,
     itemType,
     guid,

services/sync/modules/engines/bookmarks.js

@@ -928,9 +928,7 @@ BookmarksTracker.prototype = {
 
   onItemMoved: function BMT_onItemMoved(
     itemId,
-    oldParent,
     oldIndex,
-    newParent,
     newIndex,
     itemType,
     guid,

toolkit/components/places/Bookmarks.jsm

@@ -960,9 +960,7 @@ var Bookmarks = Object.freeze({
           ) {
             notify(observers, "onItemMoved", [
               updatedItem._id,
-              item._parentId,
               item.index,
-              updatedItem._parentId,
               updatedItem.index,
               updatedItem.type,
               updatedItem.guid,
@@ -1189,9 +1187,7 @@ var Bookmarks = Object.freeze({
         ) {
           notify(observers, "onItemMoved", [
             updatedItem._id,
-            existingItem._parentId,
             existingItem.index,
-            updatedItem._parentId,
             updatedItem.index,
             updatedItem.type,
             updatedItem.guid,
@@ -1750,9 +1746,7 @@ var Bookmarks = Object.freeze({
         let child = sortedChildren[i];
         notify(observers, "onItemMoved", [
           child._id,
-          child._parentId,
           child.index,
-          child._parentId,
           i,
           child.type,
           child.guid,

toolkit/components/places/SyncedBookmarksMirror.jsm

@@ -2204,8 +2204,7 @@ class BookmarkObserverRecorder {
 
     MirrorLog.trace("Recording observer notifications for moved items");
     await this.db.execute(
-      `SELECT b.id, b.guid, b.type, p.id AS newParentId, c.oldParentId,
+      `SELECT b.id, b.guid, b.type, p.guid AS newParentGuid, c.oldParentGuid,
-              p.guid AS newParentGuid, c.oldParentGuid,
               b.position AS newPosition, c.oldPosition,
               (SELECT h.url FROM moz_places h WHERE h.id = b.fk) AS url
        FROM itemsMoved c
@@ -2222,8 +2221,6 @@ class BookmarkObserverRecorder {
           id: row.getResultByName("id"),
           guid: row.getResultByName("guid"),
           type: row.getResultByName("type"),
-          newParentId: row.getResultByName("newParentId"),
-          oldParentId: row.getResultByName("oldParentId"),
           newParentGuid: row.getResultByName("newParentGuid"),
           oldParentGuid: row.getResultByName("oldParentGuid"),
           newPosition: row.getResultByName("newPosition"),
@@ -2326,9 +2323,7 @@ class BookmarkObserverRecorder {
   noteItemMoved(info) {
     this.itemMovedArgs.push([
       info.id,
-      info.oldParentId,
       info.oldPosition,
-      info.newParentId,
       info.newPosition,
       info.type,
       info.guid,

toolkit/components/places/TaggingService.jsm

@@ -437,16 +437,17 @@ TaggingService.prototype = {
 
   onItemMoved: function TS_onItemMoved(
     aItemId,
-    aOldParent,
     aOldIndex,
-    aNewParent,
     aNewIndex,
-    aItemType
+    aItemType,
+    aGuid,
+    aOldParentGuid,
+    aNewParentGuid
   ) {
     if (
       this._tagFolders[aItemId] &&
-      PlacesUtils.tagsFolderId == aOldParent &&
+      PlacesUtils.bookmarks.tagsGuid == aOldParentGuid &&
-      PlacesUtils.tagsFolderId != aNewParent
+      PlacesUtils.bookmarks.tagsGuid != aNewParentGuid
     ) {
       delete this._tagFolders[aItemId];
     }

toolkit/components/places/nsINavBookmarksService.idl

@@ -89,12 +89,8 @@ interface nsINavBookmarkObserver : nsISupports
    *
    * @param aItemId
    *        The id of the item that was moved.
-   * @param aOldParentId
-   *        The id of the old parent.
    * @param aOldIndex
    *        The old index inside the old parent.
-   * @param aNewParentId
-   *        The id of the new parent.
    * @param aNewIndex
    *        The index inside the new parent.
    * @param aItemType
@@ -112,9 +108,7 @@ interface nsINavBookmarkObserver : nsISupports
    *        The URI for this bookmark.
    */
   void onItemMoved(in long long aItemId,
-                   in long long aOldParentId,
                    in long aOldIndex,
-                   in long long aNewParentId,
                    in long aNewIndex,
                    in unsigned short aItemType,
                    in ACString aGuid,

toolkit/components/places/nsNavHistoryResult.cpp

@@ -2503,18 +2503,20 @@ nsNavHistoryQueryResultNode::OnItemChanged(
 }
 
 NS_IMETHODIMP
-nsNavHistoryQueryResultNode::OnItemMoved(
+nsNavHistoryQueryResultNode::OnItemMoved(int64_t aFolder, int32_t aOldIndex,
-    int64_t aFolder, int64_t aOldParent, int32_t aOldIndex, int64_t aNewParent,
+                                         int32_t aNewIndex, uint16_t aItemType,
-    int32_t aNewIndex, uint16_t aItemType, const nsACString& aGUID,
+                                         const nsACString& aGUID,
-    const nsACString& aOldParentGUID, const nsACString& aNewParentGUID,
+                                         const nsACString& aOldParentGUID,
-    uint16_t aSource, const nsACString& aURI) {
+                                         const nsACString& aNewParentGUID,
+                                         uint16_t aSource,
+                                         const nsACString& aURI) {
   // 1. The query cannot be affected by the item's position
   // 2. For the time being, we cannot optimize this not to update
   //    queries which are not restricted to some folders, due to way
   //    sub-queries are updated (see Refresh)
   if (mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS &&
       aItemType != nsINavBookmarksService::TYPE_SEPARATOR &&
-      aOldParent != aNewParent) {
+      !aNewParentGUID.Equals(aOldParentGUID)) {
     return Refresh();
   }
   return NS_OK;
@@ -2565,7 +2567,7 @@ nsNavHistoryFolderResultNode::nsNavHistoryFolderResultNode(
 
 nsNavHistoryFolderResultNode::~nsNavHistoryFolderResultNode() {
   if (mIsRegisteredFolderObserver && mResult)
-    mResult->RemoveBookmarkFolderObserver(this, mTargetFolderItemId);
+    mResult->RemoveBookmarkFolderObserver(this, mTargetFolderGuid);
 }
 
 /**
@@ -2766,7 +2768,7 @@ nsresult nsNavHistoryFolderResultNode::OnChildrenFilled() {
  */
 void nsNavHistoryFolderResultNode::EnsureRegisteredAsFolderObserver() {
   if (!mIsRegisteredFolderObserver && mResult) {
-    mResult->AddBookmarkFolderObserver(this, mTargetFolderItemId);
+    mResult->AddBookmarkFolderObserver(this, mTargetFolderGuid);
     mIsRegisteredFolderObserver = true;
   }
 }
@@ -2877,7 +2879,7 @@ void nsNavHistoryFolderResultNode::ClearChildren(bool unregister) {
 
   bool needsUnregister = unregister && (mContentsValid || mAsyncPendingStmt);
   if (needsUnregister && mResult && mIsRegisteredFolderObserver) {
-    mResult->RemoveBookmarkFolderObserver(this, mTargetFolderItemId);
+    mResult->RemoveBookmarkFolderObserver(this, mTargetFolderGuid);
     mIsRegisteredFolderObserver = false;
   }
   mContentsValid = false;
@@ -3320,14 +3322,16 @@ nsresult nsNavHistoryFolderResultNode::OnItemVisited(nsIURI* aURI,
 }
 
 NS_IMETHODIMP
-nsNavHistoryFolderResultNode::OnItemMoved(
+nsNavHistoryFolderResultNode::OnItemMoved(int64_t aItemId, int32_t aOldIndex,
-    int64_t aItemId, int64_t aOldParent, int32_t aOldIndex, int64_t aNewParent,
+                                          int32_t aNewIndex, uint16_t aItemType,
-    int32_t aNewIndex, uint16_t aItemType, const nsACString& aGUID,
+                                          const nsACString& aGUID,
-    const nsACString& aOldParentGUID, const nsACString& aNewParentGUID,
+                                          const nsACString& aOldParentGUID,
-    uint16_t aSource, const nsACString& aURI) {
+                                          const nsACString& aNewParentGUID,
-  NS_ASSERTION(
+                                          uint16_t aSource,
-      aOldParent == mTargetFolderItemId || aNewParent == mTargetFolderItemId,
+                                          const nsACString& aURI) {
-      "Got a bookmark message that doesn't belong to us");
+  MOZ_ASSERT(aOldParentGUID.Equals(mTargetFolderGuid) ||
+                 aNewParentGUID.Equals(mTargetFolderGuid),
+             "Got a bookmark message that doesn't belong to us");
 
   RESTART_AND_RETURN_IF_ASYNC_PENDING();
 
@@ -3347,15 +3351,15 @@ nsNavHistoryFolderResultNode::OnItemMoved(
   // example the Library left pane could have refreshed and replaced the
   // right pane as a consequence. In such a case our contents are already
   // up-to-date.  That's OK.
-  if (node && aNewParent == mTargetFolderItemId &&
+  if (node && aNewParentGUID.Equals(mTargetFolderGuid) &&
       index == static_cast<uint32_t>(aNewIndex))
     return NS_OK;
-  if (!node && aOldParent == mTargetFolderItemId) return NS_OK;
+  if (!node && aOldParentGUID.Equals(mTargetFolderGuid)) return NS_OK;
 
   if (!StartIncrementalUpdate())
     return NS_OK;  // entire container was refreshed for us
 
-  if (aOldParent == aNewParent) {
+  if (aNewParentGUID.Equals(aOldParentGUID)) {
     // getting moved within the same folder, we don't want to do a remove and
     // an add because that will lose your tree state.
 
@@ -3384,13 +3388,13 @@ nsNavHistoryFolderResultNode::OnItemMoved(
     NS_ENSURE_SUCCESS(rv, rv);
     NS_ENSURE_SUCCESS(rv, rv);
   }
-  if (aOldParent == mTargetFolderItemId) {
+  if (aOldParentGUID.Equals(mTargetFolderGuid)) {
-    OnItemRemoved(aItemId, aOldParent, aOldIndex, aItemType, itemURI, aGUID,
+    OnItemRemoved(aItemId, mTargetFolderItemId, aOldIndex, aItemType, itemURI,
-                  aOldParentGUID, aSource);
+                  aGUID, aOldParentGUID, aSource);
   }
-  if (aNewParent == mTargetFolderItemId) {
+  if (aNewParentGUID.Equals(mTargetFolderGuid)) {
     OnItemAdded(
-        aItemId, aNewParent, aNewIndex, aItemType, itemURI,
+        aItemId, mTargetFolderItemId, aNewIndex, aItemType, itemURI,
         RoundedPRNow(),  // This is a dummy dateAdded, not the real value.
         aGUID, aNewParentGUID, aSource);
   }
@@ -3592,12 +3596,13 @@ void nsNavHistoryResult::AddMobilePrefsObserver(
 }
 
 void nsNavHistoryResult::AddBookmarkFolderObserver(
-    nsNavHistoryFolderResultNode* aNode, int64_t aFolder) {
+    nsNavHistoryFolderResultNode* aNode, const nsACString& aFolderGUID) {
+  MOZ_ASSERT(!aFolderGUID.IsEmpty(), "aFolderGUID should not be empty");
   EnsureIsObservingBookmarks();
   // Don't add duplicate observers.  In some case we don't unregister when
   // children are cleared (see ClearChildren) and the next FillChildren call
   // will try to add the observer again.
-  FolderObserverList* list = BookmarkFolderObserversForId(aFolder, true);
+  FolderObserverList* list = BookmarkFolderObserversForGUID(aFolderGUID, true);
   if (list->IndexOf(aNode) == FolderObserverList::NoIndex) {
     list->AppendElement(aNode);
   }
@@ -3642,22 +3647,23 @@ void nsNavHistoryResult::RemoveMobilePrefsObserver(
 }
 
 void nsNavHistoryResult::RemoveBookmarkFolderObserver(
-    nsNavHistoryFolderResultNode* aNode, int64_t aFolder) {
+    nsNavHistoryFolderResultNode* aNode, const nsACString& aFolderGUID) {
-  FolderObserverList* list = BookmarkFolderObserversForId(aFolder, false);
+  MOZ_ASSERT(!aFolderGUID.IsEmpty(), "aFolderGUID should not be empty");
+  FolderObserverList* list = BookmarkFolderObserversForGUID(aFolderGUID, false);
   if (!list) return;  // we don't even have an entry for that folder
   list->RemoveElement(aNode);
 }
 
 nsNavHistoryResult::FolderObserverList*
-nsNavHistoryResult::BookmarkFolderObserversForId(int64_t aFolderId,
+nsNavHistoryResult::BookmarkFolderObserversForGUID(
-                                                 bool aCreate) {
+    const nsACString& aFolderGUID, bool aCreate) {
   FolderObserverList* list;
-  if (mBookmarkFolderObservers.Get(aFolderId, &list)) return list;
+  if (mBookmarkFolderObservers.Get(aFolderGUID, &list)) return list;
   if (!aCreate) return nullptr;
 
   // need to create a new list
   list = new FolderObserverList;
-  mBookmarkFolderObservers.InsertOrUpdate(aFolderId, list);
+  mBookmarkFolderObservers.InsertOrUpdate(aFolderGUID, list);
   return list;
 }
 
@@ -3824,15 +3830,16 @@ void nsNavHistoryResult::requestRefresh(
 // Here, it is important that we create a COPY of the observer array. Some
 // observers will requery themselves, which may cause the observer array to
 // be modified or added to.
-#define ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(_folderId, _functionCall)        \
+#define ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(_folderGUID, _functionCall) \
-  PR_BEGIN_MACRO                                                             \
+  PR_BEGIN_MACRO                                                        \
-  FolderObserverList* _fol = BookmarkFolderObserversForId(_folderId, false); \
+  FolderObserverList* _fol =                                            \
-  if (_fol) {                                                                \
+      BookmarkFolderObserversForGUID(_folderGUID, false);               \
-    FolderObserverList _listCopy(_fol->Clone());                             \
+  if (_fol) {                                                           \
-    for (uint32_t _fol_i = 0; _fol_i < _listCopy.Length(); ++_fol_i) {       \
+    FolderObserverList _listCopy(_fol->Clone());                        \
-      if (_listCopy[_fol_i]) _listCopy[_fol_i]->_functionCall;               \
+    for (uint32_t _fol_i = 0; _fol_i < _listCopy.Length(); ++_fol_i) {  \
-    }                                                                        \
+      if (_listCopy[_fol_i]) _listCopy[_fol_i]->_functionCall;          \
-  }                                                                          \
+    }                                                                   \
+  }                                                                     \
   PR_END_MACRO
 #define ENUMERATE_LIST_OBSERVERS(_listType, _functionCall, _observersList, \
                                  _conditionCall)                           \
@@ -3913,7 +3920,7 @@ nsNavHistoryResult::OnItemChanged(
   // opened, meaning we cannot optimize this code path for changes done to
   // folder-nodes.
 
-  FolderObserverList* list = BookmarkFolderObserversForId(aParentId, false);
+  FolderObserverList* list = BookmarkFolderObserversForGUID(aParentGUID, false);
   if (!list) return NS_OK;
 
   for (uint32_t i = 0; i < list->Length(); ++i) {
@@ -3945,29 +3952,28 @@ nsNavHistoryResult::OnItemChanged(
  * different).
  */
 NS_IMETHODIMP
-nsNavHistoryResult::OnItemMoved(int64_t aItemId, int64_t aOldParent,
+nsNavHistoryResult::OnItemMoved(int64_t aItemId, int32_t aOldIndex,
-                                int32_t aOldIndex, int64_t aNewParent,
                                 int32_t aNewIndex, uint16_t aItemType,
                                 const nsACString& aGUID,
                                 const nsACString& aOldParentGUID,
                                 const nsACString& aNewParentGUID,
                                 uint16_t aSource, const nsACString& aURI) {
   ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
-      aOldParent, OnItemMoved(aItemId, aOldParent, aOldIndex, aNewParent,
+      aOldParentGUID,
-                              aNewIndex, aItemType, aGUID, aOldParentGUID,
+      OnItemMoved(aItemId, aOldIndex, aNewIndex, aItemType, aGUID,
-                              aNewParentGUID, aSource, aURI));
+                  aOldParentGUID, aNewParentGUID, aSource, aURI));
-  if (aNewParent != aOldParent) {
+  if (!aNewParentGUID.Equals(aOldParentGUID)) {
     ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
-        aNewParent, OnItemMoved(aItemId, aOldParent, aOldIndex, aNewParent,
+        aNewParentGUID,
-                                aNewIndex, aItemType, aGUID, aOldParentGUID,
+        OnItemMoved(aItemId, aOldIndex, aNewIndex, aItemType, aGUID,
-                                aNewParentGUID, aSource, aURI));
+                    aOldParentGUID, aNewParentGUID, aSource, aURI));
   }
-  ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemMoved(
+  ENUMERATE_ALL_BOOKMARKS_OBSERVERS(
-      aItemId, aOldParent, aOldIndex, aNewParent, aNewIndex, aItemType, aGUID,
+      OnItemMoved(aItemId, aOldIndex, aNewIndex, aItemType, aGUID,
-      aOldParentGUID, aNewParentGUID, aSource, aURI));
+                  aOldParentGUID, aNewParentGUID, aSource, aURI));
-  ENUMERATE_HISTORY_OBSERVERS(OnItemMoved(
+  ENUMERATE_HISTORY_OBSERVERS(OnItemMoved(aItemId, aOldIndex, aNewIndex,
-      aItemId, aOldParent, aOldIndex, aNewParent, aNewIndex, aItemType, aGUID,
+                                          aItemType, aGUID, aOldParentGUID,
-      aOldParentGUID, aNewParentGUID, aSource, aURI));
+                                          aNewParentGUID, aSource, aURI));
   return NS_OK;
 }
 
@@ -4063,7 +4069,7 @@ nsresult nsNavHistoryResult::OnVisit(nsIURI* aURI, int64_t aVisitId,
       for (int32_t i = 0; i < nodes.Count(); ++i) {
         nsNavHistoryResultNode* node = nodes[i];
         ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
-            node->mParent->mItemId, OnItemVisited(aURI, aVisitId, aTime));
+            node->mParent->mBookmarkGuid, OnItemVisited(aURI, aVisitId, aTime));
       }
     }
   }
@@ -4141,7 +4147,7 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
         }
 
         ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
-            item->mParentId,
+            item->mParentGuid,
             OnItemAdded(item->mId, item->mParentId, item->mIndex,
                         item->mItemType, uri, item->mDateAdded * 1000,
                         item->mGuid, item->mParentGuid, item->mSource));
@@ -4168,7 +4174,7 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
           continue;
         }
         ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
-            item->mParentId,
+            item->mParentGuid,
             OnItemRemoved(item->mId, item->mParentId, item->mIndex,
                           item->mItemType, uri, item->mGuid, item->mParentGuid,
                           item->mSource));

toolkit/components/places/nsNavHistoryResult.h

@@ -89,12 +89,12 @@ class nsNavHistoryResult final
 
   void AddHistoryObserver(nsNavHistoryQueryResultNode* aNode);
   void AddBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode,
-                                 int64_t aFolder);
+                                 const nsACString& aFolderGUID);
   void AddAllBookmarksObserver(nsNavHistoryQueryResultNode* aNode);
   void AddMobilePrefsObserver(nsNavHistoryQueryResultNode* aNode);
   void RemoveHistoryObserver(nsNavHistoryQueryResultNode* aNode);
   void RemoveBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode,
-                                    int64_t aFolder);
+                                    const nsACString& aFolderGUID);
   void RemoveAllBookmarksObserver(nsNavHistoryQueryResultNode* aNode);
   void RemoveMobilePrefsObserver(nsNavHistoryQueryResultNode* aNode);
   void StopObserving();
@@ -137,9 +137,9 @@ class nsNavHistoryResult final
   QueryObserverList mMobilePrefObservers;
 
   typedef nsTArray<RefPtr<nsNavHistoryFolderResultNode> > FolderObserverList;
-  nsTHashMap<nsTrimInt64HashKey, FolderObserverList*> mBookmarkFolderObservers;
+  nsTHashMap<nsCStringHashKey, FolderObserverList*> mBookmarkFolderObservers;
-  FolderObserverList* BookmarkFolderObserversForId(int64_t aFolderId,
+  FolderObserverList* BookmarkFolderObserversForGUID(const nsACString& aGUID,
-                                                   bool aCreate);
+                                                     bool aCreate);
 
   typedef nsTArray<RefPtr<nsNavHistoryContainerResultNode> >
       ContainerObserverList;

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

@@ -258,9 +258,7 @@ async function testMoveToFolder(details) {
         name: "onItemMoved",
         arguments: [
           await PlacesUtils.promiseItemId(origItem.guid),
-          await PlacesUtils.promiseItemId(origItem.parentGuid),
           notification.originalIndex,
-          await PlacesUtils.promiseItemId(newFolder.guid),
           notification.newIndex,
           PlacesUtils.bookmarks.TYPE_BOOKMARK,
           origItem.guid,

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

@@ -326,7 +326,6 @@ add_task(async function update_move_same_folder() {
     url: new URL("http://move.example.com/"),
   });
   let bmItemId = await PlacesUtils.promiseItemId(bm.guid);
-  let bmParentId = await PlacesUtils.promiseItemId(bm.parentGuid);
   let bmOldIndex = bm.index;
 
   let observer = expectNotifications();
@@ -341,9 +340,7 @@ add_task(async function update_move_same_folder() {
       name: "onItemMoved",
       arguments: [
         bmItemId,
-        bmParentId,
         bmOldIndex,
-        bmParentId,
         bm.index,
         bm.type,
         bm.guid,
@@ -369,9 +366,7 @@ add_task(async function update_move_same_folder() {
       name: "onItemMoved",
       arguments: [
         bmItemId,
-        bmParentId,
         bmOldIndex,
-        bmParentId,
         bm.index,
         bm.type,
         bm.guid,
@@ -395,9 +390,6 @@ add_task(async function update_move_different_folder() {
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
   });
   let bmItemId = await PlacesUtils.promiseItemId(bm.guid);
-  let bmOldParentId = await PlacesUtils.promiseItemId(
-    PlacesUtils.bookmarks.unfiledGuid
-  );
   let bmOldIndex = bm.index;
 
   let observer = expectNotifications();
@@ -407,15 +399,12 @@ add_task(async function update_move_different_folder() {
     index: PlacesUtils.bookmarks.DEFAULT_INDEX,
   });
   Assert.equal(bm.index, 0);
-  let bmNewParentId = await PlacesUtils.promiseItemId(folder.guid);
   observer.check([
     {
       name: "onItemMoved",
       arguments: [
         bmItemId,
-        bmOldParentId,
         bmOldIndex,
-        bmNewParentId,
         bm.index,
         bm.type,
         bm.guid,
@@ -894,9 +883,6 @@ add_task(async function reorder_notification() {
   );
 
   let expectedNotifications = [];
-  let unfiledBookmarksFolderId = await PlacesUtils.promiseItemId(
-    PlacesUtils.bookmarks.unfiledGuid
-  );
   for (let i = 0; i < sorted.length; ++i) {
     let child = sorted[i];
     let childId = await PlacesUtils.promiseItemId(child.guid);
@@ -904,9 +890,7 @@ add_task(async function reorder_notification() {
       name: "onItemMoved",
       arguments: [
         childId,
-        unfiledBookmarksFolderId,
         child.index,
-        unfiledBookmarksFolderId,
         i,
         child.type,
         child.guid,

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

@@ -504,12 +504,7 @@ add_task(async function onItemMoved_bookmark() {
         name: "onItemMoved",
         args: [
           { name: "itemId", check: v => typeof v == "number" && v > 0 },
-          { name: "oldParentId", check: v => v === gUnfiledFolderId },
           { name: "oldIndex", check: v => v === 0 },
-          {
-            name: "newParentId",
-            check: v => v === PlacesUtils.toolbarFolderId,
-          },
           { name: "newIndex", check: v => v === 0 },
           {
             name: "itemType",
@@ -539,12 +534,7 @@ add_task(async function onItemMoved_bookmark() {
         name: "onItemMoved",
         args: [
           { name: "itemId", check: v => typeof v == "number" && v > 0 },
-          {
-            name: "oldParentId",
-            check: v => v === PlacesUtils.toolbarFolderId,
-          },
           { name: "oldIndex", check: v => v === 0 },
-          { name: "newParentId", check: v => v === gUnfiledFolderId },
           { name: "newIndex", check: v => v === 0 },
           {
             name: "itemType",

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

@@ -59,13 +59,7 @@ var bookmarksObserver = {
     this._itemChangedValue = value;
     this._itemChangedOldValue = oldValue;
   },
-  onItemMoved(id, oldParent, oldIndex, newParent, newIndex, itemType) {
+  onItemMoved() {},
-    this._itemMovedId = id;
-    this._itemMovedOldParent = oldParent;
-    this._itemMovedOldIndex = oldIndex;
-    this._itemMovedNewParent = newParent;
-    this._itemMovedNewIndex = newIndex;
-  },
   QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]),
 };
 

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

@@ -379,9 +379,7 @@ BookmarkObserver.prototype = {
   },
   onItemMoved(
     itemId,
-    oldParentId,
     oldIndex,
-    newParentId,
     newIndex,
     type,
     guid,
@@ -394,9 +392,7 @@ BookmarkObserver.prototype = {
       name: "onItemMoved",
       params: {
         itemId,
-        oldParentId,
         oldIndex,
-        newParentId,
         newIndex,
         type,
         guid,

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

@@ -479,9 +479,7 @@ add_task(async function test_apply_then_revert() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkEEEE"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 2,
-        newParentId: PlacesUtils.bookmarksMenuFolderId,
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkEEEE",
@@ -495,9 +493,7 @@ add_task(async function test_apply_then_revert() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("folderAAAAAA"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 0,
-        newParentId: PlacesUtils.toolbarFolderId,
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_FOLDER,
         guid: "folderAAAAAA",
@@ -511,9 +507,7 @@ add_task(async function test_apply_then_revert() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkCCCC"),
-        oldParentId: localItemIds.get("folderAAAAAA"),
         oldIndex: 1,
-        newParentId: localItemIds.get("folderAAAAAA"),
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkCCCC",
@@ -527,9 +521,7 @@ add_task(async function test_apply_then_revert() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
-        oldParentId: localItemIds.get("folderAAAAAA"),
         oldIndex: 0,
-        newParentId: localItemIds.get("folderAAAAAA"),
         newIndex: 1,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkBBBB",

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

@@ -152,9 +152,7 @@ add_task(async function test_value_structure_conflict() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkEEEE"),
-        oldParentId: localItemIds.get("folderDDDDDD"),
         oldIndex: 1,
-        newParentId: localItemIds.get("folderDDDDDD"),
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkEEEE",
@@ -168,9 +166,7 @@ add_task(async function test_value_structure_conflict() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
-        oldParentId: localItemIds.get("folderDDDDDD"),
         oldIndex: 0,
-        newParentId: localItemIds.get("folderDDDDDD"),
         newIndex: 1,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkBBBB",
@@ -405,9 +401,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("devFolder___"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 0,
-        newParentId: PlacesUtils.toolbarFolderId,
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_FOLDER,
         guid: "devFolder___",
@@ -421,11 +415,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("mozFolder___"),
-        oldParentId: localItemIds.get("devFolder___"),
         oldIndex: 1,
-        newParentId: await PlacesUtils.promiseItemId(
-          PlacesUtils.bookmarks.unfiledGuid
-        ),
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_FOLDER,
         guid: "mozFolder___",
@@ -439,9 +429,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bzBmk_______"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 1,
-        newParentId: localItemIds.get("devFolder___"),
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bzBmk_______",
@@ -455,9 +443,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("wmBmk_______"),
-        oldParentId: localItemIds.get("devFolder___"),
         oldIndex: 2,
-        newParentId: localItemIds.get("devFolder___"),
         newIndex: 1,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "wmBmk_______",
@@ -471,9 +457,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("nightlyBmk__"),
-        oldParentId: localItemIds.get("mozFolder___"),
         oldIndex: 1,
-        newParentId: localItemIds.get("mozFolder___"),
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "nightlyBmk__",
@@ -487,9 +471,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("mdnBmk______"),
-        oldParentId: localItemIds.get("devFolder___"),
         oldIndex: 0,
-        newParentId: localItemIds.get("mozFolder___"),
         newIndex: 1,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "mdnBmk______",
@@ -503,9 +485,7 @@ add_task(async function test_move() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("fxBmk_______"),
-        oldParentId: localItemIds.get("mozFolder___"),
         oldIndex: 0,
-        newParentId: localItemIds.get("mozFolder___"),
         newIndex: 2,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "fxBmk_______",
@@ -737,9 +717,7 @@ add_task(async function test_move_into_parent_sibling() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
-        oldParentId: localItemIds.get("folderAAAAAA"),
         oldIndex: 0,
-        newParentId: localItemIds.get("folderCCCCCC"),
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkBBBB",
@@ -956,9 +934,7 @@ add_task(async function test_complex_move_with_additions() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkCCCC"),
-        oldParentId: localItemIds.get("folderAAAAAA"),
         oldIndex: 1,
-        newParentId: PlacesUtils.bookmarksMenuFolderId,
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkCCCC",
@@ -972,9 +948,7 @@ add_task(async function test_complex_move_with_additions() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("folderAAAAAA"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 0,
-        newParentId: PlacesUtils.toolbarFolderId,
         newIndex: 0,
         type: PlacesUtils.bookmarks.TYPE_FOLDER,
         guid: "folderAAAAAA",

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

@@ -221,9 +221,7 @@ add_task(async function test_value_combo() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bzBmk_______"),
-        oldParentId: PlacesUtils.toolbarFolderId,
         oldIndex: 0,
-        newParentId: PlacesUtils.toolbarFolderId,
         newIndex: 2,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bzBmk_______",
@@ -1374,9 +1372,7 @@ add_task(async function test_keywords_complex() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkBBBB"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 0,
-        newParentId: PlacesUtils.bookmarksMenuFolderId,
         newIndex: 3,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkBBBB",
@@ -1390,9 +1386,7 @@ add_task(async function test_keywords_complex() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkCCCC"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 1,
-        newParentId: PlacesUtils.bookmarksMenuFolderId,
         newIndex: 4,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkCCCC",
@@ -1406,9 +1400,7 @@ add_task(async function test_keywords_complex() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkDDDD"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 2,
-        newParentId: PlacesUtils.bookmarksMenuFolderId,
         newIndex: 5,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkDDDD",
@@ -1422,9 +1414,7 @@ add_task(async function test_keywords_complex() {
       name: "onItemMoved",
       params: {
         itemId: localItemIds.get("bookmarkEEEE"),
-        oldParentId: PlacesUtils.bookmarksMenuFolderId,
         oldIndex: 3,
-        newParentId: PlacesUtils.bookmarksMenuFolderId,
         newIndex: 6,
         type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
         guid: "bookmarkEEEE",

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

@@ -93,9 +93,7 @@ var observer = {
 
   onItemMoved(
     aItemId,
-    aOldParent,
     aOldIndex,
-    aNewParent,
     aNewIndex,
     aItemType,
     aGuid,