[fyfre] Enable FirstRunService triggering on Dice platforms

Renames `LacrosFirstRunService` to `FirstRunService`, makes it reachable and usable on Dice instead of Lacros only. Bug: 1375277 Change-Id: I573f2581a46688c6e367602cf46587d1177d56d5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004712 Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/main@{#1069763}
chromium · Nov 10, 2022 · 7ccc880 · 7ccc880
1 parent 73126a5
commit 7ccc880
Show file tree

Hide file tree

Showing 20 changed files with 309 additions and 216 deletions.
diff --git a/chrome/browser/lacros/browser_service_lacros.cc b/chrome/browser/lacros/browser_service_lacros.cc
@@ -41,7 +41,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/profile_picker.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
-#include "chrome/browser/ui/startup/lacros_first_run_service.h"
+#include "chrome/browser/ui/startup/first_run_service.h"
 #include "chrome/browser/ui/startup/startup_tab.h"
 #include "chrome/browser/ui/views/tabs/tab_scrubber_chromeos.h"
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui_util.h"
@@ -104,13 +104,12 @@ void OnMainProfileInitialized(base::OnceCallback<void(Profile*)> callback,
     return;
   }
 
-  auto* fre_service =
-      LacrosFirstRunServiceFactory::GetForBrowserContext(profile);
+  auto* fre_service = FirstRunServiceFactory::GetForBrowserContext(profile);
   if (fre_service && can_trigger_fre && fre_service->ShouldOpenFirstRun()) {
     // TODO(https://crbug.com/1313848): Consider taking a
     // `ScopedProfileKeepAlive`.
     fre_service->OpenFirstRunIfNeeded(
-        LacrosFirstRunService::EntryPoint::kOther,
+        FirstRunService::EntryPoint::kOther,
         base::BindOnce(&MaybeProceedWithProfile, std::move(callback),
                        base::Unretained(profile)));
   } else {

diff --git a/chrome/browser/lacros/browser_service_lacros_browsertest.cc b/chrome/browser/lacros/browser_service_lacros_browsertest.cc
@@ -25,7 +25,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/profile_picker.h"
 #include "chrome/browser/ui/profile_ui_test_utils.h"
-#include "chrome/browser/ui/startup/lacros_first_run_service.h"
+#include "chrome/browser/ui/startup/first_run_service.h"
 #include "chrome/browser/ui/views/session_crashed_bubble_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
@@ -490,7 +490,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
 }
 IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
                        NewWindow_OpensFirstRun) {
-  EXPECT_TRUE(ShouldOpenPrimaryProfileFirstRun(GetPrimaryProfile()));
+  EXPECT_TRUE(ShouldOpenFirstRun(GetPrimaryProfile()));
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectTotalCount(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint", 0);
@@ -507,7 +507,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
   EXPECT_EQ(1u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectUniqueSample(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint",
-      LacrosFirstRunService::EntryPoint::kOther, 1);
+      FirstRunService::EntryPoint::kOther, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
@@ -518,7 +518,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
 }
 IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
                        NewWindow_OpensFirstRun_UiClose) {
-  EXPECT_TRUE(ShouldOpenPrimaryProfileFirstRun(GetPrimaryProfile()));
+  EXPECT_TRUE(ShouldOpenFirstRun(GetPrimaryProfile()));
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectTotalCount(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint", 0);
@@ -535,7 +535,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectUniqueSample(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint",
-      LacrosFirstRunService::EntryPoint::kOther, 1);
+      FirstRunService::EntryPoint::kOther, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
@@ -546,7 +546,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
 }
 IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
                        NewTab_OpensFirstRun) {
-  EXPECT_TRUE(ShouldOpenPrimaryProfileFirstRun(GetPrimaryProfile()));
+  EXPECT_TRUE(ShouldOpenFirstRun(GetPrimaryProfile()));
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectTotalCount(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint", 0);
@@ -562,7 +562,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesBrowserTest,
   EXPECT_EQ(1u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectUniqueSample(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint",
-      LacrosFirstRunService::EntryPoint::kOther, 1);
+      FirstRunService::EntryPoint::kOther, 1);
 }
 
 class BrowserServiceLacrosNonSyncingProfilesGuestBrowserTest
@@ -581,7 +581,7 @@ IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesGuestBrowserTest,
 }
 IN_PROC_BROWSER_TEST_F(BrowserServiceLacrosNonSyncingProfilesGuestBrowserTest,
                        NewWindow_OpensFirstRun) {
-  EXPECT_FALSE(ShouldOpenPrimaryProfileFirstRun(GetPrimaryProfile()));
+  EXPECT_FALSE(ShouldOpenFirstRun(GetPrimaryProfile()));
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectTotalCount(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint", 0);
@@ -611,7 +611,7 @@ IN_PROC_BROWSER_TEST_F(
 IN_PROC_BROWSER_TEST_F(
     BrowserServiceLacrosNonSyncingProfilesWebKioskBrowserTest,
     NewWindow_OpensFirstRun) {
-  EXPECT_FALSE(ShouldOpenPrimaryProfileFirstRun(GetPrimaryProfile()));
+  EXPECT_FALSE(ShouldOpenFirstRun(GetPrimaryProfile()));
   EXPECT_EQ(0u, BrowserList::GetInstance()->size());
   histogram_tester().ExpectTotalCount(
       "Profile.LacrosPrimaryProfileFirstRunEntryPoint", 0);

diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
@@ -467,7 +467,10 @@
 #include "chrome/browser/lacros/app_mode/kiosk_session_service_lacros.h"
 #include "chrome/browser/lacros/lacros_prefs.h"
 #include "chrome/browser/lacros/net/proxy_config_service_lacros.h"
-#include "chrome/browser/ui/startup/lacros_first_run_service.h"
+#endif
+
+#if BUILDFLAG(IS_CHROMEOS_LACROS) || BUILDFLAG(ENABLE_DICE_SUPPORT)
+#include "chrome/browser/ui/startup/first_run_service.h"
 #endif
 
 #if !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
@@ -1131,8 +1134,8 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
   WhatsNewUI::RegisterLocalStatePrefs(registry);
 #endif  // BUILDFLAG(IS_ANDROID)
 
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
-  LacrosFirstRunService::RegisterLocalStatePrefs(registry);
+#if BUILDFLAG(IS_CHROMEOS_LACROS) || BUILDFLAG(ENABLE_DICE_SUPPORT)
+  FirstRunService::RegisterLocalStatePrefs(registry);
 #endif
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)

diff --git a/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc b/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
@@ -249,7 +249,7 @@
 #include "chrome/browser/lacros/account_manager/profile_account_manager_factory.h"
 #include "chrome/browser/lacros/cert/cert_db_initializer_factory.h"
 #include "chrome/browser/lacros/remote_apps/remote_apps_proxy_lacros_factory.h"
-#include "chrome/browser/ui/startup/lacros_first_run_service.h"
+#include "chrome/browser/ui/startup/first_run_service.h"
 #endif
 
 #if BUILDFLAG(BUILD_WITH_TFLITE_LIB)
@@ -412,7 +412,7 @@ void ChromeBrowserMainExtraPartsProfiles::
   InstantServiceFactory::GetInstance();
 #endif
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
-  LacrosFirstRunServiceFactory::GetInstance();
+  FirstRunServiceFactory::GetInstance();
 #endif
   LanguageModelManagerFactory::GetInstance();
   if (base::FeatureList::IsEnabled(

diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
@@ -166,7 +166,7 @@
 
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
 #include "chrome/browser/lacros/account_manager/account_profile_mapper.h"
-#include "chrome/browser/ui/startup/lacros_first_run_service.h"
+#include "chrome/browser/ui/startup/first_run_service.h"
 #include "chromeos/startup/browser_params_proxy.h"
 #include "components/account_manager_core/chromeos/account_manager_facade_factory.h"
 #endif
@@ -2408,7 +2408,7 @@ void ProfileManager::OnBrowserOpened(Browser* browser) {
 
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
   // No browser should be opened before the FRE is finished.
-  DCHECK(!ShouldOpenPrimaryProfileFirstRun(profile));
+  DCHECK(!ShouldOpenFirstRun(profile));
 #endif  // BUILDFLAG(IS_CHROMEOS_LACROS)
 
   if (!profile->IsOffTheRecord() &&

diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
@@ -3398,8 +3398,6 @@ static_library("ui") {
 
   if (is_chromeos_lacros) {
     sources += [
-      "startup/lacros_first_run_service.cc",
-      "startup/lacros_first_run_service.h",
       "startup/silent_sync_enabler.cc",
       "startup/silent_sync_enabler.h",
       "views/chrome_browser_main_extra_parts_views_lacros.cc",
@@ -3548,6 +3546,8 @@ static_library("ui") {
       "startup/default_browser_infobar_delegate.h",
       "startup/default_browser_prompt.cc",
       "startup/default_browser_prompt.h",
+      "startup/first_run_service.cc",
+      "startup/first_run_service.h",
       "views/profiles/badged_profile_photo.cc",
       "views/profiles/badged_profile_photo.h",
       "views/profiles/profile_customization_bubble_sync_controller.cc",

diff --git a/chrome/browser/ui/profile_picker.cc b/chrome/browser/ui/profile_picker.cc
@@ -78,12 +78,6 @@ ProfilePicker::Params ProfilePicker::Params::ForBackgroundManager(
   return params;
 }
 
-// static
-ProfilePicker::Params ProfilePicker::Params::ForFirstRun(
-    const base::FilePath& profile_path) {
-  return ProfilePicker::Params(EntryPoint::kFirstRun, profile_path);
-}
-
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
 // static
 ProfilePicker::Params ProfilePicker::Params::ForLacrosSelectAvailableAccount(
@@ -95,35 +89,49 @@ ProfilePicker::Params ProfilePicker::Params::ForLacrosSelectAvailableAccount(
   return params;
 }
 
-// static
-ProfilePicker::Params ProfilePicker::Params::ForLacrosPrimaryProfileFirstRun(
-    FirstRunExitedCallback first_run_finished_callback) {
-  Params params(EntryPoint::kLacrosPrimaryProfileFirstRun,
-                ProfileManager::GetPrimaryUserProfilePath());
-  params.first_run_exited_callback_ = std::move(first_run_finished_callback);
-  return params;
-}
-
 void ProfilePicker::Params::NotifyAccountSelected(const std::string& gaia_id) {
   if (account_selected_callback_)
     std::move(account_selected_callback_).Run(gaia_id);
 }
+#endif
+
+// static
+ProfilePicker::Params ProfilePicker::Params::ForFirstRun(
+    const base::FilePath& profile_path,
+    FirstRunExitedCallback first_run_exited_callback) {
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+  DCHECK_EQ(profile_path, ProfileManager::GetPrimaryUserProfilePath());
+#endif
+
+  Params params(
+#if BUILDFLAG(ENABLE_DICE_SUPPORT)
+      EntryPoint::kFirstRun,
+#elif BUILDFLAG(IS_CHROMEOS_LACROS)
+      EntryPoint::kLacrosPrimaryProfileFirstRun,
+#endif
+      profile_path);
+  params.first_run_exited_callback_ = std::move(first_run_exited_callback);
+  return params;
+}
 
 void ProfilePicker::Params::NotifyFirstRunExited(
     FirstRunExitStatus exit_status,
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
     FirstRunExitSource exit_source,
+#endif
     base::OnceClosure maybe_callback) {
   if (!first_run_exited_callback_)
     return;
 
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
   LOG(ERROR) << "Notifying FirstRun exit with status="
              << static_cast<int>(exit_status)
              << " from source=" << static_cast<int>(exit_source);
+#endif
 
   std::move(first_run_exited_callback_)
       .Run(exit_status, std::move(maybe_callback));
 }
-#endif
 
 bool ProfilePicker::Params::CanReusePickerWindow(const Params& other) const {
 #if BUILDFLAG(IS_CHROMEOS_LACROS)

diff --git a/chrome/browser/ui/profile_picker.h b/chrome/browser/ui/profile_picker.h
@@ -27,7 +27,6 @@ class WebView;
 
 class ProfilePicker {
  public:
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
   enum class FirstRunExitStatus {
     // The user completed the FRE and is continuing to launch the browser.
     kCompleted = 0,
@@ -43,6 +42,7 @@ class ProfilePicker {
       base::OnceCallback<void(FirstRunExitStatus status,
                               base::OnceClosure callback)>;
 
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
   // Added for bug investigation purposes.
   // TODO(crbug.com/1340791): Remove this once the source of the bug is found.
   enum class FirstRunExitSource {
@@ -81,6 +81,7 @@ class ProfilePicker {
     // May only be used on lacros, opens a first run experience (provided no
     // policies prevent it) to let the user opt in to sync, etc. for the primary
     // profile.
+    // TODO(crbug.com/1375277): Migrate to only using kFirstRun.
     kLacrosPrimaryProfileFirstRun = 9,
     // The Profile became idle, due to the IdleProfileCloseTimeout policy.
     kProfileIdle = 10,
@@ -110,8 +111,6 @@ class ProfilePicker {
     static Params ForBackgroundManager(
         const GURL& on_select_profile_target_url);
 
-    static Params ForFirstRun(const base::FilePath& profile_path);
-
     EntryPoint entry_point() const { return entry_point_; }
 
     // Returns the path to the profile to use to display the Web UI.
@@ -139,30 +138,35 @@ class ProfilePicker {
         const base::FilePath& profile_path,
         base::OnceCallback<void(const std::string&)> account_selected_callback);
 
-    // Builds parameter with the `kLacrosPrimaryProfileFirstRun` entry point.
+    // Calls `account_selected_callback_`. See
+    // `ForLacrosSelectAvailableAccount()` for more details.
+    void NotifyAccountSelected(const std::string& gaia_id);
+#endif
+
+    // Builds parameter with the `kFirstRun` (on Dice) or the
+    //  `kLacrosPrimaryProfileFirstRun` (on Lacros) entry point.
     //
+    // `profile_path` is the profile for which to open the FRE. On Lacros we
+    // expect it to be the main profile path.
     // `first_run_exited_callback` is called when the first run experience is
     // exited, with a `FirstRunExitStatus` indicating how the user responded to
     // it, and an optional callback that must be run if the user has proceeded
     // to the browser after the FRE.
-    static Params ForLacrosPrimaryProfileFirstRun(
-        FirstRunExitedCallback first_run_exited_callback);
-
-    // Calls `account_selected_callback_`. See
-    // `ForLacrosSelectAvailableAccount()` for more details.
-    void NotifyAccountSelected(const std::string& gaia_id);
+    static Params ForFirstRun(const base::FilePath& profile_path,
+                              FirstRunExitedCallback first_run_exited_callback);
 
     // Calls `first_run_exited_callback_`, forwarding `exit_status` and
-    // `maybe_callback`. See `ForLacrosPrimaryProfileFirstRun()` for more
+    // `maybe_callback`. See `ForFirstRun()` for more
     // details.
     //
     // If this method is not called by the time this `Param` is destroyed, an
     // intent to quit will be assumed and `first_run_exited_callback_` will be
     // called by the destructor with quit-related arguments.
     void NotifyFirstRunExited(FirstRunExitStatus exit_status,
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
                               FirstRunExitSource exit_source,
-                              base::OnceClosure maybe_callback);
 #endif
+                              base::OnceClosure maybe_callback);
 
     // Returns whether the current profile picker window can be reused for
     // different parameters. If this returns false, the picker cannot be reused
@@ -176,9 +180,10 @@ class ProfilePicker {
     EntryPoint entry_point_ = EntryPoint::kOnStartup;
     GURL on_select_profile_target_url_;
     base::FilePath profile_path_;
+    FirstRunExitedCallback first_run_exited_callback_;
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
     base::OnceCallback<void(const std::string&)> account_selected_callback_;
-    FirstRunExitedCallback first_run_exited_callback_;
+
 #endif
   };
 
@@ -251,11 +256,9 @@ class ProfilePicker {
   // Returns whether the profile picker is currently open.
   static bool IsOpen();
 
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
-  // Returns whether the profile picker is currently open and showing the Lacros
-  // First Run Experience.
-  static bool IsLacrosFirstRunOpen();
-#endif
+  // Returns whether the profile picker is currently open and showing the First
+  // Run Experience.
+  static bool IsFirstRunOpen();
 
   // Returns whether the Profile picker is showing and active.
   static bool IsActive();