Bug 1440276 - Activity Stream spikes after Bookmark activity, triggering lots of main thread work draft
authorUrsula Sarracini <usarracini@mozilla.com>
Wed, 28 Feb 2018 11:17:42 -0500
changeset 761056 ed7490f732925b80eb7dbfc43d34e04c95e27070
parent 760935 ee326c976eebdca48128054022c443d3993e12b0
push id100837
push userusarracini@mozilla.com
push dateWed, 28 Feb 2018 16:20:13 +0000
bugs1440276
milestone60.0a1
Bug 1440276 - Activity Stream spikes after Bookmark activity, triggering lots of main thread work MozReview-Commit-ID: BoCZEDXpSKD
browser/extensions/activity-stream/lib/PlacesFeed.jsm
toolkit/components/places/nsNavBookmarks.cpp
--- a/browser/extensions/activity-stream/lib/PlacesFeed.jsm
+++ b/browser/extensions/activity-stream/lib/PlacesFeed.jsm
@@ -85,16 +85,17 @@ class HistoryObserver extends Observer {
 }
 
 /**
  * BookmarksObserver - observes events from PlacesUtils.bookmarks
  */
 class BookmarksObserver extends Observer {
   constructor(dispatch) {
     super(dispatch, Ci.nsINavBookmarkObserver);
+    this.skipTags = true;
   }
 
   /**
    * onItemAdded - Called when a bookmark is added
    *
    * @param  {str} id
    * @param  {str} folderId
    * @param  {int} index
@@ -142,66 +143,28 @@ class BookmarksObserver extends Observer
     if (type === PlacesUtils.bookmarks.TYPE_BOOKMARK) {
       this.dispatch({
         type: at.PLACES_BOOKMARK_REMOVED,
         data: {url: uri.spec, bookmarkGuid: guid}
       });
     }
   }
 
-  /**
-   * onItemChanged - Called when a bookmark is modified
-   *
-   * @param  {str} id           description
-   * @param  {str} property     The property that was modified (e.g. uri, title)
-   * @param  {bool} isAnnotation
-   * @param  {any} value
-   * @param  {int} lastModified
-   * @param  {int} type         Indicates if the bookmark is an actual bookmark,
-   *                             a folder, or a separator.
-   * @param  {int} parent
-   * @param  {str} guid         The unique id of the bookmark
-   */
-  async onItemChanged(...args) {
-
-    /*
-    // Disabled due to performance cost, see Issue 3203 /
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1392267.
-    //
-    // If this is used, please consider avoiding the call to
-    // NewTabUtils.activityStreamProvider.getBookmark which performs an additional
-    // fetch to the database.
-    // If you need more fields, please talk to the places team.
-
-    const property = args[1];
-    const type = args[5];
-    const guid = args[7];
-
-    // Only process this event if it is a TYPE_BOOKMARK, and uri or title was the property changed.
-    if (type !== PlacesUtils.bookmarks.TYPE_BOOKMARK || !["uri", "title"].includes(property)) {
-      return;
-    }
-    try {
-      // bookmark: {bookmarkGuid, bookmarkTitle, lastModified, url}
-      const bookmark = await NewTabUtils.activityStreamProvider.getBookmark(guid);
-      this.dispatch({type: at.PLACES_BOOKMARK_CHANGED, data: bookmark});
-    } catch (e) {
-      Cu.reportError(e);
-    }
-    */
-  }
-
   // Empty functions to make xpconnect happy
   onBeginUpdateBatch() {}
 
   onEndUpdateBatch() {}
 
   onItemVisited() {}
 
   onItemMoved() {}
+
+  // Disabled due to performance cost, see Issue 3203 /
+  // https://bugzilla.mozilla.org/show_bug.cgi?id=1392267.
+  onItemChanged() {}
 }
 
 class PlacesFeed {
   constructor() {
     this.historyObserver = new HistoryObserver(action => this.store.dispatch(ac.BroadcastToContent(action)));
     this.bookmarksObserver = new BookmarksObserver(action => this.store.dispatch(ac.BroadcastToContent(action)));
   }
 
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -1281,17 +1281,18 @@ nsNavBookmarks::SetItemLastModified(int6
     NS_ENSURE_SUCCESS(rv, rv);
   } else {
     rv = SetItemDateInternal(LAST_MODIFIED, syncChangeDelta, bookmark.id,
                              bookmark.lastModified);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Note: mDBSetItemDateAdded also sets lastModified to aDateAdded.
-  NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavBookmarkObserver,
+  NOTIFY_BOOKMARKS_OBSERVERS(mCanNotify, mObservers,
+                   SKIP_TAGS(isTagging || bookmark.parentId == tagsRootId),
                    OnItemChanged(bookmark.id,
                                  NS_LITERAL_CSTRING("lastModified"),
                                  false,
                                  nsPrintfCString("%" PRId64, bookmark.lastModified),
                                  bookmark.lastModified,
                                  bookmark.type,
                                  bookmark.parentId,
                                  bookmark.guid,