Bug 1423896 - Make the new All Bookmarks folder query only update on the mobile folder status change for better performance. r?mak draft
authorMark Banner <standard8@mozilla.com>
Mon, 05 Feb 2018 17:19:26 +0000
changeset 752055 1bf7c9fcee805a52837fdf6a559c664b23b527fd
parent 751996 1c8a09475b3b88b3a1ef2b67be79217e03f62e17
push id98153
push userbmo:standard8@mozilla.com
push dateWed, 07 Feb 2018 14:05:59 +0000
reviewersmak
bugs1423896
milestone60.0a1
Bug 1423896 - Make the new All Bookmarks folder query only update on the mobile folder status change for better performance. r?mak MozReview-Commit-ID: 4sIR6d0ZabJ
browser/components/places/content/treeView.js
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistory.h
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/places/nsNavHistoryResult.h
toolkit/components/places/tests/queries/test_results-as-roots.js
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -187,23 +187,20 @@ PlacesTreeView.prototype = {
     }
 
     // Ensure that the entire chain is open, otherwise that node is invisible.
     for (let ancestor of ancestors) {
       if (!ancestor.containerOpen)
         throw new Error("Invisible node passed to _getRowForNode");
     }
 
-    // Non-plain containers, and non-Roots queries are initially built with their
-    // contents.
+    // Non-plain containers are initially built with their contents.
     let parent = aNode.parent;
     let parentIsPlain = this._isPlainContainer(parent);
-    if (!parentIsPlain &&
-        parent.queryOptions.resultType !=
-        Ci.nsINavHistoryQueryOptions.RESULTS_AS_ROOTS_QUERY) {
+    if (!parentIsPlain) {
       if (parent == this._rootNode) {
         return this._rows.indexOf(aNode);
       }
 
       return this._rows.indexOf(aNode, aParentRow);
     }
 
     let row = -1;
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -100,20 +100,16 @@ using namespace mozilla::places;
 #define PREF_FREC_DEFAULT_VISIT_BONUS_DEF       0
 #define PREF_FREC_UNVISITED_BOOKMARK_BONUS      "places.frecency.unvisitedBookmarkBonus"
 #define PREF_FREC_UNVISITED_BOOKMARK_BONUS_DEF  140
 #define PREF_FREC_UNVISITED_TYPED_BONUS         "places.frecency.unvisitedTypedBonus"
 #define PREF_FREC_UNVISITED_TYPED_BONUS_DEF     200
 #define PREF_FREC_RELOAD_VISIT_BONUS            "places.frecency.reloadVisitBonus"
 #define PREF_FREC_RELOAD_VISIT_BONUS_DEF        0
 
-// This is a hidden pref to determine when to show the mobile bookmarks folder.
-// Note: the name here matches those used elsewhere in the code, for easier searching.
-#define MOBILE_BOOKMARKS_PREF "browser.bookmarks.showMobileBookmarks"
-
 // This is a 'hidden' pref for the purposes of unit tests.
 #define PREF_FREC_DECAY_RATE     "places.frecency.decayRate"
 #define PREF_FREC_DECAY_RATE_DEF 0.975f
 
 // In order to avoid calling PR_now() too often we use a cached "now" value
 // for repeating stuff.  These are milliseconds between "now" cache refreshes.
 #define RENEW_CACHED_NOW_TIMEOUT ((int32_t)3 * PR_MSEC_PER_SEC)
 
@@ -831,20 +827,22 @@ nsNavHistory::GetUpdateRequirements(cons
         query->Uri() != nullptr)
       nonTimeBasedItems = true;
 
     if (!query->Domain().IsVoid())
       domainBasedItems = true;
   }
 
   if (aOptions->ResultType() ==
-        nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY ||
-      aOptions->ResultType() ==
+        nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY)
+      return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS;
+
+  if (aOptions->ResultType() ==
         nsINavHistoryQueryOptions::RESULTS_AS_ROOTS_QUERY)
-    return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS;
+      return QUERYUPDATE_MOBILEPREF;
 
   // Whenever there is a maximum number of results,
   // and we are not a bookmark query we must requery. This
   // is because we can't generally know if any given addition/change causes
   // the item to be in the top N items in the database.
   if (aOptions->MaxResults() > 0)
     return QUERYUPDATE_COMPLEX;
 
@@ -1953,17 +1951,17 @@ PlacesSQLQueryBuilder::SelectAsRoots()
   nsAutoCString mobileString;
 
   if (Preferences::GetBool(MOBILE_BOOKMARKS_PREF, false)) {
     nsAutoCString mobileTitle;
     history->GetStringFromName("MobileBookmarksFolderTitle", mobileTitle);
 
     mobileString = nsPrintfCString(","
       "(null, 'place:folder=MOBILE_BOOKMARKS', '%s', null, null, null, "
-       "null, null, 0, 0, null, null, null, null, 'mobile____v', null) ",
+       "null, null, 0, 0, null, null, null, null, '" MOBILE_BOOKMARKS_VIRTUAL_GUID "', null) ",
       mobileTitle.get());
   }
 
   mQueryString = nsPrintfCString(
     "SELECT * FROM ("
         "VALUES(null, 'place:folder=TOOLBAR', '%s', null, null, null, "
                "null, null, 0, 0, null, null, null, null, 'toolbar____v', null), "
               "(null, 'place:folder=BOOKMARKS_MENU', '%s', null, null, null, "
--- a/toolkit/components/places/nsNavHistory.h
+++ b/toolkit/components/places/nsNavHistory.h
@@ -34,16 +34,17 @@
 #include <wincrypt.h>
 #endif
 
 #define QUERYUPDATE_TIME 0
 #define QUERYUPDATE_SIMPLE 1
 #define QUERYUPDATE_COMPLEX 2
 #define QUERYUPDATE_COMPLEX_WITH_BOOKMARKS 3
 #define QUERYUPDATE_HOST 4
+#define QUERYUPDATE_MOBILEPREF 5
 
 // Clamp title and URL to generously large, but not too large, length.
 // See bug 319004 for details.
 #define URI_LENGTH_MAX 65536
 #define TITLE_LENGTH_MAX 4096
 
 // Microsecond timeout for "recent" events such as typed and bookmark following.
 // If you typed it more than this time ago, it's not recent.
@@ -52,16 +53,23 @@
 #ifdef MOZ_XUL
 // Fired after autocomplete feedback has been updated.
 #define TOPIC_AUTOCOMPLETE_FEEDBACK_UPDATED "places-autocomplete-feedback-updated"
 #endif
 
 // Fired after frecency has been updated.
 #define TOPIC_FRECENCY_UPDATED "places-frecency-updated"
 
+// The preference we watch to know when the mobile bookmarks folder is filled by
+// sync.
+#define MOBILE_BOOKMARKS_PREF "browser.bookmarks.showMobileBookmarks"
+
+// The guid of the mobile bookmarks virtual query.
+#define MOBILE_BOOKMARKS_VIRTUAL_GUID "mobile____v"
+
 class nsNavHistory;
 class QueryKeyValuePair;
 class nsIEffectiveTLDService;
 class nsIIDNService;
 class PlacesSQLQueryBuilder;
 class nsIAutoCompleteController;
 
 // nsNavHistory
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -1298,16 +1298,39 @@ nsNavHistoryContainerResultNode::FindChi
         return mChildren[i];
       }
     }
   }
   return nullptr;
 }
 
 /**
+ * Searches this folder for a node with the given guid/target-folder-guid.
+ *
+ * @return the node if found, null otherwise.
+ * @note Does not addref the node!
+ */
+nsNavHistoryResultNode*
+nsNavHistoryContainerResultNode::FindChildByGuid(const nsACString& guid,
+    int32_t* nodeIndex)
+{
+  *nodeIndex = -1;
+  for (int32_t i = 0; i < mChildren.Count(); ++i) {
+    if (mChildren[i]->mBookmarkGuid == guid ||
+        mChildren[i]->mPageGuid == guid ||
+        (mChildren[i]->IsFolder() &&
+         mChildren[i]->GetAsFolder()->mTargetFolderGuid == guid)) {
+      *nodeIndex = i;
+      return mChildren[i];
+    }
+  }
+  return nullptr;
+}
+
+/**
  * This does the work of adding a child to the container.  The child can be
  * either a container or or a single item that may even be collapsed with the
  * adjacent ones.
  */
 nsresult
 nsNavHistoryContainerResultNode::InsertChildAt(nsNavHistoryResultNode* aNode,
                                                int32_t aIndex)
 {
@@ -1784,16 +1807,18 @@ nsNavHistoryQueryResultNode::nsNavHistor
 
 nsNavHistoryQueryResultNode::~nsNavHistoryQueryResultNode() {
   // Remove this node from result's observers.  We don't need to be notified
   // anymore.
   if (mResult && mResult->mAllBookmarksObservers.Contains(this))
     mResult->RemoveAllBookmarksObserver(this);
   if (mResult && mResult->mHistoryObservers.Contains(this))
     mResult->RemoveHistoryObserver(this);
+  if (mResult && mResult->mMobilePrefObservers.Contains(this))
+    mResult->RemoveMobilePrefsObserver(this);
 }
 
 /**
  * Whoever made us may want non-expanding queries. However, we always expand
  * when we are the root node, or else asking for non-expanding queries would be
  * useless.  A query node is not expandable if excludeItems is set or if
  * expandQueries is unset.
  */
@@ -2145,16 +2170,20 @@ nsNavHistoryQueryResultNode::FillChildre
   if (mOptions->QueryType() == nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS ||
       mOptions->QueryType() == nsINavHistoryQueryOptions::QUERY_TYPE_UNIFIED ||
       mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS ||
       mHasSearchTerms) {
     // register with the result for bookmark updates
     result->AddAllBookmarksObserver(this);
   }
 
+  if (mLiveUpdate == QUERYUPDATE_MOBILEPREF) {
+    result->AddMobilePrefsObserver(this);
+  }
+
   mContentsValid = true;
   return NS_OK;
 }
 
 
 /**
  * Call with unregister = false when we are going to update the children (for
  * example, when the container is open).  This will clear the list and notify
@@ -2172,16 +2201,17 @@ nsNavHistoryQueryResultNode::ClearChildr
     mChildren[i]->OnRemoving();
   mChildren.Clear();
 
   if (aUnregister && mContentsValid) {
     nsNavHistoryResult* result = GetResult();
     if (result) {
       result->RemoveHistoryObserver(this);
       result->RemoveAllBookmarksObserver(this);
+      result->RemoveMobilePrefsObserver(this);
     }
   }
   mContentsValid = false;
 }
 
 
 /**
  * This is called to update the result when something has changed that we
@@ -2361,16 +2391,20 @@ nsNavHistoryQueryResultNode::OnVisit(nsI
     NS_ENSURE_SUCCESS(rv, rv);
     return NS_OK;
   }
 
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
 
   switch(mLiveUpdate) {
+    case QUERYUPDATE_MOBILEPREF: {
+      return NS_OK;
+    }
+
     case QUERYUPDATE_HOST: {
       // For these simple yet common cases we can check the host ourselves
       // before doing the overhead of creating a new result node.
       MOZ_ASSERT(mQueries.Count() == 1,
                  "Host updated queries can have only one object");
       RefPtr<nsNavHistoryQuery> query = do_QueryObject(mQueries[0]);
 
       bool hasDomain;
@@ -2790,17 +2824,19 @@ nsNavHistoryQueryResultNode::OnItemAdded
                                          nsIURI* aURI,
                                          const nsACString& aTitle,
                                          PRTime aDateAdded,
                                          const nsACString& aGUID,
                                          const nsACString& aParentGUID,
                                          uint16_t aSource)
 {
   if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK &&
-      mLiveUpdate != QUERYUPDATE_SIMPLE &&  mLiveUpdate != QUERYUPDATE_TIME) {
+      mLiveUpdate != QUERYUPDATE_SIMPLE &&
+      mLiveUpdate != QUERYUPDATE_TIME &&
+      mLiveUpdate != QUERYUPDATE_MOBILEPREF) {
     nsresult rv = Refresh();
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
@@ -2809,17 +2845,19 @@ nsNavHistoryQueryResultNode::OnItemRemov
                                            int32_t aIndex,
                                            uint16_t aItemType,
                                            nsIURI* aURI,
                                            const nsACString& aGUID,
                                            const nsACString& aParentGUID,
                                            uint16_t aSource)
 {
   if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK &&
-      mLiveUpdate != QUERYUPDATE_SIMPLE && mLiveUpdate != QUERYUPDATE_TIME) {
+      mLiveUpdate != QUERYUPDATE_SIMPLE &&
+      mLiveUpdate != QUERYUPDATE_TIME &&
+      mLiveUpdate != QUERYUPDATE_MOBILEPREF) {
     nsresult rv = Refresh();
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
@@ -3598,16 +3636,37 @@ nsNavHistoryFolderResultNode::OnItemAdde
     // insert at natural bookmarks position
     return InsertChildAt(node, aIndex);
   }
 
   // insert at sorted position
   return InsertSortedChild(node);
 }
 
+nsresult
+nsNavHistoryQueryResultNode::OnMobilePrefChanged(bool newValue)
+{
+  RESTART_AND_RETURN_IF_ASYNC_PENDING();
+
+  if (newValue) {
+    // If the folder is being added, simply refresh the query as that is
+    // simpler than re-inserting just the mobile folder.
+    return Refresh();
+  }
+
+  // We're removing the mobile folder, so find it.
+  int32_t existingIndex;
+  FindChildByGuid(NS_LITERAL_CSTRING(MOBILE_BOOKMARKS_VIRTUAL_GUID), &existingIndex);
+
+  if (existingIndex == -1) {
+    return NS_OK;
+  }
+
+  return RemoveChildAt(existingIndex);
+}
 
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::OnItemRemoved(int64_t aItemId,
                                             int64_t aParentFolder,
                                             int32_t aIndex,
                                             uint16_t aItemType,
                                             nsIURI* aURI,
                                             const nsACString& aGUID,
@@ -3954,32 +4013,34 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(nsNavHist
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsNavHistoryResult)
   tmp->StopObserving();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mRootNode)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mObservers)
   for (auto it = tmp->mBookmarkFolderObservers.Iter(); !it.Done(); it.Next()) {
     delete it.Data();
     it.Remove();
   }
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mMobilePrefObservers)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mAllBookmarksObservers)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mHistoryObservers)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsNavHistoryResult)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRootNode)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mObservers)
   for (auto it = tmp->mBookmarkFolderObservers.Iter(); !it.Done(); it.Next()) {
     nsNavHistoryResult::FolderObserverList*& list = it.Data();
     for (uint32_t i = 0; i < list->Length(); ++i) {
       NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb,
                                          "mBookmarkFolderObservers value[i]");
       nsNavHistoryResultNode* node = list->ElementAt(i);
       cb.NoteXPCOMChild(node);
     }
   }
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMobilePrefObservers)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAllBookmarksObservers)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mHistoryObservers)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsNavHistoryResult)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsNavHistoryResult)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsNavHistoryResult)
@@ -3992,16 +4053,17 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END
 
 nsNavHistoryResult::nsNavHistoryResult(nsNavHistoryContainerResultNode* aRoot)
   : mRootNode(aRoot)
   , mNeedsToApplySortingMode(false)
   , mIsHistoryObserver(false)
   , mIsBookmarkFolderObserver(false)
   , mIsAllBookmarksObserver(false)
+  , mIsMobilePrefObserver(false)
   , mBookmarkFolderObservers(64)
   , mBatchInProgress(false)
   , mSuppressNotifications(false)
 {
   mRootNode->mResult = this;
 }
 
 nsNavHistoryResult::~nsNavHistoryResult()
@@ -4019,16 +4081,22 @@ nsNavHistoryResult::StopObserving()
   if (mIsBookmarkFolderObserver || mIsAllBookmarksObserver) {
     nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
     if (bookmarks) {
       bookmarks->RemoveObserver(this);
       mIsBookmarkFolderObserver = false;
       mIsAllBookmarksObserver = false;
     }
   }
+  if (mIsMobilePrefObserver) {
+    Preferences::UnregisterCallback(OnMobilePrefChangedCallback,
+                                    MOBILE_BOOKMARKS_PREF,
+                                    this);
+    mIsMobilePrefObserver = false;
+  }
   if (mIsHistoryObserver) {
     nsNavHistory* history = nsNavHistory::GetHistoryService();
     if (history) {
       history->RemoveObserver(this);
       mIsHistoryObserver = false;
     }
   }
 }
@@ -4133,16 +4201,34 @@ nsNavHistoryResult::AddAllBookmarksObser
   // will try to add the observer again.
   if (mAllBookmarksObservers.IndexOf(aNode) == mAllBookmarksObservers.NoIndex) {
     mAllBookmarksObservers.AppendElement(aNode);
   }
 }
 
 
 void
+nsNavHistoryResult::AddMobilePrefsObserver(nsNavHistoryQueryResultNode* aNode)
+{
+  if (!mIsMobilePrefObserver) {
+    Preferences::RegisterCallback(OnMobilePrefChangedCallback,
+                                  MOBILE_BOOKMARKS_PREF,
+                                  this);
+    mIsMobilePrefObserver = true;
+  }
+  // Don't add duplicate observers.  In some case we don't unregister when
+  // children are cleared (see ClearChildren) and the next FillChildren call
+  // will try to add the observer again.
+  if (mMobilePrefObservers.IndexOf(aNode) == mMobilePrefObservers.NoIndex) {
+    mMobilePrefObservers.AppendElement(aNode);
+  }
+}
+
+
+void
 nsNavHistoryResult::AddBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode,
                                               int64_t aFolder)
 {
   if (!mIsBookmarkFolderObserver && !mIsAllBookmarksObserver) {
     nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
     if (!bookmarks) {
       NS_NOTREACHED("Can't create bookmark service");
       return;
@@ -4170,16 +4256,23 @@ nsNavHistoryResult::RemoveHistoryObserve
 void
 nsNavHistoryResult::RemoveAllBookmarksObserver(nsNavHistoryQueryResultNode* aNode)
 {
   mAllBookmarksObservers.RemoveElement(aNode);
 }
 
 
 void
+nsNavHistoryResult::RemoveMobilePrefsObserver(nsNavHistoryQueryResultNode* aNode)
+{
+  mMobilePrefObservers.RemoveElement(aNode);
+}
+
+
+void
 nsNavHistoryResult::RemoveBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode,
                                                  int64_t aFolder)
 {
   FolderObserverList* list = BookmarkFolderObserversForId(aFolder, false);
   if (!list)
     return; // we don't even have an entry for that folder
   list->RemoveElement(aNode);
 }
@@ -4349,16 +4442,18 @@ nsNavHistoryResult::requestRefresh(nsNav
     } \
   PR_END_MACRO
 #define ENUMERATE_QUERY_OBSERVERS(_functionCall, _observersList, _conditionCall) \
   ENUMERATE_LIST_OBSERVERS(QueryObserverList, _functionCall, _observersList, _conditionCall)
 #define ENUMERATE_ALL_BOOKMARKS_OBSERVERS(_functionCall) \
   ENUMERATE_QUERY_OBSERVERS(_functionCall, mAllBookmarksObservers, IsQuery())
 #define ENUMERATE_HISTORY_OBSERVERS(_functionCall) \
   ENUMERATE_QUERY_OBSERVERS(_functionCall, mHistoryObservers, IsQuery())
+#define ENUMERATE_MOBILE_PREF_OBSERVERS(_functionCall) \
+  ENUMERATE_QUERY_OBSERVERS(_functionCall, mMobilePrefObservers, IsQuery())
 
 #define NOTIFY_REFRESH_PARTICIPANTS() \
   PR_BEGIN_MACRO \
   ENUMERATE_LIST_OBSERVERS(ContainerObserverList, Refresh(), mRefreshParticipants, IsContainer()); \
   mRefreshParticipants.Clear(); \
   PR_END_MACRO
 
 NS_IMETHODIMP
@@ -4778,8 +4873,25 @@ nsNavHistoryResult::OnDeleteVisits(nsIUR
                                    uint32_t aTransitionType)
 {
   NS_ENSURE_ARG(aURI);
 
   ENUMERATE_HISTORY_OBSERVERS(OnDeleteVisits(aURI, aVisitTime, aGUID, aReason,
                                              aTransitionType));
   return NS_OK;
 }
+
+void
+nsNavHistoryResult::OnMobilePrefChanged()
+{
+  ENUMERATE_MOBILE_PREF_OBSERVERS(OnMobilePrefChanged(Preferences::GetBool(MOBILE_BOOKMARKS_PREF, false)));
+}
+
+
+void
+nsNavHistoryResult::OnMobilePrefChangedCallback(const char *prefName,
+                                                void *closure)
+{
+  MOZ_ASSERT(!strcmp(prefName, MOBILE_BOOKMARKS_PREF),
+    "We only expected Mobile Bookmarks pref change.");
+
+  static_cast<nsNavHistoryResult*>(closure)->OnMobilePrefChanged();
+}
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -115,19 +115,21 @@ public:
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsNavHistoryResult, nsINavHistoryResult)
   NS_DECL_BOOKMARK_HISTORY_OBSERVER_EXTERNAL(override)
   NS_IMETHOD OnVisits(nsIVisitData** aVisits,
                       uint32_t aVisitsCount) override;
 
   void AddHistoryObserver(nsNavHistoryQueryResultNode* aNode);
   void AddBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode, int64_t aFolder);
   void AddAllBookmarksObserver(nsNavHistoryQueryResultNode* aNode);
+  void AddMobilePrefsObserver(nsNavHistoryQueryResultNode* aNode);
   void RemoveHistoryObserver(nsNavHistoryQueryResultNode* aNode);
   void RemoveBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode, int64_t aFolder);
   void RemoveAllBookmarksObserver(nsNavHistoryQueryResultNode* aNode);
+  void RemoveMobilePrefsObserver(nsNavHistoryQueryResultNode* aNode);
   void StopObserving();
 
   nsresult OnVisit(nsIURI* aURI, int64_t aVisitId, PRTime aTime,
                    uint32_t aTransitionType, const nsACString& aGUID,
                    bool aHidden, uint32_t aVisitCount,
                    const nsAString& aLastKnownTitle);
 
 public:
@@ -152,20 +154,22 @@ public:
 
   // The sorting annotation to be used for in SORT_BY_ANNOTATION_* modes
   nsCString mSortingAnnotation;
 
   // node observers
   bool mIsHistoryObserver;
   bool mIsBookmarkFolderObserver;
   bool mIsAllBookmarksObserver;
+  bool mIsMobilePrefObserver;
 
   typedef nsTArray< RefPtr<nsNavHistoryQueryResultNode> > QueryObserverList;
   QueryObserverList mHistoryObservers;
   QueryObserverList mAllBookmarksObservers;
+  QueryObserverList mMobilePrefObservers;
 
   typedef nsTArray< RefPtr<nsNavHistoryFolderResultNode> > FolderObserverList;
   nsDataHashtable<nsTrimInt64HashKey, FolderObserverList*> mBookmarkFolderObservers;
   FolderObserverList* BookmarkFolderObserversForId(int64_t aFolderId, bool aCreate);
 
   typedef nsTArray< RefPtr<nsNavHistoryContainerResultNode> > ContainerObserverList;
 
   void RecursiveExpandCollapse(nsNavHistoryContainerResultNode* aContainer,
@@ -176,16 +180,20 @@ public:
   bool mBatchInProgress;
 
   nsMaybeWeakPtrArray<nsINavHistoryResultObserver> mObservers;
   bool mSuppressNotifications;
 
   ContainerObserverList mRefreshParticipants;
   void requestRefresh(nsNavHistoryContainerResultNode* aContainer);
 
+  void OnMobilePrefChanged();
+
+  static void OnMobilePrefChangedCallback(const char* prefName, void* closure);
+
 protected:
   virtual ~nsNavHistoryResult();
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsNavHistoryResult, NS_NAVHISTORYRESULT_IID)
 
 // nsNavHistoryResultNode
 //
@@ -285,16 +293,20 @@ public:
                            PRTime aNewLastModified,
                            uint16_t aItemType,
                            int64_t aParentId,
                            const nsACString& aGUID,
                            const nsACString& aParentGUID,
                            const nsACString &aOldValue,
                            uint16_t aSource);
 
+  virtual nsresult OnMobilePrefChanged(bool newValue) {
+    return NS_OK;
+  };
+
 protected:
   virtual ~nsNavHistoryResultNode() {}
 
 public:
 
   nsNavHistoryResult* GetResult();
 
   // These functions test the type. We don't use a virtual function since that
@@ -565,16 +577,19 @@ public:
 
   // finding children: THESE DO NOT ADDREF
   nsNavHistoryResultNode* FindChildURI(const nsACString& aSpec,
                                        uint32_t* aNodeIndex);
   // returns the index of the given node, -1 if not found
   int32_t FindChild(nsNavHistoryResultNode* aNode)
     { return mChildren.IndexOf(aNode); }
 
+  nsNavHistoryResultNode* FindChildByGuid(const nsACString& guid,
+                                          int32_t* nodeIndex);
+
   nsresult InsertChildAt(nsNavHistoryResultNode* aNode, int32_t aIndex);
   nsresult InsertSortedChild(nsNavHistoryResultNode* aNode,
                              bool aIgnoreDuplicates = false);
   bool EnsureItemPosition(uint32_t aIndex);
 
   nsresult RemoveChildAt(int32_t aIndex);
 
   void RecursiveFindURIs(bool aOnlyOne,
@@ -631,16 +646,18 @@ public:
   NS_FORWARD_COMMON_RESULTNODE_TO_BASE
   NS_IMETHOD GetType(uint32_t* type) override
     { *type = nsNavHistoryResultNode::RESULT_TYPE_QUERY; return NS_OK; }
   NS_IMETHOD GetUri(nsACString& aURI) override; // does special lazy creation
   NS_FORWARD_CONTAINERNODE_EXCEPT_HASCHILDREN
   NS_IMETHOD GetHasChildren(bool* aHasChildren) override;
   NS_DECL_NSINAVHISTORYQUERYRESULTNODE
 
+  virtual nsresult OnMobilePrefChanged(bool newValue) override;
+
   bool CanExpand();
   bool IsContainersQuery();
 
   virtual nsresult OpenContainer() override;
 
   NS_DECL_BOOKMARK_HISTORY_OBSERVER_INTERNAL
 
   // The internal version has an output aAdded parameter, it is incremented by
--- a/toolkit/components/places/tests/queries/test_results-as-roots.js
+++ b/toolkit/components/places/tests/queries/test_results-as-roots.js
@@ -82,21 +82,12 @@ add_task(async function test_results_as_
   Services.prefs.setBoolPref(MOBILE_BOOKMARKS_PREF, true);
 
   let root = getAllBookmarksQuery();
   root.containerOpen = true;
 
   // Now un-set the pref, and poke the database to update the query.
   Services.prefs.clearUserPref(MOBILE_BOOKMARKS_PREF);
 
-  // We've un-set the pref, but we still expect the mobile folder to be there
-  // until the database is poked.
-  assertExpectedChildren(root, expectedRootsWithMobile);
-
-  await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.menuGuid,
-    url: "http://example.com",
-  });
-
   assertExpectedChildren(root, expectedRoots);
 
   root.containerOpen = false;
 });