[Test] Migrate account related methods in SyncTestRule

The sign in API uses CoreAccountInfo. So in the long run its better to
use CoreAccountInfo for helper methods that interact with sign in API.
This cl refactors SyncTestRule so that the methods to add account and
toggle sync states return CoreAccountInfo.

Bug: 1135510, 1117006
Change-Id: I333f9dd87b093d781311a7ee286c04f274ee13ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523112
Commit-Queue: Tanmoy Mollik <triploblastic@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825339}

chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkPersonalizedSigninPromoTest.java

@@ -18,7 +18,6 @@ import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
-import android.accounts.Account;
 import android.app.Activity;
 import android.content.Context;
 
@@ -38,6 +37,7 @@ import org.chromium.chrome.browser.signin.SigninActivityLauncherImpl;
 import org.chromium.chrome.browser.sync.SyncTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.BookmarkTestRule;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
 
 /**
@@ -78,12 +78,12 @@ public class BookmarkPersonalizedSigninPromoTest {
         doNothing()
                 .when(SigninActivityLauncherImpl.get())
                 .launchActivityForPromoDefaultFlow(any(Context.class), anyInt(), anyString());
-        Account account = mSyncTestRule.addTestAccount();
+        CoreAccountInfo accountInfo = mSyncTestRule.addTestAccount();
         showBookmarkManagerAndCheckSigninPromoIsDisplayed();
         onView(withId(R.id.signin_promo_signin_button)).perform(click());
         verify(mMockSigninActivityLauncherImpl)
                 .launchActivityForPromoDefaultFlow(any(Activity.class),
-                        eq(SigninAccessPoint.BOOKMARK_MANAGER), eq(account.name));
+                        eq(SigninAccessPoint.BOOKMARK_MANAGER), eq(accountInfo.getEmail()));
     }
 
     @Test
@@ -92,12 +92,12 @@ public class BookmarkPersonalizedSigninPromoTest {
         doNothing()
                 .when(SigninActivityLauncherImpl.get())
                 .launchActivityForPromoChooseAccountFlow(any(Context.class), anyInt(), anyString());
-        Account account = mSyncTestRule.addTestAccount();
+        CoreAccountInfo accountInfo = mSyncTestRule.addTestAccount();
         showBookmarkManagerAndCheckSigninPromoIsDisplayed();
         onView(withId(R.id.signin_promo_choose_account_button)).perform(click());
         verify(mMockSigninActivityLauncherImpl)
                 .launchActivityForPromoChooseAccountFlow(any(Activity.class),
-                        eq(SigninAccessPoint.BOOKMARK_MANAGER), eq(account.name));
+                        eq(SigninAccessPoint.BOOKMARK_MANAGER), eq(accountInfo.getEmail()));
     }
 
     @Test

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

@@ -12,7 +12,6 @@ import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
 import static androidx.test.espresso.matcher.ViewMatchers.withId;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 
-import android.accounts.Account;
 import android.support.test.InstrumentationRegistry;
 import android.text.Spanned;
 import android.text.style.ClickableSpan;
@@ -46,6 +45,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.ActivityUtils;
 import org.chromium.chrome.test.util.ChromeRenderTestRule;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.ui.test.util.DisableAnimationsTestRule;
@@ -96,13 +96,13 @@ public class SigninFragmentTest {
     @LargeTest
     @Feature("RenderTest")
     public void testSigninFragmentNotDefaultAccountWithPrimaryAccount() throws IOException {
-        Account account = mSyncTestRule.addTestAccount();
+        CoreAccountInfo accountInfo = mSyncTestRule.addTestAccount();
         mSyncTestRule.addAccount("test.second.account@gmail.com");
         mSigninActivity = ActivityUtils.waitForActivity(
                 InstrumentationRegistry.getInstrumentation(), SigninActivity.class, () -> {
                     SigninActivityLauncherImpl.get().launchActivityForPromoChooseAccountFlow(
                             mSyncTestRule.getActivity(), SigninAccessPoint.BOOKMARK_MANAGER,
-                            account.name);
+                            accountInfo.getEmail());
                 });
         mRenderTestRule.render(mSigninActivity.findViewById(R.id.fragment_container),
                 "signin_fragment_choose_primary_account");
@@ -129,12 +129,12 @@ public class SigninFragmentTest {
     @LargeTest
     @Feature("RenderTest")
     public void testSigninFragmentDefaultAccount() throws IOException {
-        Account account = mSyncTestRule.addTestAccount();
+        CoreAccountInfo accountInfo = mSyncTestRule.addTestAccount();
         mSigninActivity = ActivityUtils.waitForActivity(
                 InstrumentationRegistry.getInstrumentation(), SigninActivity.class, () -> {
                     SigninActivityLauncherImpl.get().launchActivityForPromoDefaultFlow(
                             mSyncTestRule.getActivity(), SigninAccessPoint.BOOKMARK_MANAGER,
-                            account.name);
+                            accountInfo.getEmail());
                 });
         mRenderTestRule.render(mSigninActivity.findViewById(R.id.fragment_container),
                 "signin_fragment_default_account");
@@ -143,13 +143,14 @@ public class SigninFragmentTest {
     @Test
     @LargeTest
     public void testClickingSettingsDoesNotSetFirstSetupComplete() {
-        Account account = mSyncTestRule.addTestAccount();
+        CoreAccountInfo accountInfo = mSyncTestRule.addTestAccount();
         mSigninActivity = ActivityUtils.waitForActivity(
                 InstrumentationRegistry.getInstrumentation(), SigninActivity.class, () -> {
                     SigninActivityLauncherImpl.get().launchActivityForPromoDefaultFlow(
-                            mSyncTestRule.getActivity(), SigninAccessPoint.SETTINGS, account.name);
+                            mSyncTestRule.getActivity(), SigninAccessPoint.SETTINGS,
+                            accountInfo.getEmail());
                 });
-        onView(withText(account.name)).check(matches(isDisplayed()));
+        onView(withText(accountInfo.getEmail())).check(matches(isDisplayed()));
         onView(withId(R.id.signin_details_description)).perform(clickOnClickableSpan());
         // Wait for sign in process to finish.
         CriteriaHelper.pollUiThread(() -> {
@@ -180,20 +181,22 @@ public class SigninFragmentTest {
     @Test
     @MediumTest
     public void testSelectNonDefaultAccountInAccountPickerDialog() {
-        Account defaultAccount = mSyncTestRule.addTestAccount();
+        CoreAccountInfo defaultAccountInfo = mSyncTestRule.addTestAccount();
         String nonDefaultAccountName = "test.account.nondefault@gmail.com";
         mSyncTestRule.addAccount(nonDefaultAccountName);
         mSigninActivity = ActivityUtils.waitForActivity(
                 InstrumentationRegistry.getInstrumentation(), SigninActivity.class, () -> {
                     SigninActivityLauncherImpl.get().launchActivityForPromoDefaultFlow(
                             mSyncTestRule.getActivity(), SigninAccessPoint.BOOKMARK_MANAGER,
-                            defaultAccount.name);
+                            defaultAccountInfo.getEmail());
                 });
-        onView(withText(defaultAccount.name)).check(matches(isDisplayed())).perform(click());
+        onView(withText(defaultAccountInfo.getEmail()))
+                .check(matches(isDisplayed()))
+                .perform(click());
         onView(withText(nonDefaultAccountName)).inRoot(isDialog()).perform(click());
         // We should return to the signin promo screen where the previous account is
         // not shown anymore.
-        onView(withText(defaultAccount.name)).check(doesNotExist());
+        onView(withText(defaultAccountInfo.getEmail())).check(doesNotExist());
         onView(withText(nonDefaultAccountName)).check(matches(isDisplayed()));
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/sync/FirstRunTest.java

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.sync;
 
-import android.accounts.Account;
 import android.app.Activity;
 import android.app.Instrumentation;
 import android.app.Instrumentation.ActivityMonitor;
@@ -38,6 +37,7 @@ import org.chromium.chrome.browser.sync.settings.AccountManagementFragment;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.ActivityUtils;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.util.concurrent.TimeoutException;
@@ -140,12 +140,12 @@ public class FirstRunTest {
     @Test
     @FlakyTest(message = "https://crbug.com/616456")
     public void testSignIn() {
-        Account testAccount = mSyncTestRule.addTestAccount();
+        CoreAccountInfo testAccountInfo = mSyncTestRule.addTestAccount();
         Assert.assertNull(mSyncTestRule.getCurrentSignedInAccount());
         Assert.assertFalse(SyncTestUtil.isSyncRequested());
 
-        processFirstRun(testAccount.name, false /* ShowSettings */);
-        Assert.assertEquals(testAccount, mSyncTestRule.getCurrentSignedInAccount());
+        processFirstRun(testAccountInfo.getEmail(), false /* ShowSettings */);
+        Assert.assertEquals(testAccountInfo, mSyncTestRule.getCurrentSignedInAccount());
         SyncTestUtil.waitForSyncActive();
     }
 
@@ -158,13 +158,13 @@ public class FirstRunTest {
     @Test
     @FlakyTest(message = "https://crbug.com/616456")
     public void testSignInWithOpenSettings() {
-        final Account testAccount = mSyncTestRule.addTestAccount();
+        CoreAccountInfo testAccountInfo = mSyncTestRule.addTestAccount();
         final SettingsActivity settingsActivity =
-                processFirstRun(testAccount.name, true /* ShowSettings */);
+                processFirstRun(testAccountInfo.getEmail(), true /* ShowSettings */);
 
         // User should be signed in and the sync backend should initialize, but sync should not
         // become fully active until the settings page is closed.
-        Assert.assertEquals(testAccount, mSyncTestRule.getCurrentSignedInAccount());
+        Assert.assertEquals(testAccountInfo, mSyncTestRule.getCurrentSignedInAccount());
         SyncTestUtil.waitForEngineInitialized();
         Assert.assertFalse(SyncTestUtil.isSyncActive());
 

chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTest.java

@@ -29,6 +29,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.signin.MockChangeEventChecker;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
+import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 /**
@@ -49,14 +50,14 @@ public class SyncTest {
     @Feature({"Sync"})
     @DisabledTest(message = "crbug.com/1144221")
     public void testSignInAndOut() {
-        Account account = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
+        CoreAccountInfo accountInfo = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
 
         // Signing out should disable sync.
         mSyncTestRule.signOut();
         Assert.assertFalse(SyncTestUtil.isSyncRequested());
 
         // Signing back in should re-enable sync.
-        mSyncTestRule.signinAndEnableSync(account);
+        mSyncTestRule.signinAndEnableSync(accountInfo);
         Assert.assertTrue("Sync should be re-enabled.", SyncTestUtil.isSyncActive());
     }
 
@@ -96,8 +97,10 @@ public class SyncTest {
     @DisabledTest(message = "crbug.com/588050,crbug.com/595893")
     public void testRename() {
         // The two accounts object that would represent the account rename.
-        final Account oldAccount = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
-        final Account newAccount = mSyncTestRule.addAccount("test2@gmail.com");
+        final Account oldAccount = CoreAccountInfo.getAndroidAccountFrom(
+                mSyncTestRule.setUpAccountAndEnableSyncForTesting());
+        final Account newAccount =
+                CoreAccountInfo.getAndroidAccountFrom(mSyncTestRule.addAccount("test2@gmail.com"));
 
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             // First, we force a call to updateAccountRenameData. In the real world,
@@ -125,7 +128,8 @@ public class SyncTest {
         });
 
         CriteriaHelper.pollInstrumentationThread(() -> {
-            Criteria.checkThat(mSyncTestRule.getCurrentSignedInAccount(), Matchers.is(newAccount));
+            Criteria.checkThat(mSyncTestRule.getCurrentSignedInAccount().getEmail(),
+                    Matchers.is(newAccount.name));
         });
         SyncTestUtil.waitForSyncActive();
     }
@@ -134,10 +138,10 @@ public class SyncTest {
     @LargeTest
     @Feature({"Sync"})
     public void testStopAndStartSync() {
-        Account account = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
+        CoreAccountInfo accountInfo = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
 
         mSyncTestRule.stopSync();
-        Assert.assertEquals(account, mSyncTestRule.getCurrentSignedInAccount());
+        Assert.assertEquals(accountInfo, mSyncTestRule.getCurrentSignedInAccount());
         Assert.assertFalse(SyncTestUtil.isSyncRequested());
 
         mSyncTestRule.startSyncAndWait();
@@ -147,7 +151,8 @@ public class SyncTest {
     @LargeTest
     @Feature({"Sync"})
     public void testStopAndStartSyncThroughAndroidChromeSync() {
-        Account account = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
+        Account account = CoreAccountInfo.getAndroidAccountFrom(
+                mSyncTestRule.setUpAccountAndEnableSyncForTesting());
         String authority = AndroidSyncSettings.getContractAuthority();
 
         Assert.assertTrue(AndroidSyncSettingsTestUtils.getIsSyncEnabledOnUiThread());
@@ -188,7 +193,8 @@ public class SyncTest {
     @Features.DisableFeatures(ChromeFeatureList.DECOUPLE_SYNC_FROM_ANDROID_MASTER_SYNC)
     @DisabledTest(message = "Test is flaky crbug.com/1100890")
     public void testReenableMasterSyncFirst() {
-        Account account = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
+        Account account = CoreAccountInfo.getAndroidAccountFrom(
+                mSyncTestRule.setUpAccountAndEnableSyncForTesting());
         String authority = AndroidSyncSettings.getContractAuthority();
 
         Assert.assertTrue(AndroidSyncSettingsTestUtils.getIsSyncEnabledOnUiThread());
@@ -221,7 +227,8 @@ public class SyncTest {
     @Features.DisableFeatures(ChromeFeatureList.DECOUPLE_SYNC_FROM_ANDROID_MASTER_SYNC)
     @Feature({"Sync"})
     public void testReenableChromeSyncFirst() {
-        Account account = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
+        Account account = CoreAccountInfo.getAndroidAccountFrom(
+                mSyncTestRule.setUpAccountAndEnableSyncForTesting());
         String authority = AndroidSyncSettings.getContractAuthority();
 
         Assert.assertTrue(AndroidSyncSettingsTestUtils.getIsSyncEnabledOnUiThread());

chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTestRule.java

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.sync;
 
-import android.accounts.Account;
 import android.app.Activity;
 import android.app.PendingIntent;
 import android.content.Context;
@@ -32,7 +31,6 @@ import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
 import org.chromium.chrome.test.util.browser.signin.SigninTestUtil;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
-import org.chromium.components.signin.AccountUtils;
 import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.sync.ModelType;
 import org.chromium.components.sync.protocol.AutofillWalletSpecifics;
@@ -169,44 +167,40 @@ public class SyncTestRule extends ChromeTabbedActivityTestRule {
 
     /**
      * Adds an account of default account name to AccountManagerFacade and waits for the seeding.
-     * TODO(https://crbug.com/1117006): Return CoreAccountInfo object
      */
-    public Account addTestAccount() {
+    public CoreAccountInfo addTestAccount() {
         return addAccount(AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
     }
 
     /**
      * Adds an account of given account name to AccountManagerFacade and waits for the seeding.
      */
-    public Account addAccount(String accountName) {
+    public CoreAccountInfo addAccount(String accountName) {
         CoreAccountInfo coreAccountInfo =
                 mAccountManagerTestRule.addAccountAndWaitForSeeding(accountName);
         Assert.assertFalse(SyncTestUtil.isSyncRequested());
-        return CoreAccountInfo.getAndroidAccountFrom(coreAccountInfo);
+        return coreAccountInfo;
     }
 
     /**
      * Returns the currently signed in account.
-     * TODO(https://crbug.com/1117006): Return CoreAccountInfo object
      */
-    public Account getCurrentSignedInAccount() {
-        return CoreAccountInfo.getAndroidAccountFrom(
-                mAccountManagerTestRule.getCurrentSignedInAccount());
+    public CoreAccountInfo getCurrentSignedInAccount() {
+        return mAccountManagerTestRule.getCurrentSignedInAccount();
     }
 
     /**
      * Set up a test account, sign in and enable sync. FirstSetupComplete bit will be set after
      * this. For most purposes this function should be used as this emulates the basic sign in flow.
      * @return the test account that is signed in.
-     * TODO(https://crbug.com/1135510): Return CoreAccountInfo object
      */
-    public Account setUpAccountAndEnableSyncForTesting() {
+    public CoreAccountInfo setUpAccountAndEnableSyncForTesting() {
         CoreAccountInfo accountInfo =
                 mAccountManagerTestRule.addTestAccountThenSigninAndEnableSync(mProfileSyncService);
         enableUKM();
         SyncTestUtil.waitForSyncActive();
         SyncTestUtil.triggerSyncAndWaitForCompletion();
-        return AccountUtils.createAccountFromName(accountInfo.getEmail());
+        return accountInfo;
     }
 
     /**
@@ -222,14 +216,13 @@ public class SyncTestRule extends ChromeTabbedActivityTestRule {
     /**
      * Set up a test account, sign in but don't mark sync setup complete.
      * @return the test account that is signed in.
-     * TODO(https://crbug.com/1135510): Return CoreAccountInfo object
      */
-    public Account setUpTestAccountAndSignInWithSyncSetupAsIncomplete() {
+    public CoreAccountInfo setUpTestAccountAndSignInWithSyncSetupAsIncomplete() {
         CoreAccountInfo accountInfo = mAccountManagerTestRule.addTestAccountThenSigninAndEnableSync(
                 /* profileSyncService= */ null);
         enableUKM();
         SyncTestUtil.waitForSyncTransportActive();
-        return AccountUtils.createAccountFromName(accountInfo.getEmail());
+        return accountInfo;
     }
 
     public void startSync() {
@@ -246,9 +239,8 @@ public class SyncTestRule extends ChromeTabbedActivityTestRule {
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
     }
 
-    public void signinAndEnableSync(final Account account) {
-        SigninTestUtil.signinAndEnableSync(
-                mAccountManagerTestRule.toCoreAccountInfo(account.name), mProfileSyncService);
+    public void signinAndEnableSync(final CoreAccountInfo accountInfo) {
+        SigninTestUtil.signinAndEnableSync(accountInfo, mProfileSyncService);
         enableUKM();
         SyncTestUtil.waitForSyncActive();
         SyncTestUtil.triggerSyncAndWaitForCompletion();