Bug 478035 - Move the code for obtaining the places built-in folder ids from nsNavBookmarks to Database. r?mak draft
authorMark Banner <standard8@mozilla.com>
Wed, 18 Apr 2018 15:53:07 +0100
changeset 785808 0e60d3c4dc2bde877a963ecb37e1b246bf9a4825
parent 785807 a10d637e167809a078442e81df24b2f0ae1c63bf
child 785809 cae78c106b8cf2413a55ed0e60682df3ab8fedfa
push id107311
push userbmo:standard8@mozilla.com
push dateFri, 20 Apr 2018 16:56:49 +0000
reviewersmak
bugs478035
milestone61.0a1
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);