[Signin][Android] Remove Activity from SigninManager.signIn

Activity instance was used before to check sign-in UI visibility and
showing dialogs. These dialog have since been removed, so the Activity
parameter can be removed to simplify the code.

Bug: 1007332
Change-Id: I2e338a555dbd57b06a29697581b16f5ef4b7313d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1823078
Auto-Submit: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699712}

chrome/android/java/src/org/chromium/chrome/browser/firstrun/ForcedSigninProcessor.java

@@ -78,7 +78,7 @@ public final class ForcedSigninProcessor {
                 Log.d(TAG, "Incorrect number of accounts (%d)", accounts.size());
                 return;
             }
-            signinManager.signIn(accounts.get(0), null, new SigninManager.SignInCallback() {
+            signinManager.signIn(accounts.get(0), new SigninManager.SignInCallback() {
                 @Override
                 public void onSignInComplete() {
                     if (onComplete != null) {

chrome/android/java/src/org/chromium/chrome/browser/preferences/sync/SyncAndServicesPreferences.java

@@ -525,8 +525,7 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
             // TODO(https://crbug.com/873116): Pass the correct reason for the signout.
             IdentityServicesProvider.getSigninManager().signOut(
                     SignoutReason.USER_CLICKED_SIGNOUT_SETTINGS,
-                    () -> IdentityServicesProvider.getSigninManager().signIn(account, null, null),
-                    false);
+                    () -> IdentityServicesProvider.getSigninManager().signIn(account, null), false);
             return;
         }
 

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

@@ -233,7 +233,7 @@ public class SigninHelper {
         // This is the correct account now.
         final Account account = AccountManagerFacade.createAccountFromName(newName);
 
-        mSigninManager.signIn(account, null, new SignInCallback() {
+        mSigninManager.signIn(account, new SignInCallback() {
             @Override
             public void onSignInComplete() {
                 validateAccountsInternal(true);

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

@@ -11,9 +11,7 @@ import android.content.Context;
 import androidx.annotation.MainThread;
 import androidx.annotation.Nullable;
 
-import org.chromium.base.ActivityState;
 import org.chromium.base.ApiCompatibilityUtils;
-import org.chromium.base.ApplicationStatus;
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
@@ -119,7 +117,6 @@ public class SigninManager
      */
     private static class SignInState {
         final Account mAccount;
-        final Activity mActivity;
         final SignInCallback mCallback;
 
         /**
@@ -138,32 +135,12 @@ public class SigninManager
 
         /**
          * @param account The account to sign in to.
-         * @param activity Reference to the UI to use for dialogs. Null means forced signin.
          * @param callback Called when the sign-in process finishes or is cancelled. Can be null.
          */
-        SignInState(
-                Account account, @Nullable Activity activity, @Nullable SignInCallback callback) {
+        SignInState(Account account, @Nullable SignInCallback callback) {
             this.mAccount = account;
-            this.mActivity = activity;
             this.mCallback = callback;
         }
-
-        /**
-         * Returns whether this is an interactive sign-in flow.
-         */
-        boolean isInteractive() {
-            return mActivity != null;
-        }
-
-        /**
-         * Returns whether the sign-in flow activity was set but is no longer visible to the user.
-         */
-        boolean isActivityInvisible() {
-            return mActivity != null
-                    && (ApplicationStatus.getStateForActivity(mActivity) == ActivityState.STOPPED
-                            || ApplicationStatus.getStateForActivity(mActivity)
-                                    == ActivityState.DESTROYED);
-        }
     }
 
     /**
@@ -407,12 +384,10 @@ public class SigninManager
      *   - Call the callback if provided.
      *
      * @param account The account to sign in to.
-     * @param activity The activity used to launch UI prompts, or null for a forced signin.
      * @param callback Optional callback for when the sign-in process is finished.
      */
     // TODO(crbug.com/1002056) SigninManager.Signin should use CoreAccountInfo as a parameter.
-    public void signIn(
-            Account account, @Nullable Activity activity, @Nullable SignInCallback callback) {
+    public void signIn(Account account, @Nullable SignInCallback callback) {
         if (account == null) {
             Log.w(TAG, "Ignoring sign-in request due to null account.");
             if (callback != null) callback.onSignInAborted();
@@ -431,7 +406,7 @@ public class SigninManager
             return;
         }
 
-        mSignInState = new SignInState(account, activity, callback);
+        mSignInState = new SignInState(account, callback);
         notifySignInAllowedChanged();
 
         progressSignInFlowSeedSystemAccounts();
@@ -444,7 +419,7 @@ public class SigninManager
     public void signIn(String accountName, @Nullable final Activity activity,
             @Nullable final SignInCallback callback) {
         AccountManagerFacade.get().getAccountFromName(
-                accountName, account -> signIn(account, activity, callback));
+                accountName, account -> signIn(account, callback));
     }
 
     private void progressSignInFlowSeedSystemAccounts() {
@@ -475,11 +450,6 @@ public class SigninManager
         // CoreAccountInfo must be set and valid to progress
         assert mSignInState.mCoreAccountInfo != null;
 
-        if (mSignInState.isActivityInvisible()) {
-            abortSignIn();
-            return;
-        }
-
         Log.d(TAG, "Checking if account has policy management enabled");
         fetchAndApplyCloudPolicy(mSignInState.mCoreAccountInfo, this::onPolicyFetchedBeforeSignIn);
     }
@@ -517,15 +487,12 @@ public class SigninManager
         mIdentityMutator.reloadAllAccountsFromSystemWithPrimaryAccount(
                 mSignInState.mCoreAccountInfo.getId());
 
-        if (mSignInState.isInteractive()) {
-            // If signin was a user action, record that it succeeded.
-            RecordUserAction.record("Signin_Signin_Succeed");
-            logSigninCompleteAccessPoint();
-            // Log signin in reason as defined in signin_metrics.h. Right now only
-            // SIGNIN_PRIMARY_ACCOUNT available on Android.
-            RecordHistogram.recordEnumeratedHistogram("Signin.SigninReason",
-                    SigninReason.SIGNIN_PRIMARY_ACCOUNT, SigninReason.MAX);
-        }
+        RecordUserAction.record("Signin_Signin_Succeed");
+        logSigninCompleteAccessPoint();
+        // Log signin in reason as defined in signin_metrics.h. Right now only
+        // SIGNIN_PRIMARY_ACCOUNT available on Android.
+        RecordHistogram.recordEnumeratedHistogram(
+                "Signin.SigninReason", SigninReason.SIGNIN_PRIMARY_ACCOUNT, SigninReason.MAX);
 
         Log.d(TAG, "Signin completed.");
         mSignInState = null;

chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java

@@ -615,7 +615,7 @@ public class HistoryActivityTest {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             IdentityServicesProvider.getSigninManager().onFirstRunCheckDone();
             IdentityServicesProvider.getSigninManager().addSignInStateObserver(mTestObserver);
-            IdentityServicesProvider.getSigninManager().signIn(account, null, null);
+            IdentityServicesProvider.getSigninManager().signIn(account, null);
         });
 
         mTestObserver.onSigninStateChangedCallback.waitForCallback(

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

@@ -159,7 +159,7 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
     public void signinAndEnableSync(final Account account) {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             IdentityServicesProvider.getSigninManager().signIn(
-                    account, null, new SigninManager.SignInCallback() {
+                    account, new SigninManager.SignInCallback() {
                         @Override
                         public void onSignInComplete() {
                             if (ChromeFeatureList.isEnabled(

chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninManagerTest.java

@@ -22,6 +22,7 @@ import static org.mockito.MockitoAnnotations.initMocks;
 
 import android.accounts.Account;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -30,6 +31,8 @@ import org.mockito.Mock;
 import org.robolectric.annotation.Config;
 
 import org.chromium.base.ContextUtils;
+import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
 import org.chromium.components.signin.AccountManagerFacade;
@@ -61,6 +64,10 @@ public class SigninManagerTest {
 
     @Before
     public void setUp() {
+        // TODO(https://crbug.com/1007903): Use DisableNativeTestRule instead.
+        RecordHistogram.setDisabledForTests(true);
+        RecordUserAction.setDisabledForTests(true);
+
         initMocks(this);
 
         mocker.mock(SigninManagerJni.TEST_HOOKS, mNativeMock);
@@ -83,6 +90,13 @@ public class SigninManagerTest {
                 AccountManagerFacade.createAccountFromName("user@domain.com"), "gaia-id-user");
     }
 
+    @After
+    public void tearDown() {
+        // TODO(https://crbug.com/1007903): Use DisableNativeTestRule instead.
+        RecordHistogram.setDisabledForTests(false);
+        RecordUserAction.setDisabledForTests(false);
+    }
+
     @Test
     public void signOutFromJavaWithManagedDomain() {
         // Stub out various native calls. Some of these are verified as never called
@@ -225,7 +239,7 @@ public class SigninManagerTest {
 
         mSigninManager.onFirstRunCheckDone(); // Allow sign-in.
 
-        mSigninManager.signIn(account.getAccount(), null, null);
+        mSigninManager.signIn(account.getAccount(), null);
         assertTrue(mSigninManager.isOperationInProgress());
         AtomicInteger callCount = new AtomicInteger(0);
         mSigninManager.runAfterOperationInProgress(callCount::incrementAndGet);