[Signin][Android] Use CoreAccountInfo in AccountPickerDelegate.signIn

This CL changes the argument of AccountPickerDelegate.signIn() from
account email string to CoreAccountInfo object.

Bug: 1093741
Change-Id: I296674da6757962c21c2956075e94d97de542525
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2351977
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799110}

chrome/android/java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerBottomSheetMediator.java

@@ -10,6 +10,7 @@ import android.text.TextUtils;
 
 import androidx.annotation.Nullable;
 
+import org.chromium.base.task.AsyncTask;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.signin.ProfileDataCache;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetProperties.AccountPickerBottomSheetState;
@@ -17,6 +18,8 @@ import org.chromium.components.signin.AccountManagerFacade;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.AccountUtils;
 import org.chromium.components.signin.AccountsChangeObserver;
+import org.chromium.components.signin.base.CoreAccountId;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.base.GoogleServiceAuthError;
 import org.chromium.ui.modelutil.PropertyModel;
 
@@ -56,6 +59,9 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste
      *
      * @param accountName The email of the selected account.
      * @param isDefaultAccount Whether the selected account is the first in the account list.
+     *
+     * TODO(https://crbug.com/1115965): Use CoreAccountInfo instead of account's email
+     * as the first argument of the method.
      */
     @Override
     public void onAccountSelected(String accountName, boolean isDefaultAccount) {
@@ -166,7 +172,20 @@ class AccountPickerBottomSheetMediator implements AccountPickerCoordinator.Liste
         } else {
             mModel.set(AccountPickerBottomSheetProperties.ACCOUNT_PICKER_BOTTOM_SHEET_STATE,
                     AccountPickerBottomSheetState.SIGNIN_IN_PROGRESS);
-            mAccountPickerDelegate.signIn(mSelectedAccountName, this::onSignInError);
+            new AsyncTask<String>() {
+                @Override
+                protected String doInBackground() {
+                    return mAccountManagerFacade.getAccountGaiaId(mSelectedAccountName);
+                }
+
+                @Override
+                protected void onPostExecute(String accountGaiaId) {
+                    CoreAccountInfo coreAccountInfo = new CoreAccountInfo(
+                            new CoreAccountId(accountGaiaId), mSelectedAccountName, accountGaiaId);
+                    mAccountPickerDelegate.signIn(
+                            coreAccountInfo, AccountPickerBottomSheetMediator.this::onSignInError);
+                }
+            }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerDelegate.java

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.signin.account_picker;
 
-import android.accounts.Account;
 import android.accounts.AccountManager;
 import android.app.Activity;
 import android.content.Intent;
@@ -21,7 +20,7 @@ import org.chromium.chrome.browser.signin.SigninUtils;
 import org.chromium.chrome.browser.signin.WebSigninBridge;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
-import org.chromium.components.signin.AccountUtils;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.base.GoogleServiceAuthError;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
 import org.chromium.content_public.browser.LoadUrlParams;
@@ -63,14 +62,13 @@ public class AccountPickerDelegate implements WebSigninBridge.Listener {
     }
 
     /**
-     * Signs the user into the account of the given accountName.
+     * Signs the user into the given account.
      */
-    public void signIn(String accountName, Callback<GoogleServiceAuthError> onSignInErrorCallback) {
-        Account account = AccountUtils.findAccountByName(
-                AccountManagerFacadeProvider.getInstance().tryGetGoogleAccounts(), accountName);
+    public void signIn(CoreAccountInfo coreAccountInfo,
+            Callback<GoogleServiceAuthError> onSignInErrorCallback) {
         mOnSignInErrorCallback = onSignInErrorCallback;
         mSigninManager.signIn(
-                SigninAccessPoint.WEB_SIGNIN, account, new SigninManager.SignInCallback() {
+                SigninAccessPoint.WEB_SIGNIN, coreAccountInfo, new SigninManager.SignInCallback() {
                     @Override
                     public void onSignInComplete() {
                         // TODO(https://crbug.com/1092399): Implement sign-in properly in delegate

chrome/android/javatests/src/org/chromium/chrome/browser/signin/AccountPickerBottomSheetTest.java

@@ -262,29 +262,7 @@ public class AccountPickerBottomSheetTest {
         View bottomSheetView = mCoordinator.getBottomSheetViewForTesting();
         ThreadUtils.runOnUiThread(
                 bottomSheetView.findViewById(R.id.account_picker_continue_as_button)::performClick);
-        CriteriaHelper.pollUiThread(() -> {
-            return !bottomSheetView.findViewById(R.id.account_picker_continue_as_button).isShown();
-        });
-        verify(mAccountPickerDelegateMock).signIn(eq(PROFILE_DATA1.getAccountName()), any());
-        Assert.assertTrue(
-                bottomSheetView.findViewById(R.id.account_picker_signin_spinner_view).isShown());
-        // Currently the ProgressBar animation cannot be disabled on android-marshmallow-arm64-rel
-        // bot with DisableAnimationsTestRule, we hide the ProgressBar manually here to enable
-        // checks of other elements on the screen.
-        // TODO(https://crbug.com/1115067): Delete this line once DisableAnimationsTestRule is
-        // fixed.
-        ThreadUtils.runOnUiThread(() -> {
-            bottomSheetView.findViewById(R.id.account_picker_signin_spinner_view)
-                    .setVisibility(View.GONE);
-        });
-        onView(withText(R.string.signin_account_picker_bottom_sheet_signin_title))
-                .check(matches(isDisplayed()));
-        onView(withId(R.id.account_picker_bottom_sheet_subtitle))
-                .check(matches(not(isDisplayed())));
-        onView(withId(R.id.account_picker_horizontal_divider)).check(matches(not(isDisplayed())));
-        onView(withId(R.id.account_picker_account_list)).check(matches(not(isDisplayed())));
-        onView(withId(R.id.account_picker_selected_account)).check(matches(not(isDisplayed())));
-        onView(withId(R.id.account_picker_continue_as_button)).check(matches(not(isDisplayed())));
+        checkSignInInProgressBottomSheet();
     }
 
     @Test
@@ -297,10 +275,7 @@ public class AccountPickerBottomSheetTest {
                 bottomSheetView.findViewById(R.id.account_picker_continue_as_button)::isShown);
         ThreadUtils.runOnUiThread(
                 bottomSheetView.findViewById(R.id.account_picker_continue_as_button)::performClick);
-        CriteriaHelper.pollUiThread(() -> {
-            return !bottomSheetView.findViewById(R.id.account_picker_continue_as_button).isShown();
-        });
-        verify(mAccountPickerDelegateMock).signIn(eq(PROFILE_DATA2.getAccountName()), any());
+        checkSignInInProgressBottomSheet();
     }
 
     @Test
@@ -313,7 +288,9 @@ public class AccountPickerBottomSheetTest {
             return null;
         })
                 .when(mAccountPickerDelegateMock)
-                .signIn(eq(PROFILE_DATA1.getAccountName()), any());
+                .signIn(eq(mAccountManagerTestRule.toCoreAccountInfo(
+                                PROFILE_DATA1.getAccountName())),
+                        any());
 
         buildAndShowCollapsedBottomSheet();
         View bottomSheetView = mCoordinator.getBottomSheetViewForTesting();
@@ -385,6 +362,34 @@ public class AccountPickerBottomSheetTest {
         onView(withId(R.id.incognito_interstitial_bottom_sheet_view)).check(matches(isDisplayed()));
     }
 
+    private void checkSignInInProgressBottomSheet() {
+        View bottomSheetView = mCoordinator.getBottomSheetViewForTesting();
+        CriteriaHelper.pollUiThread(() -> {
+            return !bottomSheetView.findViewById(R.id.account_picker_continue_as_button).isShown();
+        });
+        // TODO(https://crbug.com/1116348): Check AccountPickerDelegate.signIn() is called
+        // after solving AsyncTask wait problem in espresso
+        Assert.assertTrue(
+                bottomSheetView.findViewById(R.id.account_picker_signin_spinner_view).isShown());
+        // Currently the ProgressBar animation cannot be disabled on android-marshmallow-arm64-rel
+        // bot with DisableAnimationsTestRule, we hide the ProgressBar manually here to enable
+        // checks of other elements on the screen.
+        // TODO(https://crbug.com/1115067): Delete this line once DisableAnimationsTestRule is
+        // fixed.
+        ThreadUtils.runOnUiThread(() -> {
+            bottomSheetView.findViewById(R.id.account_picker_signin_spinner_view)
+                    .setVisibility(View.GONE);
+        });
+        onView(withText(R.string.signin_account_picker_bottom_sheet_signin_title))
+                .check(matches(isDisplayed()));
+        onView(withId(R.id.account_picker_bottom_sheet_subtitle))
+                .check(matches(not(isDisplayed())));
+        onView(withId(R.id.account_picker_horizontal_divider)).check(matches(not(isDisplayed())));
+        onView(withId(R.id.account_picker_account_list)).check(matches(not(isDisplayed())));
+        onView(withId(R.id.account_picker_selected_account)).check(matches(not(isDisplayed())));
+        onView(withId(R.id.account_picker_continue_as_button)).check(matches(not(isDisplayed())));
+    }
+
     private void checkZeroAccountBottomSheet() {
         onView(allOf(withText(PROFILE_DATA1.getAccountName()), withEffectiveVisibility(VISIBLE)))
                 .check(doesNotExist());

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/signin/AccountManagerTestRule.java

@@ -13,6 +13,8 @@ import org.junit.runners.model.Statement;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.AccountUtils;
 import org.chromium.components.signin.ProfileDataSource;
+import org.chromium.components.signin.base.CoreAccountId;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.test.util.FakeAccountManagerFacade;
 import org.chromium.components.signin.test.util.FakeProfileDataSource;
 
@@ -156,6 +158,14 @@ public class AccountManagerTestRule implements TestRule {
         return SigninTestUtil.getCurrentAccount();
     }
 
+    /**
+     * Converts an account email to its corresponding CoreAccountInfo object.
+     */
+    public CoreAccountInfo toCoreAccountInfo(String accountEmail) {
+        String accountGaiaId = mFakeAccountManagerFacade.getAccountGaiaId(accountEmail);
+        return new CoreAccountInfo(new CoreAccountId(accountGaiaId), accountEmail, accountGaiaId);
+    }
+
     /**
      * Sign out from the current account.
      *