From 3ddb5240c21b83a7a8e665699d89288f0d25afc1 Mon Sep 17 00:00:00 2001 From: Daisuke Akatsuka Date: Thu, 6 May 2021 02:29:44 +0000 Subject: [PATCH] Bug 1511062: Remove onItemMoved from nsINavBookmarkObserver. r=mak Depends on D102574 Differential Revision: https://phabricator.services.mozilla.com/D102575 --- .../extensions/parent/ext-bookmarks.js | 19 ----------- browser/components/newtab/lib/PlacesFeed.jsm | 4 +-- .../newtab/test/unit/lib/PlacesFeed.test.js | 1 - .../components/places/content/editBookmark.js | 32 ------------------ .../browser/browser_editBookmark_keywords.js | 1 - .../tests/browser/browser_views_liveupdate.js | 17 ---------- services/sync/modules/engines/bookmarks.js | 18 ---------- toolkit/components/places/TaggingService.jsm | 18 ---------- .../places/nsINavBookmarksService.idl | 33 ------------------- .../components/places/nsNavHistoryResult.cpp | 30 ----------------- .../components/places/nsNavHistoryResult.h | 10 ++++++ .../bookmarks/test_nsINavBookmarkObserver.js | 6 ---- .../places/tests/chrome/test_371798.xhtml | 1 - .../components/places/tests/head_common.js | 1 - .../places/tests/legacy/test_bookmarks.js | 1 - .../components/places/tests/sync/head_sync.js | 26 --------------- .../tests/unit/test_async_transactions.js | 18 ---------- .../tests/unit/test_onItemChanged_tags.js | 2 -- 18 files changed, 11 insertions(+), 227 deletions(-) diff --git a/browser/components/extensions/parent/ext-bookmarks.js b/browser/components/extensions/parent/ext-bookmarks.js index d4bdfc446c40..5f5d021b1c27 100644 --- a/browser/components/extensions/parent/ext-bookmarks.js +++ b/browser/components/extensions/parent/ext-bookmarks.js @@ -181,25 +181,6 @@ let observer = new (class extends EventEmitter { } } - onItemMoved( - id, - oldIndex, - newIndex, - itemType, - guid, - oldParentGuid, - newParentGuid, - source - ) { - let info = { - parentId: newParentGuid, - index: newIndex, - oldParentId: oldParentGuid, - oldIndex, - }; - this.emit("moved", { guid, info }); - } - onItemChanged( id, prop, diff --git a/browser/components/newtab/lib/PlacesFeed.jsm b/browser/components/newtab/lib/PlacesFeed.jsm index 03eb0c222939..bb5d7ec4162b 100644 --- a/browser/components/newtab/lib/PlacesFeed.jsm +++ b/browser/components/newtab/lib/PlacesFeed.jsm @@ -50,9 +50,7 @@ class BookmarksObserver extends Observer { this.skipTags = true; } - // Empty functions to make xpconnect happy - onItemMoved() {} - + // Empty functions to make xpconnect happy. // Disabled due to performance cost, see Issue 3203 / // https://bugzilla.mozilla.org/show_bug.cgi?id=1392267. onItemChanged() {} diff --git a/browser/components/newtab/test/unit/lib/PlacesFeed.test.js b/browser/components/newtab/test/unit/lib/PlacesFeed.test.js index f53baa9833ad..89becde4793b 100644 --- a/browser/components/newtab/test/unit/lib/PlacesFeed.test.js +++ b/browser/components/newtab/test/unit/lib/PlacesFeed.test.js @@ -1092,7 +1092,6 @@ describe("PlacesFeed", () => { }); describe("Other empty methods (to keep code coverage happy)", () => { it("should have a various empty functions for xpconnect happiness", () => { - observer.onItemMoved(); observer.onItemChanged(); }); }); diff --git a/browser/components/places/content/editBookmark.js b/browser/components/places/content/editBookmark.js index b2c61664332d..9f6e9950f543 100644 --- a/browser/components/places/content/editBookmark.js +++ b/browser/components/places/content/editBookmark.js @@ -1312,38 +1312,6 @@ var gEditItemOverlay = { break; } }, - - onItemMoved( - id, - oldIndex, - newIndex, - type, - guid, - oldParentGuid, - newParentGuid - ) { - if (!this._paneInfo.isItem || this._paneInfo.itemId != id) { - return; - } - - this._paneInfo.parentGuid = newParentGuid; - - if ( - !this._paneInfo.visibleRows.has("folderRow") || - newParentGuid == this._folderMenuList.selectedItem.folderGuid - ) { - return; - } - - // Just setting selectItem _does not_ trigger oncommand, so we don't - // recurse. - PlacesUtils.bookmarks.fetch(newParentGuid).then(bm => { - this._folderMenuList.selectedItem = this._getFolderMenuItem( - newParentGuid, - bm.title - ); - }); - }, }; XPCOMUtils.defineLazyGetter(gEditItemOverlay, "_folderTree", () => { diff --git a/browser/components/places/tests/browser/browser_editBookmark_keywords.js b/browser/components/places/tests/browser/browser_editBookmark_keywords.js index 8fc550275927..9256cc153a42 100644 --- a/browser/components/places/tests/browser/browser_editBookmark_keywords.js +++ b/browser/components/places/tests/browser/browser_editBookmark_keywords.js @@ -6,7 +6,6 @@ add_task(async function() { function promiseOnItemChanged() { return new Promise(resolve => { PlacesUtils.bookmarks.addObserver({ - onItemMoved() {}, onItemChanged(id, property, isAnno, value) { PlacesUtils.bookmarks.removeObserver(this); resolve({ property, value }); diff --git a/browser/components/places/tests/browser/browser_views_liveupdate.js b/browser/components/places/tests/browser/browser_views_liveupdate.js index 780a87c4ec86..762d469cd2ff 100644 --- a/browser/components/places/tests/browser/browser_views_liveupdate.js +++ b/browser/components/places/tests/browser/browser_views_liveupdate.js @@ -190,23 +190,6 @@ var bookmarksObserver = { } }, - onItemMoved( - itemId, - oldIndex, - newIndex, - itemType, - guid, - oldParentGuid, - newParentGuid - ) { - this._notifications.push([ - "assertItemMoved", - newParentGuid, - guid, - newIndex, - ]); - }, - onItemChanged( itemId, property, diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 92ff10e7133f..d7f6792fdf7d 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -933,24 +933,6 @@ BookmarksTracker.prototype = { ); this._upScore(); }, - - onItemMoved: function BMT_onItemMoved( - itemId, - oldIndex, - newIndex, - itemType, - guid, - oldParentGuid, - newParentGuid, - source - ) { - if (IGNORED_SOURCES.includes(source)) { - return; - } - - this._log.trace("onItemMoved: " + itemId); - this._upScore(); - }, }; /** diff --git a/toolkit/components/places/TaggingService.jsm b/toolkit/components/places/TaggingService.jsm index fec13666247e..b710f0d434c0 100644 --- a/toolkit/components/places/TaggingService.jsm +++ b/toolkit/components/places/TaggingService.jsm @@ -444,24 +444,6 @@ TaggingService.prototype = { } }, - onItemMoved: function TS_onItemMoved( - aItemId, - aOldIndex, - aNewIndex, - aItemType, - aGuid, - aOldParentGuid, - aNewParentGuid - ) { - if ( - this._tagFolders[aItemId] && - PlacesUtils.bookmarks.tagsGuid == aOldParentGuid && - PlacesUtils.bookmarks.tagsGuid != aNewParentGuid - ) { - delete this._tagFolders[aItemId]; - } - }, - // nsISupports classID: Components.ID("{bbc23860-2553-479d-8b78-94d9038334f7}"), diff --git a/toolkit/components/places/nsINavBookmarksService.idl b/toolkit/components/places/nsINavBookmarksService.idl index 4e1cd85b2ecf..49a94279fb2f 100644 --- a/toolkit/components/places/nsINavBookmarksService.idl +++ b/toolkit/components/places/nsINavBookmarksService.idl @@ -83,39 +83,6 @@ interface nsINavBookmarkObserver : nsISupports in ACString aParentGuid, in AUTF8String aOldValue, in unsigned short aSource); - - /** - * Notifies that an item has been moved. - * - * @param aItemId - * The id of the item that was moved. - * @param aOldIndex - * The old index inside the old parent. - * @param aNewIndex - * The index inside the new parent. - * @param aItemType - * The type of the item to be removed (see TYPE_* constants below). - * @param aGuid - * The unique ID associated with the item. - * @param aOldParentGuid - * The unique ID associated with the old item's parent. - * @param aNewParentGuid - * The unique ID associated with the new item's parent. - * @param aSource - * A change source constant from nsINavBookmarksService::SOURCE_*, - * passed to the method that notifies the observer. - * @param aURI - * The URI for this bookmark. - */ - void onItemMoved(in long long aItemId, - in long aOldIndex, - in long aNewIndex, - in unsigned short aItemType, - in ACString aGuid, - in ACString aOldParentGuid, - in ACString aNewParentGuid, - in unsigned short aSource, - in AUTF8String aURI); }; /** diff --git a/toolkit/components/places/nsNavHistoryResult.cpp b/toolkit/components/places/nsNavHistoryResult.cpp index 04373dc9d36d..3edd5007eb23 100644 --- a/toolkit/components/places/nsNavHistoryResult.cpp +++ b/toolkit/components/places/nsNavHistoryResult.cpp @@ -3950,36 +3950,6 @@ nsNavHistoryResult::OnItemChanged( return NS_OK; } -/** - * Need to notify both the source and the destination folders (if they are - * different). - */ -NS_IMETHODIMP -nsNavHistoryResult::OnItemMoved(int64_t aItemId, int32_t aOldIndex, - 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( - aOldParentGUID, - OnItemMoved(aItemId, aOldIndex, aNewIndex, aItemType, aGUID, - aOldParentGUID, aNewParentGUID, aSource, aURI)); - if (!aNewParentGUID.Equals(aOldParentGUID)) { - ENUMERATE_BOOKMARK_FOLDER_OBSERVERS( - aNewParentGUID, - OnItemMoved(aItemId, aOldIndex, aNewIndex, aItemType, aGUID, - aOldParentGUID, aNewParentGUID, aSource, aURI)); - } - ENUMERATE_ALL_BOOKMARKS_OBSERVERS( - OnItemMoved(aItemId, aOldIndex, aNewIndex, aItemType, aGUID, - aOldParentGUID, aNewParentGUID, aSource, aURI)); - ENUMERATE_HISTORY_OBSERVERS(OnItemMoved(aItemId, aOldIndex, aNewIndex, - aItemType, aGUID, aOldParentGUID, - aNewParentGUID, aSource, aURI)); - return NS_OK; -} - nsresult nsNavHistoryResult::OnVisit(nsIURI* aURI, int64_t aVisitId, PRTime aTime, uint32_t aTransitionType, const nsACString& aGUID, bool aHidden, diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index 1660f4e8256e..ce24d10b984e 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -688,6 +688,11 @@ class nsNavHistoryQueryResultNode final uint16_t aItemType, nsIURI* aURI, const nsACString& aGUID, const nsACString& aParentGUID, uint16_t aSource); + nsresult OnItemMoved(int64_t aFolder, int32_t aOldIndex, int32_t aNewIndex, + uint16_t aItemType, const nsACString& aGUID, + const nsACString& aOldParentGUID, + const nsACString& aNewParentGUID, uint16_t aSource, + const nsACString& aURI); // 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, @@ -786,6 +791,11 @@ class nsNavHistoryFolderResultNode final uint16_t aItemType, nsIURI* aURI, const nsACString& aGUID, const nsACString& aParentGUID, uint16_t aSource); + nsresult OnItemMoved(int64_t aFolder, int32_t aOldIndex, int32_t aNewIndex, + uint16_t aItemType, const nsACString& aGUID, + const nsACString& aOldParentGUID, + const nsACString& aNewParentGUID, uint16_t aSource, + const nsACString& aURI); nsresult OnItemVisited(nsIURI* aURI, int64_t aVisitId, PRTime aTime); virtual void OnRemoving() override; diff --git a/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js b/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js index 929c561d2854..6c130478f5a4 100644 --- a/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js +++ b/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js @@ -60,9 +60,6 @@ var gBookmarksObserver = { onItemChanged() { return this.validate("onItemChanged", arguments); }, - onItemMoved() { - return this.validate("onItemMoved", arguments); - }, // nsISupports QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]), @@ -106,9 +103,6 @@ var gBookmarkSkipObserver = { onItemChanged() { return this.validate("onItemChanged", arguments); }, - onItemMoved() { - return this.validate("onItemMoved", arguments); - }, // nsISupports QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]), diff --git a/toolkit/components/places/tests/chrome/test_371798.xhtml b/toolkit/components/places/tests/chrome/test_371798.xhtml index 562029ab5104..afc02c2f462d 100644 --- a/toolkit/components/places/tests/chrome/test_371798.xhtml +++ b/toolkit/components/places/tests/chrome/test_371798.xhtml @@ -25,7 +25,6 @@ function promiseOnItemChanged() { return new Promise(resolve => { PlacesUtils.bookmarks.addObserver({ onItemRemoved() {}, - onItemMoved() {}, onItemChanged() { PlacesUtils.bookmarks.removeObserver(this); diff --git a/toolkit/components/places/tests/head_common.js b/toolkit/components/places/tests/head_common.js index 10e19ca930d9..f77437a32c56 100644 --- a/toolkit/components/places/tests/head_common.js +++ b/toolkit/components/places/tests/head_common.js @@ -722,7 +722,6 @@ function NavBookmarkObserver() {} NavBookmarkObserver.prototype = { onItemRemoved() {}, onItemChanged() {}, - onItemMoved() {}, QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]), }; diff --git a/toolkit/components/places/tests/legacy/test_bookmarks.js b/toolkit/components/places/tests/legacy/test_bookmarks.js index 58a4e7671a52..90d9a2c29e81 100644 --- a/toolkit/components/places/tests/legacy/test_bookmarks.js +++ b/toolkit/components/places/tests/legacy/test_bookmarks.js @@ -59,7 +59,6 @@ var bookmarksObserver = { this._itemChangedValue = value; this._itemChangedOldValue = oldValue; }, - onItemMoved() {}, QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]), }; diff --git a/toolkit/components/places/tests/sync/head_sync.js b/toolkit/components/places/tests/sync/head_sync.js index 7bd9956ec458..0c6b01db76bc 100644 --- a/toolkit/components/places/tests/sync/head_sync.js +++ b/toolkit/components/places/tests/sync/head_sync.js @@ -393,32 +393,6 @@ BookmarkObserver.prototype = { } this.notifications.push({ name: "onItemChanged", params }); }, - onItemMoved( - itemId, - oldIndex, - newIndex, - type, - guid, - oldParentGuid, - newParentGuid, - source, - urlHref - ) { - this.notifications.push({ - name: "onItemMoved", - params: { - itemId, - oldIndex, - newIndex, - type, - guid, - oldParentGuid, - newParentGuid, - source, - urlHref, - }, - }); - }, QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]), diff --git a/toolkit/components/places/tests/unit/test_async_transactions.js b/toolkit/components/places/tests/unit/test_async_transactions.js index 10599ede62b0..1fde01a9b47c 100644 --- a/toolkit/components/places/tests/unit/test_async_transactions.js +++ b/toolkit/components/places/tests/unit/test_async_transactions.js @@ -100,24 +100,6 @@ var observer = { }; changesForGuid.set(aProperty, change); }, - - onItemMoved( - aItemId, - aOldIndex, - aNewIndex, - aItemType, - aGuid, - aOldParentGuid, - aNewParentGuid - ) { - this.itemsMoved.set(aGuid, { - oldParentGuid: aOldParentGuid, - oldIndex: aOldIndex, - newParentGuid: aNewParentGuid, - newIndex: aNewIndex, - itemType: aItemType, - }); - }, }; observer.reset(); diff --git a/toolkit/components/places/tests/unit/test_onItemChanged_tags.js b/toolkit/components/places/tests/unit/test_onItemChanged_tags.js index 48bf0e18fa16..76f6664617b6 100644 --- a/toolkit/components/places/tests/unit/test_onItemChanged_tags.js +++ b/toolkit/components/places/tests/unit/test_onItemChanged_tags.js @@ -51,8 +51,6 @@ add_task(async function run_test() { } } }, - - onItemMoved() {}, }; PlacesUtils.bookmarks.addObserver(bookmarksObserver); bookmarksObserver.handlePlacesEvents = bookmarksObserver.handlePlacesEvents.bind(