Bug 897954: Remove onBeginUpdateBatch and onEndUpdateBatch functions. r=mak
☠☠ backed out by 3703efb779d0 ☠ ☠
authorDaisuke Akatsuka <daisuke@birchill.co.jp>
Thu, 25 Feb 2021 00:12:40 +0000
changeset 568689 9443814d173b8b626aa6891c899190856b64e6e2
parent 568688 2e558ac8e419279b92528383e9f1e26664c11e86
child 568690 0a60eb3f7ba821199cb2d1b445b9295757af9867
push id38236
push usersmolnar@mozilla.com
push dateThu, 25 Feb 2021 04:16:11 +0000
treeherdermozilla-central@7be47f00f8cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs897954
milestone88.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 897954: Remove onBeginUpdateBatch and onEndUpdateBatch functions. r=mak Differential Revision: https://phabricator.services.mozilla.com/D105441
browser/base/content/browser-places.js
browser/components/extensions/parent/ext-bookmarks.js
browser/components/newtab/lib/PlacesFeed.jsm
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/editBookmark.js
browser/components/places/tests/browser/browser_editBookmark_keywords.js
browser/components/places/tests/browser/browser_views_liveupdate.js
services/sync/modules/engines/bookmarks.js
toolkit/components/downloads/test/unit/head.js
toolkit/components/places/History.jsm
toolkit/components/places/PlacesExpiration.jsm
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/TaggingService.jsm
toolkit/components/places/nsINavBookmarksService.idl
toolkit/components/places/nsINavHistoryService.idl
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistory.h
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/places/nsNavHistoryResult.h
toolkit/components/places/tests/PlacesTestUtils.jsm
toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
toolkit/components/places/tests/chrome/test_371798.xhtml
toolkit/components/places/tests/head_common.js
toolkit/components/places/tests/history/test_async_history_api.js
toolkit/components/places/tests/legacy/test_bookmarks.js
toolkit/components/places/tests/sync/head_sync.js
toolkit/components/places/tests/unit/test_async_transactions.js
toolkit/components/places/tests/unit/test_onItemChanged_tags.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -2391,19 +2391,16 @@ var BookmarkingUI = {
         // Only need to update the UI if it wasn't marked as starred before:
         if (this._itemGuids.size == 1) {
           this._updateStar();
         }
       }
     }
   },
 
-  onBeginUpdateBatch() {},
-  onEndUpdateBatch() {},
-  onBeforeItemRemoved() {},
   onItemMoved(
     aItemId,
     aProperty,
     aIsAnnotationProperty,
     aNewValue,
     aLastModified,
     aItemType,
     aGuid,
--- a/browser/components/extensions/parent/ext-bookmarks.js
+++ b/browser/components/extensions/parent/ext-bookmarks.js
@@ -121,19 +121,16 @@ let observer = new (class extends EventE
   constructor() {
     super();
 
     this.skipTags = true;
 
     this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
   }
 
-  onBeginUpdateBatch() {}
-  onEndUpdateBatch() {}
-
   handlePlacesEvents(events) {
     for (let event of events) {
       switch (event.type) {
         case "bookmark-added":
           if (event.isTagging) {
             continue;
           }
           let bookmark = {
--- a/browser/components/newtab/lib/PlacesFeed.jsm
+++ b/browser/components/newtab/lib/PlacesFeed.jsm
@@ -51,20 +51,16 @@ class Observer {
  */
 class BookmarksObserver extends Observer {
   constructor(dispatch) {
     super(dispatch, Ci.nsINavBookmarkObserver);
     this.skipTags = true;
   }
 
   // Empty functions to make xpconnect happy
-  onBeginUpdateBatch() {}
-
-  onEndUpdateBatch() {}
-
   onItemMoved() {}
 
   // Disabled due to performance cost, see Issue 3203 /
   // https://bugzilla.mozilla.org/show_bug.cgi?id=1392267.
   onItemChanged() {}
 }
 
 /**
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -983,18 +983,16 @@ var PlacesUIUtils = {
    * @param {Function} functionToWrap The function to
    */
   async batchUpdatesForNode(resultNode, itemsBeingChanged, functionToWrap) {
     if (!resultNode) {
       await functionToWrap();
       return;
     }
 
-    resultNode = resultNode.QueryInterface(Ci.nsINavBookmarkObserver);
-
     if (itemsBeingChanged > ITEM_CHANGED_BATCH_NOTIFICATION_THRESHOLD) {
       resultNode.onBeginUpdateBatch();
     }
 
     try {
       await functionToWrap();
     } finally {
       if (itemsBeingChanged > ITEM_CHANGED_BATCH_NOTIFICATION_THRESHOLD) {
@@ -1178,18 +1176,16 @@ var PlacesUIUtils = {
             1
           );
         }
       }
     };
 
     // This listener is for tracking bookmark moves
     let placesUtilsBookmarksObserver = {
-      onBeginUpdateBatch() {},
-      onEndUpdateBatch() {},
       onItemChanged() {},
       onItemMoved(
         aItemId,
         aProperty,
         aIsAnnotationProperty,
         aNewValue,
         aLastModified,
         aItemType,
--- a/browser/components/places/content/editBookmark.js
+++ b/browser/components/places/content/editBookmark.js
@@ -1289,19 +1289,16 @@ var gEditItemOverlay = {
     // recurse.
     PlacesUtils.bookmarks.fetch(newParentGuid).then(bm => {
       this._folderMenuList.selectedItem = this._getFolderMenuItem(
         newParentGuid,
         bm.title
       );
     });
   },
-
-  onBeginUpdateBatch() {},
-  onEndUpdateBatch() {},
 };
 
 XPCOMUtils.defineLazyGetter(gEditItemOverlay, "_folderTree", () => {
   if (!customElements.get("places-tree")) {
     Services.scriptloader.loadSubScript(
       "chrome://browser/content/places/places-tree.js",
       window
     );
--- a/browser/components/places/tests/browser/browser_editBookmark_keywords.js
+++ b/browser/components/places/tests/browser/browser_editBookmark_keywords.js
@@ -1,18 +1,16 @@
 "use strict";
 
 const TEST_URL = "about:blank";
 
 add_task(async function() {
   function promiseOnItemChanged() {
     return new Promise(resolve => {
       PlacesUtils.bookmarks.addObserver({
-        onBeginUpdateBatch() {},
-        onEndUpdateBatch() {},
         onItemMoved() {},
         onItemChanged(id, property, isAnno, value) {
           PlacesUtils.bookmarks.removeObserver(this);
           resolve({ property, value });
         },
         QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]),
       });
     });
--- a/browser/components/places/tests/browser/browser_views_liveupdate.js
+++ b/browser/components/places/tests/browser/browser_views_liveupdate.js
@@ -204,19 +204,16 @@ var bookmarksObserver = {
     this._notifications.push([
       "assertItemMoved",
       newParentGuid,
       guid,
       newIndex,
     ]);
   },
 
-  onBeginUpdateBatch() {},
-  onEndUpdateBatch() {},
-
   onItemChanged(
     itemId,
     property,
     annoProperty,
     newValue,
     lastModified,
     itemType,
     parentId,
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -790,18 +790,16 @@ BookmarksStore.prototype = {
 
 // The bookmarks tracker is a special flower. Instead of listening for changes
 // via observer notifications, it queries Places for the set of items that have
 // changed since the last sync. Because it's a "pull-based" tracker, it ignores
 // all concepts of "add a changed ID." However, it still registers an observer
 // to bump the score, so that changed bookmarks are synced immediately.
 function BookmarksTracker(name, engine) {
   Tracker.call(this, name, engine);
-  this._batchDepth = 0;
-  this._batchSawScoreIncrement = false;
 }
 BookmarksTracker.prototype = {
   __proto__: Tracker.prototype,
 
   onStart() {
     PlacesUtils.bookmarks.addObserver(this, true);
     this._placesListener = new PlacesWeakCallbackWrapper(
       this.handlePlacesEvents.bind(this)
@@ -857,24 +855,19 @@ BookmarksTracker.prototype = {
     }
   },
 
   QueryInterface: ChromeUtils.generateQI([
     "nsINavBookmarkObserver",
     "nsISupportsWeakReference",
   ]),
 
-  /* Every add/remove/change will trigger a sync for MULTI_DEVICE (except in
-     a batch operation, where we do it at the end of the batch) */
+  /* Every add/remove/change will trigger a sync for MULTI_DEVICE */
   _upScore: function BMT__upScore() {
-    if (this._batchDepth == 0) {
-      this.score += SCORE_INCREMENT_XLARGE;
-    } else {
-      this._batchSawScoreIncrement = true;
-    }
+    this.score += SCORE_INCREMENT_XLARGE;
   },
 
   handlePlacesEvents(events) {
     for (let event of events) {
       switch (event.type) {
         case "bookmark-added":
           if (IGNORED_SOURCES.includes(event.source)) {
             continue;
@@ -947,26 +940,16 @@ BookmarksTracker.prototype = {
   ) {
     if (IGNORED_SOURCES.includes(source)) {
       return;
     }
 
     this._log.trace("onItemMoved: " + itemId);
     this._upScore();
   },
-
-  onBeginUpdateBatch() {
-    ++this._batchDepth;
-  },
-  onEndUpdateBatch() {
-    if (--this._batchDepth === 0 && this._batchSawScoreIncrement) {
-      this.score += SCORE_INCREMENT_XLARGE;
-      this._batchSawScoreIncrement = false;
-    }
-  },
 };
 
 /**
  * A changeset that stores extra metadata in a change record for each ID. The
  * engine updates this metadata when uploading Sync records, and writes it back
  * to Places in `BookmarksEngine#trackRemainingChanges`.
  *
  * The `synced` property on a change record means its corresponding item has
--- a/toolkit/components/downloads/test/unit/head.js
+++ b/toolkit/components/downloads/test/unit/head.js
@@ -252,17 +252,17 @@ function promiseTimeout(aTime) {
 
 /**
  * Waits for a new history visit to be notified for the specified URI.
  *
  * @param aUrl
  *        String containing the URI that will be visited.
  *
  * @return {Promise}
- * @resolves Array [aTime, aTransitionType] from nsINavHistoryObserver.onVisit.
+ * @resolves Array [aTime, aTransitionType] from page-visited places event.
  * @rejects Never.
  */
 function promiseWaitForVisit(aUrl) {
   return new Promise(resolve => {
     function listener(aEvents) {
       Assert.equal(aEvents.length, 1);
       let event = aEvents[0];
       Assert.equal(event.type, "page-visited");
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -57,20 +57,20 @@
  *     as argument may hold `nsIURI` or `string` values for property `referrer`,
  *     but `VisitInfo` objects returned by this module always hold `URL`
  *     values.
  * See the documentation of individual methods to find out which properties
  * are required for `VisitInfo` arguments or returned for `VisitInfo` results.
  *
  *
  *
- * Each successful operation notifies through the nsINavHistoryObserver
- * interface. To listen to such notifications you must register using
- * nsINavHistoryService `addObserver` and `removeObserver` methods.
- * @see nsINavHistoryObserver
+ * Each successful operation notifies through the PlacesObservers. To listen to such
+ * notifications you must register using
+ * PlacesObservers `addListener` and `removeListener` methods.
+ * @see PlacesObservers
  */
 
 var EXPORTED_SYMBOLS = ["History"];
 
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 
@@ -217,17 +217,17 @@ var History = Object.freeze({
     return PlacesUtils.promiseDBConnection().then(db =>
       fetchAnnotatedPages(db, annotations)
     );
   },
 
   /**
    * Adds a number of visits for a single page.
    *
-   * Any change may be observed through nsINavHistoryObserver
+   * Any change may be observed through PlacesObservers.
    *
    * @param pageInfo: (PageInfo)
    *      Information on a page. This `PageInfo` MUST contain
    *        - a property `url`, as specified by the definition of `PageInfo`.
    *        - a property `visits`, as specified by the definition of
    *          `PageInfo`, which MUST contain at least one visit.
    *      If a property `title` is provided, the title of the page
    *      is updated.
@@ -264,17 +264,17 @@ var History = Object.freeze({
     return PlacesUtils.withConnectionWrapper("History.jsm: insert", db =>
       insert(db, info)
     );
   },
 
   /**
    * Adds a number of visits for a number of pages.
    *
-   * Any change may be observed through nsINavHistoryObserver
+   * Any change may be observed through PlacesObservers.
    *
    * @param pageInfos: (Array<PageInfo>)
    *      Information on a page. This `PageInfo` MUST contain
    *        - a property `url`, as specified by the definition of `PageInfo`.
    *        - a property `visits`, as specified by the definition of
    *          `PageInfo`, which MUST contain at least one visit.
    *      If a property `title` is provided, the title of the page
    *      is updated.
@@ -334,17 +334,17 @@ var History = Object.freeze({
     return PlacesUtils.withConnectionWrapper("History.jsm: insertMany", db =>
       insertMany(db, infos, onResult, onError)
     );
   },
 
   /**
    * Remove pages from the database.
    *
-   * Any change may be observed through nsINavHistoryObserver
+   * Any change may be observed through PlacesObservers.
    *
    *
    * @param page: (URL or nsIURI)
    *      The full URI of the page.
    *             or (string)
    *      Either the full URI of the page or the GUID of the page.
    *             or (Array<URL|nsIURI|string>)
    *      An array of the above, to batch requests.
@@ -417,17 +417,17 @@ var History = Object.freeze({
       }
       return removedPages;
     })();
   },
 
   /**
    * Remove visits matching specific characteristics.
    *
-   * Any change may be observed through nsINavHistoryObserver.
+   * Any change may be observed through PlacesObservers.
    *
    * @param filter: (object)
    *      The `object` may contain some of the following
    *      properties:
    *          - beginDate: (Date) Remove visits that have
    *                been added since this date (inclusive).
    *          - endDate: (Date) Remove visits that have
    *                been added before this date (inclusive).
@@ -511,17 +511,17 @@ var History = Object.freeze({
       "History.jsm: removeVisitsByFilter",
       db => removeVisitsByFilter(db, filter, onResult)
     );
   },
 
   /**
    * Remove pages from the database based on a filter.
    *
-   * Any change may be observed through nsINavHistoryObserver
+   * Any change may be observed through PlacesObservers
    *
    *
    * @param filter: An object containing a non empty subset of the following
    * properties:
    * - host: (string)
    *     Hostname with or without subhost. Examples:
    *       "mozilla.org" removes pages from mozilla.org but not its subdomains
    *       ".mozilla.org" removes pages from mozilla.org and its subdomains
--- a/toolkit/components/places/PlacesExpiration.jsm
+++ b/toolkit/components/places/PlacesExpiration.jsm
@@ -929,15 +929,14 @@ nsPlacesExpiration.prototype = {
     }
     return (this._timer = timer);
   },
 
   classID: Components.ID("705a423f-2f69-42f3-b9fe-1517e0dee56f"),
 
   QueryInterface: ChromeUtils.generateQI([
     "nsIObserver",
-    "nsINavHistoryObserver",
     "nsITimerCallback",
     "nsISupportsWeakReference",
   ]),
 };
 
 var EXPORTED_SYMBOLS = ["nsPlacesExpiration"];
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -2402,19 +2402,16 @@ class BookmarkObserverRecorder {
         isDescendantRemoval: false,
       })
     );
   }
 
   async notifyBookmarkObservers() {
     MirrorLog.trace("Notifying bookmark observers");
     let observers = PlacesUtils.bookmarks.getObservers();
-    // ideally we'd send `onBeginUpdateBatch` here (and `onEndUpdateBatch` at
-    // the end) to all observers, but batching is somewhat broken currently.
-    // See bug 1605881 for all the gory details...
     await Async.yieldingForEach(
       this.guidChangedArgs,
       args => {
         if (this.signal.aborted) {
           throw new SyncedBookmarksMirror.InterruptedError(
             "Interrupted while notifying observers for changed GUIDs"
           );
         }
--- a/toolkit/components/places/TaggingService.jsm
+++ b/toolkit/components/places/TaggingService.jsm
@@ -447,19 +447,16 @@ TaggingService.prototype = {
       this._tagFolders[aItemId] &&
       PlacesUtils.tagsFolderId == aOldParent &&
       PlacesUtils.tagsFolderId != aNewParent
     ) {
       delete this._tagFolders[aItemId];
     }
   },
 
-  onBeginUpdateBatch() {},
-  onEndUpdateBatch() {},
-
   // nsISupports
 
   classID: Components.ID("{bbc23860-2553-479d-8b78-94d9038334f7}"),
 
   _xpcom_factory: ComponentUtils.generateSingletonFactory(TaggingService),
 
   QueryInterface: ChromeUtils.generateQI([
     "nsITaggingService",
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -17,31 +17,16 @@ interface nsINavHistoryBatchCallback;
 interface nsINavBookmarkObserver : nsISupports
 {
   /*
    * This observer should not be called for items that are tags.
    */
   readonly attribute boolean skipTags;
 
   /**
-   * Notifies that a batch transaction has started.
-   * Other notifications will be sent during the batch, but the observer is
-   * guaranteed that onEndUpdateBatch() will be called at its completion.
-   * During a batch the observer should do its best to reduce the work done to
-   * handle notifications, since multiple changes are going to happen in a short
-   * timeframe.
-   */
-  void onBeginUpdateBatch();
-
-  /**
-   * Notifies that a batch transaction has ended.
-   */
-  void onEndUpdateBatch();
-
-  /**
    * Notifies that an item's information has changed.  This will be called
    * whenever any attributes like "title" are changed.
    *
    * @param aItemId
    *        The id of the item that was changed.
    * @param aProperty
    *        The property which changed.
    * @param aIsAnnotationProperty
--- a/toolkit/components/places/nsINavHistoryService.idl
+++ b/toolkit/components/places/nsINavHistoryService.idl
@@ -552,26 +552,17 @@ interface nsINavHistoryResult : nsISuppo
    * When a result goes out of scope it will continue to observe changes till
    * it is cycle collected.  While the result waits to be collected it will stay
    * in memory, and continue to update itself, potentially causing unwanted
    * additional work.  When you close the root node the result will stop
    * observing changes, so it is good practice to close the root node when you
    * are done with a result, since that will avoid unwanted performance hits.
    */
   readonly attribute nsINavHistoryContainerResultNode root;
-};
 
-
-/**
- * DANGER! If you are in the middle of a batch transaction, there may be a
- * database transaction active. You can still access the DB, but be careful.
- */
-[scriptable, uuid(0f0f45b0-13a1-44ae-a0ab-c6046ec6d4da)]
-interface nsINavHistoryObserver : nsISupports
-{
   /**
    * Notifies you that a bunch of things are about to change, don't do any
    * heavy-duty processing until onEndUpdateBatch is called.
    */
   void onBeginUpdateBatch();
 
   /**
    * Notifies you that we are done doing a bunch of things and you should go
@@ -1099,35 +1090,16 @@ interface nsINavHistoryService : nsISupp
   /**
    * Converts a query into an equivalent string that can be persisted. Inverse
    * of queryStringToQuery()
    */
   AUTF8String queryToQueryString(in nsINavHistoryQuery aQuery,
                                  in nsINavHistoryQueryOptions options);
 
   /**
-   * Adds a history observer. If ownsWeak is false, the history service will
-   * keep an owning reference to the observer.  If ownsWeak is true, then
-   * aObserver must implement nsISupportsWeakReference, and the history service
-   * will keep a weak reference to the observer.
-   */
-  void addObserver(in nsINavHistoryObserver observer,
-                   [optional] in boolean ownsWeak);
-
-  /**
-   * Removes a history observer.
-   */
-  void removeObserver(in nsINavHistoryObserver observer);
-
-  /**
-   * Gets an array of registered nsINavHistoryObserver objects.
-   */
-  Array<nsINavHistoryObserver> getObservers();
-
-  /**
    * True if history is disabled. currently,
    * history is disabled if the places.history.enabled pref is false.
    */
   readonly attribute boolean historyDisabled;
 
   /**
    * Generate a guid.
    * Guids can be used for any places purposes (history, bookmarks, etc.)
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -382,18 +382,17 @@ nsNavHistory::nsNavHistory()
       mRecentLink(RECENT_EVENTS_INITIAL_CACHE_LENGTH),
       mRecentBookmark(RECENT_EVENTS_INITIAL_CACHE_LENGTH),
       mHistoryEnabled(true),
       mMatchDiacritics(false),
       mNumVisitsForFrecency(10),
       mDecayFrecencyPendingCount(0),
       mTagsFolder(-1),
       mLastCachedStartOfDay(INT64_MAX),
-      mLastCachedEndOfDay(0),
-      mCanNotify(true)
+      mLastCachedEndOfDay(0)
 #ifdef XP_WIN
       ,
       mCryptoProviderInitialized(false)
 #endif
 {
   NS_ASSERTION(!gHistoryService,
                "Attempting to create two instances of the service!");
 #ifdef XP_WIN
@@ -1913,58 +1912,16 @@ nsresult nsNavHistory::GetQueryResults(
     rv = ResultsAsList(statement, aOptions, aResults);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsNavHistory::AddObserver(nsINavHistoryObserver* aObserver, bool aOwnsWeak) {
-  NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
-  NS_ENSURE_ARG(aObserver);
-
-  if (NS_WARN_IF(!mCanNotify)) return NS_ERROR_UNEXPECTED;
-
-  return mObservers.AppendWeakElementUnlessExists(aObserver, aOwnsWeak);
-}
-
-NS_IMETHODIMP
-nsNavHistory::RemoveObserver(nsINavHistoryObserver* aObserver) {
-  NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
-  NS_ENSURE_ARG(aObserver);
-
-  return mObservers.RemoveWeakElement(aObserver);
-}
-
-NS_IMETHODIMP
-nsNavHistory::GetObservers(
-    nsTArray<RefPtr<nsINavHistoryObserver>>& aObservers) {
-  aObservers.Clear();
-
-  // Clear any cached value, cause it's very likely the consumer has made
-  // changes to history and is now trying to notify them.
-  InvalidateDaysOfHistory();
-
-  if (!mCanNotify) return NS_OK;
-
-  // Then add the other observers.
-  for (uint32_t i = 0; i < mObservers.Length(); ++i) {
-    nsCOMPtr<nsINavHistoryObserver> observer =
-        mObservers.ElementAt(i).GetValue();
-    // Skip nullified weak observers.
-    if (observer) {
-      aObservers.AppendElement(observer.forget());
-    }
-  }
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 nsNavHistory::GetHistoryDisabled(bool* _retval) {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG_POINTER(_retval);
 
   *_retval = IsHistoryDisabled();
   return NS_OK;
 }
 
@@ -2151,23 +2108,16 @@ nsNavHistory::Observe(nsISupports* aSubj
   if (strcmp(aTopic, TOPIC_PROFILE_TEARDOWN) == 0 ||
       strcmp(aTopic, TOPIC_PROFILE_CHANGE) == 0 ||
       strcmp(aTopic, TOPIC_SIMULATE_PLACES_SHUTDOWN) == 0) {
     // These notifications are used by tests to simulate a Places shutdown.
     // They should just be forwarded to the Database handle.
     mDB->Observe(aSubject, aTopic, aData);
   }
 
-  else if (strcmp(aTopic, TOPIC_PLACES_CONNECTION_CLOSED) == 0) {
-    // Don't even try to notify observers from this point on, the category
-    // cache would init services that could try to use our APIs.
-    mCanNotify = false;
-    mObservers.Clear();
-  }
-
   else if (strcmp(aTopic, TOPIC_PREF_CHANGED) == 0) {
     LoadPrefs();
   }
 
   else if (strcmp(aTopic, TOPIC_IDLE_DAILY) == 0) {
     (void)DecayFrecency();
   }
 
--- a/toolkit/components/places/nsNavHistory.h
+++ b/toolkit/components/places/nsNavHistory.h
@@ -233,23 +233,16 @@ class nsNavHistory final : public nsSupp
    */
   int32_t GetDaysOfHistory();
 
   void DomainNameFromURI(nsIURI* aURI, nsACString& aDomainName);
   static PRTime NormalizeTime(uint32_t aRelative, PRTime aOffset);
 
   typedef nsDataHashtable<nsCStringHashKey, nsCString> StringHash;
 
-  /**
-   * Indicates if it is OK to notify history observers or not.
-   *
-   * @return true if it is OK to notify, false otherwise.
-   */
-  bool canNotify() { return mCanNotify; }
-
   enum RecentEventFlags {
     RECENT_TYPED = 1 << 0,      // User typed in URL recently
     RECENT_ACTIVATED = 1 << 1,  // User tapped URL link recently
     RECENT_BOOKMARKED = 1 << 2  // User bookmarked URL recently
   };
 
   /**
    * Returns any recent activity done with a URL.
@@ -428,19 +421,16 @@ class nsNavHistory final : public nsSupp
       mozIStorageBaseStatement* statement,
       const RefPtr<nsNavHistoryQuery>& aQuery,
       const RefPtr<nsNavHistoryQueryOptions>& aOptions);
 
   nsresult ResultsAsList(mozIStorageStatement* statement,
                          nsNavHistoryQueryOptions* aOptions,
                          nsCOMArray<nsNavHistoryResultNode>* aResults);
 
-  // observers
-  nsMaybeWeakPtrArray<nsINavHistoryObserver> mObservers;
-
   // effective tld service
   nsCOMPtr<nsIEffectiveTLDService> mTLDService;
   nsCOMPtr<nsIIDNService> mIDNService;
 
   // localization
   nsCOMPtr<nsIStringBundle> mBundle;
   nsCOMPtr<nsICollation> mCollation;
 
@@ -495,19 +485,16 @@ class nsNavHistory final : public nsSupp
       const nsTArray<mozilla::places::QueryKeyValuePair>& aTokens,
       nsNavHistoryQuery* aQuery, nsNavHistoryQueryOptions* aOptions);
 
   int64_t mTagsFolder;
 
   int64_t mLastCachedStartOfDay;
   int64_t mLastCachedEndOfDay;
 
-  // Used to enable and disable the observer notifications
-  bool mCanNotify;
-
   // Used to cache the call to CryptAcquireContext, which is expensive
   // when called thousands of times
 #ifdef XP_WIN
   HCRYPTPROV mCryptoProvider;
   bool mCryptoProviderInitialized;
 #endif
 };
 
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -1987,21 +1987,19 @@ void nsNavHistoryQueryResultNode::Recurs
 }
 
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::GetSkipTags(bool* aSkipTags) {
   *aSkipTags = false;
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNavHistoryQueryResultNode::OnBeginUpdateBatch() { return NS_OK; }
-
-NS_IMETHODIMP
-nsNavHistoryQueryResultNode::OnEndUpdateBatch() {
+nsresult nsNavHistoryQueryResultNode::OnBeginUpdateBatch() { return NS_OK; }
+
+nsresult nsNavHistoryQueryResultNode::OnEndUpdateBatch() {
   // If the query has no children it's possible it's not yet listening to
   // bookmarks changes, in such a case it's safer to force a refresh to gather
   // eventual new nodes matching query options.
   if (mChildren.Count() == 0) {
     nsresult rv = Refresh();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -2985,21 +2983,19 @@ nsNavHistoryResultNode* nsNavHistoryFold
   }
 
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::GetSkipTags(bool* aSkipTags) {
   *aSkipTags = false;
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNavHistoryFolderResultNode::OnBeginUpdateBatch() { return NS_OK; }
-
-NS_IMETHODIMP
-nsNavHistoryFolderResultNode::OnEndUpdateBatch() { return NS_OK; }
+nsresult nsNavHistoryFolderResultNode::OnBeginUpdateBatch() { return NS_OK; }
+
+nsresult nsNavHistoryFolderResultNode::OnEndUpdateBatch() { return NS_OK; }
 
 nsresult nsNavHistoryFolderResultNode::OnItemAdded(
     int64_t aItemId, int64_t aParentFolder, int32_t aIndex, uint16_t aItemType,
     nsIURI* aURI, PRTime aDateAdded, const nsACString& aGUID,
     const nsACString& aParentGUID, uint16_t aSource) {
   MOZ_ASSERT(aParentFolder == mTargetFolderItemId, "Got wrong bookmark update");
 
   RESTART_AND_RETURN_IF_ASYNC_PENDING();
@@ -3446,17 +3442,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsNavHistoryResult)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsNavHistoryResult)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsNavHistoryResult)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsINavHistoryResult)
   NS_INTERFACE_MAP_STATIC_AMBIGUOUS(nsNavHistoryResult)
   NS_INTERFACE_MAP_ENTRY(nsINavHistoryResult)
   NS_INTERFACE_MAP_ENTRY(nsINavBookmarkObserver)
-  NS_INTERFACE_MAP_ENTRY(nsINavHistoryObserver)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
 NS_INTERFACE_MAP_END
 
 nsNavHistoryResult::nsNavHistoryResult(
     nsNavHistoryContainerResultNode* aRoot,
     const RefPtr<nsNavHistoryQuery>& aQuery,
     const RefPtr<nsNavHistoryQueryOptions>& aOptions)
     : mRootNode(aRoot),
@@ -3508,20 +3503,16 @@ void nsNavHistoryResult::StopObserving()
     events.AppendElement(PlacesEventType::Bookmark_removed);
   }
   if (mIsMobilePrefObserver) {
     Preferences::UnregisterCallback(OnMobilePrefChangedCallback,
                                     MOBILE_BOOKMARKS_PREF, this);
     mIsMobilePrefObserver = false;
   }
   if (mIsHistoryObserver) {
-    nsNavHistory* history = nsNavHistory::GetHistoryService();
-    if (history) {
-      history->RemoveObserver(this);
-    }
     mIsHistoryObserver = false;
     events.AppendElement(PlacesEventType::History_cleared);
     events.AppendElement(PlacesEventType::Page_removed);
   }
   if (mIsHistoryDetailsObserver) {
     events.AppendElement(PlacesEventType::Page_visited);
     events.AppendElement(PlacesEventType::Page_title_changed);
     mIsHistoryDetailsObserver = false;
@@ -3543,19 +3534,16 @@ bool nsNavHistoryResult::CanSkipHistoryD
          mSortingMode !=
              nsINavHistoryQueryOptions::SORT_BY_FRECENCY_ASCENDING &&
          mSortingMode != nsINavHistoryQueryOptions::SORT_BY_FRECENCY_DESCENDING;
 }
 
 void nsNavHistoryResult::AddHistoryObserver(
     nsNavHistoryQueryResultNode* aNode) {
   if (!mIsHistoryObserver) {
-    nsNavHistory* history = nsNavHistory::GetHistoryService();
-    NS_ASSERTION(history, "Can't create history service");
-    history->AddObserver(this, true);
     mIsHistoryObserver = true;
 
     AutoTArray<PlacesEventType, 3> events;
     events.AppendElement(PlacesEventType::History_cleared);
     events.AppendElement(PlacesEventType::Page_removed);
     if (!mIsHistoryDetailsObserver) {
       events.AppendElement(PlacesEventType::Page_visited);
       events.AppendElement(PlacesEventType::Page_title_changed);
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -72,17 +72,16 @@ class nsTrimInt64HashKey : public PLDHas
       0xa6, 0x41, 0x8b, 0xb7, 0xe8, 0x82, 0x23, 0x87 \
     }                                                \
   }
 
 class nsNavHistoryResult final
     : public nsSupportsWeakReference,
       public nsINavHistoryResult,
       public nsINavBookmarkObserver,
-      public nsINavHistoryObserver,
       public mozilla::places::INativePlacesEventCallback {
  public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_NAVHISTORYRESULT_IID)
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_NSINAVHISTORYRESULT
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsNavHistoryResult,
                                            nsINavHistoryResult)
@@ -691,16 +690,19 @@ class nsNavHistoryQueryResultNode final
   nsresult OnPageRemovedFromStore(nsIURI* aURI, const nsACString& aGUID,
                                   uint16_t aReason);
   nsresult OnPageRemovedVisits(nsIURI* aURI, bool aPartialRemoval,
                                const nsACString& aGUID, uint16_t aReason,
                                uint32_t aTransitionType);
 
   virtual void OnRemoving() override;
 
+  nsresult OnBeginUpdateBatch();
+  nsresult OnEndUpdateBatch();
+
  public:
   RefPtr<nsNavHistoryQuery> mQuery;
   uint32_t mLiveUpdate;  // one of QUERYUPDATE_* in nsNavHistory.h
   bool mHasSearchTerms;
 
   // safe options getter, ensures query is parsed
   nsNavHistoryQueryOptions* Options();
 
@@ -792,16 +794,19 @@ class nsNavHistoryFolderResultNode final
   void ClearChildren(bool aUnregister);
   nsresult Refresh() override;
 
   bool StartIncrementalUpdate();
   void ReindexRange(int32_t aStartIndex, int32_t aEndIndex, int32_t aDelta);
 
   nsNavHistoryResultNode* FindChildById(int64_t aItemId, uint32_t* aNodeIndex);
 
+  nsresult OnBeginUpdateBatch();
+  nsresult OnEndUpdateBatch();
+
  protected:
   virtual ~nsNavHistoryFolderResultNode();
 
  private:
   nsresult OnChildrenFilled();
   void EnsureRegisteredAsFolderObserver();
   nsresult FillChildrenAsync();
 
--- a/toolkit/components/places/tests/PlacesTestUtils.jsm
+++ b/toolkit/components/places/tests/PlacesTestUtils.jsm
@@ -426,27 +426,23 @@ var PlacesTestUtils = Object.freeze({
             PlacesObservers.removeListener([notification], listener);
             resolve(events);
           }
         }
         PlacesObservers.addListener([notification], listener);
       });
     }
 
-    let iface =
-      type == "bookmarks"
-        ? Ci.nsINavBookmarkObserver
-        : Ci.nsINavHistoryObserver;
     return new Promise(resolve => {
       let proxifiedObserver = new Proxy(
         {},
         {
           get: (target, name) => {
             if (name == "QueryInterface") {
-              return ChromeUtils.generateQI([iface]);
+              return ChromeUtils.generateQI([Ci.nsINavBookmarkObserver]);
             }
             if (name == notification) {
               return (...args) => {
                 if (!conditionFn || conditionFn.apply(this, args)) {
                   PlacesUtils[type].removeObserver(proxifiedObserver);
                   resolve();
                 }
               };
--- a/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
+++ b/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
@@ -52,22 +52,16 @@ var gBookmarksObserver = {
     }
   },
 
   handlePlacesEvents(events) {
     this.validateEvents(events);
   },
 
   // nsINavBookmarkObserver
-  onBeginUpdateBatch() {
-    return this.validate("onBeginUpdateBatch", arguments);
-  },
-  onEndUpdateBatch() {
-    return this.validate("onEndUpdateBatch", arguments);
-  },
   onItemChanged() {
     return this.validate("onItemChanged", arguments);
   },
   onItemMoved() {
     return this.validate("onItemMoved", arguments);
   },
 
   // nsISupports
@@ -104,22 +98,16 @@ var gBookmarkSkipObserver = {
     }
   },
 
   handlePlacesEvents(events) {
     this.validateEvents(events);
   },
 
   // nsINavBookmarkObserver
-  onBeginUpdateBatch() {
-    return this.validate("onBeginUpdateBatch", arguments);
-  },
-  onEndUpdateBatch() {
-    return this.validate("onEndUpdateBatch", arguments);
-  },
   onItemChanged() {
     return this.validate("onItemChanged", arguments);
   },
   onItemMoved() {
     return this.validate("onItemMoved", arguments);
   },
 
   // nsISupports
--- a/toolkit/components/places/tests/chrome/test_371798.xhtml
+++ b/toolkit/components/places/tests/chrome/test_371798.xhtml
@@ -19,18 +19,16 @@ const {NetUtil} = ChromeUtils.import("re
 
 const {PlacesUtils} = ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
 
 const TEST_URI = NetUtil.newURI("http://foo.com");
 
 function promiseOnItemChanged() {
   return new Promise(resolve => {
     PlacesUtils.bookmarks.addObserver({
-      onBeginUpdateBatch() {},
-      onEndUpdateBatch() {},
       onItemRemoved() {},
       onItemMoved() {},
 
       onItemChanged() {
         PlacesUtils.bookmarks.removeObserver(this);
         resolve();
       },
 
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -715,18 +715,16 @@ function do_compare_arrays(a1, a2, sorte
 
 /**
  * Generic nsINavBookmarkObserver that doesn't implement anything, but provides
  * dummy methods to prevent errors about an object not having a certain method.
  */
 function NavBookmarkObserver() {}
 
 NavBookmarkObserver.prototype = {
-  onBeginUpdateBatch() {},
-  onEndUpdateBatch() {},
   onItemRemoved() {},
   onItemChanged() {},
   onItemMoved() {},
   QueryInterface: ChromeUtils.generateQI(["nsINavBookmarkObserver"]),
 };
 
 /**
  * Generic nsINavHistoryResultObserver that doesn't implement anything, but
--- a/toolkit/components/places/tests/history/test_async_history_api.js
+++ b/toolkit/components/places/tests/history/test_async_history_api.js
@@ -1018,18 +1018,18 @@ add_task(async function test_title_chang
   place.visits = [new VisitInfo()];
   asyncHistory.updatePlaces(place);
 
   await titleChangePromise;
   await PlacesTestUtils.promiseAsyncUpdates();
 });
 
 add_task(async function test_visit_notifies() {
-  // There are two observers we need to see for each visit.  One is an
-  // nsINavHistoryObserver and the other is the uri-visit-saved observer topic.
+  // There are two observers we need to see for each visit. One is an
+  // PlacesObservers and the other is the uri-visit-saved observer topic.
   let place = {
     guid: "abcdefghijkl",
     uri: NetUtil.newURI(TEST_DOMAIN + "test_visit_notifies"),
     visits: [new VisitInfo()],
   };
   Assert.equal(false, await PlacesUtils.history.hasVisits(place.uri));
 
   function promiseVisitObserver(aPlace) {
--- a/toolkit/components/places/tests/legacy/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -37,23 +37,16 @@ var bookmarksObserver = {
         break;
       case "bookmark-removed":
         bookmarksObserver._itemRemovedId = event.id;
         bookmarksObserver._itemRemovedFolder = event.parentId;
         bookmarksObserver._itemRemovedIndex = event.index;
     }
   },
 
-  onBeginUpdateBatch() {
-    this._beginUpdateBatch = true;
-  },
-  onEndUpdateBatch() {
-    this._endUpdateBatch = true;
-  },
-
   onItemChanged(
     id,
     property,
     isAnnotationProperty,
     value,
     lastModified,
     itemType,
     parentId,
--- a/toolkit/components/places/tests/sync/head_sync.js
+++ b/toolkit/components/places/tests/sync/head_sync.js
@@ -342,18 +342,16 @@ BookmarkObserver.prototype = {
             source: event.source,
           };
           this.notifications.push({ name: "bookmark-removed", params });
           break;
         }
       }
     }
   },
-  onBeginUpdateBatch() {},
-  onEndUpdateBatch() {},
   onItemChanged(
     itemId,
     property,
     isAnnoProperty,
     newValue,
     lastModified,
     type,
     parentId,
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -22,18 +22,16 @@ var observer = {
 
   tagRelatedGuids: new Set(),
 
   reset() {
     this.itemsAdded = new Map();
     this.itemsRemoved = new Map();
     this.itemsChanged = new Map();
     this.itemsMoved = new Map();
-    this.beginUpdateBatch = false;
-    this.endUpdateBatch = false;
   },
 
   handlePlacesEvents(events) {
     for (let event of events) {
       switch (event.type) {
         case "bookmark-added":
           // Ignore tag items.
           if (event.isTagging) {
@@ -59,24 +57,16 @@ var observer = {
             parentGuid: event.parentGuid,
             index: event.index,
             itemType: event.itemType,
           });
       }
     }
   },
 
-  onBeginUpdateBatch() {
-    this.beginUpdateBatch = true;
-  },
-
-  onEndUpdateBatch() {
-    this.endUpdateBatch = true;
-  },
-
   onItemChanged(
     aItemId,
     aProperty,
     aIsAnnoProperty,
     aNewValue,
     aLastModified,
     aItemType,
     aParentId,
--- a/toolkit/components/places/tests/unit/test_onItemChanged_tags.js
+++ b/toolkit/components/places/tests/unit/test_onItemChanged_tags.js
@@ -47,18 +47,16 @@ add_task(async function run_test() {
               );
               Assert.equal(this._changedCount, 2);
               promise.resolve();
             }
         }
       }
     },
 
-    onBeginUpdateBatch() {},
-    onEndUpdateBatch() {},
     onItemMoved() {},
   };
   PlacesUtils.bookmarks.addObserver(bookmarksObserver);
   bookmarksObserver.handlePlacesEvents = bookmarksObserver.handlePlacesEvents.bind(
     bookmarksObserver
   );
   PlacesUtils.observers.addListener(
     ["bookmark-removed"],