From b2fdf0b9c07155d7d8c1b82a38aeb3fffb0f609c Mon Sep 17 00:00:00 2001 From: rockot Date: Thu, 22 Jan 2015 19:42:35 -0800 Subject: [PATCH] Identity API: Show signin for new failure modes This change causes Chrome signin UI to be displayed in these additional cases during an interactive chrome.identity.getAuthToken run: - Token minting failure - General Gaia service (non-network) error BUG=356904 Review URL: https://codereview.chromium.org/810483006 Cr-Commit-Position: refs/heads/master@{#312766} --- chrome/browser/app_controller_mac.mm | 3 +- .../extensions/api/identity/identity_api.cc | 24 +++++++-- .../browser/extensions/api/identity/identity_api.h | 1 + chrome/browser/ui/browser_command_controller.cc | 2 +- chrome/browser/ui/browser_mac.cc | 2 +- chrome/browser/ui/chrome_pages.cc | 62 ++++++++++++---------- chrome/browser/ui/chrome_pages.h | 8 ++- 7 files changed, 66 insertions(+), 36 deletions(-) diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm index 087091162eb3..41043f7feab9 100644 --- a/chrome/browser/app_controller_mac.mm +++ b/chrome/browser/app_controller_mac.mm @@ -1136,7 +1136,8 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver { break; case IDC_SHOW_SYNC_SETUP: if (Browser* browser = ActivateBrowser(lastProfile)) { - chrome::ShowBrowserSignin(browser, signin_metrics::SOURCE_MENU); + chrome::ShowBrowserSigninOrSettings(browser, + signin_metrics::SOURCE_MENU); } else { chrome::OpenSyncSetupWindow(lastProfile, signin_metrics::SOURCE_MENU); } diff --git a/chrome/browser/extensions/api/identity/identity_api.cc b/chrome/browser/extensions/api/identity/identity_api.cc index 3cd86688e91b..ca52c1a7aa19 100644 --- a/chrome/browser/extensions/api/identity/identity_api.cc +++ b/chrome/browser/extensions/api/identity/identity_api.cc @@ -288,6 +288,7 @@ ExtensionFunction::ResponseAction IdentityGetAccountsFunction::Run() { IdentityGetAuthTokenFunction::IdentityGetAuthTokenFunction() : OAuth2TokenService::Consumer("extensions_identity_api"), + interactive_(false), should_prompt_for_scopes_(false), should_prompt_for_signin_(false) { } @@ -311,12 +312,12 @@ bool IdentityGetAuthTokenFunction::RunAsync() { scoped_ptr params( identity::GetAuthToken::Params::Create(*args_)); EXTENSION_FUNCTION_VALIDATE(params.get()); - bool interactive = params->details.get() && + interactive_ = params->details.get() && params->details->interactive.get() && *params->details->interactive; - should_prompt_for_scopes_ = interactive; - should_prompt_for_signin_ = interactive; + should_prompt_for_scopes_ = interactive_; + should_prompt_for_signin_ = interactive_; const OAuth2Info& oauth2_info = OAuth2Info::GetOAuth2Info(extension()); @@ -567,8 +568,13 @@ void IdentityGetAuthTokenFunction::OnMintTokenFailure( "error", error.ToString()); CompleteMintTokenFlow(); - switch (error.state()) { + case GoogleServiceAuthError::SERVICE_ERROR: + if (interactive_) { + StartSigninFlow(); + return; + } + break; case GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS: case GoogleServiceAuthError::ACCOUNT_DELETED: case GoogleServiceAuthError::ACCOUNT_DISABLED: @@ -646,6 +652,16 @@ void IdentityGetAuthTokenFunction::OnGaiaFlowFailure( break; case GaiaWebAuthFlow::SERVICE_AUTH_ERROR: + // If this is really an authentication error and not just a transient + // network error, and this is an interactive request for a signed-in + // user, then we show signin UI instead of failing. + if (service_error.state() != GoogleServiceAuthError::CONNECTION_FAILED && + service_error.state() != + GoogleServiceAuthError::SERVICE_UNAVAILABLE && + interactive_ && HasLoginToken()) { + StartSigninFlow(); + return; + } error = std::string(identity_constants::kAuthFailure) + service_error.ToString(); break; diff --git a/chrome/browser/extensions/api/identity/identity_api.h b/chrome/browser/extensions/api/identity/identity_api.h index 489029cff4fd..c60563a334eb 100644 --- a/chrome/browser/extensions/api/identity/identity_api.h +++ b/chrome/browser/extensions/api/identity/identity_api.h @@ -280,6 +280,7 @@ class IdentityGetAuthTokenFunction : public ChromeAsyncExtensionFunction, std::string GetOAuth2ClientId() const; + bool interactive_; bool should_prompt_for_scopes_; IdentityMintRequestQueue::MintType mint_token_flow_type_; scoped_ptr mint_token_flow_; diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc index b0ce210b3c06..49486f6bb7b0 100644 --- a/chrome/browser/ui/browser_command_controller.cc +++ b/chrome/browser/ui/browser_command_controller.cc @@ -760,7 +760,7 @@ void BrowserCommandController::ExecuteCommandWithDisposition( ShowHelp(browser_, HELP_SOURCE_MENU); break; case IDC_SHOW_SIGNIN: - ShowBrowserSignin(browser_, signin_metrics::SOURCE_MENU); + ShowBrowserSigninOrSettings(browser_, signin_metrics::SOURCE_MENU); break; case IDC_TOGGLE_SPEECH_INPUT: ToggleSpeechInput(browser_); diff --git a/chrome/browser/ui/browser_mac.cc b/chrome/browser/ui/browser_mac.cc index c7455fb081a5..96068352ae59 100644 --- a/chrome/browser/ui/browser_mac.cc +++ b/chrome/browser/ui/browser_mac.cc @@ -55,7 +55,7 @@ void OpenSyncSetupWindow(Profile* profile, signin_metrics::Source source) { Browser* browser = new Browser(Browser::CreateParams(profile, chrome::HOST_DESKTOP_TYPE_NATIVE)); - ShowBrowserSignin(browser, source); + ShowBrowserSigninOrSettings(browser, source); browser->window()->Show(); } diff --git a/chrome/browser/ui/chrome_pages.cc b/chrome/browser/ui/chrome_pages.cc index bf306abd9101..99bbac230622 100644 --- a/chrome/browser/ui/chrome_pages.cc +++ b/chrome/browser/ui/chrome_pages.cc @@ -334,36 +334,44 @@ void ShowBrowserSignin(Browser* browser, signin_metrics::Source source) { SigninManagerBase* manager = SigninManagerFactory::GetForProfile(original_profile); DCHECK(manager->IsSigninAllowed()); - // If we're signed in, just show settings. - if (manager->IsAuthenticated()) { - ShowSettings(browser); - } else { - // If the browser's profile is an incognito profile, make sure to use - // a browser window from the original profile. The user cannot sign in - // from an incognito window. - scoped_ptr displayer; - if (browser->profile()->IsOffTheRecord()) { - displayer.reset(new ScopedTabbedBrowserDisplayer( - original_profile, chrome::HOST_DESKTOP_TYPE_NATIVE)); - browser = displayer->browser(); - } + // If the browser's profile is an incognito profile, make sure to use + // a browser window from the original profile. The user cannot sign in + // from an incognito window. + scoped_ptr displayer; + if (browser->profile()->IsOffTheRecord()) { + displayer.reset(new ScopedTabbedBrowserDisplayer( + original_profile, chrome::HOST_DESKTOP_TYPE_NATIVE)); + browser = displayer->browser(); + } - signin_metrics::LogSigninSource(source); - - // Since the app launcher is a separate application, it might steal focus - // away from Chrome, and accidentally close the avatar bubble. In this case, - // fallback to the full-tab signin page. - if (switches::IsNewAvatarMenu() && - source != signin_metrics::SOURCE_APP_LAUNCHER) { - browser->window()->ShowAvatarBubbleFromAvatarButton( - BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, - signin::ManageAccountsParams()); - } else { - NavigateToSingletonTab(browser, GURL(signin::GetPromoURL(source, false))); - DCHECK_GT(browser->tab_strip_model()->count(), 0); - } + signin_metrics::LogSigninSource(source); + + // Since the app launcher is a separate application, it might steal focus + // away from Chrome, and accidentally close the avatar bubble. In this case, + // fallback to the full-tab signin page. + if (switches::IsNewAvatarMenu() && + source != signin_metrics::SOURCE_APP_LAUNCHER) { + browser->window()->ShowAvatarBubbleFromAvatarButton( + BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, + signin::ManageAccountsParams()); + } else { + NavigateToSingletonTab(browser, GURL(signin::GetPromoURL(source, false))); + DCHECK_GT(browser->tab_strip_model()->count(), 0); } } + +void ShowBrowserSigninOrSettings( + Browser* browser, + signin_metrics::Source source) { + Profile* original_profile = browser->profile()->GetOriginalProfile(); + SigninManagerBase* manager = + SigninManagerFactory::GetForProfile(original_profile); + DCHECK(manager->IsSigninAllowed()); + if (manager->IsAuthenticated()) + ShowSettings(browser); + else + ShowBrowserSignin(browser, source); +} #endif } // namespace chrome diff --git a/chrome/browser/ui/chrome_pages.h b/chrome/browser/ui/chrome_pages.h index 63c6f6613483..2dc9feaee1e7 100644 --- a/chrome/browser/ui/chrome_pages.h +++ b/chrome/browser/ui/chrome_pages.h @@ -82,9 +82,13 @@ void ShowAboutChrome(Browser* browser); void ShowSearchEngineSettings(Browser* browser); #if !defined(OS_ANDROID) && !defined(OS_IOS) -// If the user is already signed in, shows the "Signin" portion of Settings, -// otherwise initiates signin. +// Initiates signin in a new browser tab. void ShowBrowserSignin(Browser* browser, signin_metrics::Source source); + +// If the user is already signed in, shows the "Signin" portion of Settings, +// otherwise initiates signin in a new browser tab. +void ShowBrowserSigninOrSettings(Browser* browser, + signin_metrics::Source source); #endif } // namespace chrome -- 2.11.4.GIT