Commit 3c456415 authored by Scott Chen's avatar Scott Chen Committed by Commit Bot

Settings: fix corner case for showing sync page.

CL 969741 accidentally broke parity during refactoring, this CL fixes it,
so in the case that sync has the "Confirm Sync Settings" error, the page
is still reachable.

Bug: None
Change-Id: I9fa811b8c3e5f220b9d856dbbf860a06f51b1e4a
Reviewed-on: https://chromium-review.googlesource.com/973946Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Scott Chen <scottchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544925}
parent 70927ec9
...@@ -383,7 +383,7 @@ void PeopleHandler::DisplayTimeout() { ...@@ -383,7 +383,7 @@ void PeopleHandler::DisplayTimeout() {
void PeopleHandler::OnDidClosePage(const base::ListValue* args) { void PeopleHandler::OnDidClosePage(const base::ListValue* args) {
// Don't mark setup as complete if "didAbort" is true, or if authentication // Don't mark setup as complete if "didAbort" is true, or if authentication
// is still needed. // is still needed.
if (!args->GetList()[0].GetBool() && !IsProfileAuthNeeded()) { if (!args->GetList()[0].GetBool() && !IsProfileAuthNeededOrHasErrors()) {
MarkFirstSetupComplete(); MarkFirstSetupComplete();
} }
...@@ -623,7 +623,10 @@ void PeopleHandler::HandleShowSetupUI(const base::ListValue* args) { ...@@ -623,7 +623,10 @@ void PeopleHandler::HandleShowSetupUI(const base::ListValue* args) {
return; return;
} }
if (IsProfileAuthNeeded()) { // This if-statement is not using IsProfileAuthNeededOrHasErrors(), because
// in some error cases (e.g. "confirmSyncSettings") the UI still needs to
// show.
if (!SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated()) {
// For web-based signin, the signin page is not displayed in an overlay // For web-based signin, the signin page is not displayed in an overlay
// on the settings page. So if we get here, it must be due to the user // on the settings page. So if we get here, it must be due to the user
// cancelling signin (by reloading the sync settings page during initial // cancelling signin (by reloading the sync settings page during initial
...@@ -683,7 +686,7 @@ void PeopleHandler::HandleStartSignin(const base::ListValue* args) { ...@@ -683,7 +686,7 @@ void PeopleHandler::HandleStartSignin(const base::ListValue* args) {
// Should only be called if the user is not already signed in or has an auth // Should only be called if the user is not already signed in or has an auth
// error. // error.
DCHECK(IsProfileAuthNeeded()); DCHECK(IsProfileAuthNeededOrHasErrors());
DisplayGaiaLogin(signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS); DisplayGaiaLogin(signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS);
} }
...@@ -884,7 +887,7 @@ PeopleHandler::GetSyncStatusDictionary() { ...@@ -884,7 +887,7 @@ PeopleHandler::GetSyncStatusDictionary() {
void PeopleHandler::PushSyncPrefs() { void PeopleHandler::PushSyncPrefs() {
#if !defined(OS_CHROMEOS) #if !defined(OS_CHROMEOS)
// Early exit if the user has not signed in yet. // Early exit if the user has not signed in yet.
if (IsProfileAuthNeeded()) if (IsProfileAuthNeededOrHasErrors())
return; return;
#endif #endif
...@@ -1015,7 +1018,7 @@ void PeopleHandler::MarkFirstSetupComplete() { ...@@ -1015,7 +1018,7 @@ void PeopleHandler::MarkFirstSetupComplete() {
FireWebUIListener("sync-settings-saved"); FireWebUIListener("sync-settings-saved");
} }
bool PeopleHandler::IsProfileAuthNeeded() { bool PeopleHandler::IsProfileAuthNeededOrHasErrors() {
return !SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated() || return !SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated() ||
SigninErrorControllerFactory::GetForProfile(profile_)->HasError(); SigninErrorControllerFactory::GetForProfile(profile_)->HasError();
} }
......
...@@ -213,7 +213,7 @@ class PeopleHandler : public SettingsPageUIHandler, ...@@ -213,7 +213,7 @@ class PeopleHandler : public SettingsPageUIHandler,
void MarkFirstSetupComplete(); void MarkFirstSetupComplete();
// True if profile needs authentication before sync can run. // True if profile needs authentication before sync can run.
bool IsProfileAuthNeeded(); bool IsProfileAuthNeededOrHasErrors();
// If we're directly loading the sync setup page, we acquire a // If we're directly loading the sync setup page, we acquire a
// SetupInProgressHandle early in order to prevent a lapse in // SetupInProgressHandle early in order to prevent a lapse in
......
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