Commit 99646a9a authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

Revert "[Dice sync delegate] Split sync confirmation and sync disabled"

This reverts commit d42167eb.

Reason for revert: This seems to have made ProfilePickerEnterpriseCreationFlowBrowserTest.CreateSignedInProfileSettings fail on win7 bot: https://ci.chromium.org/p/chromium/builders/ci/Win7%20%2832%29%20Tests/65702 . Output: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8865640044637153696/+/steps/browser_tests/0/logs/Deterministic_failure:_ProfilePickerEnterpriseCreationFlowBrowserTest.CreateSignedInProfileSettings__status_CRASH_/0

Original change's description:
> [Dice sync delegate] Split sync confirmation and sync disabled
>
> This CL splits the ShowSyncConfirmation() function of the dice turn on
> sync helper delegate into one that shows the normal sync confirmation
> and one that shows a error message that sync is disabled by the admin.
>
> Since both the dialogs are implemented by the same webUI page, there
> is not much behavioral change. The only change is for the delegate
> for the signed-in profile creation flow which opens a new browser
> window and shows the dialog in this window instead of showing it in
> the creation flow window.
>
> In follow-up CLs, the variant of the UI will be passed as a param
> to the WebUI page replacing the logic to decide which variant to show.
>
> Bug: 1126913
> Change-Id: Ibac9a339c4fb071f960f15864482c818247f5719
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487100
> Commit-Queue: Jan Krcal <jkrcal@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Auto-Submit: Jan Krcal <jkrcal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#820222}

TBR=droger@chromium.org,jkrcal@chromium.org

Change-Id: I81c91215340e0d77530c3e1faf0a23eac31722b9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1126913
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495329Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820268}
parent 239086c8
......@@ -63,9 +63,6 @@ class TestDiceTurnSyncOnHelperDelegate : public DiceTurnSyncOnHelper::Delegate {
const std::string& email,
DiceTurnSyncOnHelper::SigninChoiceCallback callback) override;
void ShowSyncConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override {}
void ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override;
void ShowSyncSettings() override;
......@@ -166,7 +163,7 @@ class UserPolicySigninServiceTest : public InProcessBrowserTest {
std::move(callback).Run(DiceTurnSyncOnHelper::SIGNIN_CHOICE_CONTINUE);
}
void OnShowSyncDisabledConfirmation(
void OnShowSyncConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) {
sync_confirmation_callback_ = std::move(callback);
......@@ -332,10 +329,10 @@ void TestDiceTurnSyncOnHelperDelegate::ShowEnterpriseAccountConfirmation(
std::move(callback));
}
void TestDiceTurnSyncOnHelperDelegate::ShowSyncDisabledConfirmation(
void TestDiceTurnSyncOnHelperDelegate::ShowSyncConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) {
test_fixture_->OnShowSyncDisabledConfirmation(std::move(callback));
test_fixture_->OnShowSyncConfirmation(std::move(callback));
}
void TestDiceTurnSyncOnHelperDelegate::ShowSyncSettings() {
......
......@@ -59,9 +59,6 @@ class TestDiceTurnSyncOnHelperDelegate : public DiceTurnSyncOnHelper::Delegate {
callback) override {
std::move(callback).Run(LoginUIService::SYNC_WITH_DEFAULT_SETTINGS);
}
void ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override {}
void ShowSyncSettings() override {}
void SwitchToProfile(Profile* new_profile) override {}
};
......
......@@ -18,16 +18,6 @@ void OpenSettingsInBrowser(Browser* browser) {
chrome::ShowSettingsSubPage(browser, chrome::kSyncSetupSubPage);
}
void OpenSyncConfirmationDialogInBrowser(Browser* browser) {
// This is a very rare corner case (e.g. the user manages to close the only
// browser window in a very short span of time between enterprise
// confirmation and this callback), not worth handling fully. Instead, the
// flow is aborted.
if (!browser)
return;
browser->signin_view_controller()->ShowModalSyncConfirmationDialog();
}
} // namespace
ProfilePickerViewSyncDelegate::ProfilePickerViewSyncDelegate(
......@@ -81,27 +71,20 @@ void ProfilePickerViewSyncDelegate::ShowSyncConfirmation(
LoginUIServiceFactory::GetForProfile(profile_));
if (enterprise_confirmation_shown_) {
OpenSyncConfirmationDialogInBrowser(
chrome::FindLastActiveWithProfile(profile_));
Browser* browser = chrome::FindLastActiveWithProfile(profile_);
// This is a very rare corner case (e.g. the user manages to close the only
// browser window in a very short span of time between enterprise
// confirmation and this callback), not worth handling fully. Instead, the
// flow is aborted.
if (!browser)
return;
browser->signin_view_controller()->ShowModalSyncConfirmationDialog();
return;
}
ProfilePicker::SwitchToSyncConfirmation();
}
void ProfilePickerViewSyncDelegate::ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) {
DCHECK(callback);
sync_confirmation_callback_ = std::move(callback);
scoped_login_ui_service_observer_.Add(
LoginUIServiceFactory::GetForProfile(profile_));
// Open the browser and when it's done, show the confirmation dialog.
std::move(open_browser_callback_)
.Run(base::BindOnce(&OpenSyncConfirmationDialogInBrowser));
}
void ProfilePickerViewSyncDelegate::ShowSyncSettings() {
if (enterprise_confirmation_shown_) {
Browser* browser = chrome::FindLastActiveWithProfile(profile_);
......
......@@ -38,9 +38,6 @@ class ProfilePickerViewSyncDelegate : public DiceTurnSyncOnHelper::Delegate,
void ShowSyncConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override;
void ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override;
void ShowSyncSettings() override;
void SwitchToProfile(Profile* new_profile) override;
......
......@@ -528,15 +528,9 @@ void DiceTurnSyncOnHelper::SyncStartupFailed() {
}
void DiceTurnSyncOnHelper::ShowSyncConfirmationUI() {
if (GetSyncService()) {
delegate_->ShowSyncConfirmation(
base::BindOnce(&DiceTurnSyncOnHelper::FinishSyncSetupAndDelete,
weak_pointer_factory_.GetWeakPtr()));
} else {
delegate_->ShowSyncDisabledConfirmation(
base::BindOnce(&DiceTurnSyncOnHelper::FinishSyncSetupAndDelete,
weak_pointer_factory_.GetWeakPtr()));
}
delegate_->ShowSyncConfirmation(
base::BindOnce(&DiceTurnSyncOnHelper::FinishSyncSetupAndDelete,
weak_pointer_factory_.GetWeakPtr()));
}
void DiceTurnSyncOnHelper::FinishSyncSetupAndDelete(
......
......@@ -90,14 +90,6 @@ class DiceTurnSyncOnHelper
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) = 0;
// Shows a confirmation screen offering to stay signed-in or to signout.
// |callback| must be called.
// TODO(crbug.com/1126913): Use a new enum for this callback with only
// values that make sense here (stay signed-in / signout).
virtual void ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) = 0;
// Opens the Sync settings page.
virtual void ShowSyncSettings() = 0;
......
......@@ -100,13 +100,6 @@ void DiceTurnSyncOnHelperDelegateImpl::ShowSyncConfirmation(
browser_->signin_view_controller()->ShowModalSyncConfirmationDialog();
}
void DiceTurnSyncOnHelperDelegateImpl::ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) {
// This is handled by the same UI element as the normal sync confirmation.
ShowSyncConfirmation(std::move(callback));
}
void DiceTurnSyncOnHelperDelegateImpl::ShowMergeSyncDataConfirmation(
const std::string& previous_email,
const std::string& new_email,
......
......@@ -38,9 +38,6 @@ class DiceTurnSyncOnHelperDelegateImpl : public DiceTurnSyncOnHelper::Delegate,
void ShowSyncConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override;
void ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override;
void ShowSyncSettings() override;
void SwitchToProfile(Profile* new_profile) override;
......
......@@ -87,9 +87,6 @@ class TestDiceTurnSyncOnHelperDelegate : public DiceTurnSyncOnHelper::Delegate {
void ShowSyncConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override;
void ShowSyncDisabledConfirmation(
base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
callback) override {}
void ShowSyncSettings() override;
void SwitchToProfile(Profile* new_profile) override;
......
......@@ -35,8 +35,6 @@ class LoginUIService : public KeyedService {
// Used when the sync confirmation UI is closed to signify which option was
// selected by the user.
enum SyncConfirmationUIClosedResult {
// TODO(crbug.com/1141341): Rename the first option to make it work better
// for the sync-disabled variant of the UI.
// Start sync immediately.
SYNC_WITH_DEFAULT_SETTINGS,
// Show the user the sync settings before starting sync.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment