Bug 1440276 - Activity Stream spikes after Bookmark activity, triggering lots of main thread work r=mak
authorUrsula Sarracini <usarracini@mozilla.com>
Wed, 28 Feb 2018 11:17:42 -0500
changeset 461176 18dd09f4f5ba3a2d1f0f11ae8fa192b5805b728c
parent 461175 3dc52ab4b2e22b13abfc8d7ea64762dc277850df
child 461177 413a684727a20b4c4a0e0aa06244faa14e1be401
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1440276
milestone60.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 1440276 - Activity Stream spikes after Bookmark activity, triggering lots of main thread work r=mak 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,