diff --git a/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.cc b/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.cc index 6a5f91c579e1e..e1b1ffd29b0da 100644 --- a/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.cc +++ b/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.cc @@ -142,19 +142,9 @@ FirstRunFlowControllerDice::FirstRunFlowControllerDice( ClearHostClosure clear_host_callback, Profile* profile, ProfilePicker::FirstRunExitedCallback first_run_exited_callback) - : ProfileManagementFlowController(host, - std::move(clear_host_callback), - Step::kIntro), + : ProfileManagementFlowController(host, std::move(clear_host_callback)), profile_(profile), - first_run_exited_callback_(std::move(first_run_exited_callback)) { - RegisterStep( - initial_step(), - CreateIntroStep(host, - base::BindRepeating( - &FirstRunFlowControllerDice::HandleIntroSigninChoice, - weak_ptr_factory_.GetWeakPtr()), - /*enable_animations=*/true)); -} + first_run_exited_callback_(std::move(first_run_exited_callback)) {} FirstRunFlowControllerDice::~FirstRunFlowControllerDice() { if (first_run_exited_callback_) { @@ -163,6 +153,19 @@ FirstRunFlowControllerDice::~FirstRunFlowControllerDice() { } } +void FirstRunFlowControllerDice::Init( + StepSwitchFinishedCallback step_switch_finished_callback) { + RegisterStep( + Step::kIntro, + CreateIntroStep(host(), + base::BindRepeating( + &FirstRunFlowControllerDice::HandleIntroSigninChoice, + weak_ptr_factory_.GetWeakPtr()), + /*enable_animations=*/true)); + SwitchToStep(Step::kIntro, /*reset_state=*/true, + std::move(step_switch_finished_callback)); +} + bool FirstRunFlowControllerDice::PreFinishWithBrowser() { DCHECK(first_run_exited_callback_); std::move(first_run_exited_callback_) @@ -187,12 +190,13 @@ void FirstRunFlowControllerDice::HandleIntroSigninChoice(bool sign_in) { // Binding as Unretained as `this` outlives the step // controllers. base::Unretained(this), ProfileManagementFlowController::Step::kIntro, - /*reset_state=*/false, /*pop_step_callback=*/base::OnceClosure(), - /*step_switch_finished_callback=*/base::OnceCallback()); + /*reset_state=*/false, + /*step_switch_finished_callback=*/StepSwitchFinishedCallback(), + /*pop_step_callback=*/base::OnceClosure()); SwitchToStep(ProfileManagementFlowController::Step::kAccountSelection, /*reset_state=*/true, - /*pop_step_callback=*/std::move(pop_closure), /*step_switch_finished_callback=*/ base::IgnoreArgs(base::BindOnce( - &NavigateBackInOneSecond, weak_ptr_factory_.GetWeakPtr()))); + &NavigateBackInOneSecond, weak_ptr_factory_.GetWeakPtr())), + /*pop_step_callback=*/std::move(pop_closure)); } diff --git a/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.h b/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.h index 047c429b56432..ae72d767a251f 100644 --- a/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.h +++ b/chrome/browser/ui/views/profiles/first_run_flow_controller_dice.h @@ -33,8 +33,10 @@ class FirstRunFlowControllerDice : public ProfileManagementFlowController { ProfilePicker::FirstRunExitedCallback first_run_exited_callback); ~FirstRunFlowControllerDice() override; - protected: // ProfileManagementFlowController: + void Init(StepSwitchFinishedCallback step_switch_finished_callback) override; + + protected: bool PreFinishWithBrowser() override; private: diff --git a/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.cc b/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.cc index 0372600ab3efa..8f2699d1f7005 100644 --- a/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.cc +++ b/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.cc @@ -4,6 +4,7 @@ #include "chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.h" +#include "base/functional/callback.h" #include "base/logging.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/profiles/profile_manager.h" @@ -131,12 +132,28 @@ FirstRunFlowControllerLacros::FirstRunFlowControllerLacros( ClearHostClosure clear_host_callback, Profile* profile, ProfilePicker::DebugFirstRunExitedCallback first_run_exited_callback) - : ProfileManagementFlowController(host, - std::move(clear_host_callback), - Step::kPostSignInFlow), + : ProfileManagementFlowController(host, std::move(clear_host_callback)), + profile_(profile), first_run_exited_callback_(std::move(first_run_exited_callback)) { DCHECK(first_run_exited_callback_); +} + +FirstRunFlowControllerLacros::~FirstRunFlowControllerLacros() { + // Call the callback if not called yet. This happens when the user exits the + // flow by closing the window, or for intent overrides. + if (first_run_exited_callback_) { + std::move(first_run_exited_callback_) + .Run(sync_confirmation_seen_ + ? ProfilePicker::FirstRunExitStatus::kQuitAtEnd + : ProfilePicker::FirstRunExitStatus::kQuitEarly, + ProfilePicker::FirstRunExitSource::kControllerDestructor); + // Since the flow is exited already, we don't have anything to close or + // finish setting up. + } +} +void FirstRunFlowControllerLacros::Init( + StepSwitchFinishedCallback step_switch_finished_callback) { auto mark_sync_confirmation_seen_callback = base::BindOnce(&FirstRunFlowControllerLacros::MarkSyncConfirmationSeen, // Unretained ok: the callback is passed to a step that @@ -149,30 +166,19 @@ FirstRunFlowControllerLacros::FirstRunFlowControllerLacros( base::Unretained(this), // Unretained ok: `signed_in_flow` will register a profile // keep alive. - base::Unretained(profile))); + base::Unretained(profile_))); auto signed_in_flow = std::make_unique( - host, profile, - content::WebContents::Create(content::WebContents::CreateParams(profile)), + host(), profile_, + content::WebContents::Create( + content::WebContents::CreateParams(profile_)), std::move(mark_sync_confirmation_seen_callback), std::move(finish_flow_callback)); - RegisterStep(initial_step(), + RegisterStep(Step::kPostSignInFlow, ProfileManagementStepController::CreateForPostSignInFlow( - host, std::move(signed_in_flow))); -} - -FirstRunFlowControllerLacros::~FirstRunFlowControllerLacros() { - // Call the callback if not called yet. This happens when the user exits the - // flow by closing the window, or for intent overrides. - if (first_run_exited_callback_) { - std::move(first_run_exited_callback_) - .Run(sync_confirmation_seen_ - ? ProfilePicker::FirstRunExitStatus::kQuitAtEnd - : ProfilePicker::FirstRunExitStatus::kQuitEarly, - ProfilePicker::FirstRunExitSource::kControllerDestructor); - // Since the flow is exited already, we don't have anything to close or - // finish setting up. - } + host(), std::move(signed_in_flow))); + SwitchToStep(Step::kPostSignInFlow, /*reset_state=*/true, + std::move(step_switch_finished_callback)); } bool FirstRunFlowControllerLacros::PreFinishWithBrowser() { diff --git a/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.h b/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.h index f40e911272d94..3ec40a08da676 100644 --- a/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.h +++ b/chrome/browser/ui/views/profiles/first_run_flow_controller_lacros.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_FIRST_RUN_FLOW_CONTROLLER_LACROS_H_ #define CHROME_BROWSER_UI_VIEWS_PROFILES_FIRST_RUN_FLOW_CONTROLLER_LACROS_H_ +#include "base/memory/raw_ptr.h" #include "chrome/browser/ui/profile_picker.h" #include "chrome/browser/ui/views/profiles/profile_management_flow_controller.h" @@ -26,13 +27,19 @@ class FirstRunFlowControllerLacros : public ProfileManagementFlowController { ~FirstRunFlowControllerLacros() override; - protected: // ProfileManagementFlowController: + void Init(StepSwitchFinishedCallback step_switch_finished_callback) override; + + protected: bool PreFinishWithBrowser() override; private: void MarkSyncConfirmationSeen(); + // Pointer to the primary profile. Safe to keep, in particular we are going to + // register a profile keep alive through the step we create in `Init()`. + const raw_ptr profile_; + // Captures the operation that the user expected to run at the time we chose // to show them the FRE. When we exit the FRE, we MUST run this. We expect // that it will cause a UI for the primary profile to be opened. diff --git a/chrome/browser/ui/views/profiles/profile_management_flow_controller.cc b/chrome/browser/ui/views/profiles/profile_management_flow_controller.cc index 352797e9a6a71..8dc8d60c9facb 100644 --- a/chrome/browser/ui/views/profiles/profile_management_flow_controller.cc +++ b/chrome/browser/ui/views/profiles/profile_management_flow_controller.cc @@ -17,27 +17,18 @@ ProfileManagementFlowController::ProfileManagementFlowController( ProfilePickerWebContentsHost* host, - ClearHostClosure clear_host_callback, - Step initial_step) - : initial_step_(initial_step), - host_(host), - clear_host_callback_(std::move(clear_host_callback)) {} - -ProfileManagementFlowController::~ProfileManagementFlowController() = default; - -void ProfileManagementFlowController::Init( - base::OnceCallback initial_step_switch_finished_callback) { + ClearHostClosure clear_host_callback) + : host_(host), clear_host_callback_(std::move(clear_host_callback)) { DCHECK(clear_host_callback_.value()); - SwitchToStep(initial_step(), /*reset_state=*/true, - /*pop_step_callback=*/base::OnceClosure(), - std::move(initial_step_switch_finished_callback)); } +ProfileManagementFlowController::~ProfileManagementFlowController() = default; + void ProfileManagementFlowController::SwitchToStep( Step step, bool reset_state, - base::OnceClosure pop_step_callback, - base::OnceCallback step_switch_finished_callback) { + StepSwitchFinishedCallback step_switch_finished_callback, + base::OnceClosure pop_step_callback) { DCHECK_NE(Step::kUnknown, step); DCHECK_NE(current_step_, step); diff --git a/chrome/browser/ui/views/profiles/profile_management_flow_controller.h b/chrome/browser/ui/views/profiles/profile_management_flow_controller.h index 888c796052fb9..3e66518edf36a 100644 --- a/chrome/browser/ui/views/profiles/profile_management_flow_controller.h +++ b/chrome/browser/ui/views/profiles/profile_management_flow_controller.h @@ -21,9 +21,9 @@ class ProfilePickerWebContentsHost; // `ProfileManagementStepController`s and owned by this object. // // Typical usage starts with calling `Init()` on the instantiated flow, which -// will switch to the `initial_step()`. Then as the user interacts with the -// flow, this controller will handle instantiating and navigating between the -// steps. +// will register and switch to the first step. Then as the user interacts with +// the flow, this controller will handle instantiating and navigating between +// the next steps. class ProfileManagementFlowController { public: // TODO(https://crbug.com/1358843): Split the steps more granularly across @@ -52,33 +52,30 @@ class ProfileManagementFlowController { kIntro, }; - // Creates a flow controller that will advance to `initial_step` when it is - // `Init()`-ed. + // Creates a flow controller that will start showing UI when `Init()`-ed. // `clear_host_callback` will be called if `host` needs to be closed. - explicit ProfileManagementFlowController(ProfilePickerWebContentsHost* host, - ClearHostClosure clear_host_callback, - Step initial_step); + explicit ProfileManagementFlowController( + ProfilePickerWebContentsHost* host, + ClearHostClosure clear_host_callback); virtual ~ProfileManagementFlowController(); - // Switches to the `initial_step()`. - // If `initial_step_switch_finished_callback` is provided, it will be called - // with `true` when the navigation to the initial step succeeded, or with - // `false` otherwise. - virtual void Init( - base::OnceCallback initial_step_switch_finished_callback = - base::OnceCallback()); + // Starts the flow by registering and switching to the first step. + // If `step_switch_finished_callback` is provided, it will be called with + // `true` when the navigation to the initial step succeeded, or with `false` + // otherwise. + virtual void Init(StepSwitchFinishedCallback step_switch_finished_callback = + StepSwitchFinishedCallback()) = 0; // Instructs a step registered as `step` to be shown. // If `step_switch_finished_callback` is provided, it will be called // with `true` when the navigation to `step` succeeded, or with // `false` otherwise. // Also see `ProfileManagementStepController::Show()`. - void SwitchToStep( - Step step, - bool reset_state, - base::OnceClosure pop_step_callback = base::OnceClosure(), - base::OnceCallback step_switch_finished_callback = - base::OnceCallback()); + void SwitchToStep(Step step, + bool reset_state, + StepSwitchFinishedCallback step_switch_finished_callback = + StepSwitchFinishedCallback(), + base::OnceClosure pop_step_callback = base::OnceClosure()); void OnNavigateBackRequested(); @@ -115,15 +112,11 @@ class ProfileManagementFlowController { Step current_step() const { return current_step_; } - Step initial_step() const { return initial_step_; } - ProfilePickerWebContentsHost* host() { return host_; } private: Step current_step_ = Step::kUnknown; - Step initial_step_; - raw_ptr host_; ClearHostClosure clear_host_callback_; diff --git a/chrome/browser/ui/views/profiles/profile_management_step_controller.cc b/chrome/browser/ui/views/profiles/profile_management_step_controller.cc index c64d51f530195..827e349eb9fc8 100644 --- a/chrome/browser/ui/views/profiles/profile_management_step_controller.cc +++ b/chrome/browser/ui/views/profiles/profile_management_step_controller.cc @@ -80,7 +80,7 @@ class DiceSignInStepController : public ProfileManagementStepController { ~DiceSignInStepController() override = default; - void Show(base::OnceCallback step_shown_callback, + void Show(StepSwitchFinishedCallback step_shown_callback, bool reset_state) override { DCHECK(step_shown_callback); DCHECK(signed_in_callback_) << "Attempting to show Dice step again while " diff --git a/chrome/browser/ui/views/profiles/profile_management_step_controller.h b/chrome/browser/ui/views/profiles/profile_management_step_controller.h index ecdec6862c688..c5ee191af1605 100644 --- a/chrome/browser/ui/views/profiles/profile_management_step_controller.h +++ b/chrome/browser/ui/views/profiles/profile_management_step_controller.h @@ -68,7 +68,7 @@ class ProfileManagementStepController { // `reset_state` indicates that the step should reset its internal state and // appear as freshly created. Callers should pass `true` for newly created // steps. - virtual void Show(base::OnceCallback step_shown_callback, + virtual void Show(StepSwitchFinishedCallback step_shown_callback, bool reset_state = false) = 0; // Frees up unneeded resources. `Show()` will be called if it's needed again. diff --git a/chrome/browser/ui/views/profiles/profile_management_utils.h b/chrome/browser/ui/views/profiles/profile_management_utils.h index ba0fd4a78a4e4..8f00d03487177 100644 --- a/chrome/browser/ui/views/profiles/profile_management_utils.h +++ b/chrome/browser/ui/views/profiles/profile_management_utils.h @@ -14,6 +14,13 @@ class Browser; class Profile; +// Type of the callbacks that are called to be notified that the switch to a +// given step by `ProfileManagementFlowController` is completed. `success` is +// is set to false if some sort of error is detected, and `true` otherwise. +// This type is intended for documentation purposes, there is no plan to treat +// it like an opaque type. +using StepSwitchFinishedCallback = base::OnceCallback; + // Callback executed when the flow finishes, after the host was cleared and // we opened a browser for the newly set up profile. // This callback should not rely on profile management flow instances, as we diff --git a/chrome/browser/ui/views/profiles/profile_picker_flow_controller.cc b/chrome/browser/ui/views/profiles/profile_picker_flow_controller.cc index 0f9f447cf2079..f0d1caf3519dc 100644 --- a/chrome/browser/ui/views/profiles/profile_picker_flow_controller.cc +++ b/chrome/browser/ui/views/profiles/profile_picker_flow_controller.cc @@ -256,21 +256,24 @@ ProfilePickerFlowController::ProfilePickerFlowController( ProfilePickerWebContentsHost* host, ClearHostClosure clear_host_callback, ProfilePicker::EntryPoint entry_point) - : ProfileManagementFlowController(host, - std::move(clear_host_callback), - Step::kProfilePicker), - entry_point_(entry_point) { - RegisterStep(initial_step(), - ProfileManagementStepController::CreateForProfilePickerApp( - host, GetInitialURL(entry_point_))); -} + : ProfileManagementFlowController(host, std::move(clear_host_callback)), + entry_point_(entry_point) {} ProfilePickerFlowController::~ProfilePickerFlowController() = default; +void ProfilePickerFlowController::Init( + StepSwitchFinishedCallback step_switch_finished_callback) { + RegisterStep(Step::kProfilePicker, + ProfileManagementStepController::CreateForProfilePickerApp( + host(), GetInitialURL(entry_point_))); + SwitchToStep(Step::kProfilePicker, /*reset_state=*/true, + std::move(step_switch_finished_callback)); +} + #if BUILDFLAG(ENABLE_DICE_SUPPORT) void ProfilePickerFlowController::SwitchToDiceSignIn( absl::optional profile_color, - base::OnceCallback switch_finished_callback) { + StepSwitchFinishedCallback switch_finished_callback) { DCHECK_EQ(Step::kProfilePicker, current_step()); profile_color_ = profile_color; @@ -289,11 +292,12 @@ void ProfilePickerFlowController::SwitchToDiceSignIn( &ProfilePickerFlowController::SwitchToStep, // Unretained ok:`this` outlives the step controllers. base::Unretained(this), Step::kProfilePicker, - /*reset_state=*/false, /*pop_step_callback=*/base::OnceClosure(), - /*step_switch_finished_callback=*/base::OnceCallback()); + /*reset_state=*/false, + /*step_switch_finished_callback=*/StepSwitchFinishedCallback(), + /*pop_step_callback=*/base::OnceClosure()); SwitchToStep(Step::kAccountSelection, - /*reset_state=*/step_needs_registration, std::move(pop_closure), - std::move(switch_finished_callback)); + /*reset_state=*/step_needs_registration, + std::move(switch_finished_callback), std::move(pop_closure)); } #endif diff --git a/chrome/browser/ui/views/profiles/profile_picker_flow_controller.h b/chrome/browser/ui/views/profiles/profile_picker_flow_controller.h index 4175235983686..4abfc5b48ca4a 100644 --- a/chrome/browser/ui/views/profiles/profile_picker_flow_controller.h +++ b/chrome/browser/ui/views/profiles/profile_picker_flow_controller.h @@ -21,9 +21,10 @@ class ProfilePickerFlowController : public ProfileManagementFlowController { ProfilePicker::EntryPoint entry_point); ~ProfilePickerFlowController() override; - void SwitchToDiceSignIn( - absl::optional profile_color, - base::OnceCallback switch_finished_callback); + void Init(StepSwitchFinishedCallback step_switch_finished_callback) override; + + void SwitchToDiceSignIn(absl::optional profile_color, + StepSwitchFinishedCallback switch_finished_callback); void SwitchToPostSignIn(Profile* signed_in_profile, #if BUILDFLAG(ENABLE_DICE_SUPPORT) diff --git a/chrome/browser/ui/views/profiles/profile_picker_view_test_utils.cc b/chrome/browser/ui/views/profiles/profile_picker_view_test_utils.cc index c70168bba7e17..161751495b27d 100644 --- a/chrome/browser/ui/views/profiles/profile_picker_view_test_utils.cc +++ b/chrome/browser/ui/views/profiles/profile_picker_view_test_utils.cc @@ -56,25 +56,25 @@ class TestProfileManagementFlowController Step step, ProfileManagementStepTestView::StepControllerFactory factory, base::OnceClosure initial_step_load_finished_closure) - : ProfileManagementFlowController(host, - std::move(clear_host_callback), - step), + : ProfileManagementFlowController(host, std::move(clear_host_callback)), + step_(step), + step_controller_factory_(std::move(factory)), initial_step_load_finished_closure_( - std::move(initial_step_load_finished_closure)) { - RegisterStep(initial_step(), factory.Run(host)); + std::move(initial_step_load_finished_closure)) {} + + void Init(StepSwitchFinishedCallback step_switch_finished_callback) override { + RegisterStep(step_, step_controller_factory_.Run(host())); + SwitchToStep( + step_, /*reset_state=*/true, + /*step_switch_finished_callback=*/ + base::BindOnce( + &TestProfileManagementFlowController::OnInitialStepSwitchFinished, + weak_ptr_factory_.GetWeakPtr(), + std::move(step_switch_finished_callback))); } - void Init(base::OnceCallback - initial_step_switch_finished_callback) override { - ProfileManagementFlowController::Init(base::BindOnce( - &TestProfileManagementFlowController::OnInitialStepSwitchFinished, - weak_ptr_factory_.GetWeakPtr(), - std::move(initial_step_switch_finished_callback))); - } - - void OnInitialStepSwitchFinished( - base::OnceCallback original_callback, - bool success) { + void OnInitialStepSwitchFinished(StepSwitchFinishedCallback original_callback, + bool success) { if (original_callback) { std::move(original_callback).Run(success); } @@ -93,6 +93,8 @@ class TestProfileManagementFlowController std::move(initial_step_load_finished_closure_).Run(); } + Step step_; + ProfileManagementStepTestView::StepControllerFactory step_controller_factory_; base::OnceClosure initial_step_load_finished_closure_; base::WeakPtrFactory weak_ptr_factory_{ this};