Skip to content

Commit

Permalink
Use SigninChoice for the enterprise welcome dialog
Browse files Browse the repository at this point in the history
Bug: 1275359
Change-Id: Ibeea6f996973d4fa02c831128b5d77eca44979c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3530729
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Yann Dago <ydago@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985553}
  • Loading branch information
Yann Dago authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 219f435 commit 5a0f11f
Show file tree
Hide file tree
Showing 17 changed files with 73 additions and 50 deletions.
25 changes: 19 additions & 6 deletions chrome/browser/ui/signin/dice_web_signin_interceptor_delegate.cc
Expand Up @@ -17,6 +17,7 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "components/signin/public/base/signin_metrics.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "google_apis/gaia/core_account_id.h"
Expand Down Expand Up @@ -69,13 +70,25 @@ class ForcedEnterpriseSigninInterceptionHandle
base::Unretained(this)));
}

void OnEnterpriseInterceptionDialogClosed(bool create_profile) {
if (!create_profile)
browser_->signin_view_controller()->CloseModalSignin();
std::move(callback_).Run(create_profile
? SigninInterceptionResult::kAccepted
: SigninInterceptionResult::kDeclined);
void OnEnterpriseInterceptionDialogClosed(signin::SigninChoice choice) {
switch (choice) {
case signin::SIGNIN_CHOICE_NEW_PROFILE:
std::move(callback_).Run(SigninInterceptionResult::kAccepted);
break;
case signin::SIGNIN_CHOICE_CANCEL:
browser_->signin_view_controller()->CloseModalSignin();
std::move(callback_).Run(SigninInterceptionResult::kDeclined);
break;
case signin::SIGNIN_CHOICE_CONTINUE:
// TODO (crbug/1275359): Add support for this once the "Link Data"
// checkbox is added to the interception dialog.
case signin::SIGNIN_CHOICE_SIZE:
default:
NOTREACHED();
break;
}
}

raw_ptr<Browser> browser_;
base::OnceCallback<void(SigninInterceptionResult)> callback_;
};
Expand Down
9 changes: 3 additions & 6 deletions chrome/browser/ui/signin_view_controller.cc
Expand Up @@ -20,6 +20,7 @@
#include "chrome/browser/ui/signin_modal_dialog.h"
#include "chrome/browser/ui/signin_modal_dialog_impl.h"
#include "chrome/browser/ui/signin_view_controller_delegate.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "components/signin/public/base/consent_level.h"
#include "components/signin/public/base/signin_buildflags.h"
#include "components/signin/public/identity_manager/account_info.h"
Expand Down Expand Up @@ -257,17 +258,13 @@ void SigninViewController::ShowModalSyncConfirmationDialog() {
void SigninViewController::ShowModalEnterpriseConfirmationDialog(
const AccountInfo& account_info,
SkColor profile_color,
base::OnceCallback<void(bool)> callback) {
signin::SigninChoiceCallback callback) {
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || \
BUILDFLAG(IS_CHROMEOS_LACROS)
CloseModalSignin();
dialog_ = std::make_unique<SigninModalDialogImpl>(
SigninViewControllerDelegate::CreateEnterpriseConfirmationDelegate(
browser_, account_info, profile_color,
base::BindOnce(
[](Browser* browser, base::OnceCallback<void(bool)> callback,
bool result) { std::move(callback).Run(result); },
base::Unretained(browser_), std::move(callback))),
browser_, account_info, profile_color, std::move(callback)),
GetOnModalDialogClosedCallback());
chrome::RecordDialogCreation(
chrome::DialogIdentifier::SIGNIN_ENTERPRISE_INTERCEPTION);
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/signin_view_controller.h
Expand Up @@ -14,6 +14,7 @@
#include "build/build_config.h"
#include "chrome/browser/ui/profile_chooser_constants.h"
#include "chrome/browser/ui/signin_modal_dialog.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "components/signin/public/base/signin_buildflags.h"
#include "third_party/skia/include/core/SkColor.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -144,7 +145,7 @@ class SigninViewController {
void ShowModalEnterpriseConfirmationDialog(
const AccountInfo& account_info,
SkColor profile_color,
base::OnceCallback<void(bool)> callback);
signin::SigninChoiceCallback callback);

// Shows the modal sign-in error dialog as a browser-modal dialog on top of
// the |browser_|'s window.
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/signin_view_controller_delegate.h
Expand Up @@ -10,6 +10,7 @@
#include "base/observer_list_types.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "components/signin/public/base/signin_buildflags.h"
#include "third_party/skia/include/core/SkColor.h"

Expand Down Expand Up @@ -82,7 +83,7 @@ class SigninViewControllerDelegate {
Browser* browser,
const AccountInfo& account_info,
SkColor profile_color,
base::OnceCallback<void(bool)> callback);
signin::SigninChoiceCallback callback);
#endif

void AddObserver(Observer* observer);
Expand Down
Expand Up @@ -205,13 +205,14 @@ IN_PROC_BROWSER_TEST_F(SignInViewControllerBrowserTest,
content::TestNavigationObserver content_observer(
GURL("chrome://enterprise-profile-welcome/"));
content_observer.StartWatchingNewWebContents();
bool result;
signin::SigninChoice result;
browser()->signin_view_controller()->ShowModalEnterpriseConfirmationDialog(
account_info, SK_ColorWHITE,
base::BindOnce(
[](Browser* browser, bool* result, bool create) {
[](Browser* browser, signin::SigninChoice* result,
signin::SigninChoice choice) {
browser->signin_view_controller()->CloseModalSignin();
*result = create;
*result = choice;
},
browser(), &result));
EXPECT_TRUE(browser()->signin_view_controller()->ShowsModalDialog());
Expand All @@ -227,6 +228,6 @@ IN_PROC_BROWSER_TEST_F(SignInViewControllerBrowserTest,
/*command=*/false));

dialog_destroyed_watcher.Wait();
EXPECT_TRUE(result);
EXPECT_EQ(result, signin::SigninChoice::SIGNIN_CHOICE_NEW_PROFILE);
EXPECT_FALSE(browser()->signin_view_controller()->ShowsModalDialog());
}
Expand Up @@ -78,7 +78,7 @@ void ProfilePickerSignedInFlowController::SwitchToSyncConfirmation() {

void ProfilePickerSignedInFlowController::SwitchToEnterpriseProfileWelcome(
EnterpriseProfileWelcomeUI::ScreenType type,
base::OnceCallback<void(bool)> proceed_callback) {
signin::SigninChoiceCallback proceed_callback) {
DCHECK(IsInitialized());
host_->ShowScreen(contents(),
GURL(chrome::kChromeUIEnterpriseProfileWelcomeURL),
Expand Down Expand Up @@ -150,7 +150,7 @@ void ProfilePickerSignedInFlowController::SwitchToSyncConfirmationFinished() {
void ProfilePickerSignedInFlowController::
SwitchToEnterpriseProfileWelcomeFinished(
EnterpriseProfileWelcomeUI::ScreenType type,
base::OnceCallback<void(bool)> proceed_callback) {
signin::SigninChoiceCallback proceed_callback) {
DCHECK(IsInitialized());
// Initialize the WebUI page once we know it's committed.
EnterpriseProfileWelcomeUI* enterprise_profile_welcome_ui =
Expand Down
Expand Up @@ -61,7 +61,7 @@ class ProfilePickerSignedInFlowController
// screen.
void SwitchToEnterpriseProfileWelcome(
EnterpriseProfileWelcomeUI::ScreenType type,
base::OnceCallback<void(bool)> proceed_callback);
signin::SigninChoiceCallback proceed_callback);

// When the sign-in flow cannot be completed because another profile at
// `profile_path` is already syncing with a chosen account, shows the profile
Expand Down Expand Up @@ -94,7 +94,7 @@ class ProfilePickerSignedInFlowController
void SwitchToSyncConfirmationFinished();
void SwitchToEnterpriseProfileWelcomeFinished(
EnterpriseProfileWelcomeUI::ScreenType type,
base::OnceCallback<void(bool)> proceed_callback);
signin::SigninChoiceCallback proceed_callback);

// Returns whether the flow is initialized (i.e. whether `Init()` has been
// called).
Expand Down
Expand Up @@ -224,8 +224,8 @@ void ProfilePickerTurnSyncOnDelegate::ShowEnterpriseWelcome(

void ProfilePickerTurnSyncOnDelegate::OnEnterpriseWelcomeClosed(
EnterpriseProfileWelcomeUI::ScreenType type,
bool proceed) {
if (!proceed) {
signin::SigninChoice choice) {
if (choice == signin::SIGNIN_CHOICE_CANCEL) {
LogOutcome(ProfileMetrics::ProfileSignedInFlowOutcome::
kAbortedOnEnterpriseWelcome);
// The callback provided by TurnSyncOnHelper must be called, UI_CLOSED
Expand All @@ -237,6 +237,8 @@ void ProfilePickerTurnSyncOnDelegate::OnEnterpriseWelcomeClosed(
return;
}

DCHECK_EQ(choice, signin::SIGNIN_CHOICE_NEW_PROFILE);

switch (type) {
case EnterpriseProfileWelcomeUI::ScreenType::kEntepriseAccountSyncEnabled:
ShowSyncConfirmationScreen();
Expand Down
Expand Up @@ -67,7 +67,7 @@ class ProfilePickerTurnSyncOnDelegate : public TurnSyncOnHelper::Delegate,
// Shows the enterprise welcome screen.
void ShowEnterpriseWelcome(EnterpriseProfileWelcomeUI::ScreenType type);
void OnEnterpriseWelcomeClosed(EnterpriseProfileWelcomeUI::ScreenType type,
bool proceed);
signin::SigninChoice choice);

// Reports metric with the outcome of the turn-sync-on flow.
void LogOutcome(ProfileMetrics::ProfileSignedInFlowOutcome outcome);
Expand Down
Expand Up @@ -53,6 +53,7 @@
#include "chrome/browser/ui/webui/signin/profile_picker_handler.h"
#include "chrome/browser/ui/webui/signin/profile_picker_ui.h"
#include "chrome/browser/ui/webui/signin/signin_url_utils.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/profile_deletion_observer.h"
Expand Down Expand Up @@ -1270,7 +1271,7 @@ class ProfilePickerEnterpriseCreationFlowBrowserTest

void ExpectEnterpriseScreenTypeAndProceed(
EnterpriseProfileWelcomeUI::ScreenType expected_type,
bool proceed) {
signin::SigninChoice choice) {
EnterpriseProfileWelcomeHandler* handler =
web_contents()
->GetWebUI()
Expand All @@ -1280,7 +1281,7 @@ class ProfilePickerEnterpriseCreationFlowBrowserTest
EXPECT_EQ(handler->GetTypeForTesting(), expected_type);

// Simulate clicking on the next button.
handler->CallProceedCallbackForTesting(proceed);
handler->CallProceedCallbackForTesting(choice);
}
};

Expand All @@ -1299,7 +1300,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerEnterpriseCreationFlowBrowserTest,
ExpectEnterpriseScreenTypeAndProceed(
/*expected_type=*/EnterpriseProfileWelcomeUI::ScreenType::
kEntepriseAccountSyncEnabled,
/*proceed=*/true);
/*choice=*/signin::SIGNIN_CHOICE_NEW_PROFILE);

WaitForLoadStop(GetSyncConfirmationURL());
// Simulate finishing the flow with "No, thanks".
Expand Down Expand Up @@ -1356,7 +1357,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerEnterpriseCreationFlowBrowserTest,
ExpectEnterpriseScreenTypeAndProceed(
/*expected_type=*/EnterpriseProfileWelcomeUI::ScreenType::
kConsumerAccountSyncDisabled,
/*proceed=*/true);
/*choice=*/signin::SIGNIN_CHOICE_NEW_PROFILE);

Browser* new_browser = BrowserAddedWaiter(2u).Wait();
WaitForLoadStop(GURL("chrome://newtab/"),
Expand Down Expand Up @@ -1400,7 +1401,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerEnterpriseCreationFlowBrowserTest,
ExpectEnterpriseScreenTypeAndProceed(
/*expected_type=*/EnterpriseProfileWelcomeUI::ScreenType::
kEntepriseAccountSyncEnabled,
/*proceed=*/true);
/*choice=*/signin::SIGNIN_CHOICE_NEW_PROFILE);

WaitForLoadStop(GetSyncConfirmationURL());
// Simulate finishing the flow with "Configure sync".
Expand Down Expand Up @@ -1454,7 +1455,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerEnterpriseCreationFlowBrowserTest, Cancel) {
ExpectEnterpriseScreenTypeAndProceed(
/*expected_type=*/EnterpriseProfileWelcomeUI::ScreenType::
kEntepriseAccountSyncEnabled,
/*proceed=*/false);
/*choice=*/signin::SIGNIN_CHOICE_CANCEL);

// As the profile creation flow was opened directly, the window is closed now.
WaitForPickerClosed();
Expand Down
Expand Up @@ -19,6 +19,7 @@
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/webui/signin/profile_customization_ui.h"
#include "chrome/browser/ui/webui/signin/signin_url_utils.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "chrome/browser/ui/webui/signin/sync_confirmation_ui.h"
#include "chrome/common/url_constants.h"
#include "chrome/common/webui_url_constants.h"
Expand Down Expand Up @@ -144,7 +145,7 @@ SigninViewControllerDelegateViews::CreateEnterpriseConfirmationWebView(
Browser* browser,
const AccountInfo& account_info,
SkColor profile_color,
base::OnceCallback<void(bool)> callback) {
signin::SigninChoiceCallback callback) {
std::unique_ptr<views::WebView> web_view = CreateDialogWebView(
browser, GURL(chrome::kChromeUIEnterpriseProfileWelcomeURL),
kSyncConfirmationDialogHeight, kSyncConfirmationDialogWidth,
Expand Down Expand Up @@ -421,7 +422,7 @@ SigninViewControllerDelegate::CreateEnterpriseConfirmationDelegate(
Browser* browser,
const AccountInfo& account_info,
SkColor profile_color,
base::OnceCallback<void(bool)> callback) {
signin::SigninChoiceCallback callback) {
return new SigninViewControllerDelegateViews(
SigninViewControllerDelegateViews::CreateEnterpriseConfirmationWebView(
browser, account_info, profile_color, std::move(callback)),
Expand Down
Expand Up @@ -14,6 +14,7 @@
#include "chrome/browser/ui/profile_chooser_constants.h"
#include "chrome/browser/ui/signin_view_controller_delegate.h"
#include "chrome/browser/ui/webui/signin/enterprise_profile_welcome_ui.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "components/signin/public/base/signin_buildflags.h"
#include "content/public/browser/web_contents_delegate.h"
#include "third_party/skia/include/core/SkColor.h"
Expand Down Expand Up @@ -76,7 +77,7 @@ class SigninViewControllerDelegateViews
Browser* browser,
const AccountInfo& account_info,
SkColor profile_color,
base::OnceCallback<void(bool)> callback);
signin::SigninChoiceCallback callback);
#endif

// views::DialogDelegateView:
Expand Down
Expand Up @@ -94,7 +94,7 @@ EnterpriseProfileWelcomeHandler::EnterpriseProfileWelcomeHandler(
EnterpriseProfileWelcomeUI::ScreenType type,
const AccountInfo& account_info,
absl::optional<SkColor> profile_color,
base::OnceCallback<void(bool)> proceed_callback)
signin::SigninChoiceCallback proceed_callback)
: browser_(browser),
type_(type),
email_(base::UTF8ToUTF16(account_info.email)),
Expand Down Expand Up @@ -199,13 +199,13 @@ void EnterpriseProfileWelcomeHandler::HandleInitializedWithSize(
void EnterpriseProfileWelcomeHandler::HandleProceed(
const base::ListValue* args) {
if (proceed_callback_)
std::move(proceed_callback_).Run(true);
std::move(proceed_callback_).Run(signin::SIGNIN_CHOICE_NEW_PROFILE);
}

void EnterpriseProfileWelcomeHandler::HandleCancel(
const base::ListValue* args) {
if (proceed_callback_)
std::move(proceed_callback_).Run(false);
std::move(proceed_callback_).Run(signin::SIGNIN_CHOICE_CANCEL);
}

void EnterpriseProfileWelcomeHandler::UpdateProfileInfo(
Expand Down Expand Up @@ -306,7 +306,7 @@ EnterpriseProfileWelcomeHandler::GetTypeForTesting() {
}

void EnterpriseProfileWelcomeHandler::CallProceedCallbackForTesting(
bool proceed) {
signin::SigninChoice choice) {
if (proceed_callback_)
std::move(proceed_callback_).Run(proceed);
std::move(proceed_callback_).Run(choice);
}
Expand Up @@ -15,6 +15,7 @@
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/ui/browser_list_observer.h"
#include "chrome/browser/ui/webui/signin/enterprise_profile_welcome_ui.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "content/public/browser/web_ui_message_handler.h"
#include "google_apis/gaia/core_account_id.h"
Expand All @@ -41,7 +42,7 @@ class EnterpriseProfileWelcomeHandler
EnterpriseProfileWelcomeUI::ScreenType type,
const AccountInfo& account_info,
absl::optional<SkColor> profile_color,
base::OnceCallback<void(bool)> proceed_callback);
signin::SigninChoiceCallback proceed_callback);
~EnterpriseProfileWelcomeHandler() override;

EnterpriseProfileWelcomeHandler(const EnterpriseProfileWelcomeHandler&) =
Expand Down Expand Up @@ -69,7 +70,7 @@ class EnterpriseProfileWelcomeHandler

// Access to construction parameters for tests.
EnterpriseProfileWelcomeUI::ScreenType GetTypeForTesting();
void CallProceedCallbackForTesting(bool proceed);
void CallProceedCallbackForTesting(signin::SigninChoice choice);

private:
void HandleInitialized(const base::ListValue* args);
Expand Down Expand Up @@ -108,7 +109,7 @@ class EnterpriseProfileWelcomeHandler
const std::string domain_name_;
const CoreAccountId account_id_;
absl::optional<SkColor> profile_color_;
base::OnceCallback<void(bool)> proceed_callback_;
signin::SigninChoiceCallback proceed_callback_;
};

#endif // CHROME_BROWSER_UI_WEBUI_SIGNIN_ENTERPRISE_PROFILE_WELCOME_HANDLER_H_
Expand Up @@ -7,6 +7,7 @@
#include "base/callback_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/signin/enterprise_profile_welcome_handler.h"
#include "chrome/browser/ui/webui/signin/signin_utils.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/chromium_strings.h"
Expand Down Expand Up @@ -55,7 +56,7 @@ void EnterpriseProfileWelcomeUI::Initialize(
EnterpriseProfileWelcomeUI::ScreenType type,
const AccountInfo& account_info,
absl::optional<SkColor> profile_color,
base::OnceCallback<void(bool)> proceed_callback) {
signin::SigninChoiceCallback proceed_callback) {
auto handler = std::make_unique<EnterpriseProfileWelcomeHandler>(
browser, type, account_info, profile_color, std::move(proceed_callback));
handler_ = handler.get();
Expand Down

0 comments on commit 5a0f11f

Please sign in to comment.