Bug 1197098 - have Sync notify history observers of a batch update and have NewTabUtils defer most work until the batch is complete. r=adw

services/sync/modules/engines/history.js

@@ -44,6 +44,25 @@ HistoryEngine.prototype = {
   applyIncomingBatchSize: HISTORY_STORE_BATCH_SIZE,
 
   syncPriority: 7,
+
+  _processIncoming: function (newitems) {
+    // We want to notify history observers that a batch operation is underway
+    // so they don't do lots of work for each incoming record.
+    let observers = PlacesUtils.history.getObservers();
+    function notifyHistoryObservers(notification) {
+      for (let observer of observers) {
+        try {
+          observer[notification]();
+        } catch (ex) { }
+      }
+    }
+    notifyHistoryObservers("onBeginUpdateBatch");
+    try {
+      return SyncEngine.prototype._processIncoming.call(this, newitems);
+    } finally {
+      notifyHistoryObservers("onEndUpdateBatch");
+    }
+  },
 };
 
 function HistoryStore(name, engine) {

toolkit/modules/NewTabUtils.jsm

@@ -607,6 +607,19 @@ let BlockedLinks = {
  * the history to retrieve the most frequently visited sites.
  */
 let PlacesProvider = {
+  /**
+   * A count of how many batch updates are under way (batches may be nested, so
+   * we keep a counter instead of a simple bool).
+   **/
+  _batchProcessingDepth: 0,
+
+  /**
+   * A flag that tracks whether onFrecencyChanged was notified while a batch
+   * operation was in progress, to tell us whether to take special action after
+   * the batch operation completes.
+   **/
+  _batchCalledFrecencyChanged: false,
+
   /**
    * Set this to change the maximum number of links the provider will provide.
    */
@@ -713,6 +726,18 @@ let PlacesProvider = {
   /**
    * Called by the history service.
    */
+  onBeginUpdateBatch: function() {
+    this._batchProcessingDepth += 1;
+  },
+
+  onEndUpdateBatch: function() {
+    this._batchProcessingDepth -= 1;
+    if (this._batchProcessingDepth == 0 && this._batchCalledFrecencyChanged) {
+      this.onManyFrecenciesChanged();
+      this._batchCalledFrecencyChanged = false;
+    }
+  },
+
   onDeleteURI: function PlacesProvider_onDeleteURI(aURI, aGUID, aReason) {
     // let observers remove sensetive data associated with deleted visit
     this._callObservers("onDeleteURI", {
@@ -728,6 +753,13 @@ let PlacesProvider = {
    * Called by the history service.
    */
   onFrecencyChanged: function PlacesProvider_onFrecencyChanged(aURI, aNewFrecency, aGUID, aHidden, aLastVisitDate) {
+    // If something is doing a batch update of history entries we don't want
+    // to do lots of work for each record. So we just track the fact we need
+    // to call onManyFrecenciesChanged() once the batch is complete.
+    if (this._batchProcessingDepth > 0) {
+      this._batchCalledFrecencyChanged = true;
+      return;
+    }
     // The implementation of the query in getLinks excludes hidden and
     // unvisited pages, so it's important to exclude them here, too.
     if (!aHidden && aLastVisitDate) {