Bug 1518639: Move profile locking outside of SelectProfile. r=froydnj
authorDave Townsend <dtownsend@oxymoronical.com>
Tue, 12 Feb 2019 20:14:54 +0000
changeset 458800 1acf0670a51063d1230ba5efc3492e9fdf6d30cb
parent 458799 63fbf50615f9519be64418f8465b7444701d2c46
child 458801 0359a76f3fc79c7625471945a414fddc8f84abe0
push id111893
push useropoprus@mozilla.com
push dateWed, 13 Feb 2019 10:39:30 +0000
treeherdermozilla-inbound@1aaea0fdd040 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1518639
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 1518639: Move profile locking outside of SelectProfile. r=froydnj So the remoting clients can know what the selected profile is before an attempt to lock it is made we move the locking code to after the call to SelectProfile. Differential Revision: https://phabricator.services.mozilla.com/D19420
toolkit/xre/nsAppRunner.cpp
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -2071,32 +2071,32 @@ static nsresult LockProfile(nsINativeApp
     }
     PR_Sleep(kLockRetrySleepMS);
   } while (TimeStamp::Now() - start <
            TimeDuration::FromSeconds(kLockRetrySeconds));
 
   return ProfileLockedDialog(aRootDir, aLocalDir, unlocker, aNative, aResult);
 }
 
-// Pick a profile. We need to end up with a profile lock.
+// Pick a profile. We need to end up with a profile root dir, local dir and
+// potentially an nsIToolkitProfile instance.
 //
 // 1) check for --profile <path>
 // 2) check for -P <name>
 // 3) check for --ProfileManager
 // 4) use the default profile, if there is one
 // 5) if there are *no* profiles, set up profile-migration
 // 6) display the profile-manager UI
-static nsresult SelectProfile(nsIProfileLock** aResult,
-                              nsToolkitProfileService* aProfileSvc,
-                              nsINativeAppSupport* aNative,
-                              nsACString* aProfileName) {
+static nsresult SelectProfile(nsToolkitProfileService* aProfileSvc,
+                              nsINativeAppSupport* aNative, nsIFile** aRootDir,
+                              nsIFile** aLocalDir,
+                              nsIToolkitProfile** aProfile) {
   StartupTimeline::Record(StartupTimeline::SELECT_PROFILE);
 
   nsresult rv;
-  *aResult = nullptr;
 
   if (EnvHasValue("MOZ_RESET_PROFILE_RESTART")) {
     gDoProfileReset = true;
     gDoMigration = true;
     SaveToEnv("MOZ_RESET_PROFILE_RESTART=");
     // We only want to restore the previous session if the profile refresh was
     // triggered by user. And if it was a user-triggered profile refresh
     // through, say, the safeMode dialog or the troubleshooting page, the
@@ -2127,88 +2127,45 @@ static nsresult SelectProfile(nsIProfile
                "Error: argument --migration is invalid when argument --osint "
                "is specified\n");
     return NS_ERROR_FAILURE;
   }
   if (ar == ARG_FOUND) {
     gDoMigration = true;
   }
 
-  nsCOMPtr<nsIFile> rootDir;
-  nsCOMPtr<nsIFile> localDir;
-  nsCOMPtr<nsIToolkitProfile> profile;
   // Ask the profile manager to select the profile directories to use.
   bool didCreate = false;
-  rv = aProfileSvc->SelectStartupProfile(
-      &gArgc, gArgv, gDoProfileReset, getter_AddRefs(rootDir),
-      getter_AddRefs(localDir), getter_AddRefs(profile), &didCreate);
+  rv = aProfileSvc->SelectStartupProfile(&gArgc, gArgv, gDoProfileReset,
+      aRootDir, aLocalDir, aProfile, &didCreate);
 
   if (rv == NS_ERROR_SHOW_PROFILE_MANAGER) {
     return ShowProfileManager(aProfileSvc, aNative);
   }
 
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (didCreate) {
     // For a fresh install, we would like to let users decide
     // to do profile migration on their own later after using.
     gDoProfileReset = false;
     gDoMigration = false;
   }
 
-  if (gDoProfileReset && !profile) {
+  if (gDoProfileReset && !*aProfile) {
     NS_WARNING("Profile reset is only supported for named profiles.");
     return NS_ERROR_ABORT;
   }
 
   // No profile could be found. This generally shouldn't happen, a new profile
   // should be created in all cases except for profile reset which is covered
   // above, but just in case...
-  if (!rootDir) {
-    return NS_ERROR_FAILURE;
-  }
-
-  // We always want to lock the profile even if we're actually going to reset
-  // it later.
-  nsCOMPtr<nsIProfileLock> tempLock;
-  rv = LockProfile(aNative, rootDir, localDir, profile, getter_AddRefs(tempLock));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (gDoProfileReset) {
-    // Unlock the old profile
-    tempLock->Unlock();
-
-    // If we're resetting a profile, create a new one and use it to startup.
-    gResetOldProfile = profile;
-    rv = aProfileSvc->CreateResetProfile(getter_AddRefs(profile));
-    if (NS_SUCCEEDED(rv)) {
-      rv = profile->GetRootDir(getter_AddRefs(rootDir));
-      NS_ENSURE_SUCCESS(rv, rv);
-      SaveFileToEnv("XRE_PROFILE_PATH", rootDir);
-
-      rv = profile->GetLocalDir(getter_AddRefs(localDir));
-      NS_ENSURE_SUCCESS(rv, rv);
-      SaveFileToEnv("XRE_PROFILE_LOCAL_PATH", localDir);
-
-      // Lock the new profile
-      rv = LockProfile(aNative, rootDir, localDir, profile, aResult);
-      NS_ENSURE_SUCCESS(rv, rv);
-    } else {
-      NS_WARNING("Profile reset failed.");
-      return NS_ERROR_ABORT;
-    }
-  } else {
-    tempLock.forget(aResult);
-  }
-
-  if (aProfileName && profile) {
-    rv = profile->GetName(*aProfileName);
-    if (NS_FAILED(rv)) {
-      aProfileName->Truncate(0);
-    }
+  if (!*aRootDir) {
+    NS_WARNING("Failed to select or create profile.");
+    return NS_ERROR_ABORT;
   }
 
   return NS_OK;
 }
 
 #ifdef MOZ_BLOCK_PROFILE_DOWNGRADE
 struct FileWriteFunc : public JSONWriteFunc {
   FILE* mFile;
@@ -4167,36 +4124,81 @@ int XREMain::XRE_mainStartup(bool* aExit
                "your profile directory.\n");
   }
   if (NS_FAILED(rv)) {
     // We failed to choose or create profile - notify user and quit
     ProfileMissingDialog(mNativeApp);
     return 1;
   }
 
-  rv = SelectProfile(getter_AddRefs(mProfileLock), mProfileSvc, mNativeApp,
-                     &mProfileName);
+  nsCOMPtr<nsIToolkitProfile> profile;
+  rv = SelectProfile(mProfileSvc, mNativeApp, getter_AddRefs(mProfD),
+                     getter_AddRefs(mProfLD), getter_AddRefs(profile));
   if (rv == NS_ERROR_LAUNCHED_CHILD_PROCESS || rv == NS_ERROR_ABORT) {
     *aExitFlag = true;
     return 0;
   }
 
   if (NS_FAILED(rv)) {
     // We failed to choose or create profile - notify user and quit
     ProfileMissingDialog(mNativeApp);
     return 1;
   }
+
+  // We always want to lock the profile even if we're actually going to reset
+  // it later.
+  rv = LockProfile(mNativeApp, mProfD, mProfLD, profile,
+      getter_AddRefs(mProfileLock));
+  if (rv == NS_ERROR_LAUNCHED_CHILD_PROCESS || rv == NS_ERROR_ABORT) {
+    *aExitFlag = true;
+    return 0;
+  } else if (NS_FAILED(rv)) {
+    return 1;
+  }
+
+  if (gDoProfileReset) {
+    // Unlock the source profile.
+    mProfileLock->Unlock();
+
+    // If we're resetting a profile, create a new one and use it to startup.
+    gResetOldProfile = profile;
+    rv = mProfileSvc->CreateResetProfile(getter_AddRefs(profile));
+    if (NS_SUCCEEDED(rv)) {
+      rv = profile->GetRootDir(getter_AddRefs(mProfD));
+      NS_ENSURE_SUCCESS(rv, 1);
+      SaveFileToEnv("XRE_PROFILE_PATH", mProfD);
+
+      rv = profile->GetLocalDir(getter_AddRefs(mProfLD));
+      NS_ENSURE_SUCCESS(rv, 1);
+      SaveFileToEnv("XRE_PROFILE_LOCAL_PATH", mProfLD);
+
+      // Lock the new profile
+      rv = LockProfile(mNativeApp, mProfD, mProfLD, profile,
+                       getter_AddRefs(mProfileLock));
+      if (rv == NS_ERROR_LAUNCHED_CHILD_PROCESS || rv == NS_ERROR_ABORT) {
+        *aExitFlag = true;
+        return 0;
+      } else if (NS_FAILED(rv)) {
+        return 1;
+      }
+    } else {
+      NS_WARNING("Profile reset failed.");
+      return 1;
+    }
+  }
+
+  if (profile) {
+    rv = profile->GetName(mProfileName);
+    if (NS_FAILED(rv)) {
+      mProfileName.Truncate(0);
+    }
+  }
+
   gProfileLock = mProfileLock;
 
-  rv = mProfileLock->GetDirectory(getter_AddRefs(mProfD));
-  NS_ENSURE_SUCCESS(rv, 1);
-
-  rv = mProfileLock->GetLocalDirectory(getter_AddRefs(mProfLD));
-  NS_ENSURE_SUCCESS(rv, 1);
-
   nsAutoCString version;
   BuildVersion(version);
 
 #ifdef TARGET_OS_ABI
   NS_NAMED_LITERAL_CSTRING(osABI, TARGET_OS_ABI);
 #else
   // No TARGET_XPCOM_ABI, but at least the OS is known
   NS_NAMED_LITERAL_CSTRING(osABI, OS_TARGET "_UNKNOWN");