diff options
author | fgorski@chromium.org <fgorski@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-17 07:35:58 +0000 |
---|---|---|
committer | fgorski@chromium.org <fgorski@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-17 07:35:58 +0000 |
commit | 7df5ef237e07608b08c11216fbdf91f9ae27f779 (patch) | |
tree | fb94f2256a9f04e30c59830958f503aa695e8573 /chrome/browser/services/gcm | |
parent | 44594085abf90e33fbf7136c4c978176cc22f37a (diff) | |
download | chromium_src-7df5ef237e07608b08c11216fbdf91f9ae27f779.zip chromium_src-7df5ef237e07608b08c11216fbdf91f9ae27f779.tar.gz chromium_src-7df5ef237e07608b08c11216fbdf91f9ae27f779.tar.bz2 |
Adding a step to the check-in process that ensures the correct account information is present before device
check-in happen, in order to maintain a relationship
between signed in accounts and the device.
Behavior of the check-in is not symmetric:
* Adding an account converges slowly - newly added
account will be associated to device with next periodic
check, to avoid checking in too often.
* Removing account triggers check-in immediately to ensure
users privacy.
BUG=374969
Review URL: https://codereview.chromium.org/378643002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283693 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/services/gcm')
4 files changed, 71 insertions, 33 deletions
diff --git a/chrome/browser/services/gcm/gcm_account_tracker.cc b/chrome/browser/services/gcm/gcm_account_tracker.cc index 00bb150..d2636ad 100644 --- a/chrome/browser/services/gcm/gcm_account_tracker.cc +++ b/chrome/browser/services/gcm/gcm_account_tracker.cc @@ -13,8 +13,7 @@ namespace gcm { namespace { -const char kGCMGroupServerScope[] = - "oauth2:https://www.googleapis.com/auth/gcm"; +const char kGCMGroupServerScope[] = "https://www.googleapis.com/auth/gcm"; const char kGCMAccountTrackerName[] = "gcm_account_tracker"; } // namespace @@ -75,10 +74,12 @@ void GCMAccountTracker::Stop() { } void GCMAccountTracker::OnAccountAdded(const gaia::AccountIds& ids) { + DVLOG(1) << "Account added: " << ids.email; // We listen for the account signing in, which happens after account is added. } void GCMAccountTracker::OnAccountRemoved(const gaia::AccountIds& ids) { + DVLOG(1) << "Account removed: " << ids.email; // We listen for the account signing out, which happens before account is // removed. } @@ -97,6 +98,7 @@ void GCMAccountTracker::OnGetTokenSuccess( const base::Time& expiration_time) { DCHECK(request); DCHECK(!request->GetAccountId().empty()); + DVLOG(1) << "Get token success: " << request->GetAccountId(); AccountInfos::iterator iter = account_infos_.find(request->GetAccountId()); DCHECK(iter != account_infos_.end()); @@ -120,6 +122,7 @@ void GCMAccountTracker::OnGetTokenFailure( const GoogleServiceAuthError& error) { DCHECK(request); DCHECK(!request->GetAccountId().empty()); + DVLOG(1) << "Get token failure: " << request->GetAccountId(); AccountInfos::iterator iter = account_infos_.find(request->GetAccountId()); DCHECK(iter != account_infos_.end()); @@ -181,7 +184,13 @@ void GCMAccountTracker::CompleteCollectingTokens() { } } - callback_.Run(account_tokens, account_removed); + // Make sure that there is something to report, otherwise bail out. + if (!account_tokens.empty() || account_removed) { + DVLOG(1) << "Calling callback: " << account_tokens.size(); + callback_.Run(account_tokens); + } else { + DVLOG(1) << "No tokens and nothing removed. Skipping callback."; + } } void GCMAccountTracker::DeleteTokenRequest( @@ -215,6 +224,7 @@ void GCMAccountTracker::GetToken(AccountInfos::iterator& account_iter) { } void GCMAccountTracker::OnAccountSignedIn(const gaia::AccountIds& ids) { + DVLOG(1) << "Account signed in: " << ids.email; AccountInfos::iterator iter = account_infos_.find(ids.account_key); if (iter == account_infos_.end()) { DCHECK(!ids.email.empty()); @@ -228,6 +238,7 @@ void GCMAccountTracker::OnAccountSignedIn(const gaia::AccountIds& ids) { } void GCMAccountTracker::OnAccountSignedOut(const gaia::AccountIds& ids) { + DVLOG(1) << "Account signed out: " << ids.email; AccountInfos::iterator iter = account_infos_.find(ids.account_key); if (iter == account_infos_.end()) return; diff --git a/chrome/browser/services/gcm/gcm_account_tracker.h b/chrome/browser/services/gcm/gcm_account_tracker.h index 9093ba7..9da40a6 100644 --- a/chrome/browser/services/gcm/gcm_account_tracker.h +++ b/chrome/browser/services/gcm/gcm_account_tracker.h @@ -50,11 +50,9 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer, }; // Callback for the GetAccountsForCheckin call. |account_tokens| maps email - // addresses to OAuth2 access tokens. |account_removed| indicates whether an - // account has been removed since the last time the callback was called. - typedef base::Callback< - void(const std::map<std::string, std::string>& account_tokens, - bool account_removed)> UpdateAccountsCallback; + // addresses to OAuth2 access tokens. + typedef base::Callback<void(const std::map<std::string, std::string>& + account_tokens)> UpdateAccountsCallback; // Creates an instance of GCMAccountTracker. |account_tracker| is used to // deliver information about the account, while |callback| will be called diff --git a/chrome/browser/services/gcm/gcm_account_tracker_unittest.cc b/chrome/browser/services/gcm/gcm_account_tracker_unittest.cc index 24c3ab3..cd9deb5 100644 --- a/chrome/browser/services/gcm/gcm_account_tracker_unittest.cc +++ b/chrome/browser/services/gcm/gcm_account_tracker_unittest.cc @@ -45,8 +45,7 @@ class GCMAccountTrackerTest : public testing::Test { virtual ~GCMAccountTrackerTest(); // Callback for the account tracker. - void UpdateAccounts(const std::map<std::string, std::string>& accounts, - bool account_removed); + void UpdateAccounts(const std::map<std::string, std::string>& accounts); // Helpers to pass fake events to the tracker. Tests should have either a pair // of Start/FinishAccountSignIn or SignInAccount per account. Don't mix. @@ -63,7 +62,6 @@ class GCMAccountTrackerTest : public testing::Test { // Test results and helpers. void ResetResults(); bool update_accounts_called() const { return update_accounts_called_; } - bool account_removed() const { return account_removed_; } const std::map<std::string, std::string>& accounts() const { return accounts_; } @@ -74,7 +72,6 @@ class GCMAccountTrackerTest : public testing::Test { private: std::map<std::string, std::string> accounts_; bool update_accounts_called_; - bool account_removed_; base::MessageLoop message_loop_; net::TestURLFetcherFactory test_fetcher_factory_; @@ -84,7 +81,7 @@ class GCMAccountTrackerTest : public testing::Test { }; GCMAccountTrackerTest::GCMAccountTrackerTest() - : update_accounts_called_(false), account_removed_(false) { + : update_accounts_called_(false) { fake_token_service_.reset(new FakeOAuth2TokenService()); fake_identity_provider_.reset( @@ -107,17 +104,14 @@ GCMAccountTrackerTest::~GCMAccountTrackerTest() { } void GCMAccountTrackerTest::UpdateAccounts( - const std::map<std::string, std::string>& accounts, - bool account_removed) { + const std::map<std::string, std::string>& accounts) { update_accounts_called_ = true; accounts_ = accounts; - account_removed_ = account_removed; } void GCMAccountTrackerTest::ResetResults() { accounts_.clear(); update_accounts_called_ = false; - account_removed_ = false; } void GCMAccountTrackerTest::StartAccountSignIn(const std::string& account_key) { @@ -159,10 +153,9 @@ void GCMAccountTrackerTest::IssueError(const std::string& account_key) { TEST_F(GCMAccountTrackerTest, NoAccounts) { EXPECT_FALSE(update_accounts_called()); - EXPECT_FALSE(account_removed()); tracker()->Start(); - EXPECT_TRUE(update_accounts_called()); - EXPECT_FALSE(account_removed()); + // Callback should not be called if there where no accounts provided. + EXPECT_FALSE(update_accounts_called()); EXPECT_TRUE(accounts().empty()); tracker()->Stop(); } @@ -183,7 +176,6 @@ TEST_F(GCMAccountTrackerTest, SingleAccount) { IssueAccessToken(kAccountId1); EXPECT_TRUE(update_accounts_called()); - EXPECT_FALSE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); @@ -201,12 +193,10 @@ TEST_F(GCMAccountTrackerTest, MultipleAccounts) { FinishAccountSignIn(kAccountId1); IssueAccessToken(kAccountId1); EXPECT_FALSE(update_accounts_called()); - EXPECT_FALSE(account_removed()); FinishAccountSignIn(kAccountId2); IssueAccessToken(kAccountId2); EXPECT_TRUE(update_accounts_called()); - EXPECT_FALSE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); @@ -225,7 +215,6 @@ TEST_F(GCMAccountTrackerTest, AccountAdded) { IssueAccessToken(kAccountId1); EXPECT_TRUE(update_accounts_called()); - EXPECT_FALSE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); @@ -248,7 +237,6 @@ TEST_F(GCMAccountTrackerTest, AccountRemoved) { SignOutAccount(kAccountId2); EXPECT_TRUE(update_accounts_called()); - EXPECT_TRUE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); @@ -267,7 +255,6 @@ TEST_F(GCMAccountTrackerTest, GetTokenFailed) { IssueError(kAccountId2); EXPECT_TRUE(update_accounts_called()); - EXPECT_FALSE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); @@ -287,7 +274,6 @@ TEST_F(GCMAccountTrackerTest, GetTokenFailedAccountRemoved) { ResetResults(); SignOutAccount(kAccountId2); EXPECT_TRUE(update_accounts_called()); - EXPECT_TRUE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); @@ -307,7 +293,6 @@ TEST_F(GCMAccountTrackerTest, AccountRemovedWhileRequestsPending) { SignOutAccount(kAccountId2); IssueAccessToken(kAccountId2); EXPECT_TRUE(update_accounts_called()); - EXPECT_TRUE(account_removed()); std::map<std::string, std::string> expected_accounts; expected_accounts[kAccountId1] = MakeAccessToken(kAccountId1); diff --git a/chrome/browser/services/gcm/gcm_profile_service.cc b/chrome/browser/services/gcm/gcm_profile_service.cc index 3e549a7..4b90897 100644 --- a/chrome/browser/services/gcm/gcm_profile_service.cc +++ b/chrome/browser/services/gcm/gcm_profile_service.cc @@ -4,6 +4,8 @@ #include "chrome/browser/services/gcm/gcm_profile_service.h" +#include <map> + #include "base/logging.h" #include "base/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" @@ -13,7 +15,10 @@ #if defined(OS_ANDROID) #include "components/gcm_driver/gcm_driver_android.h" #else +#include "base/bind.h" #include "base/files/file_path.h" +#include "base/memory/weak_ptr.h" +#include "chrome/browser/services/gcm/gcm_account_tracker.h" #include "chrome/browser/services/gcm/gcm_desktop_utils.h" #include "chrome/browser/signin/profile_identity_provider.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" @@ -21,7 +26,9 @@ #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h" #include "chrome/common/chrome_constants.h" #include "components/gcm_driver/gcm_client_factory.h" +#include "components/gcm_driver/gcm_driver_desktop.h" #include "components/signin/core/browser/signin_manager.h" +#include "google_apis/gaia/account_tracker.h" #include "google_apis/gaia/identity_provider.h" #include "net/url_request/url_request_context_getter.h" #endif @@ -29,9 +36,11 @@ namespace gcm { #if !defined(OS_ANDROID) +// Identity observer only has actual work to do when the user is actually signed +// in. It ensures that account tracker is taking class GCMProfileService::IdentityObserver : public IdentityProvider::Observer { public: - IdentityObserver(Profile* profile, GCMDriver* driver); + IdentityObserver(Profile* profile, GCMDriverDesktop* driver); virtual ~IdentityObserver(); // IdentityProvider::Observer: @@ -40,20 +49,29 @@ class GCMProfileService::IdentityObserver : public IdentityProvider::Observer { std::string SignedInUserName() const; + // Called to inform IdentityObserver that a list of accounts was updated. + // |account_tokens| maps email addresses to OAuth2 access tokens. + void AccountsUpdated( + const std::map<std::string, std::string>& account_tokens); + private: - GCMDriver* driver_; + Profile* profile_; + GCMDriverDesktop* driver_; scoped_ptr<IdentityProvider> identity_provider_; + scoped_ptr<GCMAccountTracker> gcm_account_tracker_; // The account ID that this service is responsible for. Empty when the service // is not running. std::string account_id_; + base::WeakPtrFactory<GCMProfileService::IdentityObserver> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(IdentityObserver); }; GCMProfileService::IdentityObserver::IdentityObserver(Profile* profile, - GCMDriver* driver) - : driver_(driver) { + GCMDriverDesktop* driver) + : profile_(profile), driver_(driver), weak_ptr_factory_(this) { identity_provider_.reset(new ProfileIdentityProvider( SigninManagerFactory::GetForProfile(profile), ProfileOAuth2TokenServiceFactory::GetForProfile(profile), @@ -64,6 +82,8 @@ GCMProfileService::IdentityObserver::IdentityObserver(Profile* profile, } GCMProfileService::IdentityObserver::~IdentityObserver() { + if (gcm_account_tracker_) + gcm_account_tracker_->Shutdown(); identity_provider_->RemoveObserver(this); } @@ -75,15 +95,38 @@ void GCMProfileService::IdentityObserver::OnActiveAccountLogin() { account_id_ = account_id; driver_->OnSignedIn(); + + if (!gcm_account_tracker_) { + scoped_ptr<gaia::AccountTracker> gaia_account_tracker( + new gaia::AccountTracker(identity_provider_.get(), + profile_->GetRequestContext())); + + gcm_account_tracker_.reset(new GCMAccountTracker( + gaia_account_tracker.Pass(), + base::Bind(&GCMProfileService::IdentityObserver::AccountsUpdated, + weak_ptr_factory_.GetWeakPtr()))); + } + + gcm_account_tracker_->Start(); } void GCMProfileService::IdentityObserver::OnActiveAccountLogout() { + // Check is necessary to not crash browser_tests. + if (gcm_account_tracker_) + gcm_account_tracker_->Stop(); + // TODO(fgorski): If we purge here, what should happen when we get + // OnActiveAccountLogin() right after that? driver_->Purge(); } std::string GCMProfileService::IdentityObserver::SignedInUserName() const { return driver_->IsStarted() ? account_id_ : std::string(); } + +void GCMProfileService::IdentityObserver::AccountsUpdated( + const std::map<std::string, std::string>& account_tokens) { + driver_->SetAccountsForCheckin(account_tokens); +} #endif // !defined(OS_ANDROID) // static @@ -122,7 +165,8 @@ GCMProfileService::GCMProfileService( profile_->GetPath().Append(chrome::kGCMStoreDirname), profile_->GetRequestContext()); - identity_observer_.reset(new IdentityObserver(profile, driver_.get())); + identity_observer_.reset(new IdentityObserver( + profile, static_cast<gcm::GCMDriverDesktop*>(driver_.get()))); } #endif // defined(OS_ANDROID) |