Making OAuth2TokenService multi-login aware, updating callers, minor fixes

chrome/browser/signin/profile_oauth2_token_service.cc

Index: chrome/browser/signin/profile_oauth2_token_service.cc
diff --git a/chrome/browser/signin/profile_oauth2_token_service.cc b/chrome/browser/signin/profile_oauth2_token_service.cc
index 6c00938c6acde13cdb628931cba2d4e0ba0954c3..11219f6d0bccc759229e3f3059291da455c0aecf 100644
--- a/chrome/browser/signin/profile_oauth2_token_service.cc
+++ b/chrome/browser/signin/profile_oauth2_token_service.cc
@@ -100,12 +100,9 @@ void ProfileOAuth2TokenService::Shutdown() {
   signin_global_error_.reset();
 }
 
-std::string ProfileOAuth2TokenService::GetRefreshToken() {
-  TokenService* token_service = TokenServiceFactory::GetForProfile(profile_);
-  if (!token_service || !token_service->HasOAuthLoginToken()) {
-    return std::string();
-  }
-  return token_service->GetOAuth2LoginRefreshToken();
+std::string ProfileOAuth2TokenService::GetRefreshToken(
+    const std::string& account_id) {
+  return refresh_tokens_[account_id];
 }
 
 void ProfileOAuth2TokenService::UpdateAuthError(
@@ -148,7 +145,6 @@ void ProfileOAuth2TokenService::Observe(
       CancelAllRequests();
       ClearCache();
       UpdateAuthError(GoogleServiceAuthError::AuthErrorNone());
-      FireRefreshTokensCleared();
       break;
     }
     case chrome::NOTIFICATION_TOKEN_LOADING_FINISHED:
@@ -173,8 +169,7 @@ void ProfileOAuth2TokenService::RegisterCacheEntry(
     const ScopeSet& scopes,
     const std::string& access_token,
     const base::Time& expiration_date) {
-  if (ShouldCacheForRefreshToken(TokenServiceFactory::GetForProfile(profile_),
-                                 refresh_token)) {
+  if (ShouldCacheForRefreshToken(refresh_token)) {
     OAuth2TokenService::RegisterCacheEntry(refresh_token,
                                            scopes,
                                            access_token,
@@ -183,16 +178,32 @@ void ProfileOAuth2TokenService::RegisterCacheEntry(
 }
 
 bool ProfileOAuth2TokenService::ShouldCacheForRefreshToken(
-    TokenService *token_service,
     const std::string& refresh_token) {
-  if (!token_service ||
-      !token_service->HasOAuthLoginToken() ||
-      token_service->GetOAuth2LoginRefreshToken().compare(refresh_token) != 0) {
-    DLOG(INFO) <<
-        "Received a token with a refresh token not maintained by TokenService.";
-    return false;
+  for (std::map<std::string, std::string>::const_iterator iter =

[Andrew T Wilson (Slow), 2013/08/23 09:38:13]

Add a comment as to what this is doing. Can users hit this case in reality (say,
after a re-auth) or should this be a CHECK/DCHECK instead?

[fgorski, 2013/08/23 19:13:35]

Done. Added a comment and a TODO to investigate CHECK/DCHECK option.

+           refresh_tokens_.begin(); iter != refresh_tokens_.end(); ++iter) {
+    if (iter->second == refresh_token)
+      return true;
+  }
+
+  DLOG(INFO) <<
+      "Received a token with a refresh token not maintained by TokenService.";
+  return false;
+}
+
+std::string ProfileOAuth2TokenService::GetPrimaryAccountId() {
+  SigninManagerBase* signin_manager =
+      SigninManagerFactory::GetForProfileIfExists(profile_);
+  return signin_manager ? signin_manager->GetAuthenticatedUsername() :
+      std::string();
+}
+
+std::vector<std::string> ProfileOAuth2TokenService::GetAccounts() {
+  std::vector<std::string> account_ids;
+  for (std::map<std::string, std::string>::const_iterator iter =
+           refresh_tokens_.begin(); iter != refresh_tokens_.end(); ++iter) {
+    account_ids.push_back(iter->first);
   }
-  return true;
+  return account_ids;
 }
 
 void ProfileOAuth2TokenService::UpdateCredentials(
@@ -204,9 +215,12 @@ void ProfileOAuth2TokenService::UpdateCredentials(
   bool refresh_token_present = refresh_tokens_.count(account_id) > 0;
   if (!refresh_token_present ||
       refresh_tokens_[account_id] != refresh_token) {
-    // If token present, and different from the new one, cancel its requests.
-    if (refresh_token_present)
+    // If token present, and different from the new one, cancel its requests,
+    // and clear the entries in cache related to that account.
+    if (refresh_token_present) {
       CancelRequestsForToken(refresh_tokens_[account_id]);
+      // ClearCacheForAccount(account_id);

[Andrew T Wilson (Slow), 2013/08/23 09:38:13]

Add a TODO here?

[fgorski, 2013/08/23 19:13:35]

Done.

+    }
 
     // Save the token in memory and in persistent store.
     refresh_tokens_[account_id] = refresh_token;
@@ -216,6 +230,7 @@ void ProfileOAuth2TokenService::UpdateCredentials(
       token_web_data->SetTokenForService(ApplyAccountIdPrefix(account_id),
                                          refresh_token);
 
+    UpdateAuthError(GoogleServiceAuthError::AuthErrorNone());

[Andrew T Wilson (Slow), 2013/08/23 09:38:13]

Just a note that we probably have to figure out how we want SigninGlobalError to
work in this multi-profile world. Do we need multiple SigninGlobalError objects
(one per account?) or just a single one and somehow have SigninGlobalError deal
with multi-accounts internally.

My point is that if I have two accounts, A and B, and they both have auth
errors, I probably shouldn't clear the "global" auth error (as we're doing here)
just because I've re-auth one of the accounts.

[fgorski, 2013/08/23 19:13:35]

Done. Included a section in design doc. Updated the signature of UpdateAuthError
method.

     FireRefreshTokenAvailable(account_id);
     // TODO(fgorski): Notify diagnostic observers.
   }
@@ -227,6 +242,7 @@ void ProfileOAuth2TokenService::RevokeCredentials(
 
   if (refresh_tokens_.count(account_id) > 0) {
     CancelRequestsForToken(refresh_tokens_[account_id]);
+    // ClearCacheForAccount(account_id);
     refresh_tokens_.erase(account_id);
     scoped_refptr<TokenWebData> token_web_data =
         TokenWebData::FromBrowserContext(profile_);
@@ -254,7 +270,6 @@ void ProfileOAuth2TokenService::RevokeAllCredentials() {
       TokenWebData::FromBrowserContext(profile_);
   if (token_web_data.get())
     token_web_data->RemoveAllTokens();
-  FireRefreshTokensCleared();
 
   // TODO(fgorski): Notify diagnostic observers.
 }