Bug 1338280 - Clear the GUIDs cache and don't clear `mCanNotify` on Places restart. draft
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 22 Feb 2017 14:34:17 -0800
changeset 488408 b8c700b5254dabdbaf915c54ea71e9d44bfd595e
parent 488407 03e6a3f5f3782719de19bb103e1d722073050e41
child 489925 cfa091fb6c4d13a46d5c2ec6d2aab5b3690a2a6f
push id46512
push userbmo:kit@mozilla.com
push dateThu, 23 Feb 2017 02:18:17 +0000
bugs1338280
milestone54.0a1
Bug 1338280 - Clear the GUIDs cache and don't clear `mCanNotify` on Places restart. If we're shutting down and replacing the database, the cache might not be valid on the next restart. Also, if we expect to restart the database, we shouldn't remove the bookmark annotation observer or disable observer notifications. This is far from complete, but enough to make the Sync bookmark repair integration test work. We'll also need to re-initialize the transaction manager and the keywords cache in `PlacesUtils`, which this patch doesn't handle. MozReview-Commit-ID: DotcIGVc1yi
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavHistory.cpp
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2544,18 +2544,23 @@ var GuidHelper = {
   },
 
   invalidateCacheForItemId(aItemId) {
     let guid = this.guidsForIds.get(aItemId);
     this.guidsForIds.delete(aItemId);
     this.idsForGuids.delete(guid);
   },
 
+  invalidateCache() {
+    this.guidsForIds.clear();
+    this.idsForGuids.clear();
+  },
+
   ensureObservingRemovedItems() {
-    if (!("observer" in this)) {
+    if (!this.observer) {
       /**
        * This observers serves two purposes:
        * (1) Invalidate cached id<->GUID paris on when items are removed.
        * (2) Cache GUIDs given us free of charge by onItemAdded/onItemRemoved.
       *      So, for exmaple, when the NewBookmark needs the new GUID, we already
       *      have it cached.
       */
       this.observer = {
@@ -2577,16 +2582,21 @@ var GuidHelper = {
         onEndUpdateBatch() {},
         onItemChanged() {},
         onItemVisited() {},
         onItemMoved() {},
       };
       PlacesUtils.bookmarks.addObserver(this.observer, false);
       PlacesUtils.registerShutdownFunction(() => {
         PlacesUtils.bookmarks.removeObserver(this.observer);
+        if (PlacesUtils._isDBTestingEnabled()) {
+          // Invalidate the entire cache if we expect to restart the database.
+          this.invalidateCache();
+          this.observer = null;
+        }
       });
     }
   }
 };
 
 // Transactions handlers.
 
 /**
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -3360,27 +3360,30 @@ nsNavBookmarks::NotifyItemChanged(const 
 //// 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.
+  ReadTestingEnabled();
+  if (strcmp(aTopic, TOPIC_PLACES_SHUTDOWN) == 0 && !mTestingEnabled) {
+    // Stop Observing annotations if we don't expect to restart the database.
     nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
     if (annosvc) {
       annosvc->RemoveObserver(this);
     }
   }
   else 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;
+    if (!mTestingEnabled) {
+      mCanNotify = false;
+    }
     mObservers.Clear();
   }
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsINavHistoryObserver
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -3084,17 +3084,20 @@ nsNavHistory::Observe(nsISupports *aSubj
     RefPtr<Database> DB = GetDatabase();
     NS_ENSURE_STATE(DB);
     DB->Observe(aSubject, aTopic, aData);
   }
 
   else 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;
+    ReadTestingEnabled();
+    if (!mTestingEnabled) {
+      mCanNotify = false;
+    }
     mObservers.Clear();
   }
 
 #ifdef MOZ_XUL
   else if (strcmp(aTopic, TOPIC_AUTOCOMPLETE_FEEDBACK_INCOMING) == 0) {
     nsCOMPtr<nsIAutoCompleteInput> input = do_QueryInterface(aSubject);
     if (!input)
       return NS_OK;