Bug 1531902: Enable launcher process pref on beta; r=mhowell
authorAaron Klotz <aklotz@mozilla.com>
Mon, 11 Mar 2019 23:26:00 +0000
changeset 521453 f9de9668b5e87be6b0461ef8c0295f48bc71dcd1
parent 521452 985e05a4b54024ae40be6d94aaf8d9fb3d66f9b2
child 521454 5242fa07735cd4f39f3357747d4ac332d90e6df9
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1531902
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 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
browser/app/profile/firefox.js
toolkit/xre/LauncherRegistryInfo.cpp
toolkit/xre/nsAppRunner.cpp
toolkit/xre/test/win/TestLauncherRegistryInfo.cpp
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1834,19 +1834,9 @@ pref("prio.enabled", true);
 pref("browser.discovery.enabled", true);
 pref("browser.discovery.containers.enabled", true);
 pref("browser.discovery.sites", "addons.mozilla.org");
 
 pref("browser.engagement.recent_visited_origins.expiry", 86400); // 24 * 60 * 60 (24 hours in seconds)
 
 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);
--- a/toolkit/xre/LauncherRegistryInfo.cpp
+++ b/toolkit/xre/LauncherRegistryInfo.cpp
@@ -51,19 +51,27 @@ LauncherResult<LauncherRegistryInfo::Dis
   }
 
   MOZ_ASSERT_UNREACHABLE("Invalid disposition from RegCreateKeyExW");
   return LAUNCHER_ERROR_GENERIC();
 }
 
 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
   LauncherResult<bool> clearedLauncherTimestamp =
       ClearStartTimestamp(ProcessType::Launcher);
   MOZ_ASSERT(clearedLauncherTimestamp.isOk());
   if (clearedLauncherTimestamp.isErr()) {
     return LAUNCHER_ERROR_FROM_RESULT(clearedLauncherTimestamp);
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -866,28 +866,33 @@ nsXULAppInfo::GetWindowsDLLBlocklistStat
 }
 
 NS_IMETHODIMP
 nsXULAppInfo::GetRestartedByOS(bool* aResult) {
   *aResult = gRestartedByOS;
   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;
-
-  LauncherResult<LauncherRegistryInfo::EnabledState> state =
-      launcherInfo.IsEnabled();
-  if (state.isErr()) {
+  SetupLauncherProcessPref();
+
+  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;
 #endif
 }
 
 #ifdef XP_WIN
 // Matches the enum in WinNT.h for the Vista SDK but renamed so that we can
@@ -1523,59 +1528,76 @@ static void RegisterApplicationRestartCh
   }
 }
 
 #  if defined(MOZ_LAUNCHER_PROCESS)
 
 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 =
       launcherRegInfo.ReflectPrefToRegistry(prefVal);
   MOZ_ASSERT(reflectResult.isOk());
 }
 
 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;
 
   mozilla::LauncherResult<mozilla::LauncherRegistryInfo::EnabledState>
       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)
 
 #endif  // XP_WIN
 
 // If aBlankCommandLine is true, then the application will be launched with a
 // blank command line instead of being launched with the same command line that
--- a/toolkit/xre/test/win/TestLauncherRegistryInfo.cpp
+++ b/toolkit/xre/test/win/TestLauncherRegistryInfo.cpp
@@ -557,100 +557,94 @@ static VoidResult TestDisableDueToFailur
 
 static VoidResult TestPrefReflection() {
   // Reset the registry to a known good state.
   VoidResult vr = SetupEnabledScenario();
   if (vr.isErr()) {
     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));
   }
 
   if (launcherTs.unwrapErr() !=
       mozilla::WindowsError::FromWin32Error(ERROR_FILE_NOT_FOUND)) {
     return mozilla::Err(launcherTs.unwrapErr());
   }
 
+  // Browser timestamp should be zero
   QWordResult browserTs = GetBrowserTimestamp();
+  if (browserTs.isErr()) {
+    return mozilla::Err(browserTs.unwrapErr());
+  }
+
+  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()) {
+    return mozilla::Err(enabled.unwrapErr().mError);
+  }
+
+  if (enabled.unwrap() !=
+      mozilla::LauncherRegistryInfo::EnabledState::ForceDisabled) {
+    return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
+  }
+
+  // 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 and browser timestamps should be non-existent.
+  launcherTs = GetLauncherTimestamp();
+  if (launcherTs.isOk()) {
+    return mozilla::Err(mozilla::WindowsError::FromHResult(E_UNEXPECTED));
+  }
+
+  if (launcherTs.unwrapErr() !=
+      mozilla::WindowsError::FromWin32Error(ERROR_FILE_NOT_FOUND)) {
+    return mozilla::Err(launcherTs.unwrapErr());
+  }
+
+  browserTs = GetBrowserTimestamp();
   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());
   }
 
   // IsEnabled should give us Enabled.
-  mozilla::LauncherResult<mozilla::LauncherRegistryInfo::EnabledState> enabled =
-      info.IsEnabled();
+  enabled = info.IsEnabled();
   if (enabled.isErr()) {
     return mozilla::Err(enabled.unwrapErr().mError);
   }
 
   if (enabled.unwrap() !=
       mozilla::LauncherRegistryInfo::EnabledState::Enabled) {
     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);
-  if (reflectOk.isErr()) {
-    return mozilla::Err(reflectOk.unwrapErr().mError);
-  }
-
-  // Launcher timestamp should be non-existent.
-  launcherTs = GetLauncherTimestamp();
-  if (launcherTs.isOk()) {
-    return mozilla::Err(mozilla::WindowsError::FromHResult(E_UNEXPECTED));
-  }
-
-  if (launcherTs.unwrapErr() !=
-      mozilla::WindowsError::FromWin32Error(ERROR_FILE_NOT_FOUND)) {
-    return mozilla::Err(launcherTs.unwrapErr());
-  }
-
-  // Browser timestamp should be zero
-  browserTs = GetBrowserTimestamp();
-  if (browserTs.isErr()) {
-    return mozilla::Err(browserTs.unwrapErr());
-  }
-
-  if (browserTs.unwrap() != 0ULL) {
-    return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
-  }
-
-  // IsEnabled should give us ForceDisabled
-  enabled = info.IsEnabled();
-  if (enabled.isErr()) {
-    return mozilla::Err(enabled.unwrapErr().mError);
-  }
-
-  if (enabled.unwrap() !=
-      mozilla::LauncherRegistryInfo::EnabledState::ForceDisabled) {
-    return mozilla::Err(mozilla::WindowsError::FromHResult(E_FAIL));
-  }
-
   return mozilla::Ok();
 }
 
 int main(int argc, char* argv[]) {
   auto fullPath = mozilla::GetFullBinaryPath();
   if (!fullPath) {
     return 1;
   }