Commit 3bfd83b0 authored by Alice Wang's avatar Alice Wang Committed by Commit Bot

[Android][WebSignin] Extract interface for AccountPickerDelegate

This CL extracts an interface for the AccountPickerDelegate
implementation to facilitate the account_picker package modularization
in the future.

Bug: 1149858
Change-Id: I9ddd1504da50e7feb71289a88831f1128537a28b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542788Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828634}
parent 473d8bb3
......@@ -1296,6 +1296,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerBottomSheetViewBinder.java",
"java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerCoordinator.java",
"java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerDelegate.java",
"java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerDelegateImpl.java",
"java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerMediator.java",
"java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerProperties.java",
"java/src/org/chromium/chrome/browser/signin/account_picker/AddAccountRowViewBinder.java",
......
......@@ -25,6 +25,7 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.signin.account_picker.AccountConsistencyPromoAction;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetCoordinator;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerDelegate;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerDelegateImpl;
import org.chromium.chrome.browser.sync.settings.AccountManagementFragment;
import org.chromium.chrome.browser.tabmodel.TabCreator;
import org.chromium.chrome.browser.tabmodel.TabModel;
......@@ -120,7 +121,7 @@ public class SigninUtils {
AccountPickerBottomSheetCoordinator coordinator =
new AccountPickerBottomSheetCoordinator(activity, bottomSheetController,
new AccountPickerDelegate(windowAndroid, activity.getActivityTab(),
new AccountPickerDelegateImpl(windowAndroid, activity.getActivityTab(),
new WebSigninBridge.Factory(), continueUrl),
incognitoInterstitialDelegate);
}
......
......@@ -10,7 +10,6 @@ import android.view.View;
import androidx.annotation.MainThread;
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.IncognitoInterstitialDelegate;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
......@@ -69,7 +68,7 @@ public class AccountPickerBottomSheetCoordinator {
mView = new AccountPickerBottomSheetView(activity, mAccountPickerBottomSheetMediator);
mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(),
mAccountPickerBottomSheetMediator, /* selectedAccountName= */ null,
/* showIncognitoRow= */ IncognitoUtils.isIncognitoModeEnabled());
/* showIncognitoRow= */ accountPickerDelegate.isIncognitoModeEnabled());
IncognitoInterstitialCoordinator incognitoInterstitialCoordinator =
new IncognitoInterstitialCoordinator(
mView.getIncognitoInterstitialView(), incognitoInterstitialDelegate, () -> {
......
......@@ -4,183 +4,47 @@
package org.chromium.chrome.browser.signin.account_picker;
import android.accounts.AccountManager;
import android.app.Activity;
import android.content.Intent;
import androidx.annotation.MainThread;
import androidx.annotation.Nullable;
import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.signin.IdentityServicesProvider;
import org.chromium.chrome.browser.signin.SigninManager;
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.identitymanager.ConsentLevel;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.metrics.SignoutReason;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.ui.base.WindowAndroid;
/**
* This class is used in web sign-in flow for the account picker bottom sheet.
*
* It is responsible for the sign-in and adding account functions needed for the
* web sign-in flow.
* This interface is used in web sign-in flow for the account picker bottom sheet.
*/
public class AccountPickerDelegate implements WebSigninBridge.Listener {
private final WindowAndroid mWindowAndroid;
private final Activity mActivity;
private final Tab mCurrentTab;
private final WebSigninBridge.Factory mWebSigninBridgeFactory;
private final String mContinueUrl;
private final SigninManager mSigninManager;
private final IdentityManager mIdentityManager;
private @Nullable WebSigninBridge mWebSigninBridge;
private Callback<GoogleServiceAuthError> mOnSignInErrorCallback;
public interface AccountPickerDelegate {
/**
* @param windowAndroid The {@link WindowAndroid} instance of the activity.
* @param currentTab The current tab where the account picker bottom sheet is displayed.
* @param webSigninBridgeFactory A {@link WebSigninBridge.Factory} to create {@link
* WebSigninBridge} instances.
* @param continueUrl The URL that the user would be redirected to after sign-in succeeds.
* Records Signin.AccountConsistencyPromoAction histogram.
*/
public AccountPickerDelegate(WindowAndroid windowAndroid, Tab currentTab,
WebSigninBridge.Factory webSigninBridgeFactory, String continueUrl) {
mWindowAndroid = windowAndroid;
mActivity = mWindowAndroid.getActivity().get();
assert mActivity != null : "Activity should not be null!";
mCurrentTab = currentTab;
mWebSigninBridgeFactory = webSigninBridgeFactory;
mContinueUrl = continueUrl;
mSigninManager = IdentityServicesProvider.get().getSigninManager(
Profile.getLastUsedRegularProfile());
mIdentityManager = IdentityServicesProvider.get().getIdentityManager(
Profile.getLastUsedRegularProfile());
static void recordAccountConsistencyPromoAction(
@AccountConsistencyPromoAction int promoAction) {
RecordHistogram.recordEnumeratedHistogram("Signin.AccountConsistencyPromoAction",
promoAction, AccountConsistencyPromoAction.MAX);
}
/**
* Releases resources used by this class.
* Called when the delegate is dismissed to release resources used by this class.
*/
public void onDismiss() {
destroyWebSigninBridge();
mOnSignInErrorCallback = null;
}
void onDismiss();
/**
* Signs the user into the given account.
* Signs in the user with the given account.
*/
public void signIn(CoreAccountInfo coreAccountInfo,
Callback<GoogleServiceAuthError> onSignInErrorCallback) {
if (mIdentityManager.getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED) != null) {
// In case an error is fired because cookies are taking longer to generate than usual,
// if user retries the sign-in from the error screen, we need to sign out the user
// first before signing in again.
destroyWebSigninBridge();
// TODO(https://crbug.com/1133752): Revise sign-out reason
mSigninManager.signOut(SignoutReason.ABORT_SIGNIN);
}
mOnSignInErrorCallback = onSignInErrorCallback;
mWebSigninBridge = mWebSigninBridgeFactory.create(
Profile.getLastUsedRegularProfile(), coreAccountInfo, this);
mSigninManager.signin(
coreAccountInfo, new SigninManager.SignInCallback() {
@Override
public void onSignInComplete() {
// After the sign-in is finished in Chrome, we still need to wait for
// WebSigninBridge to be called to redirect to the continue url.
}
@Override
public void onSignInAborted() {
AccountPickerDelegate.this.destroyWebSigninBridge();
}
});
}
void signIn(CoreAccountInfo coreAccountInfo,
Callback<GoogleServiceAuthError> onSignInErrorCallback);
/**
* Notifies when the user clicked the "add account" button.
*
* TODO(https//crbug.com/1117000): Change the callback argument to CoreAccountInfo
* Adds account to device.
*/
public void addAccount(Callback<String> callback) {
AccountManagerFacadeProvider.getInstance().createAddAccountIntent(
(@Nullable Intent intent) -> {
if (intent != null) {
WindowAndroid.IntentCallback intentCallback =
new WindowAndroid.IntentCallback() {
@Override
public void onIntentCompleted(
WindowAndroid window, int resultCode, Intent data) {
if (resultCode == Activity.RESULT_OK) {
callback.onResult(data.getStringExtra(
AccountManager.KEY_ACCOUNT_NAME));
}
}
};
mWindowAndroid.showIntent(intent, intentCallback, null);
} else {
// AccountManagerFacade couldn't create intent, use SigninUtils to open
// settings instead.
SigninUtils.openSettingsForAllAccounts(mActivity);
}
});
}
void addAccount(Callback<String> callback);
/**
* Updates credentials of the given account name.
*/
public void updateCredentials(
String accountName, Callback<Boolean> onUpdateCredentialsCallback) {
AccountManagerFacadeProvider.getInstance().updateCredentials(
AccountUtils.createAccountFromName(accountName), mActivity,
onUpdateCredentialsCallback);
}
void updateCredentials(String accountName, Callback<Boolean> onUpdateCredentialsCallback);
/**
* Sign-in completed successfully and the primary account is available in the cookie jar.
* Whether the incognito mode is enabled by policy.
*/
@MainThread
@Override
public void onSigninSucceeded() {
ThreadUtils.assertOnUiThread();
mCurrentTab.loadUrl(new LoadUrlParams(mContinueUrl));
}
/**
* Sign-in process failed.
*
* @param error Details about the error that occurred in the sign-in process.
*/
@MainThread
@Override
public void onSigninFailed(GoogleServiceAuthError error) {
ThreadUtils.assertOnUiThread();
mOnSignInErrorCallback.onResult(error);
}
/**
* Records Signin.AccountConsistencyPromoAction histogram.
*/
public static void recordAccountConsistencyPromoAction(
@AccountConsistencyPromoAction int promoAction) {
RecordHistogram.recordEnumeratedHistogram("Signin.AccountConsistencyPromoAction",
promoAction, AccountConsistencyPromoAction.MAX);
}
private void destroyWebSigninBridge() {
if (mWebSigninBridge != null) {
mWebSigninBridge.destroy();
mWebSigninBridge = null;
}
}
boolean isIncognitoModeEnabled();
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.signin.account_picker;
import android.accounts.AccountManager;
import android.app.Activity;
import android.content.Intent;
import androidx.annotation.MainThread;
import androidx.annotation.Nullable;
import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.signin.IdentityServicesProvider;
import org.chromium.chrome.browser.signin.SigninManager;
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.identitymanager.ConsentLevel;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.metrics.SignoutReason;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.ui.base.WindowAndroid;
/**
* This class is used in web sign-in flow for the account picker bottom sheet.
*
* It is responsible for the sign-in and adding account functions needed for the
* web sign-in flow.
*/
public class AccountPickerDelegateImpl implements WebSigninBridge.Listener, AccountPickerDelegate {
private final WindowAndroid mWindowAndroid;
private final Activity mActivity;
private final Tab mCurrentTab;
private final WebSigninBridge.Factory mWebSigninBridgeFactory;
private final String mContinueUrl;
private final SigninManager mSigninManager;
private final IdentityManager mIdentityManager;
private @Nullable WebSigninBridge mWebSigninBridge;
private Callback<GoogleServiceAuthError> mOnSignInErrorCallback;
/**
* @param windowAndroid The {@link WindowAndroid} instance of the activity.
* @param currentTab The current tab where the account picker bottom sheet is displayed.
* @param webSigninBridgeFactory A {@link WebSigninBridge.Factory} to create {@link
* WebSigninBridge} instances.
* @param continueUrl The URL that the user would be redirected to after sign-in succeeds.
*/
public AccountPickerDelegateImpl(WindowAndroid windowAndroid, Tab currentTab,
WebSigninBridge.Factory webSigninBridgeFactory, String continueUrl) {
mWindowAndroid = windowAndroid;
mActivity = mWindowAndroid.getActivity().get();
assert mActivity != null : "Activity should not be null!";
mCurrentTab = currentTab;
mWebSigninBridgeFactory = webSigninBridgeFactory;
mContinueUrl = continueUrl;
mSigninManager = IdentityServicesProvider.get().getSigninManager(
Profile.getLastUsedRegularProfile());
mIdentityManager = IdentityServicesProvider.get().getIdentityManager(
Profile.getLastUsedRegularProfile());
}
/**
* Releases resources used by this class.
*/
@Override
public void onDismiss() {
destroyWebSigninBridge();
mOnSignInErrorCallback = null;
}
/**
* Signs the user into the given account.
*/
@Override
public void signIn(CoreAccountInfo coreAccountInfo,
Callback<GoogleServiceAuthError> onSignInErrorCallback) {
if (mIdentityManager.getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED) != null) {
// In case an error is fired because cookies are taking longer to generate than usual,
// if user retries the sign-in from the error screen, we need to sign out the user
// first before signing in again.
destroyWebSigninBridge();
// TODO(https://crbug.com/1133752): Revise sign-out reason
mSigninManager.signOut(SignoutReason.ABORT_SIGNIN);
}
mOnSignInErrorCallback = onSignInErrorCallback;
mWebSigninBridge = mWebSigninBridgeFactory.create(
Profile.getLastUsedRegularProfile(), coreAccountInfo, this);
mSigninManager.signin(coreAccountInfo, new SigninManager.SignInCallback() {
@Override
public void onSignInComplete() {
// After the sign-in is finished in Chrome, we still need to wait for
// WebSigninBridge to be called to redirect to the continue url.
}
@Override
public void onSignInAborted() {
AccountPickerDelegateImpl.this.destroyWebSigninBridge();
}
});
}
/**
* Notifies when the user clicked the "add account" button.
*
* TODO(https//crbug.com/1117000): Change the callback argument to CoreAccountInfo
*/
@Override
public void addAccount(Callback<String> callback) {
AccountManagerFacadeProvider.getInstance().createAddAccountIntent((@Nullable Intent intent)
-> {
if (intent != null) {
WindowAndroid.IntentCallback intentCallback = new WindowAndroid.IntentCallback() {
@Override
public void onIntentCompleted(
WindowAndroid window, int resultCode, Intent data) {
if (resultCode == Activity.RESULT_OK) {
callback.onResult(data.getStringExtra(AccountManager.KEY_ACCOUNT_NAME));
}
}
};
mWindowAndroid.showIntent(intent, intentCallback, null);
} else {
// AccountManagerFacade couldn't create intent, use SigninUtils to open
// settings instead.
SigninUtils.openSettingsForAllAccounts(mActivity);
}
});
}
/**
* Updates credentials of the given account name.
*/
@Override
public void updateCredentials(
String accountName, Callback<Boolean> onUpdateCredentialsCallback) {
AccountManagerFacadeProvider.getInstance().updateCredentials(
AccountUtils.createAccountFromName(accountName), mActivity,
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.
*/
@MainThread
@Override
public void onSigninSucceeded() {
ThreadUtils.assertOnUiThread();
mCurrentTab.loadUrl(new LoadUrlParams(mContinueUrl));
}
/**
* Sign-in process failed.
*
* @param error Details about the error that occurred in the sign-in process.
*/
@MainThread
@Override
public void onSigninFailed(GoogleServiceAuthError error) {
ThreadUtils.assertOnUiThread();
mOnSignInErrorCallback.onResult(error);
}
private void destroyWebSigninBridge() {
if (mWebSigninBridge != null) {
mWebSigninBridge.destroy();
mWebSigninBridge = null;
}
}
}
......@@ -14,6 +14,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
import android.view.View;
......@@ -41,7 +42,6 @@ import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
import org.chromium.chrome.browser.night_mode.ChromeNightModeTestUtils;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetCoordinator;
......@@ -116,14 +116,13 @@ public class AccountPickerBottomSheetRenderTest {
@Before
public void setUp() {
initMocks(this);
IncognitoUtils.setEnabledForTesting(true);
when(mAccountPickerDelegateMock.isIncognitoModeEnabled()).thenReturn(true);
mActivityTestRule.startMainActivityOnBlankPage();
}
@After
public void tearDown() throws Exception {
ApplicationTestUtils.finishActivity(mActivityTestRule.getActivity());
IncognitoUtils.setEnabledForTesting(null);
}
@AfterClass
......
......@@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
import android.view.View;
......@@ -32,7 +33,6 @@ import androidx.test.espresso.ViewInteraction;
import androidx.test.filters.MediumTest;
import org.hamcrest.Matcher;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.ClassRule;
......@@ -52,7 +52,6 @@ import org.chromium.base.test.util.MetricsUtils.HistogramDelta;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
import org.chromium.chrome.browser.signin.account_picker.AccountConsistencyPromoAction;
import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetCoordinator;
......@@ -130,16 +129,11 @@ public class AccountPickerBottomSheetTest {
@Before
public void setUp() {
initMocks(this);
IncognitoUtils.setEnabledForTesting(true);
when(mAccountPickerDelegateMock.isIncognitoModeEnabled()).thenReturn(true);
mAccountManagerTestRule.addAccount(PROFILE_DATA1);
mAccountManagerTestRule.addAccount(PROFILE_DATA2);
}
@After
public void tearDown() {
IncognitoUtils.setEnabledForTesting(null);
}
@Test
@MediumTest
public void testCollapsedSheetWithAccount() {
......@@ -168,7 +162,7 @@ public class AccountPickerBottomSheetTest {
@Test
@MediumTest
public void testExpandedSheetWithIncognitoModeDisabled() {
IncognitoUtils.setEnabledForTesting(false);
when(mAccountPickerDelegateMock.isIncognitoModeEnabled()).thenReturn(false);
buildAndShowExpandedBottomSheet();
onVisibleView(withText(PROFILE_DATA1.getAccountName())).check(matches(isDisplayed()));
onVisibleView(withText(PROFILE_DATA1.getFullName())).check(matches(isDisplayed()));
......
......@@ -50,7 +50,7 @@ import org.chromium.ui.base.WindowAndroid;
import java.lang.ref.WeakReference;
/**
* This class tests the {@link AccountPickerDelegate}.
* This class tests the {@link AccountPickerDelegateImpl}.
*/
@RunWith(BaseRobolectricTestRunner.class)
public class AccountPickerDelegateTest {
......@@ -89,7 +89,7 @@ public class AccountPickerDelegateTest {
private FragmentActivity mActivity;
private AccountPickerDelegate mDelegate;
private AccountPickerDelegateImpl mDelegate;
private CoreAccountInfo mCoreAccountInfo;
......@@ -108,7 +108,7 @@ public class AccountPickerDelegateTest {
mCoreAccountInfo =
mAccountManagerTestRule.addAccount(AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
mDelegate = new AccountPickerDelegate(
mDelegate = new AccountPickerDelegateImpl(
mWindowAndroidMock, mTabMock, mWebSigninBridgeFactoryMock, CONTINUE_URL);
when(mWebSigninBridgeFactoryMock.create(eq(mProfileMock), any(), eq(mDelegate)))
.thenReturn(mWebSigninBridgeMock);
......
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