Bug 478035 - Move the code for obtaining the places built-in folder ids from nsNavBookmarks to Database. r=mak
authorMark Banner <standard8@mozilla.com>
Wed, 18 Apr 2018 15:53:07 +0100
changeset 468219 db3d3eb4b46c52ee285a235c54dfc0041d092b27
parent 468218 1f04da36cb83f3db03c1629135fd71678fc2b4d6
child 468220 fe5121c793e4923e45bd622cf92c8687c001b0ff
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs478035
milestone61.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 478035 - Move the code for obtaining the places built-in folder ids from nsNavBookmarks to Database. r=mak MozReview-Commit-ID: 74we2Z47xY
toolkit/components/places/Database.cpp
toolkit/components/places/Database.h
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavBookmarks.h
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -102,16 +102,21 @@
 
 // Places string bundle, contains internationalized bookmark root names.
 #define PLACES_BUNDLE "chrome://places/locale/places.properties"
 
 // Livemarks annotations.
 #define LMANNO_FEEDURI "livemark/feedURI"
 #define LMANNO_SITEURI "livemark/siteURI"
 
+#define ROOT_GUID "root________"
+#define MENU_ROOT_GUID "menu________"
+#define TOOLBAR_ROOT_GUID "toolbar_____"
+#define UNFILED_ROOT_GUID "unfiled_____"
+#define TAGS_ROOT_GUID "tags________"
 #define MOBILE_ROOT_GUID "mobile______"
 // This is no longer used & obsolete except for during migration.
 // Note: it may still be found in older places databases.
 #define MOBILE_ROOT_ANNO "mobile/bookmarksRoot"
 
 // We use a fixed title for the mobile root to avoid marking the database as
 // corrupt if we can't look up the localized title in the string bundle. Sync
 // sets the title to the localized version when it creates the left pane query.
@@ -377,16 +382,22 @@ Database::Database()
   , mDatabaseStatus(nsINavHistoryService::DATABASE_STATUS_OK)
   , mClosed(false)
   , mShouldConvertIconPayloads(false)
   , mShouldVacuumIcons(false)
   , mClientsShutdown(new ClientsShutdownBlocker())
   , mConnectionShutdown(new ConnectionShutdownBlocker(this))
   , mMaxUrlLength(0)
   , mCacheObservers(TOPIC_PLACES_INIT_COMPLETE)
+  , mRootId(-1)
+  , mMenuRootId(-1)
+  , mTagsRootId(-1)
+  , mUnfiledRootId(-1)
+  , mToolbarRootId(-1)
+  , mMobileRootId(-1)
 {
   MOZ_ASSERT(!XRE_IsContentProcess(),
              "Cannot instantiate Places in the content process");
   // Attempting to create two instances of the service?
   MOZ_ASSERT(!gDatabase);
   gDatabase = this;
 }
 
@@ -645,16 +656,19 @@ Database::EnsureConnection()
 
     // Initialize here all the items that are not part of the on-disk database,
     // like views, temp triggers or temp tables.  The database should not be
     // considered corrupt if any of the following fails.
 
     rv = InitTempEntities();
     NS_ENSURE_SUCCESS(rv, rv);
 
+    rv = CheckRoots();
+    NS_ENSURE_SUCCESS(rv, rv);
+
     initSucceeded = true;
   }
   return NS_OK;
 }
 
 nsresult
 Database::NotifyConnectionInitalized()
 {
@@ -1355,16 +1369,58 @@ Database::InitSchema(bool* aDatabaseMigr
   // AND TRY TO REPLACE IT.
   // DO NOT PUT HERE ANYTHING THAT IS NOT RELATED TO INITIALIZATION OR MODIFYING
   // THE DISK DATABASE.
 
   return NS_OK;
 }
 
 nsresult
+Database::CheckRoots()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  nsCOMPtr<mozIStorageStatement> stmt;
+  nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT guid, id FROM moz_bookmarks WHERE guid IN ( "
+      "'" ROOT_GUID "', '" MENU_ROOT_GUID "', '" TOOLBAR_ROOT_GUID "', "
+      "'" TAGS_ROOT_GUID "', '" UNFILED_ROOT_GUID "', '" MOBILE_ROOT_GUID "' )"
+    ), getter_AddRefs(stmt));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  bool hasResult;
+  nsAutoCString guid;
+  while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
+    rv = stmt->GetUTF8String(0, guid);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    if (guid.EqualsLiteral(ROOT_GUID)) {
+      mRootId = stmt->AsInt64(1);
+    }
+    else if (guid.EqualsLiteral(MENU_ROOT_GUID)) {
+      mMenuRootId = stmt->AsInt64(1);
+    }
+    else if (guid.EqualsLiteral(TOOLBAR_ROOT_GUID)) {
+      mToolbarRootId = stmt->AsInt64(1);
+    }
+    else if (guid.EqualsLiteral(TAGS_ROOT_GUID)) {
+      mTagsRootId = stmt->AsInt64(1);
+    }
+    else if (guid.EqualsLiteral(UNFILED_ROOT_GUID)) {
+      mUnfiledRootId = stmt->AsInt64(1);
+    }
+    else if (guid.EqualsLiteral(MOBILE_ROOT_GUID)) {
+      mMobileRootId = stmt->AsInt64(1);
+    }
+  }
+
+  return NS_OK;
+}
+
+nsresult
 Database::CreateBookmarkRoots()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // The first root's title is an empty string.
   nsresult rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("places"),
                            NS_LITERAL_CSTRING("root________"), EmptyCString());
   if (NS_FAILED(rv)) return rv;
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -200,16 +200,41 @@ public:
    * @return The cached statement.
    * @note Always null check the result.
    * @note AsyncStatements are automatically reset on execution.
    */
   already_AddRefed<mozIStorageAsyncStatement> GetAsyncStatement(const nsACString& aQuery);
 
   uint32_t MaxUrlLength();
 
+  int64_t GetRootFolderId() {
+    mozilla::Unused << EnsureConnection();
+    return mRootId;
+  }
+  int64_t GetMenuFolderId() {
+    mozilla::Unused << EnsureConnection();
+    return mMenuRootId;
+  }
+  int64_t GetTagsFolderId() {
+    mozilla::Unused << EnsureConnection();
+    return mTagsRootId;
+  }
+  int64_t GetUnfiledFolderId() {
+    mozilla::Unused << EnsureConnection();
+    return mUnfiledRootId;
+  }
+  int64_t GetToolbarFolderId() {
+    mozilla::Unused << EnsureConnection();
+    return mToolbarRootId;
+  }
+  int64_t GetMobileFolderId() {
+    mozilla::Unused << EnsureConnection();
+    return mMobileRootId;
+  }
+
 protected:
   /**
    * Finalizes the cached statements and closes the database connection.
    * A TOPIC_PLACES_CONNECTION_CLOSED notification is fired when done.
    */
   void Shutdown();
 
   bool IsShutdownStarted() const;
@@ -269,16 +294,21 @@ protected:
    * database.  All migration is done inside a transaction that is rolled back
    * if any error occurs.
    * @param aDatabaseMigrated
    *        Whether a schema upgrade happened.
    */
   nsresult InitSchema(bool* aDatabaseMigrated);
 
   /**
+   * Checks the root bookmark folders are present, and saves the IDs for them.
+   */
+  nsresult CheckRoots();
+
+  /**
    * Creates bookmark roots in a new DB.
    */
   nsresult CreateBookmarkRoots();
 
   /**
    * Initializes additionale SQLite functions, defined in SQLFunctions.h
    */
   nsresult InitFunctions();
@@ -364,14 +394,22 @@ private:
   // Maximum length of a stored url.
   // For performance reasons we don't store very long urls in history, since
   // they are slower to search through and cause abnormal database growth,
   // affecting the awesomebar fetch time.
   uint32_t mMaxUrlLength;
 
   // Used to initialize components on places startup.
   nsCategoryCache<nsIObserver> mCacheObservers;
+
+  // Used to cache the places folder Ids when the connection is started.
+  int64_t mRootId;
+  int64_t mMenuRootId;
+  int64_t mTagsRootId;
+  int64_t mUnfiledRootId;
+  int64_t mToolbarRootId;
+  int64_t mMobileRootId;
 };
 
 } // namespace places
 } // namespace mozilla
 
 #endif // mozilla_places_Database_h_
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -152,23 +152,17 @@ inline bool
 NeedsTombstone(const BookmarkData& aBookmark) {
   return aBookmark.syncStatus == nsINavBookmarksService::SYNC_STATUS_NORMAL;
 }
 
 } // namespace
 
 
 nsNavBookmarks::nsNavBookmarks()
-  : mRoot(0)
-  , mMenuRoot(0)
-  , mTagsRoot(0)
-  , mUnfiledRoot(0)
-  , mToolbarRoot(0)
-  , mMobileRoot(0)
-  , mCanNotify(false)
+  : mCanNotify(false)
 {
   NS_ASSERTION(!gBookmarksService,
                "Attempting to create two instances of the service!");
   gBookmarksService = this;
 }
 
 
 nsNavBookmarks::~nsNavBookmarks()
@@ -220,71 +214,16 @@ nsNavBookmarks::Init()
   history->AddObserver(this, true);
 
   // DO NOT PUT STUFF HERE that can fail. See observer comment above.
 
   return NS_OK;
 }
 
 nsresult
-nsNavBookmarks::EnsureRoots()
-{
-  if (mRoot)
-    return NS_OK;
-
-  nsCOMPtr<mozIStorageConnection> conn = mDB->MainConn();
-  if (!conn) {
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  nsCOMPtr<mozIStorageStatement> stmt;
-  nsresult rv = conn->CreateStatement(NS_LITERAL_CSTRING(
-    "SELECT guid, id FROM moz_bookmarks WHERE guid IN ( "
-      "'root________', 'menu________', 'toolbar_____', "
-      "'tags________', 'unfiled_____', 'mobile______' )"
-  ), getter_AddRefs(stmt));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  bool hasResult;
-  while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
-    nsAutoCString guid;
-    rv = stmt->GetUTF8String(0, guid);
-    NS_ENSURE_SUCCESS(rv, rv);
-    int64_t id;
-    rv = stmt->GetInt64(1, &id);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    if (guid.EqualsLiteral("root________")) {
-      mRoot = id;
-    }
-    else if (guid.EqualsLiteral("menu________")) {
-      mMenuRoot = id;
-    }
-    else if (guid.EqualsLiteral("toolbar_____")) {
-      mToolbarRoot = id;
-    }
-    else if (guid.EqualsLiteral("tags________")) {
-      mTagsRoot = id;
-    }
-    else if (guid.EqualsLiteral("unfiled_____")) {
-      mUnfiledRoot = id;
-    }
-    else if (guid.EqualsLiteral("mobile______")) {
-      mMobileRoot = id;
-    }
-  }
-
-  if (!mRoot || !mMenuRoot || !mToolbarRoot || !mTagsRoot || !mUnfiledRoot ||
-      !mMobileRoot)
-    return NS_ERROR_FAILURE;
-
-  return NS_OK;
-}
-
-nsresult
 nsNavBookmarks::AdjustIndices(int64_t aFolderId,
                               int32_t aStartIndex,
                               int32_t aEndIndex,
                               int32_t aDelta)
 {
   NS_ASSERTION(aStartIndex >= 0 && aEndIndex <= INT32_MAX &&
                aStartIndex <= aEndIndex, "Bad indices");
 
@@ -344,69 +283,69 @@ nsNavBookmarks::AdjustSeparatorsSyncCoun
 
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::GetPlacesRoot(int64_t* aRoot)
 {
-  nsresult rv = EnsureRoots();
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aRoot = mRoot;
+  int64_t id = mDB->GetRootFolderId();
+  NS_ENSURE_TRUE(id > 0, NS_ERROR_UNEXPECTED);
+  *aRoot = id;
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::GetBookmarksMenuFolder(int64_t* aRoot)
 {
-  nsresult rv = EnsureRoots();
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aRoot = mMenuRoot;
+  int64_t id = mDB->GetMenuFolderId();
+  NS_ENSURE_TRUE(id > 0, NS_ERROR_UNEXPECTED);
+  *aRoot = id;
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
-nsNavBookmarks::GetToolbarFolder(int64_t* aFolderId)
+nsNavBookmarks::GetToolbarFolder(int64_t* aRoot)
 {
-  nsresult rv = EnsureRoots();
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aFolderId = mToolbarRoot;
+  int64_t id = mDB->GetToolbarFolderId();
+  NS_ENSURE_TRUE(id > 0, NS_ERROR_UNEXPECTED);
+  *aRoot = id;
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::GetTagsFolder(int64_t* aRoot)
 {
-  nsresult rv = EnsureRoots();
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aRoot = mTagsRoot;
+  int64_t id = mDB->GetTagsFolderId();
+  NS_ENSURE_TRUE(id > 0, NS_ERROR_UNEXPECTED);
+  *aRoot = id;
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::GetUnfiledBookmarksFolder(int64_t* aRoot)
 {
-  nsresult rv = EnsureRoots();
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aRoot = mUnfiledRoot;
+  int64_t id = mDB->GetUnfiledFolderId();
+  NS_ENSURE_TRUE(id > 0, NS_ERROR_UNEXPECTED);
+  *aRoot = id;
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::GetMobileFolder(int64_t* aRoot)
 {
-  nsresult rv = EnsureRoots();
-  NS_ENSURE_SUCCESS(rv, rv);
-  *aRoot = mMobileRoot;
+  int64_t id = mDB->GetMobileFolderId();
+  NS_ENSURE_TRUE(id > 0, NS_ERROR_UNEXPECTED);
+  *aRoot = id;
   return NS_OK;
 }
 
 
 nsresult
 nsNavBookmarks::InsertBookmarkInDB(int64_t aPlaceId,
                                    enum ItemType aItemType,
                                    int64_t aParentId,
@@ -623,17 +562,17 @@ nsNavBookmarks::InsertBookmark(int64_t a
     rv = history->UpdateFrecency(placeId);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
   NOTIFY_BOOKMARKS_OBSERVERS(mCanNotify, mObservers,
-                             SKIP_TAGS(grandParentId == mTagsRoot),
+                             SKIP_TAGS(grandParentId == mDB->GetTagsFolderId()),
                              OnItemAdded(*aNewBookmarkId, aFolder, index,
                                          TYPE_BOOKMARK, aURI, title, dateAdded,
                                          guid, folderGuid, aSource));
 
   // If the bookmark has been added to a tag container, notify all
   // bookmark-folder result nodes which contain a bookmark for the new
   // bookmark's url.
   if (grandParentId == tagsRootId) {
--- a/toolkit/components/places/nsNavBookmarks.h
+++ b/toolkit/components/places/nsNavBookmarks.h
@@ -215,22 +215,16 @@ private:
    * Checks whether or not aFolderId points to a live bookmark.
    *
    * @param aFolderId
    *        the item-id of the folder to check.
    * @return true if aFolderId points to live bookmarks, false otherwise.
    */
   bool IsLivemark(int64_t aFolderId);
 
-  /**
-   * Locates the root items in the bookmarks folder hierarchy assigning folder
-   * ids to the root properties that are exposed through the service interface.
-   */
-  nsresult EnsureRoots();
-
   nsresult AdjustIndices(int64_t aFolder,
                          int32_t aStartIndex,
                          int32_t aEndIndex,
                          int32_t aDelta);
 
   nsresult AdjustSeparatorsSyncCounter(int64_t aFolderId,
                                        int32_t aStartIndex,
                                        int64_t aSyncChangeDelta);
@@ -283,34 +277,26 @@ private:
   /**
    * This is an handle to the Places database.
    */
   RefPtr<mozilla::places::Database> mDB;
 
   nsMaybeWeakPtrArray<nsINavBookmarkObserver> mObservers;
 
   int64_t TagsRootId() {
-    nsresult rv = EnsureRoots();
-    NS_ENSURE_SUCCESS(rv, -1);
-    return mTagsRoot;
+    return mDB->GetTagsFolderId();
   }
 
-  // These are lazy loaded, so never access them directly, always use the
-  // XPIDL getters or TagsRootId().
-  int64_t mRoot;
-  int64_t mMenuRoot;
-  int64_t mTagsRoot;
-  int64_t mUnfiledRoot;
-  int64_t mToolbarRoot;
-  int64_t mMobileRoot;
-
   inline bool IsRoot(int64_t aFolderId) {
-    return aFolderId == mRoot || aFolderId == mMenuRoot ||
-           aFolderId == mTagsRoot || aFolderId == mUnfiledRoot ||
-           aFolderId == mToolbarRoot || aFolderId == mMobileRoot;
+    return aFolderId == mDB->GetRootFolderId() ||
+           aFolderId == mDB->GetMenuFolderId() ||
+           aFolderId == mDB->GetTagsFolderId() ||
+           aFolderId == mDB->GetUnfiledFolderId() ||
+           aFolderId == mDB->GetToolbarFolderId() ||
+           aFolderId == mDB->GetMobileFolderId();
   }
 
   nsresult SetItemDateInternal(enum mozilla::places::BookmarkDate aDateType,
                                int64_t aSyncChangeDelta,
                                int64_t aItemId,
                                PRTime aValue);
 
   nsresult RemoveFolderChildren(int64_t aFolderId, uint16_t aSource);