Add IdentityProvider-based AccountTracker to google_apis

google_apis/gaia/account_tracker.cc

Index: google_apis/gaia/account_tracker.cc
diff --git a/chrome/browser/extensions/api/identity/account_tracker.cc b/google_apis/gaia/account_tracker.cc
similarity index 62%
copy from chrome/browser/extensions/api/identity/account_tracker.cc
copy to google_apis/gaia/account_tracker.cc
index a93f505034a040ea673be1debff96b0d005881c4..58b634f46fde161a5e9f31e97ceda4e6a1e0eef9 100644
--- a/chrome/browser/extensions/api/identity/account_tracker.cc
+++ b/google_apis/gaia/account_tracker.cc
@@ -2,47 +2,29 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/extensions/api/identity/account_tracker.h"
+#include "google_apis/gaia/account_tracker.h"
 
 #include "base/logging.h"
 #include "base/stl_util.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
-#include "components/signin/core/browser/profile_oauth2_token_service.h"
-#include "components/signin/core/browser/signin_manager.h"
-#include "content/public/browser/notification_details.h"
-#include "extensions/browser/extension_system.h"
-
-namespace extensions {
-
-AccountTracker::AccountTracker(Profile* profile) : profile_(profile) {
-  ProfileOAuth2TokenService* service =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
-  service->AddObserver(this);
-  service->signin_error_controller()->AddProvider(this);
-  SigninManagerFactory::GetForProfile(profile_)->AddObserver(this);
-}
+#include "net/url_request/url_request_context_getter.h"
 
-AccountTracker::~AccountTracker() {}
+namespace gaia {
 
-void AccountTracker::ReportAuthError(const std::string& account_id,
-                                     const GoogleServiceAuthError& error) {
-  account_errors_.insert(make_pair(account_id, error));
-  ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->
-      signin_error_controller()->AuthStatusChanged();
-  UpdateSignInState(account_id, false);
+AccountTracker::AccountTracker(
+    scoped_ptr<IdentityProvider> identity_provider,
+    net::URLRequestContextGetter* request_context_getter)
+    : identity_provider_(identity_provider.Pass()),
+      request_context_getter_(request_context_getter) {
+  identity_provider_->AddObserver(this);
+  identity_provider_->GetTokenService()->AddObserver(this);
 }
 
+AccountTracker::~AccountTracker() {}
+
 void AccountTracker::Shutdown() {
   STLDeleteValues(&user_info_requests_);

[fgorski, 2014/06/17 04:14:00]

Is there a chance Shutdown does not get call before AccountTracker is deleted,
leading to a memory leak?

[Roger Tawa OOO till Jul 10th, 2014/06/17 15:28:20]

I'd eventually like this make this class a |KeyedService|, so shutdown should
always be called.  But good idea to DCHECK or something in the dtor that
Shutdown() was called.

[Michael Courage, 2014/06/18 23:13:15]

Done.

-  SigninManagerFactory::GetForProfile(profile_)->RemoveObserver(this);
-  ProfileOAuth2TokenService* service =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
-  service->signin_error_controller()->RemoveProvider(this);
-  service->RemoveObserver(this);
+  identity_provider_->GetTokenService()->RemoveObserver(this);
+  identity_provider_->RemoveObserver(this);
 }
 
 void AccountTracker::AddObserver(Observer* observer) {
@@ -54,7 +36,7 @@ void AccountTracker::RemoveObserver(Observer* observer) {
 }
 
 std::vector<AccountIds> AccountTracker::GetAccounts() const {
-  const std::string primary_account_id = signin_manager_account_id();
+  const std::string primary_account_id = active_account_id();

[Roger Tawa OOO till Jul 10th, 2014/06/17 15:28:20]

Rename local var to |active_account_id| ?

[Michael Courage, 2014/06/18 23:13:14]

Done. (note: giving method and var the same name does not compile)

   std::vector<AccountIds> accounts;
 
   for (std::map<std::string, AccountState>::const_iterator it =
@@ -93,11 +75,10 @@ std::string AccountTracker::FindAccountKeyByGaiaId(const std::string& gaia_id) {
 
 void AccountTracker::OnRefreshTokenAvailable(const std::string& account_id) {
   // Ignore refresh tokens if there is no primary account ID at all.

[Roger Tawa OOO till Jul 10th, 2014/06/17 15:28:20]

Maybe do a global replace of |primary| with |active| ?

[Michael Courage, 2014/06/18 23:13:15]

Done.

-  if (signin_manager_account_id().empty())
+  if (active_account_id().empty())
     return;
 
   DVLOG(1) << "AVAILABLE " << account_id;
-  ClearAuthError(account_id);
   UpdateSignInState(account_id, true);
 }
 
@@ -106,10 +87,11 @@ void AccountTracker::OnRefreshTokenRevoked(const std::string& account_id) {
   UpdateSignInState(account_id, false);
 }
 
-void AccountTracker::GoogleSigninSucceeded(const std::string& username,
-                                           const std::string& password) {
+void AccountTracker::OnActiveAccountLogin() {
   std::vector<std::string> accounts =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->GetAccounts();
+      identity_provider_->GetTokenService()->GetAccounts();
+
+  DVLOG(1) << "LOGIN " << accounts.size() << " accounts available.";
 
   for (std::vector<std::string>::const_iterator it = accounts.begin();
        it != accounts.end();
@@ -118,13 +100,9 @@ void AccountTracker::GoogleSigninSucceeded(const std::string& username,
   }
 }
 
-void AccountTracker::GoogleSignedOut(const std::string& username) {
-  if (username == signin_manager_account_id() ||
-      signin_manager_account_id().empty()) {
-    StopTrackingAllAccounts();
-  } else {
-    StopTrackingAccount(username);
-  }
+void AccountTracker::OnActiveAccountLogout() {
+  DVLOG(1) << "LOGOUT";
+  StopTrackingAllAccounts();
 }
 
 void AccountTracker::SetAccountStateForTest(AccountIds ids, bool is_signed_in) {
@@ -144,9 +122,8 @@ void AccountTracker::SetAccountStateForTest(AccountIds ids, bool is_signed_in) {
   }
 }
 
-const std::string AccountTracker::signin_manager_account_id() const {
-  return SigninManagerFactory::GetForProfile(profile_)
-      ->GetAuthenticatedAccountId();
+const std::string AccountTracker::active_account_id() const {
+  return identity_provider_->GetActiveAccountId();
 }
 
 void AccountTracker::NotifyAccountAdded(const AccountState& account) {
@@ -168,13 +145,7 @@ void AccountTracker::NotifySignInChanged(const AccountState& account) {
                     OnAccountSignInChanged(account.ids, account.is_signed_in));
 }
 
-void AccountTracker::ClearAuthError(const std::string& account_key) {
-  account_errors_.erase(account_key);
-  ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->
-      signin_error_controller()->AuthStatusChanged();
-}
-
-void AccountTracker::UpdateSignInState(const std::string& account_key,
+void AccountTracker::UpdateSignInState(const std::string account_key,

[Roger Tawa OOO till Jul 10th, 2014/06/17 15:28:20]

What is the reason to remove the & from here and below?

[Michael Courage, 2014/06/18 23:13:14]

StopTrackingAccount had a use-after-free before the change because of pointer
aliasing. Line 182: accounts_.erase(account_key) could delete the account_key
parameter. StopTrackingAllAccounts triggered this issue.

I could have made explicit copies for just the cases where an alias is possible,
but passing string values isn't all that expensive for infrequent operations
like account state change, and eliminates that class of bug.

                                        bool is_signed_in) {
   StartTrackingAccount(account_key);
   AccountState& account = accounts_[account_key];
@@ -189,7 +160,7 @@ void AccountTracker::UpdateSignInState(const std::string& account_key,
     NotifySignInChanged(account);
 }
 
-void AccountTracker::StartTrackingAccount(const std::string& account_key) {
+void AccountTracker::StartTrackingAccount(const std::string account_key) {
   if (!ContainsKey(accounts_, account_key)) {
     DVLOG(1) << "StartTracking " << account_key;
     AccountState account_state;
@@ -200,7 +171,8 @@ void AccountTracker::StartTrackingAccount(const std::string& account_key) {
   }
 }
 
-void AccountTracker::StopTrackingAccount(const std::string& account_key) {
+void AccountTracker::StopTrackingAccount(const std::string account_key) {
+  DVLOG(1) << "StopTracking " << account_key;
   if (ContainsKey(accounts_, account_key)) {
     AccountState& account = accounts_[account_key];
     if (!account.ids.gaia.empty()) {
@@ -210,8 +182,6 @@ void AccountTracker::StopTrackingAccount(const std::string& account_key) {
     accounts_.erase(account_key);
   }
 
-  ClearAuthError(account_key);
-
   if (ContainsKey(user_info_requests_, account_key))
     DeleteFetcher(user_info_requests_[account_key]);
 }
@@ -221,13 +191,16 @@ void AccountTracker::StopTrackingAllAccounts() {
     StopTrackingAccount(accounts_.begin()->first);
 }
 
-void AccountTracker::StartFetchingUserInfo(const std::string& account_key) {
+void AccountTracker::StartFetchingUserInfo(const std::string account_key) {
   if (ContainsKey(user_info_requests_, account_key))
     DeleteFetcher(user_info_requests_[account_key]);
 
   DVLOG(1) << "StartFetching " << account_key;
   AccountIdFetcher* fetcher =
-      new AccountIdFetcher(profile_, this, account_key);
+      new AccountIdFetcher(identity_provider_->GetTokenService(),
+                           request_context_getter_.get(),
+                           this,
+                           account_key);
   user_info_requests_[account_key] = fetcher;
   fetcher->Start();
 }
@@ -254,32 +227,8 @@ void AccountTracker::OnUserInfoFetchFailure(AccountIdFetcher* fetcher) {
   StopTrackingAccount(key);
 }
 
-std::string AccountTracker::GetAccountId() const {
-  if (account_errors_.size() == 0)
-    return std::string();
-  else
-    return account_errors_.begin()->first;
-}
-
-std::string AccountTracker::GetUsername() const {
-  std::string id = GetAccountId();
-  if (!id.empty()) {
-    std::map<std::string, AccountState>::const_iterator it =
-        accounts_.find(id);
-    if (it != accounts_.end())
-      return it->second.ids.email;
-  }
-  return std::string();
-}
-
-GoogleServiceAuthError AccountTracker::GetAuthStatus() const {
-  if (account_errors_.size() == 0)
-    return GoogleServiceAuthError::AuthErrorNone();
-  else
-    return account_errors_.begin()->second;
-}
-
 void AccountTracker::DeleteFetcher(AccountIdFetcher* fetcher) {
+  DVLOG(1) << "DeleteFetcher " << fetcher->account_key();
   const std::string& account_key = fetcher->account_key();
   DCHECK(ContainsKey(user_info_requests_, account_key));
   DCHECK_EQ(fetcher, user_info_requests_[account_key]);
@@ -287,20 +236,22 @@ void AccountTracker::DeleteFetcher(AccountIdFetcher* fetcher) {
   delete fetcher;
 }
 
-AccountIdFetcher::AccountIdFetcher(Profile* profile,
-                                   AccountTracker* tracker,
-                                   const std::string& account_key)
-    : OAuth2TokenService::Consumer("extensions_account_tracker"),
-      profile_(profile),
+AccountIdFetcher::AccountIdFetcher(
+    OAuth2TokenService* token_service,
+    net::URLRequestContextGetter* request_context_getter,
+    AccountTracker* tracker,
+    const std::string& account_key)
+    : OAuth2TokenService::Consumer("gaia_account_tracker"),
+      token_service_(token_service),
+      request_context_getter_(request_context_getter),
       tracker_(tracker),
-      account_key_(account_key) {}
+      account_key_(account_key) {
+}
 
 AccountIdFetcher::~AccountIdFetcher() {}
 
 void AccountIdFetcher::Start() {
-  ProfileOAuth2TokenService* service =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
-  login_token_request_ = service->StartRequest(
+  login_token_request_ = token_service_->StartRequest(
       account_key_, OAuth2TokenService::ScopeSet(), this);
 }
 
@@ -310,8 +261,7 @@ void AccountIdFetcher::OnGetTokenSuccess(
     const base::Time& expiration_time) {
   DCHECK_EQ(request, login_token_request_.get());
 
-  gaia_oauth_client_.reset(new gaia::GaiaOAuthClient(
-      g_browser_process->system_request_context()));
+  gaia_oauth_client_.reset(new gaia::GaiaOAuthClient(request_context_getter_));
 
   const int kMaxGetUserIdRetries = 3;
   gaia_oauth_client_->GetUserId(access_token, kMaxGetUserIdRetries, this);
@@ -339,4 +289,4 @@ void AccountIdFetcher::OnNetworkError(int response_code) {
   tracker_->OnUserInfoFetchFailure(this);
 }
 
-}  // namespace extensions
+}  // namespace gaia