author Mark Banner <standard8@mozilla.com>

Wed, 18 Apr 2018 18:29:19 +0100

changeset 785809 cae78c106b8cf2413a55ed0e60682df3ab8fedfa

parent 785808 0e60d3c4dc2bde877a963ecb37e1b246bf9a4825

child 785810 55d623fc9ff610f35b8de79a2ff0c80bdf6990c7

push id 107311

push user bmo:standard8@mozilla.com

push date Fri, 20 Apr 2018 16:56:49 +0000

reviewers mak

bugs 478035

milestone 61.0a1

toolkit/components/places/Database.cpp file | annotate | diff | comparison | revisions

toolkit/components/places/Database.h file | annotate | diff | comparison | revisions

toolkit/components/places/nsNavBookmarks.h file | annotate | diff | comparison | revisions

toolkit/components/places/tests/head_common.js file | annotate | diff | comparison | revisions

toolkit/components/places/tests/migration/head_migration.js file | annotate | diff | comparison | revisions

toolkit/components/places/tests/unit/missingBuiltIn.sqlite file | annotate | diff | comparison | revisions

toolkit/components/places/tests/unit/noRoot.sqlite file | annotate | diff | comparison | revisions

toolkit/components/places/tests/unit/test_missing_builtin_folders.js file | annotate | diff | comparison | revisions

toolkit/components/places/tests/unit/test_missing_root_folder.js file | annotate | diff | comparison | revisions

toolkit/components/places/tests/unit/xpcshell.ini file | annotate | diff | comparison | revisions
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -5,21 +5,21 @@
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/ScopeExit.h"
 
 #include "Database.h"
 
 #include "nsIAnnotationService.h"
-#include "nsINavBookmarksService.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIFile.h"
 #include "nsIWritablePropertyBag2.h"
 
+#include "nsNavBookmarks.h"
 #include "nsNavHistory.h"
 #include "nsPlacesTables.h"
 #include "nsPlacesIndexes.h"
 #include "nsPlacesTriggers.h"
 #include "nsPlacesMacros.h"
 #include "nsVariant.h"
 #include "SQLFunctions.h"
 #include "Helpers.h"
@@ -228,23 +228,20 @@ SetJournalMode(nsCOMPtr<mozIStorageConne
   }
 
   return JOURNAL_DELETE;
 }
 
 nsresult
 CreateRoot(nsCOMPtr<mozIStorageConnection>& aDBConn,
            const nsCString& aRootName, const nsCString& aGuid,
-           const nsCString& titleString)
+           const nsCString& titleString, const int32_t position, int64_t& newId)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  // The position of the new item in its folder.
-  static int32_t itemPosition = 0;
-
   // A single creation timestamp for all roots so that the root folder's
   // last modification time isn't earlier than its childrens' creation time.
   static PRTime timestamp = 0;
   if (!timestamp)
     timestamp = RoundedPRNow();
 
   // Create a new bookmark folder for the root.
   nsCOMPtr<mozIStorageStatement> stmt;
@@ -257,38 +254,34 @@ CreateRoot(nsCOMPtr<mozIStorageConnectio
             "IFNULL((SELECT id FROM moz_bookmarks WHERE parent = 0), 0), "
             "1, :sync_status)"
   ), getter_AddRefs(stmt));
   if (NS_FAILED(rv)) return rv;
 
   rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("item_type"),
                              nsINavBookmarksService::TYPE_FOLDER);
   if (NS_FAILED(rv)) return rv;
-  rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("item_position"), itemPosition);
+  rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("item_position"), position);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("item_title"),
                                   titleString);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("date_added"), timestamp);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("last_modified"), timestamp);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("guid"), aGuid);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("sync_status"),
                              nsINavBookmarksService::SYNC_STATUS_NEW);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->Execute();
   if (NS_FAILED(rv)) return rv;
 
-  // The 'places' root is a folder containing the other roots.
-  // The first bookmark in a folder has position 0.
-  if (!aRootName.EqualsLiteral("places"))
-    ++itemPosition;
-
+  newId = nsNavBookmarks::sLastInsertedItemId;
   return NS_OK;
 }
 
 nsresult
 SetupDurability(nsCOMPtr<mozIStorageConnection>& aDBConn, int32_t aDBPageSize) {
   nsresult rv;
   if (PR_GetEnv(ENV_ALLOW_CORRUPTION) &&
       Preferences::GetBool(PREF_DISABLE_DURABILITY, false)) {
@@ -1348,19 +1341,17 @@ Database::InitSchema(bool* aDatabaseMigr
     NS_ENSURE_SUCCESS(rv, rv);
     rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_ITEMSANNOS_PLACEATTRIBUTE);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // moz_meta.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_META);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Initialize the bookmark roots in the new DB.
-    rv = CreateBookmarkRoots();
-    NS_ENSURE_SUCCESS(rv, rv);
+    // The bookmarks roots get initialized in CheckRoots().
   }
 
   // Set the schema version to the current one.
   rv = mMainConn->SetSchemaVersion(DATABASE_SCHEMA_VERSION);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
@@ -1373,122 +1364,151 @@ Database::InitSchema(bool* aDatabaseMigr
   return NS_OK;
 }
 
 nsresult
 Database::CheckRoots()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
+  // If the database has just been created, skip straight to the part where
+  // we create the roots.
+  if (mDatabaseStatus == nsINavHistoryService::DATABASE_STATUS_CREATE) {
+    return EnsureBookmarkRoots(0);
+  }
+
   nsCOMPtr<mozIStorageStatement> stmt;
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
-    "SELECT guid, id FROM moz_bookmarks WHERE guid IN ( "
+    "SELECT guid, id, position 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;
+  int32_t maxPosition = 0;
   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);
+    else {
+      maxPosition = std::max(stmt->AsInt32(2), maxPosition);
+
+      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);
+      }
     }
   }
 
+  rv = EnsureBookmarkRoots(maxPosition + 1);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   return NS_OK;
 }
 
 nsresult
-Database::CreateBookmarkRoots()
+Database::EnsureBookmarkRoots(const int32_t startPosition)
 {
   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;
+  nsresult rv;
+
+  // Note: If the root is missing, we recreate it but we don't fix any
+  // remaining built-in folder parent ids. We leave these to a maintenance task,
+  // so that we're not needing to do extra checks on startup.
+
+  if (mRootId < 1) {
+    // The first root's title is an empty string.
+    rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("places"),
+                    NS_LITERAL_CSTRING("root________"), EmptyCString(),
+                    0, mRootId);
+
+    if (NS_FAILED(rv)) return rv;
+  }
+
+  int32_t position = startPosition;
 
   // For the other roots, the UI doesn't rely on the value in the database, so
   // just set it to something simple to make it easier for humans to read.
-
-  rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("menu"),
-                  NS_LITERAL_CSTRING("menu________"), NS_LITERAL_CSTRING("menu"));
-  if (NS_FAILED(rv)) return rv;
-
-  rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("toolbar"),
-                  NS_LITERAL_CSTRING("toolbar_____"), NS_LITERAL_CSTRING("toolbar"));
-  if (NS_FAILED(rv)) return rv;
-
-  rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("tags"),
-                  NS_LITERAL_CSTRING("tags________"), NS_LITERAL_CSTRING("tags"));
-  if (NS_FAILED(rv)) return rv;
-
-  if (NS_FAILED(rv)) return rv;
-  rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("unfiled"),
-                  NS_LITERAL_CSTRING("unfiled_____"), NS_LITERAL_CSTRING("unfiled"));
-  if (NS_FAILED(rv)) return rv;
+  if (mMenuRootId < 1) {
+    rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("menu"),
+                    NS_LITERAL_CSTRING("menu________"), NS_LITERAL_CSTRING("menu"),
+                    position, mMenuRootId);
+    if (NS_FAILED(rv)) return rv;
+    position++;
+  }
 
-  int64_t mobileRootId = CreateMobileRoot();
-  if (mobileRootId <= 0) return NS_ERROR_FAILURE;
-  {
-    nsCOMPtr<mozIStorageStatement> mobileRootSyncStatusStmt;
-    rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
-      "UPDATE moz_bookmarks SET syncStatus = :sync_status WHERE id = :id"
-    ), getter_AddRefs(mobileRootSyncStatusStmt));
+  if (mToolbarRootId < 1) {
+    rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("toolbar"),
+                    NS_LITERAL_CSTRING("toolbar_____"), NS_LITERAL_CSTRING("toolbar"),
+                    position, mToolbarRootId);
     if (NS_FAILED(rv)) return rv;
-    mozStorageStatementScoper mobileRootSyncStatusScoper(
-      mobileRootSyncStatusStmt);
+    position++;
+  }
 
-    rv = mobileRootSyncStatusStmt->BindInt32ByName(
-      NS_LITERAL_CSTRING("sync_status"),
-      nsINavBookmarksService::SYNC_STATUS_NEW
-    );
+  if (mTagsRootId < 1) {
+    rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("tags"),
+                    NS_LITERAL_CSTRING("tags________"), NS_LITERAL_CSTRING("tags"),
+                    position, mTagsRootId);
     if (NS_FAILED(rv)) return rv;
-    rv = mobileRootSyncStatusStmt->BindInt64ByName(NS_LITERAL_CSTRING("id"),
-                                                   mobileRootId);
-    if (NS_FAILED(rv)) return rv;
-
-    rv = mobileRootSyncStatusStmt->Execute();
-    NS_ENSURE_SUCCESS(rv, rv);
+    position++;
   }
 
-#if DEBUG
-  nsCOMPtr<mozIStorageStatement> stmt;
-  rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
-    "SELECT count(*), sum(position) FROM moz_bookmarks"
-  ), getter_AddRefs(stmt));
-  if (NS_FAILED(rv)) return rv;
+  if (mUnfiledRootId < 1) {
+    if (NS_FAILED(rv)) return rv;
+    rv = CreateRoot(mMainConn, NS_LITERAL_CSTRING("unfiled"),
+                    NS_LITERAL_CSTRING("unfiled_____"), NS_LITERAL_CSTRING("unfiled"),
+                    position, mUnfiledRootId);
+    if (NS_FAILED(rv)) return rv;
+    position++;
+  }
 
-  bool hasResult;
-  rv = stmt->ExecuteStep(&hasResult);
-  if (NS_FAILED(rv)) return rv;
-  MOZ_ASSERT(hasResult);
-  int32_t bookmarkCount = stmt->AsInt32(0);
-  int32_t positionSum = stmt->AsInt32(1);
-  MOZ_ASSERT(bookmarkCount == 6 && positionSum == 10);
-#endif
+  if (mMobileRootId < 1) {
+    int64_t mobileRootId = CreateMobileRoot();
+    if (mobileRootId <= 0) return NS_ERROR_FAILURE;
+    {
+      nsCOMPtr<mozIStorageStatement> mobileRootSyncStatusStmt;
+      rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
+        "UPDATE moz_bookmarks SET syncStatus = :sync_status WHERE id = :id"
+      ), getter_AddRefs(mobileRootSyncStatusStmt));
+      if (NS_FAILED(rv)) return rv;
+      mozStorageStatementScoper mobileRootSyncStatusScoper(
+        mobileRootSyncStatusStmt);
+
+      rv = mobileRootSyncStatusStmt->BindInt32ByName(
+        NS_LITERAL_CSTRING("sync_status"),
+        nsINavBookmarksService::SYNC_STATUS_NEW
+      );
+      if (NS_FAILED(rv)) return rv;
+      rv = mobileRootSyncStatusStmt->BindInt64ByName(NS_LITERAL_CSTRING("id"),
+                                                     mobileRootId);
+      if (NS_FAILED(rv)) return rv;
+
+      rv = mobileRootSyncStatusStmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      mMobileRootId = mobileRootId;
+    }
+  }
 
   return NS_OK;
 }
 
 nsresult
 Database::InitFunctions()
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -2303,17 +2323,17 @@ Database::CreateMobileRoot()
 
   // Create the mobile root, ignoring conflicts if one already exists (for
   // example, if the user downgraded to an earlier release channel).
   nsCOMPtr<mozIStorageStatement> createStmt;
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "INSERT OR IGNORE INTO moz_bookmarks "
       "(type, title, dateAdded, lastModified, guid, position, parent) "
     "SELECT :item_type, :item_title, :timestamp, :timestamp, :guid, "
-      "(SELECT COUNT(*) FROM moz_bookmarks p WHERE p.parent = b.id), b.id "
+      "IFNULL((SELECT MAX(position) + 1 FROM moz_bookmarks p WHERE p.parent = b.id), 0), b.id "
     "FROM moz_bookmarks b WHERE b.parent = 0"
   ), getter_AddRefs(createStmt));
   if (NS_FAILED(rv)) return -1;
   mozStorageStatementScoper createScoper(createStmt);
 
   rv = createStmt->BindInt32ByName(NS_LITERAL_CSTRING("item_type"),
                                    nsINavBookmarksService::TYPE_FOLDER);
   if (NS_FAILED(rv)) return -1;
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -301,17 +301,17 @@ protected:
   /**
    * Checks the root bookmark folders are present, and saves the IDs for them.
    */
   nsresult CheckRoots();
 
   /**
    * Creates bookmark roots in a new DB.
    */
-  nsresult CreateBookmarkRoots();
+  nsresult EnsureBookmarkRoots(const int32_t startPosition);
 
   /**
    * Initializes additionale SQLite functions, defined in SQLFunctions.h
    */
   nsresult InitFunctions();
 
   /**
    * Initializes temp entities, like triggers, tables, views...
--- a/toolkit/components/places/nsNavBookmarks.h
+++ b/toolkit/components/places/nsNavBookmarks.h
@@ -260,17 +260,17 @@ private:
   // is used to update tagged bookmarks when changing or removing a tag folder.
   nsresult AddSyncChangesForBookmarksInFolder(int64_t aFolderId,
                                               int64_t aSyncChangeDelta);
 
   // Inserts a tombstone for a removed synced item.
   nsresult InsertTombstone(const BookmarkData& aBookmark);
 
   // Inserts tombstones for removed synced items.
-  nsresult InsertTombstones(const nsTArray<TombstoneData>& aTombstones);
+  nsresult InsertTombstones(const nsTArray<mozilla::places::TombstoneData>& aTombstones);
 
   // Removes a stale synced bookmark tombstone.
   nsresult RemoveTombstone(const nsACString& aGUID);
 
   nsresult SetItemTitleInternal(BookmarkData& aBookmark,
                                 const nsACString& aTitle,
                                 int64_t aSyncChangeDelta);
 
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -924,8 +924,32 @@ function mapItemIdToInternalRootName(aIt
       return "toolbarFolder";
     case PlacesUtils.unfiledBookmarksFolderId:
       return "unfiledBookmarksFolder";
     case PlacesUtils.mobileFolderId:
       return "mobileFolder";
   }
   return null;
 }
+
+const DB_FILENAME = "places.sqlite";
+
+/**
+ * Sets the database to use for the given test.  This should be the very first
+ * thing in the test, otherwise this database will not be used!
+ *
+ * @param aFileName
+ *        The filename of the database to use.  This database must exist in
+ *        toolkit/components/places/tests/migration!
+ * @return {Promise}
+ */
+var setupPlacesDatabase = async function(aFileName, aDestFileName = DB_FILENAME) {
+  let currentDir = await OS.File.getCurrentDirectory();
+
+  let src = OS.Path.join(currentDir, aFileName);
+  Assert.ok((await OS.File.exists(src)), "Database file found");
+
+  // Ensure that our database doesn't already exist.
+  let dest = OS.Path.join(OS.Constants.Path.profileDir, aDestFileName);
+  Assert.ok(!(await OS.File.exists(dest)), "Database file should not exist yet");
+
+  await OS.File.copy(src, dest);
+};
--- a/toolkit/components/places/tests/migration/head_migration.js
+++ b/toolkit/components/places/tests/migration/head_migration.js
@@ -9,34 +9,8 @@ ChromeUtils.import("resource://gre/modul
 {
   /* import-globals-from ../head_common.js */
   let commonFile = do_get_file("../head_common.js", false);
   let uri = Services.io.newFileURI(commonFile);
   Services.scriptloader.loadSubScript(uri.spec, this);
 }
 
 // Put any other stuff relative to this test folder below.
-
-const DB_FILENAME = "places.sqlite";
-
-/**
- * Sets the database to use for the given test.  This should be the very first
- * thing in the test, otherwise this database will not be used!
- *
- * @param aFileName
- *        The filename of the database to use.  This database must exist in
- *        toolkit/components/places/tests/migration!
- * @return {Promise}
- */
-var setupPlacesDatabase = async function(aFileName, aDestFileName = DB_FILENAME) {
-  let currentDir = await OS.File.getCurrentDirectory();
-
-  let src = OS.Path.join(currentDir, aFileName);
-  Assert.ok((await OS.File.exists(src)), "Database file found");
-
-  // Ensure that our database doesn't already exist.
-  let dest = OS.Path.join(OS.Constants.Path.profileDir, aDestFileName);
-  Assert.ok(!(await OS.File.exists(dest)), "Database file should not exist yet");
-
-  await OS.File.copy(src, dest);
-};
-
-// This works provided all tests in this folder use add_task.
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_missing_builtin_folders.js
@@ -0,0 +1,92 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * This file tests that a missing built-in folders (child of root) are correctly
+ * fixed when the database is loaded.
+ */
+
+const ALL_ROOT_GUIDS = [
+  PlacesUtils.bookmarks.menuGuid,
+  PlacesUtils.bookmarks.unfiledGuid,
+  PlacesUtils.bookmarks.toolbarGuid,
+  PlacesUtils.bookmarks.tagsGuid,
+  PlacesUtils.bookmarks.mobileGuid,
+];
+
+const INITIAL_ROOT_GUIDS = [
+  PlacesUtils.bookmarks.menuGuid,
+  PlacesUtils.bookmarks.tagsGuid,
+  PlacesUtils.bookmarks.unfiledGuid,
+];
+
+add_task(async function setup() {
+  // This file has the toolbar and mobile folders missing.
+  await setupPlacesDatabase("missingBuiltIn.sqlite");
+
+  // Check database contents to be migrated.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, DB_FILENAME);
+  let db = await Sqlite.openConnection({ path });
+
+  let rows = await db.execute(`
+    SELECT guid FROM moz_bookmarks
+    WHERE parent = (SELECT id from moz_bookmarks WHERE guid = :guid)
+  `, {
+    guid: PlacesUtils.bookmarks.rootGuid
+  });
+
+  let guids = rows.map(row => row.getResultByName("guid"));
+  Assert.deepEqual(guids, INITIAL_ROOT_GUIDS,
+    "Initial database should have only the expected GUIDs");
+
+  await db.close();
+});
+
+add_task(async function test_database_recreates_roots() {
+  Assert.ok(PlacesUtils.history.databaseStatus == PlacesUtils.history.DATABASE_STATUS_OK ||
+    PlacesUtils.history.databaseStatus == PlacesUtils.history.DATABASE_STATUS_UPGRADED,
+    "Should successfully access the database for the first time");
+
+  let rootId = PlacesUtils.placesRootId;
+  Assert.greaterOrEqual(rootId, 0, "Should have a valid root Id");
+
+  let db = await PlacesUtils.promiseDBConnection();
+
+  for (let guid of ALL_ROOT_GUIDS) {
+    let rows = await db.execute(`
+      SELECT id, parent FROM moz_bookmarks
+      WHERE guid = :guid
+    `, {guid});
+
+    Assert.equal(rows.length, 1, "Should have exactly one row for the root");
+
+    Assert.equal(rows[0].getResultByName("parent"), rootId,
+      "Should have been created with the correct parent");
+
+    let root = await PlacesUtils.bookmarks.fetch(guid);
+
+    Assert.equal(root.guid, guid, "GUIDs should match");
+    Assert.equal(root.parentGuid, PlacesUtils.bookmarks.rootGuid,
+      "Should have the correct parent GUID");
+    Assert.equal(root.type, PlacesUtils.bookmarks.TYPE_FOLDER,
+      "Should have the correct type");
+
+    let id = rows[0].getResultByName("id");
+    Assert.equal(await PlacesUtils.promiseItemId(guid), id,
+      "Should return the correct id from promiseItemId");
+    Assert.equal(await PlacesUtils.promiseItemGuid(id), guid,
+      "Should return the correct guid from promiseItemGuid");
+  }
+
+  let rows = await db.execute(`
+    SELECT 1 FROM moz_bookmarks
+    WHERE parent = (SELECT id from moz_bookmarks WHERE guid = :guid)
+  `, {
+    guid: PlacesUtils.bookmarks.rootGuid
+  });
+
+  Assert.equal(rows.length, ALL_ROOT_GUIDS.length,
+    "Root folder should have the expected number of children");
+});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_missing_root_folder.js
@@ -0,0 +1,81 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * This file tests that a missing root folder is correctly fixed when the
+ * database is loaded.
+ */
+
+const ALL_ROOT_GUIDS = [
+  PlacesUtils.bookmarks.menuGuid,
+  PlacesUtils.bookmarks.unfiledGuid,
+  PlacesUtils.bookmarks.toolbarGuid,
+  PlacesUtils.bookmarks.tagsGuid,
+  PlacesUtils.bookmarks.mobileGuid,
+];
+
+add_task(async function setup() {
+  // This file has no root folder.
+  await setupPlacesDatabase("noRoot.sqlite");
+
+  // Check database contents to be migrated.
+  let path = OS.Path.join(OS.Constants.Path.profileDir, DB_FILENAME);
+  let db = await Sqlite.openConnection({ path });
+
+  let rows = await db.execute(`
+    SELECT guid FROM moz_bookmarks
+    WHERE guid = :guid
+  `, {
+    guid: PlacesUtils.bookmarks.rootGuid
+  });
+
+  Assert.equal(rows.length, 0, "Root folder should not exist");
+
+  await db.close();
+});
+
+add_task(async function test_database_recreates_roots() {
+  Assert.ok(PlacesUtils.history.databaseStatus == PlacesUtils.history.DATABASE_STATUS_OK ||
+    PlacesUtils.history.databaseStatus == PlacesUtils.history.DATABASE_STATUS_UPGRADED,
+    "Should successfully access the database for the first time");
+
+  let db = await PlacesUtils.promiseDBConnection();
+
+  let rows = await db.execute(`
+    SELECT id, parent, type FROM moz_bookmarks
+    WHERE guid = :guid
+  `, {guid: PlacesUtils.bookmarks.rootGuid});
+
+  Assert.equal(rows.length, 1, "Should have added exactly one root");
+  Assert.greaterOrEqual(rows[0].getResultByName("id"), 1,
+    "Should have a valid root Id");
+  Assert.equal(rows[0].getResultByName("parent"), 0,
+    "Should have a parent of id 0");
+  Assert.equal(rows[0].getResultByName("type"), PlacesUtils.bookmarks.TYPE_FOLDER,
+    "Should have a type of folder");
+
+  let id = rows[0].getResultByName("id");
+  Assert.equal(await PlacesUtils.promiseItemId(PlacesUtils.bookmarks.rootGuid), id,
+    "Should return the correct id from promiseItemId");
+  Assert.equal(await PlacesUtils.promiseItemGuid(id), PlacesUtils.bookmarks.rootGuid,
+    "Should return the correct guid from promiseItemGuid");
+
+  // Note: Currently we do not fix the parent of the folders on initial startup.
+  // There is a maintenance task that will do it, hence we don't check the parents
+  // here, just that the built-in folders correctly exist and haven't been
+  // duplicated.
+  for (let guid of ALL_ROOT_GUIDS) {
+    rows = await db.execute(`
+      SELECT id FROM moz_bookmarks
+      WHERE guid = :guid
+    `, {guid});
+
+    Assert.equal(rows.length, 1, "Should have exactly one row for the root");
+
+    let root = await PlacesUtils.bookmarks.fetch(guid);
+
+    Assert.equal(root.guid, guid, "GUIDs should match");
+  }
+});
--- a/toolkit/components/places/tests/unit/xpcshell.ini
+++ b/toolkit/components/places/tests/unit/xpcshell.ini
@@ -80,16 +80,20 @@ skip-if = (os == "win" && os_version == 
 [test_import_mobile_bookmarks.js]
 [test_isPageInDB.js]
 [test_isURIVisited.js]
 [test_isvisited.js]
 [test_keywords.js]
 [test_lastModified.js]
 [test_markpageas.js]
 [test_metadata.js]
+[test_missing_builtin_folders.js]
+support-files = missingBuiltIn.sqlite
+[test_missing_root_folder.js]
+support-files = noRoot.sqlite
 [test_mozIAsyncLivemarks.js]
 [test_multi_word_tags.js]
 [test_nsINavHistoryViewer.js]
 [test_null_interfaces.js]
 [test_onItemChanged_tags.js]
 [test_pageGuid_bookmarkGuid.js]
 [test_frecency_observers.js]
 [test_placeURIs.js]
author	Mark Banner <standard8@mozilla.com>
	Wed, 18 Apr 2018 18:29:19 +0100
changeset 785809	cae78c106b8cf2413a55ed0e60682df3ab8fedfa
parent 785808	0e60d3c4dc2bde877a963ecb37e1b246bf9a4825
child 785810	55d623fc9ff610f35b8de79a2ff0c80bdf6990c7
push id	107311
push user	bmo:standard8@mozilla.com
push date	Fri, 20 Apr 2018 16:56:49 +0000
reviewers	mak
bugs	478035
milestone	61.0a1
toolkit/components/places/Database.cpp		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/Database.h		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/nsNavBookmarks.h		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/head_common.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/migration/head_migration.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/unit/missingBuiltIn.sqlite		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/unit/noRoot.sqlite		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/unit/test_missing_builtin_folders.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/unit/test_missing_root_folder.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/places/tests/unit/xpcshell.ini		file \| annotate \| diff \| comparison \| revisions