Bug 1962531: Re-create the original profile or reset state if the profiles created pref is accidentally set. r=profiles-reviewers,jhirsch

Bug 1953884 made the storeID pref always set and used the presence of storeID in profiles.ini to
determine whether profiles had previously been created. However it missed the startup code in
nsToolkitProfileService which always sets the storeID in profiles.ini if there is one set in prefs.
This means that users who had never used the profiles feature had the profiles created pref set to
true but no entry for the current profile was added to the database. This causes bustage when we
attempt to display various parts of the UI.

This adds a check to a point in startup when we know we should have found an entry for the current
profile in the database. If one doesn't exist we either create one if there are other profiles in
the database or we reset the prefs and profiles.ini to what they should look like if no profiles
have ever been created.

This should also resolve bug 1943559.

Differential Revision: https://phabricator.services.mozilla.com/D246775

browser/components/profiles/SelectableProfileService.sys.mjs

@@ -253,11 +253,13 @@ class SelectableProfileServiceClass extends EventEmitter {
    * At startup, store the nsToolkitProfile for the group.
    * Get the groupDBPath from the nsToolkitProfile, and connect to it.
    *
+   * @param {boolean} isInitial true if this is an init prior to creating a new profile.
+   *
    * @returns {Promise}
    */
-  init() {
+  init(isInitial = false) {
     if (!this.#initPromise) {
-      this.#initPromise = this.#init().finally(
+      this.#initPromise = this.#init(isInitial).finally(
         () => (this.#initPromise = null)
       );
     }
@@ -265,7 +267,7 @@ class SelectableProfileServiceClass extends EventEmitter {
     return this.#initPromise;
   }
 
-  async #init() {
+  async #init(isInitial = false) {
     if (this.#initialized) {
       return;
     }
@@ -292,15 +294,6 @@ class SelectableProfileServiceClass extends EventEmitter {
       return;
     }
 
-    // This can happen if profiles.ini has been reset by a version of Firefox
-    // prior to 67 and the current profile is not the current default for the
-    // group. We can recover by overwriting this.#groupToolkitProfile.storeID
-    // with the current storeID.
-    if (this.#groupToolkitProfile.storeID != this.storeID) {
-      this.#groupToolkitProfile.storeID = this.storeID;
-      this.#attemptFlushProfileService();
-    }
-
     // When we launch into the startup window, the `ProfD` is not defined so
     // getting the directory will throw. Leaving the `currentProfile` as null
     // is fine for the startup window.
@@ -312,6 +305,38 @@ class SelectableProfileServiceClass extends EventEmitter {
       );
     } catch {}
 
+    // If this isn't the first init prior to creating the first new profile and
+    // the app is started up we should have found a current profile.
+    if (!isInitial && !Services.startup.startingUp && !this.#currentProfile) {
+      let count = await this.getProfileCount();
+
+      if (count) {
+        // There are other profiles, re-create the current profile.
+        this.#currentProfile = await this.#createProfile(
+          ProfilesDatastoreService.constructor.getDirectory("ProfD")
+        );
+      } else {
+        // No other profiles. Reset our state.
+        this.#groupToolkitProfile.storeID = null;
+        this.#attemptFlushProfileService();
+        Services.prefs.setBoolPref(PROFILES_CREATED_PREF_NAME, false);
+
+        this.#connection = null;
+        this.updateEnabledState();
+
+        return;
+      }
+    }
+
+    // This can happen if profiles.ini has been reset by a version of Firefox
+    // prior to 67 and the current profile is not the current default for the
+    // group. We can recover by overwriting this.#groupToolkitProfile.storeID
+    // with the current storeID.
+    if (this.#groupToolkitProfile.storeID != this.storeID) {
+      this.#groupToolkitProfile.storeID = this.storeID;
+      this.#attemptFlushProfileService();
+    }
+
     // On macOS when other applications request we open a url the most recent
     // window becomes activated first. This would cause the default profile to
     // change before we determine which profile to open the url in. By
@@ -1041,7 +1066,7 @@ class SelectableProfileServiceClass extends EventEmitter {
     }
 
     await this.initProfilesData();
-    await this.init();
+    await this.init(true);
 
     await this.flushAllSharedPrefsToDatabase();
 

browser/components/profiles/tests/unit/test_recover_database.js

@@ -0,0 +1,67 @@
+/* Any copyright is dedicated to the Public Domain.
+https://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// If we think profiles have been created but there is no current profile in the database but there
+// are other profiles we should create the current profile entry.
+add_task(async function test_recover_database() {
+  startProfileService();
+
+  const SelectableProfileService = getSelectableProfileService();
+  const ProfilesDatastoreService = getProfilesDatastoreService();
+
+  await ProfilesDatastoreService.init();
+
+  let db = await ProfilesDatastoreService.getConnection();
+
+  let rootDir = getProfileService().currentProfile.rootDir.clone();
+  rootDir.leafName = "other";
+  let otherPath = getRelativeProfilePath(rootDir);
+
+  // Inject some other profile into the database
+  await db.execute(
+    `INSERT INTO Profiles VALUES (NULL, :path, :name, :avatar, :themeId, :themeFg, :themeBg);`,
+    {
+      path: otherPath,
+      name: "Fake Profile",
+      avatar: "book",
+      themeId: "default",
+      themeFg: "",
+      themeBg: "",
+    }
+  );
+
+  let toolkitProfile = getProfileService().currentProfile;
+  toolkitProfile.storeID = await ProfilesDatastoreService.storeID;
+
+  Services.prefs.setBoolPref("browser.profiles.enabled", true);
+  Services.prefs.setBoolPref("browser.profiles.created", true);
+  await SelectableProfileService.init();
+
+  Assert.ok(SelectableProfileService.isEnabled, "Service should be enabled");
+
+  Assert.ok(
+    Services.prefs.getBoolPref("browser.profiles.created", false),
+    "Should have kept the profile created state."
+  );
+
+  Assert.equal(
+    toolkitProfile.storeID,
+    await ProfilesDatastoreService.storeID,
+    "Should not have cleared the store ID"
+  );
+  Assert.ok(
+    SelectableProfileService.currentProfile,
+    "Should have created the current profile"
+  );
+
+  let profiles = await SelectableProfileService.getAllProfiles();
+  Assert.equal(profiles.length, 2, "Should be two profiles in the database");
+
+  let newProfile = await SelectableProfileService.createNewProfile(false);
+  Assert.ok(newProfile, "Should have created a new profile");
+
+  profiles = await SelectableProfileService.getAllProfiles();
+  Assert.equal(profiles.length, 3, "Should be three profiles in the database");
+});

browser/components/profiles/tests/unit/test_recover_empty_database.js

@@ -0,0 +1,58 @@
+/* Any copyright is dedicated to the Public Domain.
+https://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// If we think profiles have been created but there is no current profile in the database and there
+// are no other profiles we should reset state.
+add_task(async function test_recover_empty_database() {
+  startProfileService();
+
+  const SelectableProfileService = getSelectableProfileService();
+  const ProfilesDatastoreService = getProfilesDatastoreService();
+
+  await ProfilesDatastoreService.init();
+
+  let toolkitProfile = getProfileService().currentProfile;
+  toolkitProfile.storeID = await ProfilesDatastoreService.storeID;
+
+  Services.prefs.setBoolPref("browser.profiles.enabled", true);
+  Services.prefs.setBoolPref("browser.profiles.created", true);
+  await SelectableProfileService.init();
+
+  Assert.ok(SelectableProfileService.isEnabled, "Service should be enabled");
+
+  Assert.ok(
+    !Services.prefs.getBoolPref("browser.profiles.created", false),
+    "Should have reset the profile created state."
+  );
+
+  Assert.ok(!toolkitProfile.storeID, "Should have cleared the store ID");
+  Assert.ok(
+    !SelectableProfileService.currentProfile,
+    "Should be no current profile"
+  );
+
+  let profiles = await SelectableProfileService.getAllProfiles();
+  Assert.ok(!profiles.length, "No selectable profiles exist yet");
+
+  let newProfile = await SelectableProfileService.createNewProfile(false);
+  Assert.ok(newProfile, "Should have created a new profile");
+  Assert.ok(
+    Services.prefs.getBoolPref("browser.profiles.created", false),
+    "Should have set the profile created state."
+  );
+  Assert.equal(
+    toolkitProfile.storeID,
+    await ProfilesDatastoreService.storeID,
+    "Should have set the store ID"
+  );
+
+  Assert.ok(
+    SelectableProfileService.currentProfile,
+    "Should have created a current profile entry"
+  );
+
+  profiles = await SelectableProfileService.getAllProfiles();
+  Assert.equal(profiles.length, 2, "Two profiles should exist in the database");
+});

browser/components/profiles/tests/unit/xpcshell.toml

@@ -11,6 +11,10 @@ prefs = [
 
 ["test_fail_recover_storeID.js"]
 
+["test_recover_database.js"]
+
+["test_recover_empty_database.js"]
+
 ["test_recover_storeID.js"]
 
 ["test_selectable_profile_launch.js"]

toolkit/profile/nsIToolkitProfileService.idl

@@ -53,7 +53,8 @@ interface nsIToolkitProfileService : nsISupports
     [infallible] readonly attribute nsIToolkitProfile currentProfile;
 
     /**
-     * The single named profile in a profile group.
+     * The single named profile in a profile group. This is only usable after
+     * startup has complete.
      *
      * If the current named profile has a StoreID, this is the current profile.
      * If the current profile is unnamed and has a StoreID, this will be the

toolkit/profile/nsToolkitProfileService.cpp

@@ -686,35 +686,22 @@ void nsToolkitProfileService::CompleteStartup() {
   nsresult rv;
   bool needsFlush = false;
 
-  // If we started into an unmanaged profile in a profile group, set the group
-  // profile to be the managed profile belonging to the group.
   nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
-  if (!mCurrent) {
   nsCString storeID;
   rv = prefs->GetCharPref(STORE_ID_PREF, storeID);
+
   if (NS_SUCCEEDED(rv) && !storeID.IsEmpty()) {
+    // We have a storeID from prefs.
+    if (!mCurrent) {
+      // We started into an unmanaged profile. Try to set the group profile to
+      // be the managed profile belonging to the group.
       mGroupProfile = GetProfileByStoreID(storeID);
     }
-  } else {
-    // Otherwise, if the current profile has a storeID, then it must be the
-    // profile for some group.
-    if (!mCurrent->mStoreID.IsVoid()) {
+  } else if (mCurrent && !mCurrent->mStoreID.IsVoid()) {
+    // No store ID in prefs. If the current profile has one we will use it.
     mGroupProfile = mCurrent;
     rv = prefs->SetCharPref(STORE_ID_PREF, mCurrent->mStoreID);
     NS_ENSURE_SUCCESS_VOID(rv);
-    } else {
-      // Otherwise if the current profile has a store ID set in prefs but not in
-      // the database then restore it. This can happen if a version of Firefox
-      // prior to 67 has overwritten the database.
-      nsCString storeID;
-      rv = prefs->GetCharPref(STORE_ID_PREF, storeID);
-      if (NS_SUCCEEDED(rv) && !storeID.IsEmpty()) {
-        rv = mCurrent->SetStoreID(storeID);
-        if (NS_SUCCEEDED(rv)) {
-          needsFlush = true;
-        }
-      }
-    }
   }
 
   if (mMaybeLockProfile) {

toolkit/profile/test/xpcshell/test_find_groupProfile.js

@@ -0,0 +1,45 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Tests that the group profile is correctly found from the store ID.
+ */
+add_task(
+  {
+    skip_if: () => !AppConstants.MOZ_SELECTABLE_PROFILES,
+  },
+  async () => {
+    let hash = xreDirProvider.getInstallHash();
+    let defaultProfile = makeRandomProfileDir("default");
+    let otherProfile = makeRandomProfileDir("other");
+    let profilesIni = {
+      profiles: [
+        {
+          name: "default",
+          path: defaultProfile.leafName,
+          storeID: "bishbashbosh",
+          default: true,
+        },
+      ],
+      installs: {
+        [hash]: {
+          default: defaultProfile.leafName,
+        },
+      },
+    };
+    writeProfilesIni(profilesIni);
+
+    Services.prefs.setCharPref("toolkit.profiles.storeID", "bishbashbosh");
+
+    let service = getProfileService();
+    let { profile } = selectStartupProfile(["-profile", otherProfile.path]);
+
+    Assert.ok(!profile);
+    Assert.ok(!service.currentProfile);
+    Assert.ok(service.groupProfile);
+    Assert.equal(service.groupProfile.storeID, "bishbashbosh");
+    Assert.equal(service.groupProfile.rootDir.path, defaultProfile.path);
+
+    checkProfileService();
+  }
+);

toolkit/profile/test/xpcshell/test_ignore_storeID_pref.js

@@ -0,0 +1,42 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/*
+ * Tests that the StoreID in prefs isn't set in profiles.ini.
+ */
+add_task(
+  {
+    skip_if: () => !AppConstants.MOZ_SELECTABLE_PROFILES,
+  },
+  async () => {
+    let hash = xreDirProvider.getInstallHash();
+    let defaultProfile = makeRandomProfileDir("default");
+    let profilesIni = {
+      profiles: [
+        {
+          name: "default",
+          path: defaultProfile.leafName,
+          default: true,
+        },
+      ],
+      installs: {
+        [hash]: {
+          default: defaultProfile.leafName,
+        },
+      },
+    };
+    writeProfilesIni(profilesIni);
+
+    Services.prefs.setCharPref("toolkit.profiles.storeID", "bishbashbosh");
+
+    let service = getProfileService();
+    let { profile } = selectStartupProfile();
+
+    Assert.equal(profile.rootDir.path, defaultProfile.path);
+    Assert.equal(service.currentProfile, profile);
+    Assert.ok(!service.groupProfile);
+    Assert.ok(!profile.storeID);
+
+    checkProfileService();
+  }
+);

toolkit/profile/test/xpcshell/test_restore_storeID.js

@@ -2,7 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /*
- * Tests that the StoreID is restored if available in prefs.
+ * Tests that the StoreID is restored if in profiles.ini.
  */
 add_task(
   {
@@ -16,6 +16,7 @@ add_task(
         {
           name: "default",
           path: defaultProfile.leafName,
+          storeID: "bishbashbosh",
           default: true,
         },
       ],
@@ -27,15 +28,16 @@ add_task(
     };
     writeProfilesIni(profilesIni);
 
-    Services.prefs.setCharPref("toolkit.profiles.storeID", "bishbashbosh");
-
     let service = getProfileService();
     let { profile } = selectStartupProfile();
 
+    let storeID = Services.prefs.getCharPref("toolkit.profiles.storeID");
+
     Assert.equal(profile.rootDir.path, defaultProfile.path);
     Assert.equal(service.currentProfile, profile);
     Assert.equal(service.groupProfile, profile);
     Assert.equal(profile.storeID, "bishbashbosh");
+    Assert.equal(storeID, "bishbashbosh");
 
     checkProfileService();
   }

toolkit/profile/test/xpcshell/xpcshell.toml

@@ -18,10 +18,14 @@ skip-if = ["os == 'android'"]
 
 ["test_delete_dirs.js"]
 
+["test_find_groupProfile.js"]
+
 ["test_fix_directory_case.js"]
 
 ["test_ignore_legacy_directory.js"]
 
+["test_ignore_storeID_pref.js"]
+
 ["test_invalid_descriptor.js"]
 
 ["test_legacy_empty.js"]