Bug 1417101 - Remove the annotation observer from the bookmarks service. r?mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 16 Nov 2017 16:49:03 -0800
changeset 706527 a2addd54990015c50a14c38efebe3ce2414a9445
parent 706205 781485c695e1f07b8782427d556f6570e4a8072f
child 742674 127888eda33afbf337d457d08bc1638ca91db3c5
push id91819
push userbmo:kit@mozilla.com
push dateSat, 02 Dec 2017 06:03:36 +0000
reviewersmak
bugs1417101
milestone59.0a1
Bug 1417101 - Remove the annotation observer from the bookmarks service. r?mak Since `SetItemAnnotation` already queries `moz_bookmarks`, we can fetch and pass the changed bookmark's info directly to `nsNavBookmarks::NotifyItemChanged`, without going through the anno observer. This patch refactors the internal `Set*` methods to receive an optional `BookmarkData` for item annotation changes, and fire `OnItemChanged` notifications after notifying anno observers. `NotifyItemChanged` also updates the bookmark's last modified time if requested. MozReview-Commit-ID: Hz5qiOmAjsD
toolkit/components/places/nsAnnotationService.cpp
toolkit/components/places/nsAnnotationService.h
toolkit/components/places/nsIAnnotationService.idl
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavBookmarks.h
toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
toolkit/components/places/tests/unit/test_null_interfaces.js
--- a/toolkit/components/places/nsAnnotationService.cpp
+++ b/toolkit/components/places/nsAnnotationService.cpp
@@ -42,16 +42,42 @@ const int32_t nsAnnotationService::kAnno
 const int32_t nsAnnotationService::kAnnoIndex_NameID = 2;
 const int32_t nsAnnotationService::kAnnoIndex_Content = 3;
 const int32_t nsAnnotationService::kAnnoIndex_Flags = 4;
 const int32_t nsAnnotationService::kAnnoIndex_Expiration = 5;
 const int32_t nsAnnotationService::kAnnoIndex_Type = 6;
 const int32_t nsAnnotationService::kAnnoIndex_DateAdded = 7;
 const int32_t nsAnnotationService::kAnnoIndex_LastModified = 8;
 
+namespace {
+
+// Fires `onItemChanged` notifications for bookmark annotation changes.
+void
+NotifyItemChanged(const BookmarkData& aBookmark, const nsACString& aName,
+                  uint16_t aSource, bool aDontUpdateLastModified)
+{
+  if (aBookmark.id < 0) {
+    return;
+  }
+
+  ItemChangeData changeData;
+  changeData.bookmark = aBookmark;
+  changeData.isAnnotation = true;
+  changeData.updateLastModified = !aDontUpdateLastModified;
+  changeData.source = aSource;
+  changeData.property = aName;
+
+  nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+  if (bookmarks) {
+    bookmarks->NotifyItemChanged(changeData);
+  }
+}
+
+}
+
 namespace mozilla {
 namespace places {
 
 ////////////////////////////////////////////////////////////////////////////////
 //// AnnotatedResult
 
 AnnotatedResult::AnnotatedResult(const nsCString& aGUID,
                                  nsIURI* aURI,
@@ -149,27 +175,31 @@ nsAnnotationService::Init()
   }
 
   return NS_OK;
 }
 
 nsresult
 nsAnnotationService::SetAnnotationStringInternal(nsIURI* aURI,
                                                  int64_t aItemId,
+                                                 BookmarkData* aBookmark,
                                                  const nsACString& aName,
                                                  const nsAString& aValue,
                                                  int32_t aFlags,
                                                  uint16_t aExpiration)
 {
   mozStorageTransaction transaction(mDB->MainConn(), false);
   nsCOMPtr<mozIStorageStatement> statement;
-  nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
+
+  nsresult rv = StartSetAnnotation(aURI, aItemId, aBookmark, aName, aFlags,
+                                   aExpiration,
                                    nsIAnnotationService::TYPE_STRING,
                                    statement);
   NS_ENSURE_SUCCESS(rv, rv);
+
   mozStorageStatementScoper scoper(statement);
 
   rv = statement->BindStringByName(NS_LITERAL_CSTRING("content"), aValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -351,17 +381,17 @@ NS_IMETHODIMP
 nsAnnotationService::SetPageAnnotationString(nsIURI* aURI,
                                              const nsACString& aName,
                                              const nsAString& aValue,
                                              int32_t aFlags,
                                              uint16_t aExpiration)
 {
   NS_ENSURE_ARG(aURI);
 
-  nsresult rv = SetAnnotationStringInternal(aURI, 0, aName, aValue,
+  nsresult rv = SetAnnotationStringInternal(aURI, 0, nullptr, aName, aValue,
                                             aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
 
   return NS_OK;
 }
 
@@ -375,40 +405,45 @@ nsAnnotationService::SetItemAnnotationSt
                                              uint16_t aSource,
                                              bool aDontUpdateLastModified)
 {
   NS_ENSURE_ARG_MIN(aItemId, 1);
 
   if (aExpiration == EXPIRE_WITH_HISTORY)
     return NS_ERROR_INVALID_ARG;
 
-  nsresult rv = SetAnnotationStringInternal(nullptr, aItemId, aName, aValue,
-                                            aFlags, aExpiration);
+  BookmarkData bookmark;
+  nsresult rv = SetAnnotationStringInternal(nullptr, aItemId, &bookmark, aName,
+                                            aValue, aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
+  NotifyItemChanged(bookmark, aName, aSource, aDontUpdateLastModified);
 
   return NS_OK;
 }
 
 
 nsresult
 nsAnnotationService::SetAnnotationInt32Internal(nsIURI* aURI,
                                                 int64_t aItemId,
+                                                BookmarkData* aBookmark,
                                                 const nsACString& aName,
                                                 int32_t aValue,
                                                 int32_t aFlags,
                                                 uint16_t aExpiration)
 {
   mozStorageTransaction transaction(mDB->MainConn(), false);
   nsCOMPtr<mozIStorageStatement> statement;
-  nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
+  nsresult rv = StartSetAnnotation(aURI, aItemId, aBookmark, aName, aFlags,
+                                   aExpiration,
                                    nsIAnnotationService::TYPE_INT32,
                                    statement);
   NS_ENSURE_SUCCESS(rv, rv);
+
   mozStorageStatementScoper scoper(statement);
 
   rv = statement->BindInt32ByName(NS_LITERAL_CSTRING("content"), aValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -423,17 +458,17 @@ NS_IMETHODIMP
 nsAnnotationService::SetPageAnnotationInt32(nsIURI* aURI,
                                             const nsACString& aName,
                                             int32_t aValue,
                                             int32_t aFlags,
                                             uint16_t aExpiration)
 {
   NS_ENSURE_ARG(aURI);
 
-  nsresult rv = SetAnnotationInt32Internal(aURI, 0, aName, aValue,
+  nsresult rv = SetAnnotationInt32Internal(aURI, 0, nullptr, aName, aValue,
                                            aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
 
   return NS_OK;
 }
 
@@ -447,40 +482,45 @@ nsAnnotationService::SetItemAnnotationIn
                                             uint16_t aSource,
                                             bool aDontUpdateLastModified)
 {
   NS_ENSURE_ARG_MIN(aItemId, 1);
 
   if (aExpiration == EXPIRE_WITH_HISTORY)
     return NS_ERROR_INVALID_ARG;
 
-  nsresult rv = SetAnnotationInt32Internal(nullptr, aItemId, aName, aValue,
-                                           aFlags, aExpiration);
+  BookmarkData bookmark;
+  nsresult rv = SetAnnotationInt32Internal(nullptr, aItemId, &bookmark, aName,
+                                           aValue, aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
+  NotifyItemChanged(bookmark, aName, aSource, aDontUpdateLastModified);
 
   return NS_OK;
 }
 
 
 nsresult
 nsAnnotationService::SetAnnotationInt64Internal(nsIURI* aURI,
                                                 int64_t aItemId,
+                                                BookmarkData* aBookmark,
                                                 const nsACString& aName,
                                                 int64_t aValue,
                                                 int32_t aFlags,
                                                 uint16_t aExpiration)
 {
   mozStorageTransaction transaction(mDB->MainConn(), false);
   nsCOMPtr<mozIStorageStatement> statement;
-  nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
+  nsresult rv = StartSetAnnotation(aURI, aItemId, aBookmark, aName, aFlags,
+                                   aExpiration,
                                    nsIAnnotationService::TYPE_INT64,
                                    statement);
   NS_ENSURE_SUCCESS(rv, rv);
+
   mozStorageStatementScoper scoper(statement);
 
   rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("content"), aValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -495,17 +535,17 @@ NS_IMETHODIMP
 nsAnnotationService::SetPageAnnotationInt64(nsIURI* aURI,
                                             const nsACString& aName,
                                             int64_t aValue,
                                             int32_t aFlags,
                                             uint16_t aExpiration)
 {
   NS_ENSURE_ARG(aURI);
 
-  nsresult rv = SetAnnotationInt64Internal(aURI, 0, aName, aValue,
+  nsresult rv = SetAnnotationInt64Internal(aURI, 0, nullptr, aName, aValue,
                                            aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
 
   return NS_OK;
 }
 
@@ -519,40 +559,45 @@ nsAnnotationService::SetItemAnnotationIn
                                             uint16_t aSource,
                                             bool aDontUpdateLastModified)
 {
   NS_ENSURE_ARG_MIN(aItemId, 1);
 
   if (aExpiration == EXPIRE_WITH_HISTORY)
     return NS_ERROR_INVALID_ARG;
 
-  nsresult rv = SetAnnotationInt64Internal(nullptr, aItemId, aName, aValue,
-                                           aFlags, aExpiration);
+  BookmarkData bookmark;
+  nsresult rv = SetAnnotationInt64Internal(nullptr, aItemId, &bookmark, aName,
+                                           aValue, aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
+  NotifyItemChanged(bookmark, aName, aSource, aDontUpdateLastModified);
 
   return NS_OK;
 }
 
 
 nsresult
 nsAnnotationService::SetAnnotationDoubleInternal(nsIURI* aURI,
                                                  int64_t aItemId,
+                                                 BookmarkData* aBookmark,
                                                  const nsACString& aName,
                                                  double aValue,
                                                  int32_t aFlags,
                                                  uint16_t aExpiration)
 {
   mozStorageTransaction transaction(mDB->MainConn(), false);
   nsCOMPtr<mozIStorageStatement> statement;
-  nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
+  nsresult rv = StartSetAnnotation(aURI, aItemId, aBookmark, aName, aFlags,
+                                   aExpiration,
                                    nsIAnnotationService::TYPE_DOUBLE,
                                    statement);
   NS_ENSURE_SUCCESS(rv, rv);
+
   mozStorageStatementScoper scoper(statement);
 
   rv = statement->BindDoubleByName(NS_LITERAL_CSTRING("content"), aValue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -567,17 +612,17 @@ NS_IMETHODIMP
 nsAnnotationService::SetPageAnnotationDouble(nsIURI* aURI,
                                              const nsACString& aName,
                                              double aValue,
                                              int32_t aFlags,
                                              uint16_t aExpiration)
 {
   NS_ENSURE_ARG(aURI);
 
-  nsresult rv = SetAnnotationDoubleInternal(aURI, 0, aName, aValue,
+  nsresult rv = SetAnnotationDoubleInternal(aURI, 0, nullptr, aName, aValue,
                                             aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
 
   return NS_OK;
 }
 
@@ -591,21 +636,23 @@ nsAnnotationService::SetItemAnnotationDo
                                              uint16_t aSource,
                                              bool aDontUpdateLastModified)
 {
   NS_ENSURE_ARG_MIN(aItemId, 1);
 
   if (aExpiration == EXPIRE_WITH_HISTORY)
     return NS_ERROR_INVALID_ARG;
 
-  nsresult rv = SetAnnotationDoubleInternal(nullptr, aItemId, aName, aValue,
-                                            aFlags, aExpiration);
+  BookmarkData bookmark;
+  nsresult rv = SetAnnotationDoubleInternal(nullptr, aItemId, &bookmark,
+                                            aName, aValue, aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
+  NotifyItemChanged(bookmark, aName, aSource, aDontUpdateLastModified);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAnnotationService::GetPageAnnotationString(nsIURI* aURI,
                                              const nsACString& aName,
                                              nsAString& _retval)
@@ -1362,16 +1409,17 @@ nsAnnotationService::ItemHasAnnotation(i
 /**
  * @note We don't remove anything from the moz_anno_attributes table. If we
  *       delete the last item of a given name, that item really should go away.
  *       It will be cleaned up by expiration.
  */
 nsresult
 nsAnnotationService::RemoveAnnotationInternal(nsIURI* aURI,
                                               int64_t aItemId,
+                                              BookmarkData* aBookmark,
                                               const nsACString& aName)
 {
   bool isItemAnnotation = (aItemId > 0);
   nsCOMPtr<mozIStorageStatement> statement;
   if (isItemAnnotation) {
     statement = mDB->GetStatement(
       "DELETE FROM moz_items_annos "
       "WHERE item_id = :item_id "
@@ -1399,46 +1447,58 @@ nsAnnotationService::RemoveAnnotationInt
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->BindUTF8StringByName(NS_LITERAL_CSTRING("anno_name"), aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  if (isItemAnnotation) {
+    nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+    if (bookmarks) {
+      MOZ_ASSERT(aBookmark);
+      if (NS_FAILED(bookmarks->FetchItemInfo(aItemId, *aBookmark))) {
+        aBookmark->id = -1;
+      }
+    }
+  }
+
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsAnnotationService::RemovePageAnnotation(nsIURI* aURI,
                                           const nsACString& aName)
 {
   NS_ENSURE_ARG(aURI);
 
-  nsresult rv = RemoveAnnotationInternal(aURI, 0, aName);
+  nsresult rv = RemoveAnnotationInternal(aURI, 0, nullptr, aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationRemoved(aURI, aName));
 
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsAnnotationService::RemoveItemAnnotation(int64_t aItemId,
                                           const nsACString& aName,
                                           uint16_t aSource)
 {
   NS_ENSURE_ARG_MIN(aItemId, 1);
 
-  nsresult rv = RemoveAnnotationInternal(nullptr, aItemId, aName);
+  BookmarkData bookmark;
+  nsresult rv = RemoveAnnotationInternal(nullptr, aItemId, &bookmark, aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, aName, aSource));
+  NotifyItemChanged(bookmark, aName, aSource, false);
 
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsAnnotationService::RemovePageAnnotations(nsIURI* aURI)
 {
@@ -1464,34 +1524,44 @@ nsAnnotationService::RemovePageAnnotatio
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsAnnotationService::RemoveItemAnnotations(int64_t aItemId,
                                            uint16_t aSource)
 {
+  nsresult rv = RemoveItemAnnotationsWithoutNotifying(aItemId);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, EmptyCString(),
+                                                 aSource));
+
+  return NS_OK;
+}
+
+
+nsresult
+nsAnnotationService::RemoveItemAnnotationsWithoutNotifying(int64_t aItemId)
+{
   NS_ENSURE_ARG_MIN(aItemId, 1);
 
   // Should this be precompiled or a getter?
   nsCOMPtr<mozIStorageStatement> statement = mDB->GetStatement(
     "DELETE FROM moz_items_annos WHERE item_id = :item_id"
   );
   NS_ENSURE_STATE(statement);
   mozStorageStatementScoper scoper(statement);
 
   nsresult rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, EmptyCString(),
-                                                 aSource));
-
   return NS_OK;
 }
 
 
 /**
  * @note If we use annotations for some standard items like GeckoFlags, it
  *       might be a good idea to blacklist these standard annotations from this
  *       copy function.
@@ -1644,16 +1714,24 @@ nsAnnotationService::CopyItemAnnotations
     NS_ENSURE_SUCCESS(rv, rv);
     rv = copyStmt->BindInt64ByName(NS_LITERAL_CSTRING("date"), PR_Now());
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = copyStmt->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
 
     NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aDestItemId, annoName, aSource, false));
+
+    nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+    if (bookmarks) {
+      BookmarkData bookmark;
+      if (NS_SUCCEEDED(bookmarks->FetchItemInfo(aDestItemId, bookmark))) {
+        NotifyItemChanged(bookmark, annoName, aSource, false);
+      }
+    }
   }
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
@@ -1833,16 +1911,17 @@ nsAnnotationService::StartGetAnnotation(
  *
  * @note The aStatement RESULT IS NOT ADDREFED.  This is just one of the class
  *       vars, which control its scope.  DO NOT RELEASE.
  *       The caller must take care of resetting the statement if this succeeds.
  */
 nsresult
 nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
                                         int64_t aItemId,
+                                        BookmarkData* aBookmark,
                                         const nsACString& aName,
                                         int32_t aFlags,
                                         uint16_t aExpiration,
                                         uint16_t aType,
                                         nsCOMPtr<mozIStorageStatement>& aStatement)
 {
   bool isItemAnnotation = (aItemId > 0);
 
@@ -1870,18 +1949,19 @@ nsAnnotationService::StartSetAnnotation(
   // - whether the annotation already exists.
   // - the nameID associated with the annotation name.
   // - the id and dateAdded of the old annotation, if it exists.
   nsCOMPtr<mozIStorageStatement> stmt;
   if (isItemAnnotation) {
     stmt = mDB->GetStatement(
       "SELECT b.id, "
              "(SELECT id FROM moz_anno_attributes WHERE name = :anno_name) AS nameid, "
-             "a.id, a.dateAdded "
+             "a.id, a.dateAdded, b.parent, b.type, b.lastModified, b.guid, p.guid "
       "FROM moz_bookmarks b "
+      "JOIN moz_bookmarks p ON p.id = b.parent "
       "LEFT JOIN moz_items_annos a ON a.item_id = b.id "
                                  "AND a.anno_attribute_id = nameid "
       "WHERE b.id = :item_id"
     );
   }
   else {
     stmt = mDB->GetStatement(
       "SELECT h.id, "
@@ -1921,16 +2001,29 @@ nsAnnotationService::StartSetAnnotation(
   if (isItemAnnotation) {
     aStatement = mDB->GetStatement(
       "INSERT OR REPLACE INTO moz_items_annos "
         "(id, item_id, anno_attribute_id, content, flags, "
          "expiration, type, dateAdded, lastModified) "
       "VALUES (:id, :fk, :name_id, :content, :flags, "
       ":expiration, :type, :date_added, :last_modified)"
     );
+
+    // Since we're already querying `moz_bookmarks`, we fetch the changed
+    // bookmark's info here, instead of using `FetchItemInfo`.
+    MOZ_ASSERT(aBookmark);
+    aBookmark->id = fkId;
+    aBookmark->parentId = stmt->AsInt64(4);
+    aBookmark->type = stmt->AsInt64(5);
+
+    aBookmark->lastModified = static_cast<PRTime>(stmt->AsInt64(6));
+    if (NS_FAILED(stmt->GetUTF8String(7, aBookmark->guid)) ||
+        NS_FAILED(stmt->GetUTF8String(8, aBookmark->parentGuid))) {
+      aBookmark->id = -1;
+    }
   }
   else {
     aStatement = mDB->GetStatement(
       "INSERT OR REPLACE INTO moz_annos "
         "(id, place_id, anno_attribute_id, content, flags, "
          "expiration, type, dateAdded, lastModified) "
       "VALUES (:id, :fk, :name_id, :content, :flags, "
       ":expiration, :type, :date_added, :last_modified)"
--- a/toolkit/components/places/nsAnnotationService.h
+++ b/toolkit/components/places/nsAnnotationService.h
@@ -108,54 +108,61 @@ protected:
 
   nsresult StartGetAnnotation(nsIURI* aURI,
                               int64_t aItemId,
                               const nsACString& aName,
                               nsCOMPtr<mozIStorageStatement>& aStatement);
 
   nsresult StartSetAnnotation(nsIURI* aURI,
                               int64_t aItemId,
+                              BookmarkData* aBookmark,
                               const nsACString& aName,
                               int32_t aFlags,
                               uint16_t aExpiration,
                               uint16_t aType,
                               nsCOMPtr<mozIStorageStatement>& aStatement);
 
   nsresult SetAnnotationStringInternal(nsIURI* aURI,
                                        int64_t aItemId,
+                                       BookmarkData* aBookmark,
                                        const nsACString& aName,
                                        const nsAString& aValue,
                                        int32_t aFlags,
                                        uint16_t aExpiration);
   nsresult SetAnnotationInt32Internal(nsIURI* aURI,
                                       int64_t aItemId,
+                                      BookmarkData* aBookmark,
                                       const nsACString& aName,
                                       int32_t aValue,
                                       int32_t aFlags,
                                       uint16_t aExpiration);
   nsresult SetAnnotationInt64Internal(nsIURI* aURI,
                                       int64_t aItemId,
+                                      BookmarkData* aBookmark,
                                       const nsACString& aName,
                                       int64_t aValue,
                                       int32_t aFlags,
                                       uint16_t aExpiration);
   nsresult SetAnnotationDoubleInternal(nsIURI* aURI,
                                        int64_t aItemId,
+                                       BookmarkData* aBookmark,
                                        const nsACString& aName,
                                        double aValue,
                                        int32_t aFlags,
                                        uint16_t aExpiration);
 
   nsresult RemoveAnnotationInternal(nsIURI* aURI,
                                     int64_t aItemId,
+                                    BookmarkData* aBookmark,
                                     const nsACString& aName);
 
 public:
   nsresult GetPagesWithAnnotationCOMArray(const nsACString& aName,
                                           nsCOMArray<nsIURI>* _results);
   nsresult GetItemsWithAnnotationTArray(const nsACString& aName,
                                         nsTArray<int64_t>* _result);
   nsresult GetAnnotationNamesTArray(nsIURI* aURI,
                                     int64_t aItemId,
                                     nsTArray<nsCString>* _result);
+  nsresult RemoveItemAnnotationsWithoutNotifying(int64_t aItemId);
 };
 
 #endif /* nsAnnotationService_h___ */
--- a/toolkit/components/places/nsIAnnotationService.idl
+++ b/toolkit/components/places/nsIAnnotationService.idl
@@ -107,17 +107,18 @@ interface nsIAnnotationService : nsISupp
      * If there is an important annotation that the user or extension wants to
      * keep, you should add a bookmark for the page and use an EXPIRE_NEVER
      * annotation.  This will ensure the annotation exists until the item is
      * removed by the user.
      * See EXPIRE_* constants above for further information.
      *
      * For item annotations, aSource should be a change source constant from
      * nsINavBookmarksService::SOURCE_*, and defaults to SOURCE_DEFAULT if
-     * omitted.
+     * omitted. Setting an item annotation also notifies
+     * `nsINavBookmarkObserver::onItemChanged` for the affected item.
      *
      * The annotation "favicon" is special. Favicons are stored in the favicon
      * service, but are special cased in the protocol handler so they look like
      * annotations. Do not set favicons using this service, it will not work.
      *
      * Only C++ consumers may use the type-specific methods.
      *
      * @throws NS_ERROR_ILLEGAL_VALUE if the page or the bookmark doesn't exist.
@@ -335,27 +336,33 @@ interface nsIAnnotationService : nsISupp
     boolean pageHasAnnotation(in nsIURI aURI,
                               in AUTF8String aName);
     boolean itemHasAnnotation(in long long aItemId,
                               in AUTF8String aName);
 
     /**
      * Removes a specific annotation. Succeeds even if the annotation is
      * not found.
+     *
+     * Removing an item annotation also notifies
+     * `nsINavBookmarkObserver::onItemChanged` for the affected item.
      */
     void removePageAnnotation(in nsIURI aURI,
                               in AUTF8String aName);
     void removeItemAnnotation(in long long aItemId,
                               in AUTF8String aName,
                               [optional] in unsigned short aSource);
 
     /**
      * Removes all annotations for the given page/item.
      * We may want some other similar functions to get annotations with given
      * flags (once we have flags defined).
+     *
+     * Unlike the other item methods, `removeItemAnnotations` does *not* notify
+     * `nsINavBookmarkObserver::onItemChanged` for the affected item.
      */
     void removePageAnnotations(in nsIURI aURI);
     void removeItemAnnotations(in long long aItemId,
                                [optional] in unsigned short aSource);
 
     /**
      * Copies all annotations from the source to the destination URI/item. If
      * the destination already has an annotation with the same name as one on
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -179,17 +179,16 @@ nsNavBookmarks::~nsNavBookmarks()
   if (gBookmarksService == this)
     gBookmarksService = nullptr;
 }
 
 
 NS_IMPL_ISUPPORTS(nsNavBookmarks
 , nsINavBookmarksService
 , nsINavHistoryObserver
-, nsIAnnotationObserver
 , nsIObserver
 , nsISupportsWeakReference
 )
 
 
 Atomic<int64_t> nsNavBookmarks::sLastInsertedItemId(0);
 
 
@@ -204,27 +203,21 @@ nsNavBookmarks::StoreLastInsertedId(cons
 nsresult
 nsNavBookmarks::Init()
 {
   mDB = Database::GetDatabase();
   NS_ENSURE_STATE(mDB);
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
-    (void)os->AddObserver(this, TOPIC_PLACES_SHUTDOWN, true);
     (void)os->AddObserver(this, TOPIC_PLACES_CONNECTION_CLOSED, true);
   }
 
   mCanNotify = true;
 
-  // Observe annotations.
-  nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
-  NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY);
-  annosvc->AddObserver(this);
-
   // Allows us to notify on title changes. MUST BE LAST so it is impossible
   // to fail after this call, or the history service will have a reference to
   // us and we won't go away.
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ENSURE_STATE(history);
   history->AddObserver(this, true);
 
   // DO NOT PUT STUFF HERE that can fail. See observer comment above.
@@ -682,23 +675,25 @@ nsNavBookmarks::RemoveItem(int64_t aItem
   NS_ENSURE_ARG(!IsRoot(aItemId));
 
   BookmarkData bookmark;
   nsresult rv = FetchItemInfo(aItemId, bookmark);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mozStorageTransaction transaction(mDB->MainConn(), false);
 
-  // First, if not a tag, remove item annotations.
+  // First, if not a tag, remove item annotations. We remove annos without
+  // notifying to avoid firing `onItemAnnotationRemoved` for an item that
+  // we're about to remove.
   int64_t tagsRootId = TagsRootId();
   bool isUntagging = bookmark.grandParentId == tagsRootId;
   if (bookmark.parentId != tagsRootId && !isUntagging) {
     nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
     NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY);
-    rv = annosvc->RemoveItemAnnotations(bookmark.id, aSource);
+    rv = annosvc->RemoveItemAnnotationsWithoutNotifying(bookmark.id);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   if (bookmark.type == TYPE_FOLDER) {
     // Remove all of the folder's children.
     rv = RemoveFolderChildren(bookmark.id, aSource);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -3087,52 +3082,50 @@ nsNavBookmarks::NotifyItemVisited(const 
   }
 }
 
 void
 nsNavBookmarks::NotifyItemChanged(const ItemChangeData& aData)
 {
   // A guid must always be defined.
   MOZ_ASSERT(!aData.bookmark.guid.IsEmpty());
+
+  PRTime lastModified = aData.bookmark.lastModified;
+  if (aData.updateLastModified) {
+    lastModified = RoundedPRNow();
+    MOZ_ALWAYS_SUCCEEDS(SetItemDateInternal(
+      LAST_MODIFIED, DetermineSyncChangeDelta(aData.source),
+      aData.bookmark.id, lastModified));
+  }
+
   NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                    nsINavBookmarkObserver,
                    OnItemChanged(aData.bookmark.id,
                                  aData.property,
                                  aData.isAnnotation,
                                  aData.newValue,
-                                 aData.bookmark.lastModified,
+                                 lastModified,
                                  aData.bookmark.type,
                                  aData.bookmark.parentId,
                                  aData.bookmark.guid,
                                  aData.bookmark.parentGuid,
                                  aData.oldValue,
-                                 // We specify the default source here because
-                                 // this method is only called for history
-                                 // visits, and we don't track sources in
-                                 // history.
-                                 SOURCE_DEFAULT));
+                                 aData.source));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsIObserver
 
 NS_IMETHODIMP
 nsNavBookmarks::Observe(nsISupports *aSubject, const char *aTopic,
                         const char16_t *aData)
 {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
 
-  if (strcmp(aTopic, TOPIC_PLACES_SHUTDOWN) == 0) {
-    // Stop Observing annotations.
-    nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
-    if (annosvc) {
-      annosvc->RemoveObserver(this);
-    }
-  }
-  else if (strcmp(aTopic, TOPIC_PLACES_CONNECTION_CLOSED) == 0) {
+  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();
   }
 
   return NS_OK;
 }
@@ -3301,68 +3294,8 @@ nsNavBookmarks::OnDeleteVisits(nsIURI* a
     changeData.bookmark.type = TYPE_BOOKMARK;
 
     RefPtr< AsyncGetBookmarksForURI<ItemChangeMethod, ItemChangeData> > notifier =
       new AsyncGetBookmarksForURI<ItemChangeMethod, ItemChangeData>(this, &nsNavBookmarks::NotifyItemChanged, changeData);
     notifier->Init();
   }
   return NS_OK;
 }
-
-
-// nsIAnnotationObserver
-
-NS_IMETHODIMP
-nsNavBookmarks::OnPageAnnotationSet(nsIURI* aPage, const nsACString& aName)
-{
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavBookmarks::OnItemAnnotationSet(int64_t aItemId, const nsACString& aName,
-                                    uint16_t aSource, bool aDontUpdateLastModified)
-{
-  BookmarkData bookmark;
-  nsresult rv = FetchItemInfo(aItemId, bookmark);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (!aDontUpdateLastModified) {
-    bookmark.lastModified = RoundedPRNow();
-    rv = SetItemDateInternal(LAST_MODIFIED, DetermineSyncChangeDelta(aSource),
-                             bookmark.id, bookmark.lastModified);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
-  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
-                   nsINavBookmarkObserver,
-                   OnItemChanged(bookmark.id,
-                                 aName,
-                                 true,
-                                 EmptyCString(),
-                                 bookmark.lastModified,
-                                 bookmark.type,
-                                 bookmark.parentId,
-                                 bookmark.guid,
-                                 bookmark.parentGuid,
-                                 EmptyCString(),
-                                 aSource));
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavBookmarks::OnPageAnnotationRemoved(nsIURI* aPage, const nsACString& aName)
-{
-  return NS_OK;
-}
-
-
-NS_IMETHODIMP
-nsNavBookmarks::OnItemAnnotationRemoved(int64_t aItemId, const nsACString& aName,
-                                        uint16_t aSource)
-{
-  // As of now this is doing the same as OnItemAnnotationSet, so just forward
-  // the call.
-  nsresult rv = OnItemAnnotationSet(aItemId, aName, aSource, false);
-  NS_ENSURE_SUCCESS(rv, rv);
-  return NS_OK;
-}
--- a/toolkit/components/places/nsNavBookmarks.h
+++ b/toolkit/components/places/nsNavBookmarks.h
@@ -2,17 +2,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsNavBookmarks_h_
 #define nsNavBookmarks_h_
 
 #include "nsINavBookmarksService.h"
-#include "nsIAnnotationService.h"
 #include "nsITransaction.h"
 #include "nsNavHistory.h"
 #include "nsToolkitCompsCID.h"
 #include "nsCategoryCache.h"
 #include "nsTHashtable.h"
 #include "nsWeakReference.h"
 #include "mozilla/Attributes.h"
 #include "prtime.h"
@@ -23,43 +22,45 @@ namespace mozilla {
 namespace places {
 
   enum BookmarkStatementId {
     DB_FIND_REDIRECTED_BOOKMARK = 0
   , DB_GET_BOOKMARKS_FOR_URI
   };
 
   struct BookmarkData {
-    int64_t id;
+    int64_t id = -1;
     nsCString url;
     nsCString title;
-    int32_t position;
-    int64_t placeId;
-    int64_t parentId;
-    int64_t grandParentId;
-    int32_t type;
-    int32_t syncStatus;
+    int32_t position = -1;
+    int64_t placeId = -1;
+    int64_t parentId = -1;
+    int64_t grandParentId = -1;
+    int32_t type = 0;
+    int32_t syncStatus = nsINavBookmarksService::SYNC_STATUS_UNKNOWN;
     nsCString serviceCID;
-    PRTime dateAdded;
-    PRTime lastModified;
+    PRTime dateAdded = 0;
+    PRTime lastModified = 0;
     nsCString guid;
     nsCString parentGuid;
   };
 
   struct ItemVisitData {
     BookmarkData bookmark;
     int64_t visitId;
     uint32_t transitionType;
     PRTime time;
   };
 
   struct ItemChangeData {
     BookmarkData bookmark;
+    bool isAnnotation = false;
+    bool updateLastModified = false;
+    uint16_t source = nsINavBookmarksService::SOURCE_DEFAULT;
     nsCString property;
-    bool isAnnotation;
     nsCString newValue;
     nsCString oldValue;
   };
 
   struct TombstoneData {
     nsCString guid;
     PRTime dateRemoved;
   };
@@ -72,25 +73,23 @@ namespace places {
   , LAST_MODIFIED
   };
 
 } // namespace places
 } // namespace mozilla
 
 class nsNavBookmarks final : public nsINavBookmarksService
                            , public nsINavHistoryObserver
-                           , public nsIAnnotationObserver
                            , public nsIObserver
                            , public nsSupportsWeakReference
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSINAVBOOKMARKSSERVICE
   NS_DECL_NSINAVHISTORYOBSERVER
-  NS_DECL_NSIANNOTATIONOBSERVER
   NS_DECL_NSIOBSERVER
 
   nsNavBookmarks();
 
   /**
    * Obtains the service's object.
    */
   static already_AddRefed<nsNavBookmarks> GetSingleton();
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
@@ -226,18 +226,16 @@ add_task(async function remove_bookmark(
   let bm = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                 parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                 url: new URL("http://remove.example.com/") });
   let itemId = await PlacesUtils.promiseItemId(bm.guid);
   let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
 
   let observer = expectNotifications();
   await PlacesUtils.bookmarks.remove(bm.guid);
-  // TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no
-  // annotations.
   observer.check([ { name: "onItemRemoved",
                      arguments: [ itemId, parentId, bm.index, bm.type, bm.url,
                                   bm.guid, bm.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
                  ]);
 });
 
 add_task(async function remove_multiple_bookmarks() {
@@ -249,18 +247,16 @@ add_task(async function remove_multiple_
                                                  url: "http://remove1.example.com/" });
   let itemId1 = await PlacesUtils.promiseItemId(bm1.guid);
   let parentId1 = await PlacesUtils.promiseItemId(bm1.parentGuid);
   let itemId2 = await PlacesUtils.promiseItemId(bm2.guid);
   let parentId2 = await PlacesUtils.promiseItemId(bm2.parentGuid);
 
   let observer = expectNotifications();
   await PlacesUtils.bookmarks.remove([bm1, bm2]);
-  // TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no
-  // annotations.
   observer.check([ { name: "onItemRemoved",
                      arguments: [ itemId1, parentId1, bm1.index, bm1.type, bm1.url,
                                   bm1.guid, bm1.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] },
                    { name: "onItemRemoved",
                      arguments: [ itemId2, parentId2, bm2.index, bm2.type, bm2.url,
                                   bm2.guid, bm2.parentGuid,
                                   Ci.nsINavBookmarksService.SOURCE_DEFAULT ] }
--- a/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
+++ b/toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
@@ -206,17 +206,17 @@ add_task(async function onItemAdded_fold
 add_task(async function onItemChanged_title_bookmark() {
   let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
   const TITLE = "New title";
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
       "onItemChanged"
     ]),
     gBookmarksObserver.setup([
-      { name: "onItemChanged", // This is an unfortunate effect of bug 653910.
+      { name: "onItemChanged",
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
           { name: "property", check: v => v === "title" },
           { name: "isAnno", check: v => v === false },
           { name: "newValue", check: v => v === TITLE },
           { name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK },
           { name: "parentId", check: v => v === PlacesUtils.unfiledBookmarksFolderId },
@@ -388,33 +388,19 @@ add_task(async function onItemMoved_book
 
 add_task(async function onItemRemoved_bookmark() {
   let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
   let guid = await PlacesUtils.promiseItemGuid(id);
   let url = (await PlacesUtils.bookmarks.fetch(guid)).url;
   let uri = Services.io.newURI(url);
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
-      "onItemChanged", "onItemRemoved"
+      "onItemRemoved"
     ]),
     gBookmarksObserver.setup([
-      { name: "onItemChanged", // This is an unfortunate effect of bug 653910.
-        args: [
-          { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "property", check: v => v === "" },
-          { name: "isAnno", check: v => v === true },
-          { name: "newValue", check: v => v === "" },
-          { name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
-          { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK },
-          { name: "parentId", check: v => v === PlacesUtils.unfiledBookmarksFolderId },
-          { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "oldValue", check: v => typeof(v) == "string" },
-          { name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
-        ] },
       { name: "onItemRemoved",
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
           { name: "parentId", check: v => v === PlacesUtils.unfiledBookmarksFolderId },
           { name: "index", check: v => v === 0 },
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK },
           { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
           { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
@@ -425,33 +411,19 @@ add_task(async function onItemRemoved_bo
   PlacesUtils.bookmarks.removeItem(id);
   await promise;
 });
 
 add_task(async function onItemRemoved_separator() {
   let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
-      "onItemChanged", "onItemRemoved"
+      "onItemRemoved"
     ]),
     gBookmarksObserver.setup([
-      { name: "onItemChanged", // This is an unfortunate effect of bug 653910.
-        args: [
-          { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "property", check: v => v === "" },
-          { name: "isAnno", check: v => v === true },
-          { name: "newValue", check: v => v === "" },
-          { name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
-          { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_SEPARATOR },
-          { name: "parentId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "oldValue", check: v => typeof(v) == "string" },
-          { name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
-        ] },
       { name: "onItemRemoved",
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
           { name: "parentId", check: v => typeof(v) == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_SEPARATOR },
           { name: "uri", check: v => v === null },
           { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
@@ -462,33 +434,19 @@ add_task(async function onItemRemoved_se
   PlacesUtils.bookmarks.removeItem(id);
   await promise;
 });
 
 add_task(async function onItemRemoved_folder() {
   let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
-      "onItemChanged", "onItemRemoved"
+      "onItemRemoved"
     ]),
     gBookmarksObserver.setup([
-      { name: "onItemChanged", // This is an unfortunate effect of bug 653910.
-        args: [
-          { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "property", check: v => v === "" },
-          { name: "isAnno", check: v => v === true },
-          { name: "newValue", check: v => v === "" },
-          { name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
-          { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER },
-          { name: "parentId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "oldValue", check: v => typeof(v) == "string" },
-          { name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
-        ] },
       { name: "onItemRemoved",
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
           { name: "parentId", check: v => typeof(v) == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER },
           { name: "uri", check: v => v === null },
           { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
@@ -502,17 +460,17 @@ add_task(async function onItemRemoved_fo
 
 add_task(async function onItemRemoved_folder_recursive() {
   const TITLE = "Folder 3";
   const BMTITLE = "Bookmark 1";
   let uri = NetUtil.newURI("http://1.mozilla.org/");
   let promise = Promise.all([
     gBookmarkSkipObserver.setup([
       "onItemAdded", "onItemAdded", "onItemAdded", "onItemAdded",
-      "onItemChanged", "onItemRemoved"
+      "onItemRemoved"
     ]),
     gBookmarksObserver.setup([
       { name: "onItemAdded",
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
           { name: "parentId", check: v => v === PlacesUtils.unfiledBookmarksFolderId },
           { name: "index", check: v => v === 0 },
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER },
@@ -557,30 +515,16 @@ add_task(async function onItemRemoved_fo
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK },
           { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
           { name: "title", check: v => v === BMTITLE },
           { name: "dateAdded", check: v => typeof(v) == "number" && v > 0 },
           { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
           { name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
           { name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
         ] },
-      { name: "onItemChanged", // This is an unfortunate effect of bug 653910.
-        args: [
-          { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "property", check: v => v === "" },
-          { name: "isAnno", check: v => v === true },
-          { name: "newValue", check: v => v === "" },
-          { name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
-          { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER },
-          { name: "parentId", check: v => typeof(v) == "number" && v > 0 },
-          { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
-          { name: "oldValue", check: v => typeof(v) == "string" },
-          { name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
-        ] },
       { name: "onItemRemoved",
         args: [
           { name: "itemId", check: v => typeof(v) == "number" && v > 0 },
           { name: "parentId", check: v => typeof(v) == "number" && v > 0 },
           { name: "index", check: v => v === 0 },
           { name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK },
           { name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
           { name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
--- a/toolkit/components/places/tests/unit/test_null_interfaces.js
+++ b/toolkit/components/places/tests/unit/test_null_interfaces.js
@@ -9,19 +9,18 @@
 // Make an array of services to test, each specifying a class id, interface
 // and an array of function names that don't throw when passed nulls
 var testServices = [
   ["browser/nav-history-service;1",
     ["nsINavHistoryService"],
     ["queryStringToQueries", "removePagesByTimeframe", "removePagesFromHost", "getObservers"]
   ],
   ["browser/nav-bookmarks-service;1",
-    ["nsINavBookmarksService", "nsINavHistoryObserver", "nsIAnnotationObserver"],
-    ["createFolder", "getObservers", "onFrecencyChanged", "onTitleChanged",
-     "onPageAnnotationSet", "onPageAnnotationRemoved", "onDeleteURI"]
+    ["nsINavBookmarksService", "nsINavHistoryObserver"],
+    ["createFolder", "getObservers", "onFrecencyChanged", "onTitleChanged", "onDeleteURI"]
   ],
   ["browser/livemark-service;2", ["mozIAsyncLivemarks"], ["reloadLivemarks"]],
   ["browser/annotation-service;1", ["nsIAnnotationService"], ["getObservers"]],
   ["browser/favicon-service;1", ["nsIFaviconService"], []],
   ["browser/tagging-service;1", ["nsITaggingService"], []],
 ];
 do_print(testServices.join("\n"));