Commit 5c9e565a authored by Alice Wang's avatar Alice Wang Committed by Chromium LUCI CQ

[Signin][Android] Make AccountPickerDelegateImpl independent of incognito package

This CL makes AccountPickerDelegateImpl independent of incognito package
by moving the incognito mode policy check to SigninBridge.

Bug: 1149857
Change-Id: Ib5ae49f71ea84a08131c70ecd3998eaefe5ac66e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640654Reviewed-by: default avatarTanmoy Mollik <triploblastic@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846057}
parent 7574d4ad
...@@ -12,6 +12,7 @@ import org.chromium.base.ThreadUtils; ...@@ -12,6 +12,7 @@ import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.chrome.browser.app.ChromeActivity; import org.chromium.chrome.browser.app.ChromeActivity;
import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncherImpl; import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncherImpl;
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.account_picker.AccountPickerDelegateImpl; import org.chromium.chrome.browser.signin.account_picker.AccountPickerDelegateImpl;
import org.chromium.chrome.browser.signin.services.IdentityServicesProvider; import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
...@@ -103,7 +104,8 @@ final class SigninBridge { ...@@ -103,7 +104,8 @@ final class SigninBridge {
activity, bottomSheetController, activity, bottomSheetController,
new AccountPickerDelegateImpl(windowAndroid, activity.getActivityTab(), new AccountPickerDelegateImpl(windowAndroid, activity.getActivityTab(),
new WebSigninBridge.Factory(), continueUrl), new WebSigninBridge.Factory(), continueUrl),
regularTabModel, incognitoTabCreator, HelpAndFeedbackLauncherImpl.getInstance()); regularTabModel, incognitoTabCreator, HelpAndFeedbackLauncherImpl.getInstance(),
/* showIncognitoRow= */ IncognitoUtils.isIncognitoModeEnabled());
} }
private SigninBridge() {} private SigninBridge() {}
......
...@@ -13,7 +13,6 @@ import androidx.annotation.Nullable; ...@@ -13,7 +13,6 @@ import androidx.annotation.Nullable;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
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.services.IdentityServicesProvider; import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
import org.chromium.chrome.browser.signin.services.SigninManager; import org.chromium.chrome.browser.signin.services.SigninManager;
...@@ -148,14 +147,6 @@ public class AccountPickerDelegateImpl implements WebSigninBridge.Listener, Acco ...@@ -148,14 +147,6 @@ public class AccountPickerDelegateImpl implements WebSigninBridge.Listener, Acco
onUpdateCredentialsCallback); onUpdateCredentialsCallback);
} }
/**
* Whether the incognito mode is enabled by policy.
*/
@Override
public boolean isIncognitoModeEnabled() {
return IncognitoUtils.isIncognitoModeEnabled();
}
/** /**
* Sign-in completed successfully and the primary account is available in the cookie jar. * Sign-in completed successfully and the primary account is available in the cookie jar.
*/ */
......
...@@ -14,7 +14,6 @@ import static org.mockito.ArgumentMatchers.any; ...@@ -14,7 +14,6 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks; import static org.mockito.MockitoAnnotations.initMocks;
import android.view.View; import android.view.View;
...@@ -114,7 +113,6 @@ public class AccountPickerBottomSheetRenderTest { ...@@ -114,7 +113,6 @@ public class AccountPickerBottomSheetRenderTest {
@Before @Before
public void setUp() { public void setUp() {
initMocks(this); initMocks(this);
when(mAccountPickerDelegateMock.isIncognitoModeEnabled()).thenReturn(true);
mActivityTestRule.startMainActivityOnBlankPage(); mActivityTestRule.startMainActivityOnBlankPage();
} }
...@@ -332,7 +330,7 @@ public class AccountPickerBottomSheetRenderTest { ...@@ -332,7 +330,7 @@ public class AccountPickerBottomSheetRenderTest {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mCoordinator = new AccountPickerBottomSheetCoordinator(mActivityTestRule.getActivity(), mCoordinator = new AccountPickerBottomSheetCoordinator(mActivityTestRule.getActivity(),
getBottomSheetController(), mAccountPickerDelegateMock, getBottomSheetController(), mAccountPickerDelegateMock,
mIncognitoInterstitialDelegateMock); mIncognitoInterstitialDelegateMock, true);
}); });
CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById( CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById(
R.id.account_picker_selected_account)::isShown); R.id.account_picker_selected_account)::isShown);
......
...@@ -24,7 +24,6 @@ import static org.mockito.ArgumentMatchers.notNull; ...@@ -24,7 +24,6 @@ import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks; import static org.mockito.MockitoAnnotations.initMocks;
import android.view.View; import android.view.View;
...@@ -145,7 +144,6 @@ public class AccountPickerBottomSheetTest { ...@@ -145,7 +144,6 @@ public class AccountPickerBottomSheetTest {
@Before @Before
public void setUp() { public void setUp() {
initMocks(this); initMocks(this);
when(mAccountPickerDelegateMock.isIncognitoModeEnabled()).thenReturn(true);
mAccountManagerTestRule.addAccount(PROFILE_DATA1); mAccountManagerTestRule.addAccount(PROFILE_DATA1);
mAccountManagerTestRule.addAccount(PROFILE_DATA2); mAccountManagerTestRule.addAccount(PROFILE_DATA2);
SharedPreferencesManager.getInstance().removeKey( SharedPreferencesManager.getInstance().removeKey(
...@@ -202,8 +200,14 @@ public class AccountPickerBottomSheetTest { ...@@ -202,8 +200,14 @@ public class AccountPickerBottomSheetTest {
@Test @Test
@MediumTest @MediumTest
public void testExpandedSheetWithIncognitoModeDisabled() { public void testExpandedSheetWithIncognitoModeDisabled() {
when(mAccountPickerDelegateMock.isIncognitoModeEnabled()).thenReturn(false); TestThreadUtils.runOnUiThreadBlocking(() -> {
buildAndShowExpandedBottomSheet(); mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(),
getBottomSheetController(), mAccountPickerDelegateMock,
mIncognitoInterstitialDelegateMock, false);
});
CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById(
R.id.account_picker_selected_account)::isShown);
onView(withText(PROFILE_DATA1.getFullName())).perform(click());
onVisibleView(withText(PROFILE_DATA1.getAccountEmail())).check(matches(isDisplayed())); onVisibleView(withText(PROFILE_DATA1.getAccountEmail())).check(matches(isDisplayed()));
onVisibleView(withText(PROFILE_DATA1.getFullName())).check(matches(isDisplayed())); onVisibleView(withText(PROFILE_DATA1.getFullName())).check(matches(isDisplayed()));
onView(withText(PROFILE_DATA2.getAccountEmail())).check(matches(isDisplayed())); onView(withText(PROFILE_DATA2.getAccountEmail())).check(matches(isDisplayed()));
...@@ -226,7 +230,7 @@ public class AccountPickerBottomSheetTest { ...@@ -226,7 +230,7 @@ public class AccountPickerBottomSheetTest {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(), mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(),
getBottomSheetController(), mAccountPickerDelegateMock, getBottomSheetController(), mAccountPickerDelegateMock,
mIncognitoInterstitialDelegateMock); mIncognitoInterstitialDelegateMock, true);
}); });
checkZeroAccountBottomSheet(); checkZeroAccountBottomSheet();
} }
...@@ -331,7 +335,7 @@ public class AccountPickerBottomSheetTest { ...@@ -331,7 +335,7 @@ public class AccountPickerBottomSheetTest {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(), mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(),
getBottomSheetController(), mAccountPickerDelegateMock, getBottomSheetController(), mAccountPickerDelegateMock,
mIncognitoInterstitialDelegateMock); mIncognitoInterstitialDelegateMock, true);
}); });
checkZeroAccountBottomSheet(); checkZeroAccountBottomSheet();
...@@ -695,7 +699,7 @@ public class AccountPickerBottomSheetTest { ...@@ -695,7 +699,7 @@ public class AccountPickerBottomSheetTest {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(), mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(),
getBottomSheetController(), mAccountPickerDelegateMock, mTabModelMock, getBottomSheetController(), mAccountPickerDelegateMock, mTabModelMock,
mTabCreatorMock, mHelpAndFeedbackLauncherMock); mTabCreatorMock, mHelpAndFeedbackLauncherMock, true);
}); });
CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById( CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById(
R.id.account_picker_selected_account)::isShown); R.id.account_picker_selected_account)::isShown);
...@@ -705,7 +709,7 @@ public class AccountPickerBottomSheetTest { ...@@ -705,7 +709,7 @@ public class AccountPickerBottomSheetTest {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(), mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(),
getBottomSheetController(), mAccountPickerDelegateMock, getBottomSheetController(), mAccountPickerDelegateMock,
mIncognitoInterstitialDelegateMock); mIncognitoInterstitialDelegateMock, true);
}); });
CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById( CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById(
R.id.account_picker_selected_account)::isShown); R.id.account_picker_selected_account)::isShown);
......
...@@ -32,7 +32,6 @@ import org.robolectric.Robolectric; ...@@ -32,7 +32,6 @@ import org.robolectric.Robolectric;
import org.chromium.base.Callback; import org.chromium.base.Callback;
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.services.IdentityServicesProvider; import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
import org.chromium.chrome.browser.signin.services.SigninManager; import org.chromium.chrome.browser.signin.services.SigninManager;
...@@ -117,7 +116,6 @@ public class AccountPickerDelegateTest { ...@@ -117,7 +116,6 @@ public class AccountPickerDelegateTest {
@After @After
public void tearDown() { public void tearDown() {
IncognitoUtils.setEnabledForTesting(null);
mDelegate.onDismiss(); mDelegate.onDismiss();
} }
...@@ -186,12 +184,4 @@ public class AccountPickerDelegateTest { ...@@ -186,12 +184,4 @@ public class AccountPickerDelegateTest {
AccountManagerTestRule.TEST_ACCOUNT_EMAIL), AccountManagerTestRule.TEST_ACCOUNT_EMAIL),
mActivity, callback); mActivity, callback);
} }
@Test
public void testIsIncognitoModeEnabled() {
IncognitoUtils.setEnabledForTesting(true);
Assert.assertTrue(mDelegate.isIncognitoModeEnabled());
IncognitoUtils.setEnabledForTesting(false);
Assert.assertFalse(mDelegate.isIncognitoModeEnabled());
}
} }
...@@ -65,10 +65,12 @@ public class AccountPickerBottomSheetCoordinator { ...@@ -65,10 +65,12 @@ public class AccountPickerBottomSheetCoordinator {
public AccountPickerBottomSheetCoordinator(Activity activity, public AccountPickerBottomSheetCoordinator(Activity activity,
BottomSheetController bottomSheetController, BottomSheetController bottomSheetController,
AccountPickerDelegate accountPickerDelegate, TabModel regularTabModel, AccountPickerDelegate accountPickerDelegate, TabModel regularTabModel,
TabCreator incognitoTabCreator, HelpAndFeedbackLauncher helpAndFeedbackLauncher) { TabCreator incognitoTabCreator, HelpAndFeedbackLauncher helpAndFeedbackLauncher,
boolean showIncognitoRow) {
this(activity, bottomSheetController, accountPickerDelegate, this(activity, bottomSheetController, accountPickerDelegate,
new IncognitoInterstitialDelegate( new IncognitoInterstitialDelegate(
activity, regularTabModel, incognitoTabCreator, helpAndFeedbackLauncher)); activity, regularTabModel, incognitoTabCreator, helpAndFeedbackLauncher),
showIncognitoRow);
} }
/** /**
...@@ -80,7 +82,7 @@ public class AccountPickerBottomSheetCoordinator { ...@@ -80,7 +82,7 @@ public class AccountPickerBottomSheetCoordinator {
public AccountPickerBottomSheetCoordinator(Activity activity, public AccountPickerBottomSheetCoordinator(Activity activity,
BottomSheetController bottomSheetController, BottomSheetController bottomSheetController,
AccountPickerDelegate accountPickerDelegate, AccountPickerDelegate accountPickerDelegate,
IncognitoInterstitialDelegate incognitoInterstitialDelegate) { IncognitoInterstitialDelegate incognitoInterstitialDelegate, boolean showIncognitoRow) {
SigninPreferencesManager.getInstance().incrementAccountPickerBottomSheetShownCount(); SigninPreferencesManager.getInstance().incrementAccountPickerBottomSheetShownCount();
SigninMetricsUtils.logAccountConsistencyPromoAction(AccountConsistencyPromoAction.SHOWN); SigninMetricsUtils.logAccountConsistencyPromoAction(AccountConsistencyPromoAction.SHOWN);
SigninMetricsUtils.logAccountConsistencyPromoShownCount( SigninMetricsUtils.logAccountConsistencyPromoShownCount(
...@@ -91,13 +93,16 @@ public class AccountPickerBottomSheetCoordinator { ...@@ -91,13 +93,16 @@ public class AccountPickerBottomSheetCoordinator {
mView = new AccountPickerBottomSheetView(activity, mAccountPickerBottomSheetMediator); mView = new AccountPickerBottomSheetView(activity, mAccountPickerBottomSheetMediator);
mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(), mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(),
mAccountPickerBottomSheetMediator, /* selectedAccountName= */ null, mAccountPickerBottomSheetMediator, /* selectedAccountName= */ null,
/* showIncognitoRow= */ accountPickerDelegate.isIncognitoModeEnabled()); /* showIncognitoRow= */ showIncognitoRow);
if (showIncognitoRow) {
IncognitoInterstitialCoordinator incognitoInterstitialCoordinator = IncognitoInterstitialCoordinator incognitoInterstitialCoordinator =
new IncognitoInterstitialCoordinator( new IncognitoInterstitialCoordinator(mView.getIncognitoInterstitialView(),
mView.getIncognitoInterstitialView(), incognitoInterstitialDelegate, () -> { incognitoInterstitialDelegate, () -> {
SigninMetricsUtils.logAccountConsistencyPromoAction( SigninMetricsUtils.logAccountConsistencyPromoAction(
AccountConsistencyPromoAction.STARTED_INCOGNITO_SESSION); AccountConsistencyPromoAction.STARTED_INCOGNITO_SESSION);
}); });
}
mBottomSheetController = bottomSheetController; mBottomSheetController = bottomSheetController;
PropertyModelChangeProcessor.create(mAccountPickerBottomSheetMediator.getModel(), mView, PropertyModelChangeProcessor.create(mAccountPickerBottomSheetMediator.getModel(), mView,
AccountPickerBottomSheetViewBinder::bind); AccountPickerBottomSheetViewBinder::bind);
......
...@@ -32,9 +32,4 @@ public interface AccountPickerDelegate { ...@@ -32,9 +32,4 @@ public interface AccountPickerDelegate {
* Updates credentials of the given account name. * Updates credentials of the given account name.
*/ */
void updateCredentials(String accountName, Callback<Boolean> onUpdateCredentialsCallback); void updateCredentials(String accountName, Callback<Boolean> onUpdateCredentialsCallback);
/**
* Whether the incognito mode is enabled by policy.
*/
boolean isIncognitoModeEnabled();
} }
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