Commit f77834f2 authored by Alice Wang's avatar Alice Wang Committed by Commit Bot

[Android][WebSignin] Use boolean to control the visibility of incognito row

This CL replaces the enum AccessPoint in account picker with a boolean
to decide whether to show the incognito row.
This is a refactoring. There is no behavior change with this CL.

Bug: 1136802
Change-Id: I624b15ae24782428ff8f25ea0352c7af4790bd2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460904Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816575}
parent 74e8dfb3
...@@ -18,7 +18,6 @@ import androidx.recyclerview.widget.RecyclerView; ...@@ -18,7 +18,6 @@ import androidx.recyclerview.widget.RecyclerView;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator; 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 * This class implements dialog-based account picker that is used by SigninFragmentBase. This
...@@ -57,7 +56,7 @@ public class AccountPickerDialogFragment extends DialogFragment { ...@@ -57,7 +56,7 @@ public class AccountPickerDialogFragment extends DialogFragment {
mCoordinator = new AccountPickerCoordinator(recyclerView, mCoordinator = new AccountPickerCoordinator(recyclerView,
(AccountPickerCoordinator.Listener) getTargetFragment(), (AccountPickerCoordinator.Listener) getTargetFragment(),
getArguments().getString(ARGUMENT_SELECTED_ACCOUNT_NAME), getArguments().getString(ARGUMENT_SELECTED_ACCOUNT_NAME),
AccountPickerAccessPoint.SETTING); /* showIncognitoRow= */ false);
return builder.setTitle(R.string.signin_account_picker_dialog_title) return builder.setTitle(R.string.signin_account_picker_dialog_title)
.setView(recyclerView) .setView(recyclerView)
.create(); .create();
......
...@@ -10,9 +10,9 @@ import android.view.View; ...@@ -10,9 +10,9 @@ import android.view.View;
import androidx.annotation.MainThread; import androidx.annotation.MainThread;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialCoordinator; import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialCoordinator;
import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate; import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
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.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver; import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver; import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
...@@ -52,7 +52,8 @@ public class AccountPickerBottomSheetCoordinator { ...@@ -52,7 +52,8 @@ public class AccountPickerBottomSheetCoordinator {
new AccountPickerBottomSheetMediator(context, accountPickerDelegate); new AccountPickerBottomSheetMediator(context, accountPickerDelegate);
mView = new AccountPickerBottomSheetView(context, mAccountPickerBottomSheetMediator); mView = new AccountPickerBottomSheetView(context, mAccountPickerBottomSheetMediator);
mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(), mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(),
mAccountPickerBottomSheetMediator, null, AccountPickerAccessPoint.WEB); mAccountPickerBottomSheetMediator, /* selectedAccountName= */ null,
/* showIncognitoRow= */ IncognitoUtils.isIncognitoModeEnabled());
IncognitoInterstitialCoordinator incognitoInterstitialCoordinator = IncognitoInterstitialCoordinator incognitoInterstitialCoordinator =
new IncognitoInterstitialCoordinator( new IncognitoInterstitialCoordinator(
mView.getIncognitoInterstitialView(), incognitoInterstitialDelegate); mView.getIncognitoInterstitialView(), incognitoInterstitialDelegate);
......
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
package org.chromium.chrome.browser.signin.account_picker; package org.chromium.chrome.browser.signin.account_picker;
import androidx.annotation.IntDef;
import androidx.annotation.MainThread; import androidx.annotation.MainThread;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.recyclerview.widget.RecyclerView; import androidx.recyclerview.widget.RecyclerView;
...@@ -13,9 +12,6 @@ import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties ...@@ -13,9 +12,6 @@ import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties
import org.chromium.ui.modelutil.MVCListAdapter; import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.SimpleRecyclerViewAdapter; 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. * The coordinator of account picker is the only public class in the account_picker package.
* *
...@@ -45,25 +41,6 @@ public class AccountPickerCoordinator { ...@@ -45,25 +41,6 @@ public class AccountPickerCoordinator {
default void goIncognitoMode() {} 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; private final AccountPickerMediator mMediator;
/** /**
...@@ -73,11 +50,11 @@ public class AccountPickerCoordinator { ...@@ -73,11 +50,11 @@ public class AccountPickerCoordinator {
* @param listener Listener to notify when an account is selected or the user wants to add an * @param listener Listener to notify when an account is selected or the user wants to add an
* account. * account.
* @param selectedAccountName The name of the account that should be marked as selected. * @param selectedAccountName The name of the account that should be marked as selected.
* @param accessPoint Access point of the account picker * @param showIncognitoRow whether to show the incognito row in the account picker.
*/ */
@MainThread @MainThread
public AccountPickerCoordinator(RecyclerView view, Listener listener, public AccountPickerCoordinator(RecyclerView view, Listener listener,
@Nullable String selectedAccountName, @AccountPickerAccessPoint int accessPoint) { @Nullable String selectedAccountName, boolean showIncognitoRow) {
assert listener != null : "The argument AccountPickerCoordinator.Listener cannot be null!"; assert listener != null : "The argument AccountPickerCoordinator.Listener cannot be null!";
MVCListAdapter.ModelList listModel = new MVCListAdapter.ModelList(); MVCListAdapter.ModelList listModel = new MVCListAdapter.ModelList();
...@@ -92,7 +69,7 @@ public class AccountPickerCoordinator { ...@@ -92,7 +69,7 @@ public class AccountPickerCoordinator {
view.setAdapter(adapter); view.setAdapter(adapter);
mMediator = new AccountPickerMediator( mMediator = new AccountPickerMediator(
view.getContext(), listModel, listener, selectedAccountName, accessPoint); view.getContext(), listModel, listener, selectedAccountName, showIncognitoRow);
} }
/** /**
......
...@@ -12,10 +12,8 @@ import androidx.annotation.Nullable; ...@@ -12,10 +12,8 @@ import androidx.annotation.Nullable;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.signin.DisplayableProfileData; import org.chromium.chrome.browser.signin.DisplayableProfileData;
import org.chromium.chrome.browser.signin.ProfileDataCache; 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.AddAccountRowProperties;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ExistingAccountRowProperties; import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ExistingAccountRowProperties;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.IncognitoAccountRowProperties; import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.IncognitoAccountRowProperties;
...@@ -39,7 +37,7 @@ class AccountPickerMediator { ...@@ -39,7 +37,7 @@ class AccountPickerMediator {
private final MVCListAdapter.ModelList mListModel; private final MVCListAdapter.ModelList mListModel;
private final AccountPickerCoordinator.Listener mAccountPickerListener; private final AccountPickerCoordinator.Listener mAccountPickerListener;
private final ProfileDataCache mProfileDataCache; private final ProfileDataCache mProfileDataCache;
private final @AccountPickerAccessPoint int mAccessPoint; private final boolean mShowIncognitoRow;
private @Nullable String mSelectedAccountName; private @Nullable String mSelectedAccountName;
private final AccountManagerFacade mAccountManagerFacade; private final AccountManagerFacade mAccountManagerFacade;
...@@ -49,12 +47,12 @@ class AccountPickerMediator { ...@@ -49,12 +47,12 @@ class AccountPickerMediator {
@MainThread @MainThread
AccountPickerMediator(Context context, MVCListAdapter.ModelList listModel, AccountPickerMediator(Context context, MVCListAdapter.ModelList listModel,
AccountPickerCoordinator.Listener listener, @Nullable String selectedAccountName, AccountPickerCoordinator.Listener listener, @Nullable String selectedAccountName,
@AccountPickerAccessPoint int accessPoint) { boolean showIncognitoRow) {
mListModel = listModel; mListModel = listModel;
mAccountPickerListener = listener; mAccountPickerListener = listener;
mProfileDataCache = new ProfileDataCache( mProfileDataCache = new ProfileDataCache(
context, context.getResources().getDimensionPixelSize(R.dimen.user_picture_size)); context, context.getResources().getDimensionPixelSize(R.dimen.user_picture_size));
mAccessPoint = accessPoint; mShowIncognitoRow = showIncognitoRow;
mSelectedAccountName = selectedAccountName; mSelectedAccountName = selectedAccountName;
mAccountManagerFacade = AccountManagerFacadeProvider.getInstance(); mAccountManagerFacade = AccountManagerFacadeProvider.getInstance();
...@@ -105,10 +103,7 @@ class AccountPickerMediator { ...@@ -105,10 +103,7 @@ class AccountPickerMediator {
mListModel.add(new MVCListAdapter.ListItem(ItemType.ADD_ACCOUNT_ROW, model)); mListModel.add(new MVCListAdapter.ListItem(ItemType.ADD_ACCOUNT_ROW, model));
// Add a "Go incognito mode" row // Add a "Go incognito mode" row
// TODO(https://crbug.com/1136802): Let ctor receive a boolean directly to control if (mShowIncognitoRow) {
// the visibility of the incognito row.
if (mAccessPoint == AccountPickerAccessPoint.WEB
&& IncognitoUtils.isIncognitoModeEnabled()) {
PropertyModel incognitoModel = IncognitoAccountRowProperties.createModel( PropertyModel incognitoModel = IncognitoAccountRowProperties.createModel(
mAccountPickerListener::goIncognitoMode); mAccountPickerListener::goIncognitoMode);
mListModel.add( mListModel.add(
......
...@@ -122,6 +122,7 @@ public class AccountPickerBottomSheetRenderTest { ...@@ -122,6 +122,7 @@ public class AccountPickerBottomSheetRenderTest {
@After @After
public void tearDown() throws Exception { public void tearDown() throws Exception {
ApplicationTestUtils.finishActivity(mActivityTestRule.getActivity()); ApplicationTestUtils.finishActivity(mActivityTestRule.getActivity());
IncognitoUtils.setEnabledForTesting(null);
} }
@AfterClass @AfterClass
......
...@@ -31,6 +31,7 @@ import android.view.View; ...@@ -31,6 +31,7 @@ import android.view.View;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
import org.junit.After;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.ClassRule; import org.junit.ClassRule;
...@@ -131,6 +132,11 @@ public class AccountPickerBottomSheetTest { ...@@ -131,6 +132,11 @@ public class AccountPickerBottomSheetTest {
mAccountManagerTestRule.addAccount(PROFILE_DATA2); mAccountManagerTestRule.addAccount(PROFILE_DATA2);
} }
@After
public void tearDown() {
IncognitoUtils.setEnabledForTesting(null);
}
@Test @Test
@MediumTest @MediumTest
public void testCollapsedSheetWithAccount() { public void testCollapsedSheetWithAccount() {
......
...@@ -20,11 +20,9 @@ import org.mockito.Mock; ...@@ -20,11 +20,9 @@ import org.mockito.Mock;
import org.robolectric.RuntimeEnvironment; import org.robolectric.RuntimeEnvironment;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.signin.DisplayableProfileData; import org.chromium.chrome.browser.signin.DisplayableProfileData;
import org.chromium.chrome.browser.signin.IdentityServicesProvider; import org.chromium.chrome.browser.signin.IdentityServicesProvider;
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.AddAccountRowProperties;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ExistingAccountRowProperties; import org.chromium.chrome.browser.signin.account_picker.AccountPickerProperties.ExistingAccountRowProperties;
import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule; import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
...@@ -71,7 +69,6 @@ public class AccountPickerMediatorTest { ...@@ -71,7 +69,6 @@ public class AccountPickerMediatorTest {
.findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress( .findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
anyString())) anyString()))
.thenReturn(null); .thenReturn(null);
IncognitoUtils.setEnabledForTesting(true);
} }
@After @After
...@@ -79,7 +76,6 @@ public class AccountPickerMediatorTest { ...@@ -79,7 +76,6 @@ public class AccountPickerMediatorTest {
if (mMediator != null) { if (mMediator != null) {
mMediator.destroy(); mMediator.destroy();
} }
IncognitoUtils.setEnabledForTesting(null);
IdentityServicesProvider.setInstanceForTests(null); IdentityServicesProvider.setInstanceForTests(null);
Profile.setLastUsedProfileForTesting(null); Profile.setLastUsedProfileForTesting(null);
} }
...@@ -88,8 +84,8 @@ public class AccountPickerMediatorTest { ...@@ -88,8 +84,8 @@ public class AccountPickerMediatorTest {
public void testModelPopulatedWhenStartedFromWeb() { public void testModelPopulatedWhenStartedFromWeb() {
addAccount(ACCOUNT_NAME1, FULL_NAME1); addAccount(ACCOUNT_NAME1, FULL_NAME1);
addAccount(ACCOUNT_NAME2, ""); addAccount(ACCOUNT_NAME2, "");
mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList, mMediator = new AccountPickerMediator(
mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.WEB); RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1, true);
// ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT, INCOGNITO MODE. // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT, INCOGNITO MODE.
Assert.assertEquals(4, mModelList.size()); Assert.assertEquals(4, mModelList.size());
checkItemForExistingAccountRow(0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ true); checkItemForExistingAccountRow(0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ true);
...@@ -102,8 +98,8 @@ public class AccountPickerMediatorTest { ...@@ -102,8 +98,8 @@ public class AccountPickerMediatorTest {
public void testModelPopulatedWhenStartedFromSettings() { public void testModelPopulatedWhenStartedFromSettings() {
addAccount(ACCOUNT_NAME1, FULL_NAME1); addAccount(ACCOUNT_NAME1, FULL_NAME1);
addAccount(ACCOUNT_NAME2, ""); addAccount(ACCOUNT_NAME2, "");
mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList, mMediator = new AccountPickerMediator(
mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.SETTING); RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1, false);
// ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT
Assert.assertEquals(3, mModelList.size()); Assert.assertEquals(3, mModelList.size());
checkItemForExistingAccountRow(0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ true); checkItemForExistingAccountRow(0, ACCOUNT_NAME1, FULL_NAME1, /* isSelectedAccount= */ true);
...@@ -115,8 +111,8 @@ public class AccountPickerMediatorTest { ...@@ -115,8 +111,8 @@ public class AccountPickerMediatorTest {
public void testModelUpdatedAfterSetSelectedAccountNameFromSettings() { public void testModelUpdatedAfterSetSelectedAccountNameFromSettings() {
addAccount(ACCOUNT_NAME1, FULL_NAME1); addAccount(ACCOUNT_NAME1, FULL_NAME1);
addAccount(ACCOUNT_NAME2, ""); addAccount(ACCOUNT_NAME2, "");
mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList, mMediator = new AccountPickerMediator(
mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.SETTING); RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1, false);
mMediator.setSelectedAccountName(ACCOUNT_NAME2); mMediator.setSelectedAccountName(ACCOUNT_NAME2);
// ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT // ACCOUNT_NAME1, ACCOUNT_NAME2, ADD_ACCOUNT
Assert.assertEquals(3, mModelList.size()); Assert.assertEquals(3, mModelList.size());
...@@ -130,8 +126,8 @@ public class AccountPickerMediatorTest { ...@@ -130,8 +126,8 @@ public class AccountPickerMediatorTest {
public void testProfileDataUpdateWhenAccountPickerIsShownFromSettings() { public void testProfileDataUpdateWhenAccountPickerIsShownFromSettings() {
addAccount(ACCOUNT_NAME1, FULL_NAME1); addAccount(ACCOUNT_NAME1, FULL_NAME1);
addAccount(ACCOUNT_NAME2, ""); addAccount(ACCOUNT_NAME2, "");
mMediator = new AccountPickerMediator(RuntimeEnvironment.application, mModelList, mMediator = new AccountPickerMediator(
mListenerMock, ACCOUNT_NAME1, AccountPickerAccessPoint.SETTING); RuntimeEnvironment.application, mModelList, mListenerMock, ACCOUNT_NAME1, false);
String fullName2 = "Full Name2"; String fullName2 = "Full Name2";
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mFakeProfileDataSource.setProfileData(ACCOUNT_NAME2, mFakeProfileDataSource.setProfileData(ACCOUNT_NAME2,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment