diff options
12 files changed, 68 insertions, 9 deletions
diff --git a/chrome/browser/signin/account_reconcilor_unittest.cc b/chrome/browser/signin/account_reconcilor_unittest.cc index 73d79c1..fe13849 100644 --- a/chrome/browser/signin/account_reconcilor_unittest.cc +++ b/chrome/browser/signin/account_reconcilor_unittest.cc @@ -323,8 +323,12 @@ TEST_F(AccountReconcilorTest, GetAccountsFromCookieSuccess) { AccountReconcilorFactory::GetForProfile(profile()); ASSERT_TRUE(reconcilor); + ASSERT_EQ(AccountReconcilor::State::NOT_RECONCILING, reconcilor->GetState()); reconcilor->StartReconcile(); + ASSERT_EQ(AccountReconcilor::State::GATHERING_INFORMATION, + reconcilor->GetState()); base::RunLoop().RunUntilIdle(); + ASSERT_EQ(AccountReconcilor::State::APPLYING_CHANGES, reconcilor->GetState()); std::vector<std::pair<std::string, bool> > accounts; ASSERT_TRUE(cookie_manager_service()->ListAccounts(&accounts)); @@ -334,18 +338,25 @@ TEST_F(AccountReconcilorTest, GetAccountsFromCookieSuccess) { TEST_F(AccountReconcilorTest, GetAccountsFromCookieFailure) { ConnectProfileToAccount("12345", "user@gmail.com"); - cookie_manager_service()->SetListAccountsResponseHttpNotFound(); + cookie_manager_service()->SetListAccountsResponseWebLoginRequired(); AccountReconcilor* reconcilor = AccountReconcilorFactory::GetForProfile(profile()); ASSERT_TRUE(reconcilor); + ASSERT_EQ(AccountReconcilor::State::NOT_RECONCILING, reconcilor->GetState()); reconcilor->StartReconcile(); + ASSERT_EQ(AccountReconcilor::State::GATHERING_INFORMATION, + reconcilor->GetState()); base::RunLoop().RunUntilIdle(); std::vector<std::pair<std::string, bool> > accounts; ASSERT_FALSE(cookie_manager_service()->ListAccounts(&accounts)); ASSERT_EQ(0u, accounts.size()); + + base::RunLoop().RunUntilIdle(); + ASSERT_EQ(AccountReconcilor::State::NOT_RECONCILING_ERROR_OCCURED, + reconcilor->GetState()); } TEST_P(AccountReconcilorTest, StartReconcileNoop) { diff --git a/chrome/browser/signin/fake_gaia_cookie_manager_service.cc b/chrome/browser/signin/fake_gaia_cookie_manager_service.cc index 430f8ab..0203d46 100644 --- a/chrome/browser/signin/fake_gaia_cookie_manager_service.cc +++ b/chrome/browser/signin/fake_gaia_cookie_manager_service.cc @@ -34,6 +34,16 @@ void FakeGaiaCookieManagerService::SetListAccountsResponseHttpNotFound() { net::URLRequestStatus::SUCCESS); } +void FakeGaiaCookieManagerService::SetListAccountsResponseWebLoginRequired() { + DCHECK(url_fetcher_factory_); + url_fetcher_factory_->SetFakeResponse( + GaiaUrls::GetInstance()->ListAccountsURLWithSource( + GaiaConstants::kChromeSource), + "Info=WebLoginRequired", + net::HTTP_OK, + net::URLRequestStatus::SUCCESS); +} + void FakeGaiaCookieManagerService::SetListAccountsResponseNoAccounts() { DCHECK(url_fetcher_factory_); url_fetcher_factory_->SetFakeResponse( diff --git a/chrome/browser/signin/fake_gaia_cookie_manager_service.h b/chrome/browser/signin/fake_gaia_cookie_manager_service.h index b03750c..5321482 100644 --- a/chrome/browser/signin/fake_gaia_cookie_manager_service.h +++ b/chrome/browser/signin/fake_gaia_cookie_manager_service.h @@ -21,6 +21,7 @@ class FakeGaiaCookieManagerService : public GaiaCookieManagerService { void Init(net::FakeURLFetcherFactory* url_fetcher_factory); void SetListAccountsResponseHttpNotFound(); + void SetListAccountsResponseWebLoginRequired(); void SetListAccountsResponseNoAccounts(); void SetListAccountsResponseOneAccount(const char* account); void SetListAccountsResponseOneAccountWithExpiry( diff --git a/chrome/browser/signin/signin_tracker_unittest.cc b/chrome/browser/signin/signin_tracker_unittest.cc index 34483b7..1fb52ec 100644 --- a/chrome/browser/signin/signin_tracker_unittest.cc +++ b/chrome/browser/signin/signin_tracker_unittest.cc @@ -45,7 +45,7 @@ class MockObserver : public SigninTracker::Observer { MOCK_METHOD1(SigninFailed, void(const GoogleServiceAuthError&)); MOCK_METHOD0(SigninSuccess, void(void)); - MOCK_METHOD1(MergeSessionComplete, void(const GoogleServiceAuthError&)); + MOCK_METHOD1(AccountAddedToCookie, void(const GoogleServiceAuthError&)); }; } // namespace diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc index 9c347ba..10f04cb 100644 --- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc +++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc @@ -432,9 +432,9 @@ void OneClickSigninSyncStarter::SigninFailed( void OneClickSigninSyncStarter::SigninSuccess() { } -void OneClickSigninSyncStarter::MergeSessionComplete( +void OneClickSigninSyncStarter::AccountAddedToCookie( const GoogleServiceAuthError& error) { - // Regardless of whether the merge session completed sucessfully or not, + // Regardless of whether the account was successfully added or not, // continue with sync starting. if (!sync_setup_completed_callback_.is_null()) diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.h b/chrome/browser/ui/sync/one_click_signin_sync_starter.h index f5b6466..0e4c815 100644 --- a/chrome/browser/ui/sync/one_click_signin_sync_starter.h +++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.h @@ -129,7 +129,7 @@ class OneClickSigninSyncStarter : public SigninTracker::Observer, // SigninTracker::Observer override. void SigninFailed(const GoogleServiceAuthError& error) override; void SigninSuccess() override; - void MergeSessionComplete(const GoogleServiceAuthError& error) override; + void AccountAddedToCookie(const GoogleServiceAuthError& error) override; // LoginUIService::Observer override. void OnSyncConfirmationUIClosed(bool configure_sync_first) override; diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter_unittest.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter_unittest.cc index 571f624..a2720e7 100644 --- a/chrome/browser/ui/sync/one_click_signin_sync_starter_unittest.cc +++ b/chrome/browser/ui/sync/one_click_signin_sync_starter_unittest.cc @@ -130,7 +130,7 @@ TEST_F(OneClickSigninSyncStarterTest, LoadContinueUrl) { CreateSyncStarter(base::Bind(&OneClickSigninSyncStarterTest::Callback, base::Unretained(this)), kTestURL); - sync_starter_->MergeSessionComplete( + sync_starter_->AccountAddedToCookie( GoogleServiceAuthError(GoogleServiceAuthError::NONE)); EXPECT_EQ(1, succeeded_count_); EXPECT_EQ(kTestURL, controller.GetPendingEntry()->GetURL()); diff --git a/chrome/browser/ui/webui/signin/login_ui_test_utils.cc b/chrome/browser/ui/webui/signin/login_ui_test_utils.cc index 7da4dc0..0f9189b 100644 --- a/chrome/browser/ui/webui/signin/login_ui_test_utils.cc +++ b/chrome/browser/ui/webui/signin/login_ui_test_utils.cc @@ -55,7 +55,7 @@ class SignInObserver : public SigninTracker::Observer { running_ = false; } - void MergeSessionComplete(const GoogleServiceAuthError& error) override {} + void AccountAddedToCookie(const GoogleServiceAuthError& error) override {} void SigninSuccess() override { DVLOG(1) << "Google signin succeeded."; diff --git a/components/signin/core/browser/account_reconcilor.cc b/components/signin/core/browser/account_reconcilor.cc index aecbe9b1..975dc0c8 100644 --- a/components/signin/core/browser/account_reconcilor.cc +++ b/components/signin/core/browser/account_reconcilor.cc @@ -64,6 +64,7 @@ AccountReconcilor::AccountReconcilor( registered_with_content_settings_(false), is_reconcile_started_(false), first_execution_(true), + error_during_last_reconcile_(false), chrome_accounts_changed_(false) { VLOG(1) << "AccountReconcilor::AccountReconcilor"; } @@ -176,6 +177,18 @@ bool AccountReconcilor::IsProfileConnected() { return signin_manager_->IsAuthenticated(); } +AccountReconcilor::State AccountReconcilor::GetState() { + if (!is_reconcile_started_) { + return error_during_last_reconcile_ + ? AccountReconcilor::State::NOT_RECONCILING_ERROR_OCCURED + : AccountReconcilor::State::NOT_RECONCILING; + } + + return add_to_cookie_.empty() + ? AccountReconcilor::State::GATHERING_INFORMATION + : AccountReconcilor::State::APPLYING_CHANGES; +} + void AccountReconcilor::OnContentSettingChanged( const ContentSettingsPattern& primary_pattern, const ContentSettingsPattern& secondary_pattern, @@ -249,6 +262,7 @@ void AccountReconcilor::StartReconcile() { return; is_reconcile_started_ = true; + error_during_last_reconcile_ = false; // Reset state for validating gaia cookie. gaia_accounts_.clear(); @@ -269,6 +283,9 @@ void AccountReconcilor::StartReconcile() { void AccountReconcilor::OnGaiaAccountsInCookieUpdated( const std::vector<std::pair<std::string, bool> >& accounts, const GoogleServiceAuthError& error) { + VLOG(1) << "AccountReconcilor::OnGaiaAccountsInCookieUpdated: " + << "CookieJar " << accounts.size() << " accounts, " + << "Error was " << error.ToString(); if (error.state() == GoogleServiceAuthError::NONE) { gaia_accounts_ = accounts; @@ -278,6 +295,8 @@ void AccountReconcilor::OnGaiaAccountsInCookieUpdated( is_reconcile_started_ ? FinishReconcile() : StartReconcile(); } else { + if (is_reconcile_started_) + error_during_last_reconcile_ = true; AbortReconcile(); } } @@ -432,8 +451,13 @@ bool AccountReconcilor::MarkAccountAsAddedToCookie( void AccountReconcilor::OnAddAccountToCookieCompleted( const std::string& account_id, const GoogleServiceAuthError& error) { + VLOG(1) << "AccountReconcilor::OnAddAccountToCookieCompleted: " + << "Account added: " << account_id << ", " + << "Error was " << error.ToString(); // Always listens to GaiaCookieManagerService. Only proceed if reconciling. if (is_reconcile_started_ && MarkAccountAsAddedToCookie(account_id)) { + if (error.state() != GoogleServiceAuthError::State::NONE) + error_during_last_reconcile_ = true; CalculateIfReconcileIsDone(); ScheduleStartReconcileIfChromeAccountsChanged(); } diff --git a/components/signin/core/browser/account_reconcilor.h b/components/signin/core/browser/account_reconcilor.h index 8dbe06c..35ec8c1 100644 --- a/components/signin/core/browser/account_reconcilor.h +++ b/components/signin/core/browser/account_reconcilor.h @@ -40,6 +40,13 @@ class AccountReconcilor : public KeyedService, public OAuth2TokenService::Observer, public SigninManagerBase::Observer { public: + enum State { + NOT_RECONCILING, + NOT_RECONCILING_ERROR_OCCURED, + GATHERING_INFORMATION, + APPLYING_CHANGES + }; + AccountReconcilor(ProfileOAuth2TokenService* token_service, SigninManagerBase* signin_manager, SigninClient* client, @@ -56,6 +63,9 @@ class AccountReconcilor : public KeyedService, // KeyedService implementation. void Shutdown() override; + // Determine what the reconcilor is currently doing. + State GetState(); + private: bool IsRegisteredWithTokenService() const { return registered_with_token_service_; @@ -174,6 +184,9 @@ class AccountReconcilor : public KeyedService, // True iff this is the first time the reconcilor is executing. bool first_execution_; + // True iff an error occured during the last attempt to reconcile. + bool error_during_last_reconcile_; + // Used during reconcile action. // These members are used to validate the gaia cookie. |gaia_accounts_| // holds the state of google accounts in the gaia cookie. Each element is diff --git a/components/signin/core/browser/signin_tracker.cc b/components/signin/core/browser/signin_tracker.cc index 9e76b86..1249fd7 100644 --- a/components/signin/core/browser/signin_tracker.cc +++ b/components/signin/core/browser/signin_tracker.cc @@ -53,5 +53,5 @@ void SigninTracker::OnRefreshTokenRevoked(const std::string& account_id) { void SigninTracker::OnAddAccountToCookieCompleted( const std::string& account_id, const GoogleServiceAuthError& error) { - observer_->MergeSessionComplete(error); + observer_->AccountAddedToCookie(error); } diff --git a/components/signin/core/browser/signin_tracker.h b/components/signin/core/browser/signin_tracker.h index 5f82bd0..6d9c7be 100644 --- a/components/signin/core/browser/signin_tracker.h +++ b/components/signin/core/browser/signin_tracker.h @@ -64,7 +64,7 @@ class SigninTracker : public SigninManagerBase::Observer, // The signed in account has been added into the content area cookie jar. // This will be called only after a call to SigninSuccess(). - virtual void MergeSessionComplete(const GoogleServiceAuthError& error) = 0; + virtual void AccountAddedToCookie(const GoogleServiceAuthError& error) = 0; }; // Creates a SigninTracker that tracks the signin status on the passed |