diff options
Diffstat (limited to 'chrome/browser/sync')
-rw-r--r-- | chrome/browser/sync/profile_sync_service.cc | 15 | ||||
-rw-r--r-- | chrome/browser/sync/profile_sync_service.h | 5 | ||||
-rw-r--r-- | chrome/browser/sync/profile_sync_service_harness.cc | 5 | ||||
-rw-r--r-- | chrome/browser/sync/profile_sync_service_unittest.cc | 1 | ||||
-rw-r--r-- | chrome/browser/sync/sync_global_error.cc | 16 | ||||
-rw-r--r-- | chrome/browser/sync/sync_global_error.h | 4 | ||||
-rw-r--r-- | chrome/browser/sync/sync_global_error_unittest.cc | 47 | ||||
-rw-r--r-- | chrome/browser/sync/sync_ui_util.cc | 92 | ||||
-rw-r--r-- | chrome/browser/sync/sync_ui_util.h | 4 | ||||
-rw-r--r-- | chrome/browser/sync/sync_ui_util_unittest.cc | 47 |
10 files changed, 53 insertions, 183 deletions
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 7296d00..f5a1945 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -162,6 +162,8 @@ ProfileSyncService::ProfileSyncService(ProfileSyncComponentsFactory* factory, channel == chrome::VersionInfo::CHANNEL_BETA) { sync_service_url_ = GURL(kSyncServerUrl); } + if (signin_) + signin_->signin_global_error()->AddProvider(this); } ProfileSyncService::~ProfileSyncService() { @@ -532,6 +534,9 @@ void ProfileSyncService::Shutdown() { // use it. invalidator_registrar_.reset(); + if (signin_) + signin_->signin_global_error()->RemoveProvider(this); + ShutdownImpl(false); } @@ -590,7 +595,9 @@ void ProfileSyncService::ShutdownImpl(bool sync_disabled) { encrypt_everything_ = false; encrypted_types_ = syncer::SyncEncryptionHandler::SensitiveTypes(); passphrase_required_reason_ = syncer::REASON_PASSPHRASE_NOT_REQUIRED; - last_auth_error_ = AuthError::None(); + // Revert to "no auth error". + if (last_auth_error_.state() != GoogleServiceAuthError::NONE) + UpdateAuthErrorState(GoogleServiceAuthError::None()); if (sync_global_error_.get()) { GlobalErrorServiceFactory::GetForProfile(profile_)->RemoveGlobalError( @@ -916,6 +923,8 @@ void ProfileSyncService::UpdateAuthErrorState(const AuthError& error) { // Fan the notification out to interested UI-thread components. NotifyObservers(); + if (signin()) + signin()->signin_global_error()->AuthStatusChanged(); } namespace { @@ -1216,6 +1225,10 @@ const AuthError& ProfileSyncService::GetAuthError() const { return last_auth_error_; } +GoogleServiceAuthError ProfileSyncService::GetAuthStatus() const { + return GetAuthError(); +} + bool ProfileSyncService::FirstSetupInProgress() const { return !HasSyncSetupCompleted() && setup_in_progress_; } diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index 652d360..ce0f293 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -21,6 +21,7 @@ #include "chrome/browser/api/sync/profile_sync_service_base.h" #include "chrome/browser/api/sync/profile_sync_service_observer.h" #include "chrome/browser/profiles/profile_keyed_service.h" +#include "chrome/browser/signin/signin_global_error.h" #include "chrome/browser/sync/backend_unrecoverable_error_handler.h" #include "chrome/browser/sync/failed_datatypes_handler.h" #include "chrome/browser/sync/glue/data_type_controller.h" @@ -155,6 +156,7 @@ class ProfileSyncService : public ProfileSyncServiceBase, public browser_sync::SyncFrontend, public browser_sync::SyncPrefObserver, public browser_sync::DataTypeManagerObserver, + public SigninGlobalError::AuthStatusProvider, public syncer::UnrecoverableErrorHandler, public content::NotificationObserver, public ProfileKeyedService, @@ -405,6 +407,9 @@ class ProfileSyncService : public ProfileSyncServiceBase, // management. If so, the user is not allowed to configure sync. bool IsManaged() const; + // SigninGlobalError::AuthStatusProvider implementation. + virtual GoogleServiceAuthError GetAuthStatus() const OVERRIDE; + // syncer::UnrecoverableErrorHandler implementation. virtual void OnUnrecoverableError( const tracked_objects::Location& from_here, diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc index 45cd86e..7b5d337 100644 --- a/chrome/browser/sync/profile_sync_service_harness.cc +++ b/chrome/browser/sync/profile_sync_service_harness.cc @@ -121,7 +121,10 @@ ProfileSyncServiceHarness::ProfileSyncServiceHarness( } } -ProfileSyncServiceHarness::~ProfileSyncServiceHarness() {} +ProfileSyncServiceHarness::~ProfileSyncServiceHarness() { + if (service_->HasObserver(this)) + service_->RemoveObserver(this); +} // static ProfileSyncServiceHarness* ProfileSyncServiceHarness::CreateAndAttach( diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc index 7c420e7..4e81dfa 100644 --- a/chrome/browser/sync/profile_sync_service_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_unittest.cc @@ -250,6 +250,7 @@ TEST_F(ProfileSyncServiceTest, AbortedByShutdown) { harness_.service.get())); harness_.service->Initialize(); + harness_.service->Shutdown(); harness_.service.reset(); } diff --git a/chrome/browser/sync/sync_global_error.cc b/chrome/browser/sync/sync_global_error.cc index b4988eb..1e539c8 100644 --- a/chrome/browser/sync/sync_global_error.cc +++ b/chrome/browser/sync/sync_global_error.cc @@ -21,8 +21,6 @@ #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" -typedef GoogleServiceAuthError AuthError; - SyncGlobalError::SyncGlobalError(ProfileSyncService* service, SigninManager* signin) : service_(service), @@ -51,7 +49,7 @@ bool SyncGlobalError::HasMenuItem() { } int SyncGlobalError::MenuItemCommandID() { - return IDC_SHOW_SYNC_ERROR; + return IDC_SHOW_SIGNIN_ERROR; } string16 SyncGlobalError::MenuItemLabel() { @@ -59,14 +57,6 @@ string16 SyncGlobalError::MenuItemLabel() { } void SyncGlobalError::ExecuteMenuItem(Browser* browser) { -#if defined(OS_CHROMEOS) - if (service_->GetAuthError().state() != AuthError::NONE) { - DLOG(INFO) << "Signing out the user to fix a sync error."; - // TODO(beng): seems like this could just call browser::AttemptUserExit(). - chrome::ExecuteCommand(browser, IDC_EXIT); - return; - } -#endif LoginUIService* login_ui = LoginUIServiceFactory::GetForProfile( service_->profile()); if (login_ui->current_login_ui()) { @@ -135,7 +125,3 @@ void SyncGlobalError::OnStateChanged() { } } } - -bool SyncGlobalError::HasCustomizedSyncMenuItem() { - return !menu_label_.empty(); -} diff --git a/chrome/browser/sync/sync_global_error.h b/chrome/browser/sync/sync_global_error.h index 7be44d8..60e3bec 100644 --- a/chrome/browser/sync/sync_global_error.h +++ b/chrome/browser/sync/sync_global_error.h @@ -40,10 +40,6 @@ class SyncGlobalError : public GlobalError, // ProfileSyncServiceObserver implementation. virtual void OnStateChanged() OVERRIDE; - // For non-ChromeOS we customize the "Sign in to sync" wrench menu item - // instead of adding a new wrench menu item at the bottom. - bool HasCustomizedSyncMenuItem(); - private: string16 bubble_accept_label_; string16 bubble_message_; diff --git a/chrome/browser/sync/sync_global_error_unittest.cc b/chrome/browser/sync/sync_global_error_unittest.cc index 883c5c3..ca1de8c 100644 --- a/chrome/browser/sync/sync_global_error_unittest.cc +++ b/chrome/browser/sync/sync_global_error_unittest.cc @@ -113,8 +113,7 @@ void VerifySyncGlobalErrorResult(NiceMock<ProfileSyncServiceMock>* service, // If there is an error then a wrench button badge, menu item, and bubble view // should be shown. EXPECT_EQ(error->HasBadge(), is_error); - EXPECT_EQ(error->HasMenuItem() || error->HasCustomizedSyncMenuItem(), - is_error); + EXPECT_EQ(error->HasMenuItem() || error->HasBadge(), is_error); EXPECT_EQ(error->HasBubbleView(), is_error); // If there is an error then labels should not be empty. @@ -182,47 +181,3 @@ TEST_F(SyncGlobalErrorTest, PassphraseGlobalError) { &service, login_ui_service, browser(), &error, GoogleServiceAuthError::NONE, true, true); } - -// Test that SyncGlobalError shows an error for conditions that can be resolved -// by the user and suppresses errors for conditions that cannot be resolved by -// the user. -TEST_F(SyncGlobalErrorTest, AuthStateGlobalError) { - scoped_ptr<Profile> profile( - ProfileSyncServiceMock::MakeSignedInTestingProfile()); - NiceMock<ProfileSyncServiceMock> service(profile.get()); - SigninManager* signin = SigninManagerFactory::GetForProfile(profile.get()); - FakeLoginUIService* login_ui_service = static_cast<FakeLoginUIService*>( - LoginUIServiceFactory::GetInstance()->SetTestingFactoryAndUse( - profile.get(), BuildMockLoginUIService)); - FakeLoginUI login_ui; - login_ui_service->SetLoginUI(&login_ui); - SyncGlobalError error(&service, signin); - - browser_sync::SyncBackendHost::Status status; - EXPECT_CALL(service, QueryDetailedSyncStatus(_)) - .WillRepeatedly(Return(false)); - - struct { - GoogleServiceAuthError::State error_state; - bool is_error; - } table[] = { - { GoogleServiceAuthError::NONE, false }, - { GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS, true }, - { GoogleServiceAuthError::USER_NOT_SIGNED_UP, true }, - { GoogleServiceAuthError::CONNECTION_FAILED, false }, - { GoogleServiceAuthError::CAPTCHA_REQUIRED, true }, - { GoogleServiceAuthError::ACCOUNT_DELETED, true }, - { GoogleServiceAuthError::ACCOUNT_DISABLED, true }, - { GoogleServiceAuthError::SERVICE_UNAVAILABLE, true }, - { GoogleServiceAuthError::TWO_FACTOR, true }, - { GoogleServiceAuthError::REQUEST_CANCELED, true }, - { GoogleServiceAuthError::HOSTED_NOT_ALLOWED, true }, - }; - - for (size_t i = 0; i < sizeof(table)/sizeof(*table); ++i) { - VerifySyncGlobalErrorResult(&service, login_ui_service, browser(), &error, - table[i].error_state, true, table[i].is_error); - VerifySyncGlobalErrorResult(&service, login_ui_service, browser(), &error, - table[i].error_state, false, false); - } -} diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc index b7f53223..43db6f6 100644 --- a/chrome/browser/sync/sync_ui_util.cc +++ b/chrome/browser/sync/sync_ui_util.cc @@ -44,10 +44,7 @@ namespace { void GetStatusLabelsForAuthError(const AuthError& auth_error, const ProfileSyncService& service, string16* status_label, - string16* link_label, - string16* global_error_menu_label, - string16* global_error_bubble_message, - string16* global_error_bubble_accept_label) { + string16* link_label) { string16 username = UTF8ToUTF16(service.profile()->GetPrefs()->GetString( prefs::kGoogleServicesUsername)); string16 product_name = l10n_util::GetStringUTF16(IDS_PRODUCT_NAME); @@ -70,18 +67,6 @@ void GetStatusLabelsForAuthError(const AuthError& auth_error, status_label->assign( l10n_util::GetStringUTF16(IDS_SYNC_LOGIN_INFO_OUT_OF_DATE)); } - if (global_error_menu_label) { - global_error_menu_label->assign(l10n_util::GetStringUTF16( - IDS_SYNC_SIGN_IN_ERROR_WRENCH_MENU_ITEM)); - } - if (global_error_bubble_message) { - global_error_bubble_message->assign(l10n_util::GetStringFUTF16( - IDS_SYNC_SIGN_IN_ERROR_BUBBLE_VIEW_MESSAGE, product_name)); - } - if (global_error_bubble_accept_label) { - global_error_bubble_accept_label->assign(l10n_util::GetStringUTF16( - IDS_SYNC_SIGN_IN_ERROR_BUBBLE_VIEW_ACCEPT)); - } } break; case AuthError::SERVICE_UNAVAILABLE: @@ -91,18 +76,6 @@ void GetStatusLabelsForAuthError(const AuthError& auth_error, } if (link_label) link_label->clear(); - if (global_error_menu_label) { - global_error_menu_label->assign(l10n_util::GetStringUTF16( - IDS_SYNC_SIGN_IN_ERROR_WRENCH_MENU_ITEM)); - } - if (global_error_bubble_message) { - global_error_bubble_message->assign(l10n_util::GetStringFUTF16( - IDS_SYNC_UNAVAILABLE_ERROR_BUBBLE_VIEW_MESSAGE, product_name)); - } - if (global_error_bubble_accept_label) { - global_error_bubble_accept_label->assign(l10n_util::GetStringUTF16( - IDS_SYNC_UNAVAILABLE_ERROR_BUBBLE_VIEW_ACCEPT)); - } break; case AuthError::CONNECTION_FAILED: // Note that there is little the user can do if the server is not @@ -119,18 +92,6 @@ void GetStatusLabelsForAuthError(const AuthError& auth_error, status_label->assign(l10n_util::GetStringUTF16( IDS_SYNC_ERROR_SIGNING_IN)); } - if (global_error_menu_label) { - global_error_menu_label->assign(l10n_util::GetStringUTF16( - IDS_SYNC_SIGN_IN_ERROR_WRENCH_MENU_ITEM)); - } - if (global_error_bubble_message) { - global_error_bubble_message->assign(l10n_util::GetStringFUTF16( - IDS_SYNC_OTHER_SIGN_IN_ERROR_BUBBLE_VIEW_MESSAGE, product_name)); - } - if (global_error_bubble_accept_label) { - global_error_bubble_accept_label->assign(l10n_util::GetStringUTF16( - IDS_SYNC_SIGN_IN_ERROR_BUBBLE_VIEW_ACCEPT)); - } break; } } @@ -236,8 +197,8 @@ MessageType GetStatusInfo(ProfileSyncService* service, // No auth in progress check for an auth error. if (auth_error.state() != AuthError::NONE) { if (status_label && link_label) { - GetStatusLabelsForAuthError(auth_error, *service, - status_label, link_label, NULL, NULL, NULL); + GetStatusLabelsForAuthError( + auth_error, *service, status_label, link_label); } return SYNC_ERROR; } @@ -293,8 +254,7 @@ MessageType GetStatusInfo(ProfileSyncService* service, auth_error.state() != AuthError::TWO_FACTOR) { if (status_label) { status_label->clear(); - GetStatusLabelsForAuthError(auth_error, *service, status_label, NULL, - NULL, NULL, NULL); + GetStatusLabelsForAuthError(auth_error, *service, status_label, NULL); } result_type = SYNC_ERROR; } @@ -388,22 +348,11 @@ void GetStatusLabelsForSyncGlobalError(ProfileSyncService* service, *bubble_message = string16(); *bubble_accept_label = string16(); + // Only display an error if we've completed sync setup. if (!service->HasSyncSetupCompleted()) return; - MessageType status = GetStatus(service, signin); - if (status == SYNC_ERROR) { - const AuthError& auth_error = service->GetAuthError(); - if (auth_error.state() != AuthError::NONE) { - GetStatusLabelsForAuthError(auth_error, *service, NULL, NULL, - menu_label, bubble_message, bubble_accept_label); - // If we have an actionable auth error, display it. - if (!menu_label->empty()) - return; - } - } - - // No actionable auth error - display the passphrase error. + // Display a passphrase error if we have one. if (service->IsPassphraseRequired() && service->IsPassphraseRequiredForDecryption()) { // This is not the first machine so ask user to enter passphrase. @@ -423,18 +372,6 @@ MessageType GetStatus( return sync_ui_util::GetStatusInfo(service, signin, WITH_HTML, NULL, NULL); } -string16 GetSyncMenuLabel( - ProfileSyncService* service, const SigninManager& signin) { - MessageType type = GetStatus(service, signin); - - if (type == sync_ui_util::SYNCED) - return l10n_util::GetStringUTF16(IDS_SYNC_MENU_SYNCED_LABEL); - else if (type == sync_ui_util::SYNC_ERROR) - return l10n_util::GetStringUTF16(IDS_SYNC_MENU_SYNC_ERROR_LABEL); - else - return l10n_util::GetStringUTF16(IDS_SYNC_START_SYNC_BUTTON_LABEL); -} - string16 ConstructTime(int64 time_in_int) { base::Time time = base::Time::FromInternalValue(time_in_int); @@ -444,21 +381,4 @@ string16 ConstructTime(int64 time_in_int) { return base::TimeFormatFriendlyDateAndTime(time); } -std::string MakeSyncAuthErrorText( - const GoogleServiceAuthError::State& state) { - switch (state) { - case GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS: - case GoogleServiceAuthError::ACCOUNT_DELETED: - case GoogleServiceAuthError::ACCOUNT_DISABLED: - case GoogleServiceAuthError::SERVICE_UNAVAILABLE: - return "INVALID_GAIA_CREDENTIALS"; - case GoogleServiceAuthError::USER_NOT_SIGNED_UP: - return "USER_NOT_SIGNED_UP"; - case GoogleServiceAuthError::CONNECTION_FAILED: - return "CONNECTION_FAILED"; - default: - return std::string(); - } -} - } // namespace sync_ui_util diff --git a/chrome/browser/sync/sync_ui_util.h b/chrome/browser/sync/sync_ui_util.h index e72671d3..c533803 100644 --- a/chrome/browser/sync/sync_ui_util.h +++ b/chrome/browser/sync/sync_ui_util.h @@ -57,9 +57,5 @@ void GetStatusLabelsForSyncGlobalError(ProfileSyncService* service, MessageType GetStatus(ProfileSyncService* service, const SigninManager& signin); -// Returns a string with the synchronization status. -string16 GetSyncMenuLabel(ProfileSyncService* service, - const SigninManager& signin); - } // namespace sync_ui_util #endif // CHROME_BROWSER_SYNC_SYNC_UI_UTIL_H_ diff --git a/chrome/browser/sync/sync_ui_util_unittest.cc b/chrome/browser/sync/sync_ui_util_unittest.cc index d15be1d..839792f 100644 --- a/chrome/browser/sync/sync_ui_util_unittest.cc +++ b/chrome/browser/sync/sync_ui_util_unittest.cc @@ -92,7 +92,7 @@ TEST(SyncUIUtilTest, PassphraseGlobalError) { } // Test that GetStatusLabelsForSyncGlobalError returns an error if a -// passphrase is required. +// passphrase is required and not for auth errors. TEST(SyncUIUtilTest, AuthAndPassphraseGlobalError) { MessageLoopForUI message_loop; content::TestBrowserThread ui_thread(BrowserThread::UI, &message_loop); @@ -117,14 +117,14 @@ TEST(SyncUIUtilTest, AuthAndPassphraseGlobalError) { string16 menu_label, label2, label3; sync_ui_util::GetStatusLabelsForSyncGlobalError( &service, signin, &menu_label, &label2, &label3); - // Make sure we aren't displaying the passphrase error badge. - EXPECT_NE(menu_label, l10n_util::GetStringUTF16( + // Make sure we are still displaying the passphrase error badge (don't show + // auth errors through SyncUIUtil). + EXPECT_EQ(menu_label, l10n_util::GetStringUTF16( IDS_SYNC_PASSPHRASE_ERROR_WRENCH_MENU_ITEM)); } -// Test that GetStatusLabelsForSyncGlobalError indicates errors for conditions -// that can be resolved by the user and suppresses errors for conditions that -// cannot be resolved by the user. +// Test that GetStatusLabelsForSyncGlobalError does not indicate errors for +// auth errors (these are reported through SigninGlobalError). TEST(SyncUIUtilTest, AuthStateGlobalError) { MessageLoopForUI message_loop; content::TestBrowserThread ui_thread(BrowserThread::UI, &message_loop); @@ -136,29 +136,24 @@ TEST(SyncUIUtilTest, AuthStateGlobalError) { EXPECT_CALL(service, QueryDetailedSyncStatus(_)) .WillRepeatedly(Return(false)); - struct { - GoogleServiceAuthError::State error_state; - bool is_error; - } table[] = { - { GoogleServiceAuthError::NONE, false }, - { GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS, true }, - { GoogleServiceAuthError::USER_NOT_SIGNED_UP, true }, - { GoogleServiceAuthError::CONNECTION_FAILED, false }, - { GoogleServiceAuthError::CAPTCHA_REQUIRED, true }, - { GoogleServiceAuthError::ACCOUNT_DELETED, true }, - { GoogleServiceAuthError::ACCOUNT_DISABLED, true }, - { GoogleServiceAuthError::SERVICE_UNAVAILABLE, true }, - { GoogleServiceAuthError::TWO_FACTOR, true }, - { GoogleServiceAuthError::REQUEST_CANCELED, true }, - { GoogleServiceAuthError::HOSTED_NOT_ALLOWED, true }, + GoogleServiceAuthError::State table[] = { + GoogleServiceAuthError::NONE, + GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS, + GoogleServiceAuthError::USER_NOT_SIGNED_UP, + GoogleServiceAuthError::CONNECTION_FAILED, + GoogleServiceAuthError::CAPTCHA_REQUIRED, + GoogleServiceAuthError::ACCOUNT_DELETED, + GoogleServiceAuthError::ACCOUNT_DISABLED, + GoogleServiceAuthError::SERVICE_UNAVAILABLE, + GoogleServiceAuthError::TWO_FACTOR, + GoogleServiceAuthError::REQUEST_CANCELED, + GoogleServiceAuthError::HOSTED_NOT_ALLOWED }; FakeSigninManager signin(profile.get()); - for (size_t i = 0; i < sizeof(table)/sizeof(*table); ++i) { - VerifySyncGlobalErrorResult( - &service, signin, table[i].error_state, true, table[i].is_error); - VerifySyncGlobalErrorResult( - &service, signin, table[i].error_state, false, false); + for (size_t i = 0; i < arraysize(table); ++i) { + VerifySyncGlobalErrorResult(&service, signin, table[i], true, false); + VerifySyncGlobalErrorResult(&service, signin, table[i], false, false); } } // Loads a ProfileSyncServiceMock to emulate one of a number of distinct cases |