[Signin][Android] Add access point to AccountPickerCoordinator

This CL adds an access point to AccountPickerCoordinator so that we can
separate the where the account picker is initiated in order to set it
up in different ways. We also removed the |Go incognito mode| row in
the account picker dialog used in Settings > |Turn on Sync?|.

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

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

@@ -18,6 +18,7 @@ import androidx.recyclerview.widget.RecyclerView;
 
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator;
+import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator.AccountPickerAccessPoint;
 
 /**
  * This class implements dialog-based account picker that is used by SigninFragmentBase. This
@@ -55,7 +56,8 @@ public class AccountPickerDialogFragment extends DialogFragment {
         recyclerView.setLayoutManager(new LinearLayoutManager(getActivity()));
         mCoordinator = new AccountPickerCoordinator(recyclerView,
                 (AccountPickerCoordinator.Listener) getTargetFragment(),
-                getArguments().getString(ARGUMENT_SELECTED_ACCOUNT_NAME));
+                getArguments().getString(ARGUMENT_SELECTED_ACCOUNT_NAME),
+                AccountPickerAccessPoint.SETTING);
         return builder.setTitle(R.string.signin_account_picker_dialog_title)
                 .setView(recyclerView)
                 .create();

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

@@ -10,6 +10,7 @@ import android.view.View;
 import androidx.annotation.MainThread;
 import androidx.annotation.VisibleForTesting;
 
+import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator.AccountPickerAccessPoint;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
 import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
@@ -44,8 +45,8 @@ public class AccountPickerBottomSheetCoordinator {
         mView = new AccountPickerBottomSheetView(context);
         mAccountPickerBottomSheetMediator =
                 new AccountPickerBottomSheetMediator(context, accountPickerDelegate);
-        mAccountPickerCoordinator = new AccountPickerCoordinator(
-                mView.getAccountListView(), mAccountPickerBottomSheetMediator, null);
+        mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(),
+                mAccountPickerBottomSheetMediator, null, AccountPickerAccessPoint.WEB);
         mBottomSheetController = bottomSheetController;
         PropertyModelChangeProcessor.create(mAccountPickerBottomSheetMediator.getModel(), mView,
                 AccountPickerBottomSheetViewBinder::bind);

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

@@ -4,15 +4,18 @@
 
 package org.chromium.chrome.browser.signin.account_picker;
 
+import androidx.annotation.IntDef;
 import androidx.annotation.MainThread;
 import androidx.annotation.Nullable;
 import androidx.recyclerview.widget.RecyclerView;
 
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ItemType;
 import org.chromium.ui.modelutil.MVCListAdapter;
 import org.chromium.ui.modelutil.SimpleRecyclerViewAdapter;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
 /**
  * The coordinator of account picker is the only public class in the account_picker package.
  *
@@ -42,6 +45,25 @@ public class AccountPickerCoordinator {
         default void goIncognitoMode() {}
     }
 
+    /**
+     * Access points to record where account picker could be initiated.
+     */
+    @IntDef({AccountPickerAccessPoint.SETTING, AccountPickerAccessPoint.WEB})
+    @Retention(RetentionPolicy.SOURCE)
+    public @interface AccountPickerAccessPoint {
+        /**
+         * When the account picker is used in settings > "Turn on Sync?" page >
+         * "Choose an account" dialog.
+         */
+        int SETTING = 0;
+
+        /**
+         * When the account picker is used in the expanded account list of the
+         * web sign-in bottom sheet.
+         */
+        int WEB = 1;
+    }
+
     private final AccountPickerMediator mMediator;
 
     /**
@@ -51,10 +73,11 @@ public class AccountPickerCoordinator {
      * @param listener Listener to notify when an account is selected or the user wants to add an
      *                 account.
      * @param selectedAccountName The name of the account that should be marked as selected.
+     * @param accessPoint Access point of the account picker
      */
     @MainThread
-    public AccountPickerCoordinator(
-            RecyclerView view, Listener listener, @Nullable String selectedAccountName) {
+    public AccountPickerCoordinator(RecyclerView view, Listener listener,
+            @Nullable String selectedAccountName, @AccountPickerAccessPoint int accessPoint) {
         assert listener != null : "The argument AccountPickerCoordinator.Listener cannot be null!";
 
         MVCListAdapter.ModelList listModel = new MVCListAdapter.ModelList();
@@ -64,16 +87,12 @@ public class AccountPickerCoordinator {
                 AddAccountRowViewBinder::bindView);
         adapter.registerType(ItemType.EXISTING_ACCOUNT_ROW, ExistingAccountRowViewBinder::buildView,
                 ExistingAccountRowViewBinder::bindView);
-
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)) {
-            adapter.registerType(ItemType.INCOGNITO_ACCOUNT_ROW,
-                    IncognitoAccountRowViewBinder::buildView,
-                    IncognitoAccountRowViewBinder::bindView);
-        }
+        adapter.registerType(ItemType.INCOGNITO_ACCOUNT_ROW,
+                IncognitoAccountRowViewBinder::buildView, IncognitoAccountRowViewBinder::bindView);
 
         view.setAdapter(adapter);
         mMediator = new AccountPickerMediator(
-                view.getContext(), listModel, listener, selectedAccountName);
+                view.getContext(), listModel, listener, selectedAccountName, accessPoint);
     }
 
     /**

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

@@ -12,9 +12,9 @@ import androidx.annotation.Nullable;
 
 import org.chromium.base.Callback;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.signin.DisplayableProfileData;
 import org.chromium.chrome.browser.signin.ProfileDataCache;
+import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator.AccountPickerAccessPoint;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.AddAccountRowProperties;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ExistingAccountRowProperties;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.IncognitoAccountRowProperties;
@@ -38,6 +38,7 @@ class AccountPickerMediator {
     private final MVCListAdapter.ModelList mListModel;
     private final AccountPickerCoordinator.Listener mAccountPickerListener;
     private final ProfileDataCache mProfileDataCache;
+    private final @AccountPickerAccessPoint int mAccessPoint;
     private @Nullable String mSelectedAccountName;
 
     private final AccountManagerFacade mAccountManagerFacade;
@@ -47,11 +48,13 @@ class AccountPickerMediator {
 
     @MainThread
     AccountPickerMediator(Context context, MVCListAdapter.ModelList listModel,
-            AccountPickerCoordinator.Listener listener, @Nullable String selectedAccountName) {
+            AccountPickerCoordinator.Listener listener, @Nullable String selectedAccountName,
+            @AccountPickerAccessPoint int accessPoint) {
         mListModel = listModel;
         mAccountPickerListener = listener;
         mProfileDataCache = new ProfileDataCache(
                 context, context.getResources().getDimensionPixelSize(R.dimen.user_picture_size));
+        mAccessPoint = accessPoint;
         mSelectedAccountName = selectedAccountName;
         mAccountManagerFacade = AccountManagerFacadeProvider.getInstance();
 
@@ -100,7 +103,7 @@ class AccountPickerMediator {
         mListModel.add(new MVCListAdapter.ListItem(ItemType.ADD_ACCOUNT_ROW, model));
 
         // Add a "Go incognito mode" row
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)) {
+        if (mAccessPoint == AccountPickerAccessPoint.WEB) {
             PropertyModel incognitoModel = IncognitoAccountRowProperties.createModel(
                     mAccountPickerListener::goIncognitoMode);
             mListModel.add(

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

@@ -340,7 +340,7 @@ public class AccountPickerBottomSheetTest {
 
     @Test
     @MediumTest
-    public void testIncognitoOptionShownOnExpandedSheet() throws Exception {
+    public void testIncognitoOptionShownOnExpandedSheet() {
         buildAndShowExpandedBottomSheet();
         onView(withText(R.string.signin_incognito_mode_secondary)).check(matches(isDisplayed()));
         onView(withText(R.string.signin_incognito_mode_primary)).perform(click());

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

@@ -55,16 +55,9 @@ public class AccountPickerDialogFragmentTest extends DummyUiActivityTestCase {
     @Rule
     public final Features.JUnitProcessor mProcessor = new Features.JUnitProcessor();
 
-    // This test rule is only applied for Legacy AccountPickerDialog fragment.
-    // TODO(crbug.com/1106737): Refactor the tests into two separate files.
-    @Rule
-    public final ChromeRenderTestRule mRenderTestRuleLegacy =
-            ChromeRenderTestRule.Builder.withPublicCorpus().setRevision(1).build();
-
-    // This test rule is only applied when MOBILE_IDENTITY_CONSISTENCY feature flag is enabled.
     @Rule
     public final ChromeRenderTestRule mRenderTestRule =
-            ChromeRenderTestRule.Builder.withPublicCorpus().setRevision(2).build();
+            ChromeRenderTestRule.Builder.withPublicCorpus().setRevision(1).build();
 
     @Rule
     public final AccountManagerTestRule mAccountManagerTestRule =
@@ -132,7 +125,7 @@ public class AccountPickerDialogFragmentTest extends DummyUiActivityTestCase {
     @Feature("RenderTest")
     public void testAccountPickerDialogViewLegacy() throws IOException {
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
-        mRenderTestRuleLegacy.render(
+        mRenderTestRule.render(
                 mDialog.getDialog().getWindow().getDecorView(), "account_picker_dialog_legacy");
     }
 
@@ -143,7 +136,7 @@ public class AccountPickerDialogFragmentTest extends DummyUiActivityTestCase {
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
         TestThreadUtils.runOnUiThreadBlocking(() -> mDialog.updateSelectedAccount(mAccountName2));
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
-        mRenderTestRuleLegacy.render(mDialog.getDialog().getWindow().getDecorView(),
+        mRenderTestRule.render(mDialog.getDialog().getWindow().getDecorView(),
                 "account_picker_dialog_update_selected_account");
     }
 

chrome/android/junit/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerMediatorTest.java

@@ -12,17 +12,15 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.robolectric.RuntimeEnvironment;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.signin.DisplayableProfileData;
+import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator.AccountPickerAccessPoint;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.AddAccountRowProperties;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ExistingAccountRowProperties;
-import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
 import org.chromium.components.signin.ProfileDataSource;
 import org.chromium.components.signin.test.util.FakeProfileDataSource;
@@ -41,9 +39,6 @@ public class AccountPickerMediatorTest {
 
     private final FakeProfileDataSource mFakeProfileDataSource = new FakeProfileDataSource();
 
-    @Rule
-    public TestRule mProcessor = new Features.JUnitProcessor();
-
     @Rule
     public final AccountManagerTestRule mAccountManagerTestRule =
             new AccountManagerTestRule(mFakeProfileDataSource);
@@ -68,12 +63,11 @@ public class AccountPickerMediatorTest {
     }
 
     @Test
-    @Features.EnableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
-    public void testModelPopulatedWhenStarted() {
+    public void testModelPopulatedWhenStartedFromWeb() {
         addAccount(ACCOUNT_NAME1, FULL_NAME1);
         addAccount(ACCOUNT_NAME2, "");
-        mMediator = new AccountPickerMediator(
-                RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1);
+        mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList,
+                mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.WEB);
         // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT, INCOGNITO MODE.
         Assert.assertEquals(4, mModelList.size());
         checkItemForExistingAccountRow(0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ true);
@@ -83,12 +77,12 @@ public class AccountPickerMediatorTest {
     }
 
     @Test
-    @Features.DisableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
-    public void testModelPopulatedWhenStartedLegacy() {
+    public void testModelPopulatedWhenStartedFromSettings() {
         addAccount(ACCOUNT_NAME1, FULL_NAME1);
         addAccount(ACCOUNT_NAME2, "");
-        mMediator = new AccountPickerMediator(
-                RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1);
+        mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList,
+                mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.SETTING);
+        // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT
         Assert.assertEquals(3, mModelList.size());
         checkItemForExistingAccountRow(0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ true);
         checkItemForExistingAccountRow(1, ACCOUNT_NAME2, "", /* isSelectedAccount= */ false);
@@ -96,30 +90,13 @@ public class AccountPickerMediatorTest {
     }
 
     @Test
-    @Features.EnableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
-    public void testModelUpdatedAfterSetSelectedAccountName() {
+    public void testModelUpdatedAfterSetSelectedAccountNameFromSettings() {
         addAccount(ACCOUNT_NAME1, FULL_NAME1);
         addAccount(ACCOUNT_NAME2, "");
-        mMediator = new AccountPickerMediator(
-                RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1);
-        mMediator.setSelectedAccountName(ACCOUNT_NAME2);
-        // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT, INCOGNITO MODE.
-        Assert.assertEquals(4, mModelList.size());
-        checkItemForExistingAccountRow(
-                0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ false);
-        checkItemForExistingAccountRow(1, ACCOUNT_NAME2, "", /* isSelectedAccount= */ true);
-        checkItemForAddAccountRow(2);
-        checkItemForIncognitoAccountRow(3);
-    }
-
-    @Test
-    @Features.DisableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
-    public void testModelUpdatedAfterSetSelectedAccountNameLegacy() {
-        addAccount(ACCOUNT_NAME1, FULL_NAME1);
-        addAccount(ACCOUNT_NAME2, "");
-        mMediator = new AccountPickerMediator(
-                RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1);
+        mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList,
+                mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.SETTING);
         mMediator.setSelectedAccountName(ACCOUNT_NAME2);
+        // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT
         Assert.assertEquals(3, mModelList.size());
         checkItemForExistingAccountRow(
                 0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ false);
@@ -128,12 +105,11 @@ public class AccountPickerMediatorTest {
     }
 
     @Test
-    @Features.DisableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
-    public void testProfileDataUpdateWhenAccountPickerIsShown() {
+    public void testProfileDataUpdateWhenAccountPickerIsShownFromSettings() {
         addAccount(ACCOUNT_NAME1, FULL_NAME1);
         addAccount(ACCOUNT_NAME2, "");
-        mMediator = new AccountPickerMediator(
-                RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1);
+        mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList,
+                mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.SETTING);
         String fullName2 = "Full Name2";
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             mFakeProfileDataSource.setProfileData(ACCOUNT_NAME2,