Bug 1525297: Don't use empty profiles for dedicated profiles and don't show the welcome page in this case. r=froydnj

Using an old default profile that is empty (like from bug 1518591) causes us to
also show the user a welcome page when that isn't necessary.

Instead leave old empty profiles alone (older versions will use them as the
default), create a new profile but don't set the flag to say that the old
default was skipped.

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

--HG--
extra : moz-landing-system : lando

toolkit/profile/nsToolkitProfileService.cpp

@@ -51,6 +51,7 @@ using namespace mozilla;
 
 #define DEV_EDITION_NAME "dev-edition-default"
 #define DEFAULT_NAME "default"
+#define COMPAT_FILE NS_LITERAL_STRING("compatibility.ini")
 
 nsToolkitProfile::nsToolkitProfile(const nsACString& aName, nsIFile* aRootDir,
                                    nsIFile* aLocalDir, nsToolkitProfile* aPrev)
@@ -351,17 +352,12 @@ bool nsToolkitProfileService::IsProfileForCurrentInstall(
   rv = profileDir->Clone(getter_AddRefs(compatFile));
   NS_ENSURE_SUCCESS(rv, false);
 
-  rv = compatFile->Append(NS_LITERAL_STRING("compatibility.ini"));
+  rv = compatFile->Append(COMPAT_FILE);
   NS_ENSURE_SUCCESS(rv, false);
 
   nsINIParser compatData;
   rv = compatData.Init(compatFile);
-  // If the file is missing then either this is an empty profile (likely
-  // generated by bug 1518591) or it is from an ancient version. We'll opt to
-  // use it in this case.
-  if (NS_FAILED(rv)) {
-    return true;
-  }
+  NS_ENSURE_SUCCESS(rv, false);
 
   /**
    * In xpcshell gDirServiceProvider doesn't have all the correct directories
@@ -1046,21 +1042,38 @@ nsresult nsToolkitProfileService::SelectStartupProfile(
         profile = mDevEditionDefault;
       }
 
-      if (profile && MaybeMakeDefaultDedicatedProfile(profile)) {
-        mStartupReason = NS_LITERAL_STRING("firstrun-claimed-default");
+      if (profile) {
+        nsCOMPtr<nsIFile> rootDir;
+        profile->GetRootDir(getter_AddRefs(rootDir));
 
-        mCurrent = profile;
-        profile->GetRootDir(aRootDir);
-        profile->GetLocalDir(aLocalDir);
-        profile.forget(aProfile);
-        return NS_OK;
+        nsCOMPtr<nsIFile> compat;
+        rootDir->Clone(getter_AddRefs(compat));
+        compat->Append(COMPAT_FILE);
+
+        bool exists;
+        rv = compat->Exists(&exists);
+
+        // If the file is missing then either this is an empty profile (likely
+        // generated by bug 1518591) or it is from an ancient version. We'll opt
+        // to leave it for older versions in this case.
+        if (exists) {
+          if (MaybeMakeDefaultDedicatedProfile(profile)) {
+            mStartupReason = NS_LITERAL_STRING("firstrun-claimed-default");
+
+            mCurrent = profile;
+            rootDir.forget(aRootDir);
+            profile->GetLocalDir(aLocalDir);
+            profile.forget(aProfile);
+            return NS_OK;
+          }
+
+          // We're going to create a new profile for this install. If there was
+          // a potential previous default to use then the user may be confused
+          // over why we're not using that anymore so set a flag for the front
+          // end to use to notify the user about what has happened.
+          mCreatedAlternateProfile = true;
+        }
       }
-
-      // We're going to create a new profile for this install. If there was a
-      // potential previous default to use then the user may be confused over
-      // why we're not using that anymore so set a flag for the front-end to use
-      // to notify the user about what has happened.
-      mCreatedAlternateProfile = !!profile;
     }
 
     // Create a new default profile

toolkit/profile/xpcshell/test_update_unknown_dedicated.js

@@ -1,6 +1,7 @@
 /*
- * Tests that an old-style default profile not previously used by any build gets
- * updated to a dedicated profile for this build.
+ * Tests that an old-style default profile not previously used by any build
+ * doesn't get updated to a dedicated profile for this build and we don't set
+ * the flag to show the user info about dedicated profiles.
  */
 
 add_task(async () => {
@@ -17,27 +18,36 @@ add_task(async () => {
 
   let service = getProfileService();
   let { profile: selectedProfile, didCreate } = selectStartupProfile();
-  checkStartupReason("firstrun-claimed-default");
+  checkStartupReason("firstrun-created-default");
 
   let profileData = readProfilesIni();
   let installData = readInstallsIni();
 
   Assert.ok(profileData.options.startWithLastProfile, "Should be set to start with the last profile.");
-  Assert.equal(profileData.profiles.length, 1, "Should have the right number of profiles.");
+  Assert.equal(profileData.profiles.length, 2, "Should have the right number of profiles.");
+
+  // The name ordering is different for dev edition.
+  if (AppConstants.MOZ_DEV_EDITION) {
+    profileData.profiles.reverse();
+  }
 
   let profile = profileData.profiles[0];
   Assert.equal(profile.name, PROFILE_DEFAULT, "Should have the right name.");
   Assert.equal(profile.path, defaultProfile.leafName, "Should be the original default profile.");
   Assert.ok(profile.default, "Should be marked as the old-style default.");
+  profile = profileData.profiles[1];
+  Assert.equal(profile.name, DEDICATED_NAME, "Should have the right name.");
+  Assert.notEqual(profile.path, defaultProfile.leafName, "Should not be the original default profile.");
+  Assert.ok(!profile.default, "Should not be marked as the old-style default.");
 
   Assert.equal(Object.keys(installData.installs).length, 1, "Should be a default for installs.");
   Assert.equal(installData.installs[hash].default, profile.path, "Should have the right default profile.");
-  Assert.ok(!installData.installs[hash].locked, "Should not have locked as we didn't create this profile for this install.");
+  Assert.ok(installData.installs[hash].locked, "Should have locked as we created this profile for this install.");
 
   checkProfileService(profileData, installData);
 
-  Assert.ok(!didCreate, "Should not have created a new profile.");
+  Assert.ok(didCreate, "Should have created a new profile.");
   Assert.ok(!service.createdAlternateProfile, "Should not have created an alternate profile.");
-  Assert.ok(selectedProfile.rootDir.equals(defaultProfile), "Should be using the right directory.");
-  Assert.equal(selectedProfile.name, PROFILE_DEFAULT);
+  Assert.ok(!selectedProfile.rootDir.equals(defaultProfile), "Should not be using the old directory.");
+  Assert.equal(selectedProfile.name, DEDICATED_NAME);
 });