Bug 1417101 - Remove the annotation observer from the bookmarks service. r=mak
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 16 Nov 2017 16:49:03 -0800
changeset 394901 dd84a4ad910edc4f587dc08c722bb54c4a3d1a3c
parent 394861 8ed94b47791c36059fa4af120ec6343465dd088b
child 394902 cc8eaa26f2ed97c7a481f5a8a7e325f5d7794fe7
push id33025
push usershindli@mozilla.com
push dateTue, 05 Dec 2017 09:57:50 +0000
treeherdermozilla-central@390c1aad9d4d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1417101
milestone59.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 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"));