[M139] [ProfilePicker] Fix double clicking issues on main page
Original change's description:
> [ProfilePicker] Fix double clicking issues on main page
>
> The profile picker main page usually redirect to a secondary
> `ProfileTypeChoice` screen where the buttons are controlled upon click
> to avoid double clicking.
> However in the context of Force Signin, even the main page can start
> actions, such as Signin and Reauth, which should not be triggered
> twice in a row (possible with double clicking) and creating unexpected
> behaviors.
>
> The fix here is to disable all buttons upon first click, and only
> re-enable them when navigating back to the page or if the action
> is completed (information sent back from the browser).
>
> Fixed: 431992755
> Change-Id: Id700432391a765bf08db230f6d0a718e38e78d90
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6763618
> Commit-Queue: Ryan Sultanem <rsult@google.com>
> Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1492175}
Bug: 437911835,431992755
Change-Id: Id700432391a765bf08db230f6d0a718e38e78d90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6837357
Commit-Queue: Krishna Govind <govind@chromium.org>
Reviewed-by: Krishna Govind <govind@chromium.org>
Auto-Submit: Chrome Cherry Picker <chrome-cherry-picker@chops-service-accounts.iam.gserviceaccount.com>
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/7258@{#2580}
Cr-Branched-From: f600d0656fd5b5fe4a82981f533d31ed6939e2e4-refs/heads/main@{#1477651}
diff --git a/chrome/browser/resources/signin/profile_picker/profile_card.html.ts b/chrome/browser/resources/signin/profile_picker/profile_card.html.ts
index 1aaf591..194f419a 100644
--- a/chrome/browser/resources/signin/profile_picker/profile_card.html.ts
+++ b/chrome/browser/resources/signin/profile_picker/profile_card.html.ts
@@ -11,6 +11,7 @@
return html`<!--_html_template_start_-->
<div id="profileCardContainer">
<cr-button id="profileCardButton" @click="${this.onProfileClick_}"
+ ?disabled="${this.disabled}"
aria-label="${this.profileState.profileCardButtonLabel}">
<div id="avatarContainer">
<img class="profile-avatar" alt="" .src="${this.profileState.avatarIcon}">
diff --git a/chrome/browser/resources/signin/profile_picker/profile_card.ts b/chrome/browser/resources/signin/profile_picker/profile_card.ts
index 59d59adc..6a0a03c 100644
--- a/chrome/browser/resources/signin/profile_picker/profile_card.ts
+++ b/chrome/browser/resources/signin/profile_picker/profile_card.ts
@@ -52,10 +52,12 @@
return {
profileState: {type: Object},
pattern_: {type: String},
+ disabled: {type: Boolean},
};
}
accessor profileState: ProfileState = createDummyProfileState();
+ accessor disabled: boolean = false;
protected accessor pattern_: string = '.*\\S.*';
private manageProfilesBrowserProxy_: ManageProfilesBrowserProxy =
ManageProfilesBrowserProxyImpl.getInstance();
@@ -124,18 +126,18 @@
}
protected onProfileClick_() {
+ this.fire('disable-all-picker-buttons');
+
this.manageProfilesBrowserProxy_.launchSelectedProfile(
this.profileState.profilePath);
}
protected onNameInputPointerEnter_() {
- this.dispatchEvent(new CustomEvent(
- 'toggle-drag', {composed: true, detail: {toggle: false}}));
+ this.fire('toggle-drag', {toggle: false});
}
protected onNameInputPointerLeave_() {
- this.dispatchEvent(new CustomEvent(
- 'toggle-drag', {composed: true, detail: {toggle: true}}));
+ this.fire('toggle-drag', {toggle: true});
}
/**
diff --git a/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.html.ts b/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.html.ts
index e3b5ead..31b6cc3 100644
--- a/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.html.ts
+++ b/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.html.ts
@@ -27,13 +27,16 @@
<div id="profilesWrapper" ?hidden="${(this.shouldHideProfilesWrapper_())}">
<div id="profilesContainer" class="custom-scrollbar">
${this.profilesList_.map((item, index) => html`
- <profile-card class="profile-item" .profileState="${item}"
- data-index="${index}">
+ <profile-card class="profile-item" data-index="${index}"
+ .profileState="${item}" .disabled="${this.pickerButtonsDisabled_}"
+ @toggle-drag="${this.toggleDrag_}"
+ @disable-all-picker-buttons="${this.disableAllPickerButtons_}">
</profile-card>
`)}
<cr-button id="addProfile" class="profile-item"
@click="${this.onAddProfileClick_}"
?hidden="${!this.profileCreationAllowed_}"
+ ?disabled="${this.pickerButtonsDisabled_}"
aria-labelledby="addProfileButtonLabel">
<div id="addProfileButtonLabel"
class="profile-card-info prominent-text">
@@ -53,7 +56,8 @@
<div class="footer">
<cr-button id="browseAsGuestButton"
@click="${this.onLaunchGuestProfileClick_}"
- ?hidden="${!this.guestModeEnabled_}">
+ ?hidden="${!this.guestModeEnabled_}"
+ ?disabled="${this.pickerButtonsDisabled_}">
<cr-icon icon="profiles:account-circle" slot="prefix-icon"></cr-icon>
$i18n{browseAsGuestButton}
</cr-button>
diff --git a/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.ts b/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.ts
index 508f608..a27a013 100644
--- a/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.ts
+++ b/chrome/browser/resources/signin/profile_picker/profile_picker_main_view.ts
@@ -11,6 +11,7 @@
import '/strings.m.js';
import {HelpBubbleMixinLit} from 'chrome://resources/cr_components/help_bubble/help_bubble_mixin_lit.js';
+import type {CrButtonElement} from 'chrome://resources/cr_elements/cr_button/cr_button.js';
import type {CrCheckboxElement} from 'chrome://resources/cr_elements/cr_checkbox/cr_checkbox.js';
import type {CrDialogElement} from 'chrome://resources/cr_elements/cr_dialog/cr_dialog.js';
import {I18nMixinLit} from 'chrome://resources/cr_elements/i18n_mixin_lit.js';
@@ -32,7 +33,7 @@
export interface ProfilePickerMainViewElement {
$: {
- addProfile: HTMLElement,
+ addProfile: CrButtonElement,
askOnStartup: CrCheckboxElement,
'picker-logo': HTMLElement,
browseAsGuestButton: HTMLElement,
@@ -70,6 +71,7 @@
askOnStartup_: {type: Boolean},
guestModeEnabled_: {type: Boolean},
profileCreationAllowed_: {type: Boolean},
+ pickerButtonsDisabled_: {type: Boolean},
forceSigninErrorDialogTitle_: {type: String},
forceSigninErrorDialogBody_: {type: String},
forceSigninErrorProfilePath_: {type: String},
@@ -100,6 +102,8 @@
private dragDelegate_: DragDropReorderTileListDelegate|null = null;
private dragDuration_: number = 300;
+ protected accessor pickerButtonsDisabled_: boolean = false;
+
// TODO(crbug.com/40280498): Move the dialog into it's own element with the
// below members. This dialog state should be independent of the Profile
// Picker itself.
@@ -112,8 +116,6 @@
override firstUpdated() {
this.addEventListener('view-enter-finish', this.onViewEnterFinish_);
-
- this.addEventListener('toggle-drag', this.toggleDrag_);
}
override connectedCallback() {
@@ -127,6 +129,9 @@
'display-force-signin-error-dialog',
(title: string, body: string, profilePath: string) =>
this.showForceSigninErrorDialog(title, body, profilePath));
+ this.addWebUiListener('reset-picker-buttons', () => {
+ this.enableAllPickerButtons_();
+ });
if (!this.isGlic_) {
this.addWebUiListener(
'guest-mode-availability-updated',
@@ -170,6 +175,9 @@
override onRouteChange(route: Routes) {
if (route === Routes.MAIN) {
+ // Every time we go back to the main route, we re-enable all the profile
+ // card buttons.
+ this.enableAllPickerButtons_();
return;
}
this.previousRoute_ = route;
@@ -255,6 +263,8 @@
if (!isProfileCreationAllowed()) {
return;
}
+
+ this.disableAllPickerButtons_();
chrome.metricsPrivate.recordUserAction('ProfilePicker_AddClicked');
navigateTo(Routes.NEW_PROFILE);
}
@@ -291,7 +301,7 @@
return !isAskOnStartupAllowed() || this.profilesList_.length < 2;
}
- private toggleDrag_(e: Event) {
+ protected toggleDrag_(e: Event) {
if (!this.dragDelegate_) {
return;
}
@@ -300,6 +310,20 @@
this.dragDelegate_.toggleDrag(customEvent.detail.toggle);
}
+ protected disableAllPickerButtons_() {
+ this.pickerButtonsDisabled_ = true;
+ if (this.dragDelegate_) {
+ this.dragDelegate_.toggleDrag(false);
+ }
+ }
+
+ private enableAllPickerButtons_() {
+ this.pickerButtonsDisabled_ = false;
+ if (this.dragDelegate_) {
+ this.dragDelegate_.toggleDrag(true);
+ }
+ }
+
// Redirects the call to the handler, to create/use a browser to show the
// Help page.
private onLearnMoreClicked_(): void {
diff --git a/chrome/browser/ui/profiles/profile_picker.h b/chrome/browser/ui/profiles/profile_picker.h
index 8b88087..2e7b963 100644
--- a/chrome/browser/ui/profiles/profile_picker.h
+++ b/chrome/browser/ui/profiles/profile_picker.h
@@ -231,8 +231,12 @@
// the `profile` will be opened. On unsuccessful reauth, the user will be
// redirected to the profile picker main page, with a popup error dialog
// displayed through `on_error_callback`.
+ // `switch_finished_callback` will be called once the step was switched (or
+ // failed to switch to), the bool parameter indicating the success of the
+ // switch.
static void SwitchToReauth(
Profile* profile,
+ base::OnceCallback<void(bool)> switch_finished_callback,
base::OnceCallback<void(const ForceSigninUIError&)> on_error_callback);
#endif
@@ -252,8 +256,13 @@
};
// Picks the profile with `profile_path`.
- static void PickProfile(const base::FilePath& profile_path,
- ProfilePickingArgs args);
+ // `pick_profile_complete_callback` will be called when a browser is opened
+ // with the profile associated with `profile_path`, the boolean parameter
+ // returning whether a browser was successfully opened or not.
+ static void PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback);
// Cancel the signed-in flow and returns back to the main picker screen (if
// the original EntryPoint was to open the picker). Must only be called from
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 a3120fe..6c63b30 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
@@ -541,7 +541,8 @@
void FirstRunFlowControllerDice::PickProfile(
const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) {
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) {
NOTREACHED() << "FRE is not expected to handle this flow";
}
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 8458a1a..c0a44ad 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
@@ -42,8 +42,10 @@
// ProfileManagementFlowControllerImpl:
void Init() override;
void CancelPostSignInFlow() override;
- void PickProfile(const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) override;
+ void PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) override;
protected:
// ProfileManagementFlowControllerImpl
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 a7ae4be..f6e665f 100644
--- a/chrome/browser/ui/views/profiles/profile_management_flow_controller.h
+++ b/chrome/browser/ui/views/profiles/profile_management_flow_controller.h
@@ -101,8 +101,11 @@
virtual void CancelPostSignInFlow() = 0;
// Picks the profile with `profile_path`.
- virtual void PickProfile(const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) = 0;
+ // `pick_profile_complete_callback` will be called on profile load.
+ virtual void PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) = 0;
// Clears the current state and reset it to the initial state that shows the
// main screen. When calling this function the state should not be the
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 d50cc5c..e88a9754 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_flow_controller.cc
+++ b/chrome/browser/ui/views/profiles/profile_picker_flow_controller.cc
@@ -518,6 +518,7 @@
void ProfilePickerFlowController::SwitchToReauth(
Profile* profile,
+ StepSwitchFinishedCallback switch_finished_callback,
base::OnceCallback<void(const ForceSigninUIError&)> on_error_callback) {
DCHECK_EQ(Step::kProfilePicker, current_step());
@@ -541,7 +542,7 @@
std::move(on_error_callback))));
SwitchToStep(
- Step::kReauth, true, StepSwitchFinishedCallback(),
+ Step::kReauth, true, std::move(switch_finished_callback),
/*pop_step_callback=*/CreateSwitchToStepPopCallback(current_step()));
}
@@ -672,7 +673,8 @@
void ProfilePickerFlowController::PickProfile(
const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) {
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) {
if (args.should_record_startup_metrics &&
// Avoid overriding the picked time if already recorded. This can happen
// for example if multiple profiles are picked: https://crbug.com/1277466.
@@ -683,17 +685,26 @@
profiles::SwitchToProfile(
profile_path, /*always_create=*/false,
base::BindOnce(&ProfilePickerFlowController::OnSwitchToProfileComplete,
- weak_ptr_factory_.GetWeakPtr(), args.open_settings));
+ weak_ptr_factory_.GetWeakPtr(), args.open_settings,
+ std::move(pick_profile_complete_callback)));
}
-void ProfilePickerFlowController::OnSwitchToProfileComplete(bool open_settings,
- Browser* browser) {
+void ProfilePickerFlowController::OnSwitchToProfileComplete(
+ bool open_settings,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback,
+ Browser* browser) {
if (!browser || browser->is_delete_scheduled()) {
// The browser is destroyed or about to be destroyed.
+ if (pick_profile_complete_callback) {
+ std::move(pick_profile_complete_callback).Run(false);
+ }
return;
}
DCHECK(browser->window());
+ if (pick_profile_complete_callback) {
+ std::move(pick_profile_complete_callback).Run(true);
+ }
Profile* profile = browser->profile();
TRACE_EVENT1("browser",
"ProfilePickerFlowController::OnSwitchToProfileComplete",
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 0572aa6..967c5d6 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_flow_controller.h
+++ b/chrome/browser/ui/views/profiles/profile_picker_flow_controller.h
@@ -35,6 +35,7 @@
void SwitchToReauth(
Profile* profile,
+ StepSwitchFinishedCallback switch_finished_callback,
base::OnceCallback<void(const ForceSigninUIError&)> on_error_callback);
#endif
@@ -49,8 +50,10 @@
void SwitchToSignedOutPostIdentityFlow(Profile* profile);
// ProfileManagementFlowControllerImpl:
- void PickProfile(const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) override;
+ void PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) override;
protected:
// ProfileManagementFlowControllerImpl
@@ -78,7 +81,10 @@
std::unique_ptr<content::WebContents> contents) override;
// Callback after loading a profile and opening a browser.
- void OnSwitchToProfileComplete(bool open_settings, Browser* browser);
+ void OnSwitchToProfileComplete(
+ bool open_settings,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback,
+ Browser* browser);
const ProfilePicker::EntryPoint entry_point_;
const GURL selected_profile_target_url_;
diff --git a/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.cc b/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.cc
index b615ea5..2a79b811 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.cc
+++ b/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.cc
@@ -12,6 +12,7 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/ui/views/profiles/profile_management_step_controller.h"
+#include "chrome/browser/ui/views/profiles/profile_management_types.h"
#include "chrome/common/webui_url_constants.h"
#include "components/signin/public/identity_manager/identity_manager.h"
@@ -52,19 +53,30 @@
void ProfilePickerGlicFlowController::PickProfile(
const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) {
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) {
g_browser_process->profile_manager()->LoadProfileByPath(
profile_path, /*incognito=*/false,
base::BindOnce(&ProfilePickerGlicFlowController::OnPickedProfileLoaded,
- base::Unretained(this)));
+ base::Unretained(this),
+ std::move(pick_profile_complete_callback)));
}
-void ProfilePickerGlicFlowController::OnPickedProfileLoaded(Profile* profile) {
+void ProfilePickerGlicFlowController::OnPickedProfileLoaded(
+ base::OnceCallback<void(bool)> pick_profile_complete_callback,
+ Profile* profile) {
if (!profile) {
+ if (pick_profile_complete_callback) {
+ std::move(pick_profile_complete_callback).Run(false);
+ }
Clear();
return;
}
+ if (pick_profile_complete_callback) {
+ std::move(pick_profile_complete_callback).Run(true);
+ }
+
loaded_profile_ = profile;
signin::IdentityManager* identity_manager =
diff --git a/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.h b/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.h
index 93d8b4e..1753ff1 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.h
+++ b/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller.h
@@ -37,8 +37,10 @@
// Loads the profile with `profile_path` asynchronously then attempts to run
// the `picked_profile_callback_` callback with the loaded profile.
// `args` are actually not used in this implementation.
- void PickProfile(const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) override;
+ void PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) override;
private:
// ProfileManagementFlowController:
@@ -49,7 +51,9 @@
// Callback after loading the picked profile. Prepares `loaded_profile_` and
// attempts to exit the flow with the loaded profile.
- void OnPickedProfileLoaded(Profile* profile);
+ void OnPickedProfileLoaded(
+ base::OnceCallback<void(bool)> pick_profile_complete_callback,
+ Profile* profile);
// Returns `loaded_profile_` through `picked_profile_callback_` while ensuring
// that the refresh tokens are loaded.
diff --git a/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller_browsertest.cc b/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller_browsertest.cc
index 91d52ad0..4323d25c7 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller_browsertest.cc
+++ b/chrome/browser/ui/views/profiles/profile_picker_glic_flow_controller_browsertest.cc
@@ -81,7 +81,10 @@
ProfilePickerGlicFlowController controller(
host(), ClearHostClosure(clear_host_callback.Get()),
picked_profile_callback.Get());
- controller.PickProfile(new_profile_path, ProfilePicker::ProfilePickingArgs());
+ base::MockCallback<base::OnceCallback<void(bool)>> mock_callback;
+ EXPECT_CALL(mock_callback, Run(true));
+ controller.PickProfile(new_profile_path, ProfilePicker::ProfilePickingArgs(),
+ mock_callback.Get());
Profile* loaded_profile = profile_waiter.WaitForProfileAdded();
signin::WaitForRefreshTokensLoaded(
@@ -108,8 +111,11 @@
ProfilePickerGlicFlowController controller(
host(), ClearHostClosure(clear_host_callback.Get()),
picked_profile_callback.Get());
+ base::MockCallback<base::OnceCallback<void(bool)>> mock_callback;
+ EXPECT_CALL(mock_callback, Run(true));
controller.PickProfile(browser()->profile()->GetPath(),
- ProfilePicker::ProfilePickingArgs());
+ ProfilePicker::ProfilePickingArgs(),
+ mock_callback.Get());
}
IN_PROC_BROWSER_TEST_F(ProfilePickerGlicFlowControllerBrowserTest,
@@ -129,8 +135,11 @@
// as it was not created yet.
base::FilePath non_profile_file_path =
g_browser_process->profile_manager()->GenerateNextProfileDirectoryPath();
+ base::MockCallback<base::OnceCallback<void(bool)>> mock_callback;
+ EXPECT_CALL(mock_callback, Run(false));
controller.PickProfile(non_profile_file_path,
- ProfilePicker::ProfilePickingArgs());
+ ProfilePicker::ProfilePickingArgs(),
+ mock_callback.Get());
}
IN_PROC_BROWSER_TEST_F(ProfilePickerGlicFlowControllerBrowserTest,
diff --git a/chrome/browser/ui/views/profiles/profile_picker_view.cc b/chrome/browser/ui/views/profiles/profile_picker_view.cc
index d82ec02..d88d90f 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_view.cc
+++ b/chrome/browser/ui/views/profiles/profile_picker_view.cc
@@ -200,17 +200,21 @@
base::OnceCallback<void(bool)> switch_finished_callback) {
if (g_profile_picker_view) {
g_profile_picker_view->SwitchToDiceSignIn(
- std::move(profile_info), std::move(switch_finished_callback));
+ std::move(profile_info),
+ StepSwitchFinishedCallback(std::move(switch_finished_callback)));
}
}
// static
void ProfilePicker::SwitchToReauth(
Profile* profile,
+ base::OnceCallback<void(bool)> switch_finished_callback,
base::OnceCallback<void(const ForceSigninUIError&)> on_error_callback) {
if (g_profile_picker_view) {
- g_profile_picker_view->SwitchToReauth(profile,
- std::move(on_error_callback));
+ g_profile_picker_view->SwitchToReauth(
+ profile,
+ StepSwitchFinishedCallback(std::move(switch_finished_callback)),
+ std::move(on_error_callback));
}
}
#endif
@@ -224,10 +228,13 @@
}
// static
-void ProfilePicker::PickProfile(const base::FilePath& profile_path,
- ProfilePickingArgs args) {
+void ProfilePicker::PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) {
if (g_profile_picker_view) {
- g_profile_picker_view->flow_controller_->PickProfile(profile_path, args);
+ g_profile_picker_view->flow_controller_->PickProfile(
+ profile_path, args, std::move(pick_profile_complete_callback));
}
}
@@ -711,17 +718,18 @@
#if BUILDFLAG(ENABLE_DICE_SUPPORT)
void ProfilePickerView::SwitchToDiceSignIn(
ProfilePicker::ProfileInfo profile_info,
- base::OnceCallback<void(bool)> switch_finished_callback) {
+ StepSwitchFinishedCallback switch_finished_callback) {
GetProfilePickerFlowController()->SwitchToDiceSignIn(
- std::move(profile_info),
- StepSwitchFinishedCallback(std::move(switch_finished_callback)));
+ std::move(profile_info), std::move(switch_finished_callback));
}
void ProfilePickerView::SwitchToReauth(
Profile* profile,
+ StepSwitchFinishedCallback switch_finished_callback,
base::OnceCallback<void(const ForceSigninUIError&)> on_error_callback) {
GetProfilePickerFlowController()->SwitchToReauth(
- profile, std::move(on_error_callback));
+ profile, std::move(switch_finished_callback),
+ std::move(on_error_callback));
}
#endif
diff --git a/chrome/browser/ui/views/profiles/profile_picker_view.h b/chrome/browser/ui/views/profiles/profile_picker_view.h
index b11e3fa..fb24a5c6 100644
--- a/chrome/browser/ui/views/profiles/profile_picker_view.h
+++ b/chrome/browser/ui/views/profiles/profile_picker_view.h
@@ -15,6 +15,7 @@
#include "build/buildflag.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/profiles/profile_picker.h"
+#include "chrome/browser/ui/views/profiles/profile_management_types.h"
#include "chrome/browser/ui/views/profiles/profile_picker_web_contents_host.h"
#include "components/keep_alive_registry/scoped_keep_alive.h"
#include "components/user_education/common/feature_promo/feature_promo_controller.h"
@@ -193,9 +194,8 @@
#if BUILDFLAG(ENABLE_DICE_SUPPORT)
// Switches the layout to the sign-in screen (and creates a new profile or
// load an existing one based on the `profile_info` content).
- void SwitchToDiceSignIn(
- ProfilePicker::ProfileInfo profile_info,
- base::OnceCallback<void(bool)> switch_finished_callback);
+ void SwitchToDiceSignIn(ProfilePicker::ProfileInfo profile_info,
+ StepSwitchFinishedCallback switch_finished_callback);
// Switches the profile picker layout to display the reauth page to the main
// account of the given `profile` if needed. On success the `profile` is
@@ -204,6 +204,7 @@
// `on_error_callback`.
void SwitchToReauth(
Profile* profile,
+ StepSwitchFinishedCallback switch_finished_callback,
base::OnceCallback<void(const ForceSigninUIError&)> on_error_callback);
#endif
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 4228bcf..b62666e 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
@@ -85,8 +85,10 @@
void CancelPostSignInFlow() override { NOTREACHED(); }
- void PickProfile(const base::FilePath& profile_path,
- ProfilePicker::ProfilePickingArgs args) override {
+ void PickProfile(
+ const base::FilePath& profile_path,
+ ProfilePicker::ProfilePickingArgs args,
+ base::OnceCallback<void(bool)> pick_profile_complete_callback) override {
NOTREACHED();
}
diff --git a/chrome/browser/ui/webui/signin/profile_picker_handler.cc b/chrome/browser/ui/webui/signin/profile_picker_handler.cc
index c743fb8..d4e672c 100644
--- a/chrome/browser/ui/webui/signin/profile_picker_handler.cc
+++ b/chrome/browser/ui/webui/signin/profile_picker_handler.cc
@@ -43,6 +43,7 @@
#include "chrome/browser/ui/profiles/profile_colors_util.h"
#include "chrome/browser/ui/profiles/profile_picker.h"
#include "chrome/browser/ui/ui_features.h"
+#include "chrome/browser/ui/views/profiles/profile_management_types.h"
#include "chrome/browser/ui/webui/profile_helper.h"
#include "chrome/browser/ui/webui/signin/login_ui_service.h"
#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
@@ -377,7 +378,9 @@
*profile_path,
ProfilePicker::ProfilePickingArgs{
.open_settings = open_settings,
- .should_record_startup_metrics = should_record_startup_metrics});
+ .should_record_startup_metrics = should_record_startup_metrics},
+ base::BindOnce(&ProfilePickerHandler::OnResetPickerButtons,
+ weak_factory_.GetWeakPtr()));
}
void ProfilePickerHandler::TryLaunchLockedProfile(
@@ -398,6 +401,7 @@
DisplayForceSigninErrorDialog(
/*profile_path=*/base::FilePath(),
ForceSigninUIError::ReauthNotSupportedByGlicFlow());
+ OnResetPickerButtons(false);
return;
}
@@ -415,9 +419,14 @@
// Triggers a fresh sign in via profile picker without existing email
// address.
ProfilePicker::SwitchToDiceSignIn(
- entry.GetPath(),
- base::BindOnce(&ProfilePickerHandler::OnLoadSigninFinished,
- weak_factory_.GetWeakPtr()));
+ entry.GetPath(), CombineCallbacks<StepSwitchFinishedCallback, bool>(
+ StepSwitchFinishedCallback(base::BindOnce(
+ &ProfilePickerHandler::OnLoadSigninFinished,
+ weak_factory_.GetWeakPtr())),
+ StepSwitchFinishedCallback(base::BindOnce(
+ &ProfilePickerHandler::OnResetPickerButtons,
+ weak_factory_.GetWeakPtr())))
+ .value());
return;
}
@@ -428,6 +437,7 @@
DisplayForceSigninErrorDialog(
/*profile_path=*/base::FilePath(),
ForceSigninUIError::ReauthNotAllowed());
+ OnResetPickerButtons(false);
}
void ProfilePickerHandler::OnProfileLoadedForSwitchToReauth(Profile* profile) {
@@ -436,6 +446,8 @@
}
ProfilePicker::SwitchToReauth(
profile,
+ base::BindOnce(&ProfilePickerHandler::OnResetPickerButtons,
+ weak_factory_.GetWeakPtr()),
base::BindOnce(&ProfilePickerHandler::DisplayForceSigninErrorDialog,
weak_factory_.GetWeakPtr(), profile->GetPath()));
}
@@ -457,8 +469,10 @@
// checking has been added to the UI.
ProfilePicker::PickProfile(
ProfileManager::GetGuestProfilePath(),
- ProfilePicker::ProfilePickingArgs{
- .open_settings = false, .should_record_startup_metrics = false});
+ ProfilePicker::ProfilePickingArgs{.open_settings = false,
+ .should_record_startup_metrics = false},
+ base::BindOnce(&ProfilePickerHandler::OnResetPickerButtons,
+ weak_factory_.GetWeakPtr()));
}
void ProfilePickerHandler::HandleAskOnStartupChanged(
@@ -576,8 +590,10 @@
// flow.
ProfilePicker::PickProfile(
*profile_path,
- ProfilePicker::ProfilePickingArgs{
- .open_settings = false, .should_record_startup_metrics = false});
+ ProfilePicker::ProfilePickingArgs{.open_settings = false,
+ .should_record_startup_metrics = false},
+ base::BindOnce(&ProfilePickerHandler::OnResetPickerButtons,
+ weak_factory_.GetWeakPtr()));
}
void ProfilePickerHandler::HandleCancelProfileSwitch(
@@ -734,8 +750,14 @@
profile_color = GenerateNewProfileColor().color;
}
ProfilePicker::SwitchToDiceSignIn(
- profile_color, base::BindOnce(&ProfilePickerHandler::OnLoadSigninFinished,
- weak_factory_.GetWeakPtr()));
+ profile_color, CombineCallbacks<StepSwitchFinishedCallback, bool>(
+ StepSwitchFinishedCallback(base::BindOnce(
+ &ProfilePickerHandler::OnLoadSigninFinished,
+ weak_factory_.GetWeakPtr())),
+ StepSwitchFinishedCallback(base::BindOnce(
+ &ProfilePickerHandler::OnResetPickerButtons,
+ weak_factory_.GetWeakPtr())))
+ .value());
#else
NOTERACHED();
#endif
@@ -746,6 +768,11 @@
FireWebUIListener("load-signin-finished", base::Value(success));
}
+void ProfilePickerHandler::OnResetPickerButtons(bool success) {
+ AllowJavascript();
+ FireWebUIListener("reset-picker-buttons", base::Value(success));
+}
+
void ProfilePickerHandler::PushProfilesList() {
DCHECK(IsJavascriptAllowed());
FireWebUIListener("profiles-list-changed", GetProfilesList());
diff --git a/chrome/browser/ui/webui/signin/profile_picker_handler.h b/chrome/browser/ui/webui/signin/profile_picker_handler.h
index 8d312ff..6af2dd5 100644
--- a/chrome/browser/ui/webui/signin/profile_picker_handler.h
+++ b/chrome/browser/ui/webui/signin/profile_picker_handler.h
@@ -120,6 +120,7 @@
void HandleRecordSignInPromoImpression(const base::Value::List& args);
void OnLoadSigninFinished(bool success);
+ void OnResetPickerButtons(bool success);
void GatherProfileStatistics(Profile* profile);
void OnProfileStatisticsReceived(const base::FilePath& profile_path,
profiles::ProfileCategoryStats result);
diff --git a/chrome/test/data/webui/signin/profile_picker_browsertest.cc b/chrome/test/data/webui/signin/profile_picker_browsertest.cc
index 536bbd5a..4e0d719 100644
--- a/chrome/test/data/webui/signin/profile_picker_browsertest.cc
+++ b/chrome/test/data/webui/signin/profile_picker_browsertest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/strings/stringprintf.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/test/base/web_ui_mocha_browser_test.h"
#include "content/public/test/browser_test.h"
diff --git a/chrome/test/data/webui/signin/profile_picker_main_view_test.ts b/chrome/test/data/webui/signin/profile_picker_main_view_test.ts
index 2e8b9a74a..7fa5dab2 100644
--- a/chrome/test/data/webui/signin/profile_picker_main_view_test.ts
+++ b/chrome/test/data/webui/signin/profile_picker_main_view_test.ts
@@ -5,10 +5,10 @@
import 'chrome://profile-picker/profile_picker.js';
import type {ProfileCardElement, ProfilePickerMainViewElement, ProfileState} from 'chrome://profile-picker/profile_picker.js';
-import {loadTimeData, ManageProfilesBrowserProxyImpl, NavigationMixin, Routes} from 'chrome://profile-picker/profile_picker.js';
+import {loadTimeData, ManageProfilesBrowserProxyImpl, navigateTo, NavigationMixin, Routes} from 'chrome://profile-picker/profile_picker.js';
import {webUIListenerCallback} from 'chrome://resources/js/cr.js';
import {CrLitElement} from 'chrome://resources/lit/v3_0/lit.rollup.js';
-import {assertDeepEquals, assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
+import {assertDeepEquals, assertEquals, assertFalse, assertGE, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {isVisible, microtasksFinished} from 'chrome://webui-test/test_util.js';
import {TestManageProfilesBrowserProxy} from './test_manage_profiles_browser_proxy.js';
@@ -118,8 +118,10 @@
const profile = profiles[i]!;
const expectedProfile = expectedProfiles[i]!;
assertTrue(!!profile.shadowRoot.querySelector('profile-card-menu'));
- profile.shadowRoot.querySelector('cr-button')!.click();
+ profile.$.profileCardButton.click();
await browserProxy.whenCalled('launchSelectedProfile');
+ webUIListenerCallback('reset-picker-buttons');
+ await microtasksFinished();
assertEquals(
profile.shadowRoot
.querySelector<HTMLElement>('#forceSigninContainer')!.hidden,
@@ -165,6 +167,8 @@
assertTrue(isVisible(mainViewElement.$.addProfile));
mainViewElement.$.browseAsGuestButton.click();
await browserProxy.whenCalled('launchGuestProfile');
+ webUIListenerCallback('reset-picker-buttons');
+ await microtasksFinished();
// Ask when chrome opens.
mainViewElement.$.askOnStartup.click();
await browserProxy.whenCalled('askOnStartupChanged');
@@ -259,6 +263,79 @@
assertFalse(navigationElement.changeCalled);
});
+ test('ProfileCreationDisableButtons', async function() {
+ resetTest();
+ const addProfileButton = mainViewElement.$.addProfile;
+ assertFalse(isVisible(addProfileButton));
+ await browserProxy.whenCalled('initializeMainView');
+ await simulateProfilesListChanged(generateProfilesList(2));
+ navigationElement.reset();
+ assertTrue(isVisible(addProfileButton));
+ assertFalse(addProfileButton.disabled);
+ const profileCards =
+ mainViewElement.shadowRoot.querySelectorAll<ProfileCardElement>(
+ 'profile-card');
+ profileCards.forEach(profileCard => {
+ assertFalse(profileCard.$.profileCardButton.disabled);
+ });
+
+ // Clicking on the add profile button should disable all buttons.
+ addProfileButton.click();
+ await microtasksFinished();
+ assertTrue(navigationElement.changeCalled);
+ assertEquals(navigationElement.route, Routes.NEW_PROFILE);
+ assertTrue(addProfileButton.disabled);
+ profileCards.forEach(profileCard => {
+ assertTrue(profileCard.$.profileCardButton.disabled);
+ });
+
+ // Navigating back to the main route should re-enable the buttons.
+ navigationElement.reset();
+ navigateTo(Routes.MAIN);
+ await microtasksFinished();
+ assertTrue(navigationElement.changeCalled);
+ assertEquals(navigationElement.route, Routes.MAIN);
+ assertFalse(addProfileButton.disabled);
+ profileCards.forEach(profileCard => {
+ assertFalse(profileCard.$.profileCardButton.disabled);
+ });
+ });
+
+ test('ProfileCardClickDisableButtons', async function() {
+ resetTest();
+ const addProfileButton = mainViewElement.$.addProfile;
+ assertFalse(isVisible(addProfileButton));
+ await browserProxy.whenCalled('initializeMainView');
+ await simulateProfilesListChanged(generateProfilesList(2));
+ assertTrue(isVisible(addProfileButton));
+ assertFalse(addProfileButton.disabled);
+ const profileCards =
+ mainViewElement.shadowRoot.querySelectorAll<ProfileCardElement>(
+ 'profile-card');
+ profileCards.forEach(profileCard => {
+ assertFalse(profileCard.$.profileCardButton.disabled);
+ });
+
+ // Clicking on a profile button should disable all buttons.
+ assertGE(profileCards.length, 1);
+ const firstProfileCard = profileCards[0]!;
+ firstProfileCard.$.profileCardButton.click();
+ await microtasksFinished();
+ assertTrue(addProfileButton.disabled);
+ profileCards.forEach(profileCard => {
+ assertTrue(profileCard.$.profileCardButton.disabled);
+ });
+
+ // Receiving the signal that the step was completed should re-enable the
+ // buttons.
+ webUIListenerCallback('reset-picker-buttons');
+ await microtasksFinished();
+ assertFalse(addProfileButton.disabled);
+ profileCards.forEach(profileCard => {
+ assertFalse(profileCard.$.profileCardButton.disabled);
+ });
+ });
+
test('AskOnStartupSingleToMultipleProfiles', async function() {
await browserProxy.whenCalled('initializeMainView');
// Hidden while profiles list is not yet defined.
