Bug 1525297: Don't use empty profiles for dedicated profiles and don't show the welcome page in this case. r=froydnj
authorDave Townsend <dtownsend@oxymoronical.com>
Mon, 11 Feb 2019 15:29:34 +0000
changeset 458514 c699c4e694a18f9db9b4b25c494babf3502c67a6
parent 458513 72d476dc74875282f2411cc7817cc9627646259e
child 458515 211b6d23cac0f3e3edff9c7ef83e8b43a2c5b2b7
push id77883
push userdtownsend@mozilla.com
push dateMon, 11 Feb 2019 15:57:44 +0000
treeherderautoland@c699c4e694a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1525297, 1518591
milestone67.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 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
toolkit/profile/nsToolkitProfileService.cpp
toolkit/profile/xpcshell/test_update_unknown_dedicated.js
--- a/toolkit/profile/nsToolkitProfileService.cpp
+++ b/toolkit/profile/nsToolkitProfileService.cpp
@@ -46,16 +46,17 @@
 #include "mozilla/UniquePtr.h"
 #include "nsIToolkitShellService.h"
 #include "mozilla/Telemetry.h"
 
 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)
     : mPrev(aPrev),
       mName(aName),
       mRootDir(aRootDir),
       mLocalDir(aLocalDir),
       mLock(nullptr) {
@@ -346,27 +347,22 @@ bool nsToolkitProfileService::IsProfileF
   nsCOMPtr<nsIFile> profileDir;
   nsresult rv = aProfile->GetRootDir(getter_AddRefs(profileDir));
   NS_ENSURE_SUCCESS(rv, false);
 
   nsCOMPtr<nsIFile> compatFile;
   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
    * set so using NS_GetSpecialDirectory works better there. But in a normal
    * app launch the component registry isn't initialized so
    * NS_GetSpecialDirectory doesn't work. So we have to use two different
    * paths to support testing.
    */
@@ -1041,31 +1037,48 @@ nsresult nsToolkitProfileService::Select
       // installs or to create a new profile.
 
       // Find what would have been the default profile for old installs.
       nsCOMPtr<nsIToolkitProfile> profile = mNormalDefault;
       if (mUseDevEditionProfile) {
         profile = mDevEditionDefault;
       }
 
-      if (profile && MaybeMakeDefaultDedicatedProfile(profile)) {
-        mStartupReason = NS_LITERAL_STRING("firstrun-claimed-default");
+      if (profile) {
+        nsCOMPtr<nsIFile> rootDir;
+        profile->GetRootDir(getter_AddRefs(rootDir));
+
+        nsCOMPtr<nsIFile> compat;
+        rootDir->Clone(getter_AddRefs(compat));
+        compat->Append(COMPAT_FILE);
+
+        bool exists;
+        rv = compat->Exists(&exists);
 
-        mCurrent = profile;
-        profile->GetRootDir(aRootDir);
-        profile->GetLocalDir(aLocalDir);
-        profile.forget(aProfile);
-        return NS_OK;
+        // 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
     nsAutoCString name;
     if (mUseDedicatedProfile) {
       name.AssignLiteral("default-" NS_STRINGIFY(MOZ_UPDATE_CHANNEL));
     } else if (mUseDevEditionProfile) {
       name.AssignLiteral(DEV_EDITION_NAME);
--- a/toolkit/profile/xpcshell/test_update_unknown_dedicated.js
+++ b/toolkit/profile/xpcshell/test_update_unknown_dedicated.js
@@ -1,43 +1,53 @@
 /*
- * 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 () => {
   let hash = xreDirProvider.getInstallHash();
   let defaultProfile = makeRandomProfileDir("default");
 
   writeProfilesIni({
     profiles: [{
       name: PROFILE_DEFAULT,
       path: defaultProfile.leafName,
       default: true,
     }],
   });
 
   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);
 });