GAIA ID migration for Android

chrome/browser/signin/oauth2_token_service_delegate_android.cc

Index: chrome/browser/signin/oauth2_token_service_delegate_android.cc
diff --git a/chrome/browser/signin/oauth2_token_service_delegate_android.cc b/chrome/browser/signin/oauth2_token_service_delegate_android.cc
index 4c322a4eeb6f49c3fc75a59e7cfcadbe250f84f7..5dc968a46097a3e72e649b43ba0911d5e8fda850 100644
--- a/chrome/browser/signin/oauth2_token_service_delegate_android.cc
+++ b/chrome/browser/signin/oauth2_token_service_delegate_android.cc
@@ -131,7 +131,10 @@ std::string AndroidAccessTokenFetcher::CombineScopes(
 
 bool OAuth2TokenServiceDelegateAndroid::is_testing_profile_ = false;
 
-OAuth2TokenServiceDelegateAndroid::OAuth2TokenServiceDelegateAndroid() {
+OAuth2TokenServiceDelegateAndroid::OAuth2TokenServiceDelegateAndroid()
+    : account_tracker_service_(nullptr),
+      accounts_have_been_validated_(false),
+      should_fire_refresh_token_loaded_(false) {
   DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::ctor";
   JNIEnv* env = AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jobject> local_java_ref =
@@ -162,8 +165,32 @@ static jobject GetForProfile(JNIEnv* env,
                                                           j_profile_android);
 }
 
-void OAuth2TokenServiceDelegateAndroid::Initialize() {
+void OAuth2TokenServiceDelegateAndroid::Initialize(
+    AccountTrackerService* account_tracker_service) {
   DVLOG(1) << "OAuth2TokenServiceDelegateAndroid::Initialize";
+
+  account_tracker_service_ = account_tracker_service;
+  DCHECK(account_tracker_service_);
+  if (account_tracker_service_->GetMigrationState() ==
+      AccountTrackerService::MIGRATION_IN_PROGRESS) {
+    std::vector<std::string> accounts = GetAccounts();
+    std::vector<std::string> accounts_id;
+    for (std::vector<std::string>::iterator it = accounts.begin();
+         it != accounts.end(); ++it) {
+      AccountTrackerService::AccountInfo account_info =
+          account_tracker_service_->FindAccountInfoByEmail(*it);
+      if (account_info.gaia.empty())
+        accounts_id.push_back(*it);

[Roger Tawa OOO till Jul 10th, 2015/08/14 15:01:22]

If this is empty there is a bug.  I don't think it is correct to push the email
back into the list.  Should add a DCHECK.

Nit: add { and }

[gogerald1, 2015/08/18 01:14:27]

Done.

+      else
+        accounts_id.push_back(account_info.gaia);
+    }
+    JNIEnv* env = AttachCurrentThread();
+    ScopedJavaLocalRef<jobjectArray> java_accounts(
+        base::android::ToJavaArrayOfStrings(env, accounts_id));
+    Java_OAuth2TokenService_saveStoredAccounts(
+        env, base::android::GetApplicationContext(), java_accounts.obj());
+  }
+
   if (!is_testing_profile_) {
     Java_OAuth2TokenService_validateAccounts(
         AttachCurrentThread(), java_ref_.obj(),
@@ -173,9 +200,10 @@ void OAuth2TokenServiceDelegateAndroid::Initialize() {
 
 bool OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable(
     const std::string& account_id) const {
+  std::string account_name = MapAccountIdToAccountName(account_id);
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jstring> j_account_id =
-      ConvertUTF8ToJavaString(env, account_id);
+      ConvertUTF8ToJavaString(env, account_name);
   jboolean refresh_token_is_available =
       Java_OAuth2TokenService_hasOAuth2RefreshToken(
           env, base::android::GetApplicationContext(), j_account_id.obj());
@@ -201,15 +229,15 @@ std::vector<std::string> OAuth2TokenServiceDelegateAndroid::GetAccounts() {
 }
 
 std::vector<std::string>
-OAuth2TokenServiceDelegateAndroid::GetSystemAccounts() {
-  std::vector<std::string> accounts;
+OAuth2TokenServiceDelegateAndroid::GetSystemAccountNames() {
+  std::vector<std::string> account_names;
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobjectArray> j_accounts =
-      Java_OAuth2TokenService_getSystemAccounts(
+      Java_OAuth2TokenService_getSystemAccountNames(
           env, base::android::GetApplicationContext());
   base::android::AppendJavaStringArrayToStringVector(env, j_accounts.obj(),
-                                                     &accounts);
-  return accounts;
+                                                     &account_names);
+  return account_names;
 }
 
 OAuth2AccessTokenFetcher*
@@ -217,8 +245,9 @@ OAuth2TokenServiceDelegateAndroid::CreateAccessTokenFetcher(
     const std::string& account_id,
     net::URLRequestContextGetter* getter,
     OAuth2AccessTokenConsumer* consumer) {
-  ValidateAccountId(account_id);
-  return new AndroidAccessTokenFetcher(consumer, account_id);
+  std::string account_name = MapAccountIdToAccountName(account_id);
+  ValidateAccountId(account_name);

[Roger Tawa OOO till Jul 10th, 2015/08/14 15:01:22]

There is no need to map the account_id to a username for validation.  Please use
the account_id directly.  Same for other calls to ValidateAccountId() below.

[gogerald1, 2015/08/18 01:14:27]

Done.

+  return new AndroidAccessTokenFetcher(consumer, account_name);
 }
 
 void OAuth2TokenServiceDelegateAndroid::InvalidateAccessToken(
@@ -226,7 +255,7 @@ void OAuth2TokenServiceDelegateAndroid::InvalidateAccessToken(
     const std::string& client_id,
     const OAuth2TokenService::ScopeSet& scopes,
     const std::string& access_token) {
-  ValidateAccountId(account_id);
+  ValidateAccountId(MapAccountIdToAccountName(account_id));
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jstring> j_access_token =
       ConvertUTF8ToJavaString(env, access_token);
@@ -245,20 +274,21 @@ void OAuth2TokenServiceDelegateAndroid::ValidateAccounts(
     signed_in_account = ConvertJavaStringToUTF8(env, j_current_acc);
   if (!signed_in_account.empty())
     signed_in_account = gaia::CanonicalizeEmail(signed_in_account);
-  ValidateAccounts(signed_in_account, j_force_notifications != JNI_FALSE);
+  ValidateAccounts(MapAccountNameToAccountId(signed_in_account),
+                   j_force_notifications != JNI_FALSE);
 }
 
 void OAuth2TokenServiceDelegateAndroid::ValidateAccounts(
     const std::string& signed_in_account,
     bool force_notifications) {
   std::vector<std::string> prev_ids = GetAccounts();
-  std::vector<std::string> curr_ids = GetSystemAccounts();
+  std::vector<std::string> curr_ids = GetSystemAccountNames();
   std::vector<std::string> refreshed_ids;
   std::vector<std::string> revoked_ids;
 
-  // Canonicalize system accounts.  |prev_ids| is already done.
   for (size_t i = 0; i < curr_ids.size(); ++i)
-    curr_ids[i] = gaia::CanonicalizeEmail(curr_ids[i]);
+    curr_ids[i] = MapAccountNameToAccountId(curr_ids[i]);
+
   for (size_t i = 0; i < prev_ids.size(); ++i)
     ValidateAccountId(prev_ids[i]);
 
@@ -289,6 +319,14 @@ void OAuth2TokenServiceDelegateAndroid::ValidateAccounts(
        it != revoked_ids.end(); it++) {
     FireRefreshTokenRevoked(*it);
   }
+
+  accounts_have_been_validated_ = true;
+  lock_.Acquire();
+  if (should_fire_refresh_token_loaded_) {
+    should_fire_refresh_token_loaded_ = false;
+    FireRefreshTokensLoaded();
+  }
+  lock_.Release();

[Roger Tawa OOO till Jul 10th, 2015/08/14 15:01:22]

There is no need for a lock.  All places that touch
should_fire_refresh_token_loaded_ are called from ui thread.  You can add a
DCHECK for that too.

(Note that you should never fire callbacks while holding a lock, this can cause
deadlocks.)

[gogerald1, 2015/08/18 01:14:27]

Done.

 }
 
 bool OAuth2TokenServiceDelegateAndroid::ValidateAccounts(
@@ -432,6 +470,36 @@ void OAuth2TokenServiceDelegateAndroid::RevokeAllCredentials() {
       env, base::android::GetApplicationContext(), java_accounts.obj());
 }
 
+void OAuth2TokenServiceDelegateAndroid::LoadCredentials(
+    const std::string& primary_account_id) {
+  lock_.Acquire();
+  if (accounts_have_been_validated_)
+    FireRefreshTokensLoaded();
+  else
+    should_fire_refresh_token_loaded_ = true;

[Roger Tawa OOO till Jul 10th, 2015/08/14 15:01:22]

Why do you need two bools?  I think accounts_have_been_validated_ should be
enough.

[gogerald1, 2015/08/18 01:14:27]

The reason is that we don't want fire refresh token loaded multiple times as on
desktop platform, however validation may not finish yet when LoadCredentials()
is called, so we need to tell validation to fire refresh token loaded. Change to
use one variable with more status.

+  lock_.Release();
+}
+
+std::string OAuth2TokenServiceDelegateAndroid::MapAccountIdToAccountName(
+    const std::string& account_id) const {
+  if (account_tracker_service_->GetMigrationState() !=
+      AccountTrackerService::MIGRATION_NOT_STARTED) {
+    return account_tracker_service_->GetAccountInfo(account_id).email;
+  }
+  return account_id;

[Roger Tawa OOO till Jul 10th, 2015/08/14 15:01:22]

You don't need the if, it should work in any state.  You can add a DCHECK that
.email is not empty.

[gogerald1, 2015/08/18 01:14:27]

Acknowledged. Always over worried about the efficiency :). 

+}
+
+std::string OAuth2TokenServiceDelegateAndroid::MapAccountNameToAccountId(
+    const std::string& account_name) const {
+  if (account_tracker_service_->GetMigrationState() !=
+      AccountTrackerService::MIGRATION_NOT_STARTED) {
+    return account_tracker_service_->FindAccountInfoByEmail(account_name)
+        .account_id;
+  }
+
+  return account_name;

[Roger Tawa OOO till Jul 10th, 2015/08/14 15:01:22]

You don't need the if, it should work in any state.  You can add a DCHECK that
.account_id is not empty.

[gogerald1, 2015/08/18 01:14:26]

Done.

+}
+
 // Called from Java when fetching of an OAuth2 token is finished. The
 // |authToken| param is only valid when |result| is true.
 void OAuth2TokenFetched(JNIEnv* env,