Move CoreAccountInfo retrieval to SigninManager.java

Move retrieval of CoreAccountInfo by account name (email), from
signin_manager_android.cc to SigninManager.java. This adds a mutable
member to SigninManager's SignInState, which will hold the
CoreAccountInfo when available, i.e. as soon as the account seeding is
complete.

This follows the creation of IdentityManager.java,
PrimaryAccountMutator.java and is a step towards migrating all
SigninManager.java accesses to IdentityManager through the java classes
directly.

A follow up CL will remove the remaining accesses by
signin_manager_android, this includes move
LegacyReloadAccountsFromSystem to IdentityManager.java.

Bug: 934688
Change-Id: Iad797a835febb1a6bbb290cabf44e2e9125989b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1789230
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Auto-Submit: Pâris Meuleman <pmeuleman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695975}

chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java

@@ -131,6 +131,12 @@ public class SigninManager
          */
         boolean mBlockedOnAccountSeeding;
 
+        /**
+         * Contains the full Core account info, which can be retrieved only once account seeding is
+         * complete
+         */
+        CoreAccountInfo mCoreAccountInfo;
+
         /**
          * @param account The account to sign in to.
          * @param activity Reference to the UI to use for dialogs. Null means forced signin.
@@ -465,6 +471,14 @@ public class SigninManager
             Log.w(TAG, "Ignoring sign in progress request as no pending sign in.");
             return;
         }
+        // TODO(crbug.com/1002056) When changing SignIn signature to use CoreAccountInfo, change the
+        // following line to use it instead of retrieving from IdentityManager.
+        mSignInState.mCoreAccountInfo =
+                mIdentityManager.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
+                        mSignInState.mAccount.name);
+
+        // CoreAccountInfo must be set and valid to progress
+        assert mSignInState.mCoreAccountInfo != null;
 
         if (mSignInState.isActivityInvisible()) {
             abortSignIn();
@@ -472,7 +486,7 @@ public class SigninManager
         }
 
         Log.d(TAG, "Checking if account has policy management enabled");
-        fetchAndApplyCloudPolicy(mSignInState.mAccount.name, this::onPolicyFetchedBeforeSignIn);
+        fetchAndApplyCloudPolicy(mSignInState.mCoreAccountInfo, this::onPolicyFetchedBeforeSignIn);
     }
 
     @VisibleForTesting
@@ -486,15 +500,9 @@ public class SigninManager
 
     private void finishSignIn() {
         // This method should be called at most once per sign-in flow.
-        assert mSignInState != null;
+        assert mSignInState != null && mSignInState.mCoreAccountInfo != null;
 
-        // TODO(crbug.com/1002056) When changing SignIn signature to use CoreAccountInfo, change the
-        // following line to use it instead of retrieving from IdentityManager.
-        if (!mPrimaryAccountMutator.setPrimaryAccount(
-                    mIdentityManager
-                            .findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
-                                    mSignInState.mAccount.name)
-                            .getId())) {
+        if (!mPrimaryAccountMutator.setPrimaryAccount(mSignInState.mCoreAccountInfo.getId())) {
             Log.w(TAG, "Failed to set the PrimaryAccount in IdentityManager, aborting signin");
             abortSignIn();
             return;
@@ -502,8 +510,9 @@ public class SigninManager
 
         // Cache the signed-in account name. This must be done after the native call, otherwise
         // sync tries to start without being signed in natively and crashes.
-        ChromeSigninController.get().setSignedInAccountName(mSignInState.mAccount.name);
-        enableSync(mSignInState.mAccount);
+        ChromeSigninController.get().setSignedInAccountName(
+                mSignInState.mCoreAccountInfo.getName());
+        enableSync(mSignInState.mCoreAccountInfo.getAccount());
 
         if (mSignInState.mCallback != null) {
             mSignInState.mCallback.onSignInComplete();
@@ -731,8 +740,14 @@ public class SigninManager
      * @param callback The callback that will receive true if the account is managed, false
      *                 otherwise.
      */
+    // TODO(crbug.com/1002408) Update API to use CoreAccountInfo instead of email
     public void isAccountManaged(String email, final Callback<Boolean> callback) {
-        SigninManagerJni.get().isAccountManaged(mNativeSigninManagerAndroid, email, callback);
+        assert email != null;
+        CoreAccountInfo account =
+                mIdentityManager.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
+                        email);
+        assert account != null;
+        SigninManagerJni.get().isAccountManaged(mNativeSigninManagerAndroid, account, callback);
     }
 
     public static String extractDomainName(String email) {
@@ -750,9 +765,9 @@ public class SigninManager
         return !ExternalAuthUtils.getInstance().isGooglePlayServicesMissing(context);
     }
 
-    private void fetchAndApplyCloudPolicy(String username, final Runnable callback) {
+    private void fetchAndApplyCloudPolicy(CoreAccountInfo account, final Runnable callback) {
         SigninManagerJni.get().fetchAndApplyCloudPolicy(
-                mNativeSigninManagerAndroid, username, callback);
+                mNativeSigninManagerAndroid, account, callback);
     }
 
     private void stopApplyingCloudPolicy() {
@@ -793,12 +808,12 @@ public class SigninManager
         boolean isMobileIdentityConsistencyEnabled();
 
         void fetchAndApplyCloudPolicy(
-                long nativeSigninManagerAndroid, String username, Runnable callback);
+                long nativeSigninManagerAndroid, CoreAccountInfo account, Runnable callback);
 
         void stopApplyingCloudPolicy(long nativeSigninManagerAndroid);
 
-        void isAccountManaged(
-                long nativeSigninManagerAndroid, String username, Callback<Boolean> callback);
+        void isAccountManaged(long nativeSigninManagerAndroid, CoreAccountInfo account,
+                Callback<Boolean> callback);
 
         String getManagementDomain(long nativeSigninManagerAndroid);
 

chrome/browser/android/signin/signin_manager_android.cc

@@ -218,21 +218,9 @@ void SigninManagerAndroid::RegisterPolicyWithAccount(
 
 void SigninManagerAndroid::FetchAndApplyCloudPolicy(
     JNIEnv* env,
-    const JavaParamRef<jstring>& j_username,
+    const base::android::JavaParamRef<jobject>& j_account_info,
     const base::android::JavaParamRef<jobject>& j_callback) {
-  std::string username =
-      base::android::ConvertJavaStringToUTF8(env, j_username);
-  DCHECK(!username.empty());
-  // TODO(bsazonov): Remove after migrating the sign-in flow to CoreAccountId.
-  // ExtractDomainName Dchecks that username is a valid email, in practice
-  // this checks that @ is present and is not the last character.
-  gaia::ExtractDomainName(username);
-  CoreAccountInfo account =
-      identity_manager_
-          ->FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
-              username)
-          .value();
-
+  CoreAccountInfo account = ConvertFromJavaCoreAccountInfo(env, j_account_info);
   auto callback =
       base::BindOnce(base::android::RunRunnableAndroid,
                      base::android::ScopedJavaGlobalRef<jobject>(j_callback));
@@ -274,19 +262,13 @@ void SigninManagerAndroid::FetchPolicyBeforeSignIn(
 
 void SigninManagerAndroid::IsAccountManaged(
     JNIEnv* env,
-    const JavaParamRef<jstring>& j_username,
+    const JavaParamRef<jobject>& j_account_info,
     const JavaParamRef<jobject>& j_callback) {
+  CoreAccountInfo account = ConvertFromJavaCoreAccountInfo(env, j_account_info);
   base::android::ScopedJavaGlobalRef<jobject> callback(env, j_callback);
-  std::string username =
-      base::android::ConvertJavaStringToUTF8(env, j_username);
-
-  base::Optional<CoreAccountInfo> account =
-      identity_manager_
-          ->FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
-              username);
 
   RegisterPolicyWithAccount(
-      account.value_or(CoreAccountInfo{}),
+      account,
       base::BindOnce(
           [](base::android::ScopedJavaGlobalRef<jobject> callback,
              const base::Optional<ManagementCredentials>& credentials) {

chrome/browser/android/signin/signin_manager_android.h

@@ -57,13 +57,14 @@ class SigninManagerAndroid : public KeyedService {
   // the policy if necessary.
   void FetchAndApplyCloudPolicy(
       JNIEnv* env,
-      const base::android::JavaParamRef<jstring>& username,
+      const base::android::JavaParamRef<jobject>& j_account_info,
       const base::android::JavaParamRef<jobject>& j_callback);
 
   void StopApplyingCloudPolicy(JNIEnv* env);
 
-  void IsAccountManaged(JNIEnv* env,
-                        const base::android::JavaParamRef<jstring>& j_username,
+  void IsAccountManaged(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& j_account_info,
       const base::android::JavaParamRef<jobject>& j_callback);
 
   base::android::ScopedJavaLocalRef<jstring> GetManagementDomain(JNIEnv* env);

components/signin/public/identity_manager/account_info.cc

@@ -144,9 +144,22 @@ base::android::ScopedJavaLocalRef<jobject> ConvertToJavaCoreAccountId(
       env, base::android::ConvertUTF8ToJavaString(env, account_id.id));
 }
 
+CoreAccountInfo ConvertFromJavaCoreAccountInfo(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& j_core_account_info) {
+  CoreAccountInfo account;
+  account.account_id = ConvertFromJavaCoreAccountId(
+      env, signin::Java_CoreAccountInfo_getId(env, j_core_account_info));
+  account.gaia = base::android::ConvertJavaStringToUTF8(
+      signin::Java_CoreAccountInfo_getGaiaId(env, j_core_account_info));
+  account.email = base::android::ConvertJavaStringToUTF8(
+      signin::Java_CoreAccountInfo_getName(env, j_core_account_info));
+  return account;
+}
+
 CoreAccountId ConvertFromJavaCoreAccountId(
     JNIEnv* env,
-    const base::android::JavaParamRef<jobject>& j_core_account_id) {
+    const base::android::JavaRef<jobject>& j_core_account_id) {
   CoreAccountId id;
   id.id = base::android::ConvertJavaStringToUTF8(
       signin::Java_CoreAccountId_getId(env, j_core_account_id));

components/signin/public/identity_manager/account_info.h

@@ -90,10 +90,15 @@ base::android::ScopedJavaLocalRef<jobject> ConvertToJavaCoreAccountId(
     JNIEnv* env,
     const CoreAccountId& account_id);
 
-// Constructs a C++ CoreAccountId from the provided java CoreAccountId
+// Constructs a C++ CoreAccountInfo from the provided Java CoreAccountInfo
+CoreAccountInfo ConvertFromJavaCoreAccountInfo(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& j_core_account_info);
+
+// Constructs a C++ CoreAccountId from the provided Java CoreAccountId
 CoreAccountId ConvertFromJavaCoreAccountId(
     JNIEnv* env,
-    const base::android::JavaParamRef<jobject>& j_core_account_id);
+    const base::android::JavaRef<jobject>& j_core_account_id);
 #endif
 
 #endif  // COMPONENTS_SIGNIN_PUBLIC_IDENTITY_MANAGER_ACCOUNT_INFO_H_

components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java

@@ -62,6 +62,7 @@ public class CoreAccountInfo {
     /**
      * Returns a unique identifier of the current account.
      */
+    @CalledByNative
     public CoreAccountId getId() {
         return mId;
     }
@@ -69,6 +70,7 @@ public class CoreAccountInfo {
     /**
      * Returns a name of the current account.
      */
+    @CalledByNative
     public String getName() {
         return mAccount.name;
     }
@@ -76,6 +78,7 @@ public class CoreAccountInfo {
     /**
      * Returns the string representation of the Gaia ID
      */
+    @CalledByNative
     public String getGaiaId() {
         return mGaiaId;
     }