Bug 1531902: Enable launcher process pref on beta; r=mhowell

* We remove the launcher process prefs from `firefox.js` and just use defaults
  at the time we access the pref;
* We set the pref to true by default on nightly and beta;
* We modify `SetupLauncherProcessPref` to save the initial state of the
  launcher process to `gLauncherProcessState` to reflect the status of the
  launcher process *at startup*;
* We modify `nxXULAppInfo::GetLauncherProcessState` to call
  `SetupLauncherProcessPref` as the former may be called from JS ahead of the
  call to `SetupLauncherProcessPref` that we do in `XRE_mainRun`;
* We modify `LauncherRegistryInfo::ReflectPrefToRegistry` to not clobber any
  registry settings unless the new pref value differs from the previous pref
  value. We also update the test to reflect this change.

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

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

browser/app/profile/firefox.js

@@ -1839,14 +1839,4 @@ pref("browser.engagement.recent_visited_origins.expiry", 86400); // 24 * 60 * 60
 
 pref("browser.aboutConfig.showWarning", true);
 
-#if defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS)
-#if defined(NIGHTLY_BUILD)
-// Enable launcher process by default on Nightly
-pref("browser.launcherProcess.enabled", true);
-#else
-// Launcher process is disabled by default, will be selectively enabled via SHIELD
-pref("browser.launcherProcess.enabled", false);
-#endif  // defined(NIGHTLY_BUILD)
-#endif // defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS)
-
 pref("browser.toolbars.keyboard_navigation", false);

toolkit/xre/LauncherRegistryInfo.cpp

@@ -56,9 +56,17 @@ LauncherResult<LauncherRegistryInfo::Disposition> LauncherRegistryInfo::Open() {
 
 LauncherVoidResult LauncherRegistryInfo::ReflectPrefToRegistry(
     const bool aEnable) {
-  LauncherResult<Disposition> disposition = Open();
-  if (disposition.isErr()) {
-    return LAUNCHER_ERROR_FROM_RESULT(disposition);
+  LauncherResult<EnabledState> curEnabledState = IsEnabled();
+  if (curEnabledState.isErr()) {
+    return LAUNCHER_ERROR_FROM_RESULT(curEnabledState);
+  }
+
+  bool isCurrentlyEnabled = curEnabledState.unwrap() !=
+    EnabledState::ForceDisabled;
+  if (isCurrentlyEnabled == aEnable) {
+    // Don't reflect to the registry unless the new enabled state is actually
+    // changing with respect to the current enabled state.
+    return Ok();
   }
 
   // Always delete the launcher timestamp

toolkit/xre/nsAppRunner.cpp

@@ -871,18 +871,23 @@ nsXULAppInfo::GetRestartedByOS(bool* aResult) {
   return NS_OK;
 }
 
+#if defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS)
+// Forward declaration
+void SetupLauncherProcessPref();
+
+static Maybe<LauncherRegistryInfo::EnabledState> gLauncherProcessState;
+#endif  // defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS)
+
 NS_IMETHODIMP
 nsXULAppInfo::GetLauncherProcessState(uint32_t* aResult) {
 #if defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS)
-  LauncherRegistryInfo launcherInfo;
+  SetupLauncherProcessPref();
 
-  LauncherResult<LauncherRegistryInfo::EnabledState> state =
-      launcherInfo.IsEnabled();
-  if (state.isErr()) {
+  if (!gLauncherProcessState) {
     return NS_ERROR_UNEXPECTED;
   }
 
-  *aResult = static_cast<uint32_t>(state.unwrap());
+  *aResult = static_cast<uint32_t>(gLauncherProcessState.value());
   return NS_OK;
 #else
   return NS_ERROR_NOT_AVAILABLE;
@@ -1528,8 +1533,16 @@ static void RegisterApplicationRestartChanged(const char* aPref, void* aData) {
 static const char kShieldPrefName[] = "app.shield.optoutstudies.enabled";
 
 static void OnLauncherPrefChanged(const char* aPref, void* aData) {
+  const bool kLauncherPrefDefaultValue =
+#    if defined(NIGHTLY_BUILD) || (MOZ_UPDATE_CHANNEL == beta)
+    true
+#    else
+    false
+#    endif  // defined(NIGHTLY_BUILD) || (MOZ_UPDATE_CHANNEL == beta)
+    ;
   bool prefVal = Preferences::GetBool(kShieldPrefName, false) &&
-                 Preferences::GetBool(PREF_WIN_LAUNCHER_PROCESS_ENABLED, false);
+                 Preferences::GetBool(PREF_WIN_LAUNCHER_PROCESS_ENABLED,
+                                      kLauncherPrefDefaultValue);
 
   mozilla::LauncherRegistryInfo launcherRegInfo;
   mozilla::LauncherVoidResult reflectResult =
@@ -1541,12 +1554,10 @@ static void SetupLauncherProcessPref() {
   // In addition to the launcher pref itself, we also tie the launcher process
   // state to the SHIELD opt-out pref.
 
-#    if defined(NIGHTLY_BUILD)
-  // On Nightly, fire the callback immediately to ensure the pref is reflected
-  // to the registry and we get immediate enablement of the launcher process
-  // for all users.
-  Preferences::RegisterCallbackAndCall(&OnLauncherPrefChanged, kShieldPrefName);
-#    endif  // defined(NIGHTLY_BUILD)
+  if (gLauncherProcessState) {
+    // We've already successfully run
+    return;
+  }
 
   mozilla::LauncherRegistryInfo launcherRegInfo;
 
@@ -1554,23 +1565,34 @@ static void SetupLauncherProcessPref() {
       enabledState = launcherRegInfo.IsEnabled();
 
   if (enabledState.isOk()) {
-    Preferences::SetBool(
-        PREF_WIN_LAUNCHER_PROCESS_ENABLED,
-        enabledState.unwrap() !=
-            mozilla::LauncherRegistryInfo::EnabledState::ForceDisabled);
+    gLauncherProcessState = Some(enabledState.unwrap());
 
     CrashReporter::AnnotateCrashReport(
         CrashReporter::Annotation::LauncherProcessState,
         static_cast<uint32_t>(enabledState.unwrap()));
+
+#    if defined(NIGHTLY_BUILD) || (MOZ_UPDATE_CHANNEL == beta)
+    // Reflect the pref states into the registry by calling
+    // OnLauncherPrefChanged.
+    OnLauncherPrefChanged(PREF_WIN_LAUNCHER_PROCESS_ENABLED, nullptr);
+
+    // Now obtain the revised state of the launcher process for reflection
+    // into prefs
+    enabledState = launcherRegInfo.IsEnabled();
+#    endif  // defined(NIGHTLY_BUILD) || (MOZ_UPDATE_CHANNEL == beta)
   }
 
+  if (enabledState.isOk()) {
+    // Reflect the launcher process registry state into user prefs
+    Preferences::SetBool(
+        PREF_WIN_LAUNCHER_PROCESS_ENABLED,
+        enabledState.unwrap() !=
+            mozilla::LauncherRegistryInfo::EnabledState::ForceDisabled);
+  }
+
+  Preferences::RegisterCallback(&OnLauncherPrefChanged, kShieldPrefName);
   Preferences::RegisterCallback(&OnLauncherPrefChanged,
                                 PREF_WIN_LAUNCHER_PROCESS_ENABLED);
-#    if !defined(NIGHTLY_BUILD)
-  // We register for SHIELD notifications, but we don't fire the callback
-  // immediately in the non-Nightly case.
-  Preferences::RegisterCallback(&OnLauncherPrefChanged, kShieldPrefName);
-#    endif  // !defined(NIGHTLY_BUILD)
 }
 
 #  endif  // defined(MOZ_LAUNCHER_PROCESS)

toolkit/xre/test/win/TestLauncherRegistryInfo.cpp

@@ -562,14 +562,14 @@ static VoidResult TestPrefReflection() {
     return vr;
   }
 
-  // First, test to see what happens when the pref is set to ON.
+  // Let's see what happens when we flip the pref to OFF.
   mozilla::LauncherRegistryInfo info;
-  mozilla::LauncherVoidResult reflectOk = info.ReflectPrefToRegistry(true);
+  mozilla::LauncherVoidResult reflectOk = info.ReflectPrefToRegistry(false);
   if (reflectOk.isErr()) {
     return mozilla::Err(reflectOk.unwrapErr().mError);
   }
 
-  // Launcher and browser timestamps should be non-existent.
+  // Launcher timestamp should be non-existent.
   QWordResult launcherTs = GetLauncherTimestamp();
   if (launcherTs.isOk()) {
     return mozilla::Err(mozilla::WindowsError::FromHResult(E_UNEXPECTED));
@@ -580,17 +580,17 @@ static VoidResult TestPrefReflection() {
     return mozilla::Err(launcherTs.unwrapErr());
   }
 
+  // Browser timestamp should be zero
   QWordResult browserTs = GetBrowserTimestamp();
-  if (browserTs.isOk()) {
-    return mozilla::Err(mozilla::WindowsError::FromHResult(E_UNEXPECTED));
-  }
-
-  if (browserTs.unwrapErr() !=
-      mozilla::WindowsError::FromWin32Error(ERROR_FILE_NOT_FOUND)) {
+  if (browserTs.isErr()) {
     return mozilla::Err(browserTs.unwrapErr());
   }
 
-  // IsEnabled should give us Enabled.
+  if (browserTs.unwrap() != 0ULL) {
+    return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
+  }
+
+  // IsEnabled should give us ForceDisabled
   mozilla::LauncherResult<mozilla::LauncherRegistryInfo::EnabledState> enabled =
       info.IsEnabled();
   if (enabled.isErr()) {
@@ -598,23 +598,17 @@ static VoidResult TestPrefReflection() {
   }
 
   if (enabled.unwrap() !=
-      mozilla::LauncherRegistryInfo::EnabledState::Enabled) {
+      mozilla::LauncherRegistryInfo::EnabledState::ForceDisabled) {
     return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
   }
 
-  // Okay, so far so good. Let's reset the board and see what happens when we
-  // flip the pref to OFF.
-  vr = SetupEnabledScenario();
-  if (vr.isErr()) {
-    return vr;
-  }
-
-  reflectOk = info.ReflectPrefToRegistry(false);
+  // Now test to see what happens when the pref is set to ON.
+  reflectOk = info.ReflectPrefToRegistry(true);
   if (reflectOk.isErr()) {
     return mozilla::Err(reflectOk.unwrapErr().mError);
   }
 
-  // Launcher timestamp should be non-existent.
+  // Launcher and browser timestamps should be non-existent.
   launcherTs = GetLauncherTimestamp();
   if (launcherTs.isOk()) {
     return mozilla::Err(mozilla::WindowsError::FromHResult(E_UNEXPECTED));
@@ -625,24 +619,24 @@ static VoidResult TestPrefReflection() {
     return mozilla::Err(launcherTs.unwrapErr());
   }
 
-  // Browser timestamp should be zero
   browserTs = GetBrowserTimestamp();
-  if (browserTs.isErr()) {
+  if (browserTs.isOk()) {
+    return mozilla::Err(mozilla::WindowsError::FromHResult(E_UNEXPECTED));
+  }
+
+  if (browserTs.unwrapErr() !=
+      mozilla::WindowsError::FromWin32Error(ERROR_FILE_NOT_FOUND)) {
     return mozilla::Err(browserTs.unwrapErr());
   }
 
-  if (browserTs.unwrap() != 0ULL) {
-    return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
-  }
-
-  // IsEnabled should give us ForceDisabled
+  // IsEnabled should give us Enabled.
   enabled = info.IsEnabled();
   if (enabled.isErr()) {
     return mozilla::Err(enabled.unwrapErr().mError);
   }
 
   if (enabled.unwrap() !=
-      mozilla::LauncherRegistryInfo::EnabledState::ForceDisabled) {
+      mozilla::LauncherRegistryInfo::EnabledState::Enabled) {
     return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
   }