[Android][Signin] Fix BookmarkPersonalizedSigninPromoTests

This CL fixes the disabled BookmarkPersonalizedSigninPromoTest
related to launching SigninActivity in different scenarios. A
SigninActivityLauncher is created to setup the intent and
launch the SigninActivity with different account settings.

Bug: 789531
Change-Id: I09ff0419fb268b2482984c3d0bd3c5cb3227860a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868878Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709836}

chrome/android/chrome_java_sources.gni

@@ -1479,6 +1479,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/signin/ProfileDownloader.java",
   "java/src/org/chromium/chrome/browser/signin/SignOutDialogFragment.java",
   "java/src/org/chromium/chrome/browser/signin/SigninActivity.java",
+  "java/src/org/chromium/chrome/browser/signin/SigninActivityLauncher.java",
   "java/src/org/chromium/chrome/browser/signin/SigninFragment.java",
   "java/src/org/chromium/chrome/browser/signin/SigninFragmentBase.java",
   "java/src/org/chromium/chrome/browser/signin/SigninHelper.java",

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java

@@ -263,7 +263,7 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs
      * @param promoState The promo state to which the header will be set to.
      */
     @VisibleForTesting
-    public static void forcePromoStateForTests(@PromoState int promoState) {
+    static void forcePromoStateForTests(@Nullable @PromoState Integer promoState) {
         sPromoStateForTests = promoState;
     }
 }

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

@@ -5,13 +5,11 @@
 package org.chromium.chrome.browser.signin;
 
 import android.content.Context;
-import android.content.Intent;
 import android.os.Bundle;
 import android.support.v4.app.Fragment;
 import android.support.v4.app.FragmentManager;
 
 import androidx.annotation.IntDef;
-
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeBaseAppCompatActivity;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
@@ -26,8 +24,7 @@ import java.lang.annotation.RetentionPolicy;
  */
 // TODO(https://crbug.com/820491): extend AsyncInitializationActivity.
 public class SigninActivity extends ChromeBaseAppCompatActivity {
-    private static final String TAG = "SigninActivity";
-    private static final String ARGUMENT_FRAGMENT_ARGS = "SigninActivity.FragmentArgs";
+    static final String ARGUMENT_FRAGMENT_ARGS = "SigninActivity.FragmentArgs";
 
     @IntDef({SigninAccessPoint.SETTINGS, SigninAccessPoint.BOOKMARK_MANAGER,
             SigninAccessPoint.RECENT_TABS, SigninAccessPoint.SIGNIN_PROMO,
@@ -35,60 +32,13 @@ public class SigninActivity extends ChromeBaseAppCompatActivity {
     @Retention(RetentionPolicy.SOURCE)
     public @interface AccessPoint {}
 
-    /**
-     * Creates an {@link Intent} which can be used to start sign-in flow.
-     * @param accessPoint {@link AccessPoint} for starting sign-in flow. Used in metrics.
-     */
-    public static Intent createIntent(Context context, @AccessPoint int accessPoint) {
-        return createIntentInternal(context, SigninFragment.createArguments(accessPoint));
-    }
-
-    /**
-     * Creates an argument bundle to start default sign-in flow from personalized sign-in promo.
-     * @param accessPoint The access point for starting sign-in flow.
-     * @param accountName The account to preselect or null to preselect the default account.
-     */
-    public static Intent createIntentForPromoDefaultFlow(
-            Context context, @SigninAccessPoint int accessPoint, String accountName) {
-        return createIntentInternal(context,
-                SigninFragment.createArgumentsForPromoDefaultFlow(accessPoint, accountName));
-    }
-
-    /**
-     * Creates an argument bundle to start "Choose account" sign-in flow from personalized sign-in
-     * promo.
-     * @param accessPoint The access point for starting sign-in flow.
-     * @param accountName The account to preselect or null to preselect the default account.
-     */
-    public static Intent createIntentForPromoChooseAccountFlow(
-            Context context, @SigninAccessPoint int accessPoint, String accountName) {
-        return createIntentInternal(context,
-                SigninFragment.createArgumentsForPromoChooseAccountFlow(accessPoint, accountName));
-    }
-
-    /**
-     * Creates an argument bundle to start "New account" sign-in flow from personalized sign-in
-     * promo.
-     * @param accessPoint The access point for starting sign-in flow.
-     */
-    public static Intent createIntentForPromoAddAccountFlow(
-            Context context, @SigninAccessPoint int accessPoint) {
-        return createIntentInternal(
-                context, SigninFragment.createArgumentsForPromoAddAccountFlow(accessPoint));
-    }
-
-    private static Intent createIntentInternal(Context context, Bundle fragmentArgs) {
-        Intent intent = new Intent(context, SigninActivity.class);
-        intent.putExtra(ARGUMENT_FRAGMENT_ARGS, fragmentArgs);
-        return intent;
-    }
-
     /**
      * A convenience method to create a SigninActivity passing the access point in the
      * intent. Checks if the sign in flow can be started before showing the activity.
      * @param accessPoint {@link AccessPoint} for starting signin flow. Used in metrics.
      * @return {@code true} if sign in has been allowed.
      */
+    // TODO(https://crbug.com/1017697): Move this method to SigninActivityLauncher
     public static boolean startIfAllowed(Context context, @AccessPoint int accessPoint) {
         SigninManager signinManager = IdentityServicesProvider.getSigninManager();
         if (!signinManager.isSignInAllowed()) {
@@ -97,8 +47,7 @@ public class SigninActivity extends ChromeBaseAppCompatActivity {
             }
             return false;
         }
-
-        context.startActivity(createIntent(context, accessPoint));
+        SigninActivityLauncher.get().launchActivity(context, accessPoint);
         return true;
     }
 

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

+// Copyright 2019 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;
+
+import android.content.Context;
+import android.content.Intent;
+import android.os.Bundle;
+
+import androidx.annotation.Nullable;
+
+import org.chromium.base.VisibleForTesting;
+import org.chromium.components.signin.metrics.SigninAccessPoint;
+
+/**
+ * SigninActivityLauncher creates the proper intent and then launches the
+ * SigninActivity in different scenarios.
+ */
+public class SigninActivityLauncher {
+    private static SigninActivityLauncher sLauncher;
+
+    /**
+     * Singleton instance getter
+     */
+    public static SigninActivityLauncher get() {
+        if (sLauncher == null) {
+            sLauncher = new SigninActivityLauncher();
+        }
+        return sLauncher;
+    }
+
+    @VisibleForTesting
+    public static void setLauncherForTest(@Nullable SigninActivityLauncher launcher) {
+        sLauncher = launcher;
+    }
+
+    private SigninActivityLauncher() {}
+
+    /**
+     * Launch the SigninActivity with default sign-in flow from personalized sign-in promo.
+     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
+     * @param accountName The account to preselect or null to preselect the default account.
+     */
+    public void launchActivityForPromoDefaultFlow(
+            Context context, @SigninAccessPoint int accessPoint, String accountName) {
+        launchInternal(context,
+                SigninFragment.createArgumentsForPromoDefaultFlow(accessPoint, accountName));
+    }
+
+    /**
+     * Launch the SigninActivity with "Choose account" sign-in flow from personalized
+     * sign-in promo.
+     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
+     * @param accountName The account to preselect or null to preselect the default account.
+     */
+    public void launchActivityForPromoChooseAccountFlow(
+            Context context, @SigninAccessPoint int accessPoint, String accountName) {
+        launchInternal(context,
+                SigninFragment.createArgumentsForPromoChooseAccountFlow(accessPoint, accountName));
+    }
+
+    /**
+     * Launch the SigninActivity with "New account" sign-in flow from personalized sign-in
+     * promo.
+     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
+     */
+    public void launchActivityForPromoAddAccountFlow(
+            Context context, @SigninAccessPoint int accessPoint) {
+        launchInternal(context, SigninFragment.createArgumentsForPromoAddAccountFlow(accessPoint));
+    }
+
+    /**
+     * Creates an {@link Intent} which can be used to start sign-in flow and launch the signin
+     * activity.
+     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
+     */
+    public void launchActivity(Context context, @SigninAccessPoint int accessPoint) {
+        launchInternal(context, SigninFragment.createArguments(accessPoint));
+    }
+
+    private void launchInternal(Context context, Bundle fragmentArgs) {
+        Intent intent = new Intent(context, SigninActivity.class);
+        intent.putExtra(SigninActivity.ARGUMENT_FRAGMENT_ARGS, fragmentArgs);
+        context.startActivity(intent);
+    }
+}

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

@@ -302,22 +302,21 @@ public class SigninPromoController {
     private void signinWithNewAccount(Context context) {
         recordSigninButtonUsed();
         RecordUserAction.record(mSigninNewAccountUserActionName);
-        context.startActivity(
-                SigninActivity.createIntentForPromoAddAccountFlow(context, mAccessPoint));
+        SigninActivityLauncher.get().launchActivityForPromoAddAccountFlow(context, mAccessPoint);
     }
 
     private void signinWithDefaultAccount(Context context) {
         recordSigninButtonUsed();
         RecordUserAction.record(mSigninWithDefaultUserActionName);
-        context.startActivity(SigninActivity.createIntentForPromoDefaultFlow(
-                context, mAccessPoint, mProfileData.getAccountName()));
+        SigninActivityLauncher.get().launchActivityForPromoDefaultFlow(
+                context, mAccessPoint, mProfileData.getAccountName());
     }
 
     private void signinWithNotDefaultAccount(Context context) {
         recordSigninButtonUsed();
         RecordUserAction.record(mSigninNotDefaultUserActionName);
-        context.startActivity(SigninActivity.createIntentForPromoChooseAccountFlow(
-                context, mAccessPoint, mProfileData.getAccountName()));
+        SigninActivityLauncher.get().launchActivityForPromoChooseAccountFlow(
+                context, mAccessPoint, mProfileData.getAccountName());
     }
 
     private void recordSigninButtonUsed() {

chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkPersonalizedSigninPromoTest.java

@@ -12,17 +12,20 @@ import static android.support.test.espresso.assertion.ViewAssertions.matches;
 import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
 import static android.support.test.espresso.matcher.ViewMatchers.withId;
 
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertFalse;
-import static junit.framework.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 import android.accounts.Account;
-import android.content.Intent;
+import android.content.Context;
 import android.support.test.filters.LargeTest;
 import android.support.test.filters.MediumTest;
-import android.support.test.runner.intent.IntentCallback;
-import android.support.test.runner.intent.IntentMonitorRegistry;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -31,14 +34,14 @@ import org.junit.runner.RunWith;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.DisableIf;
 import org.chromium.base.test.util.DisabledTest;
-import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
-import org.chromium.chrome.browser.signin.SigninActivity;
+import org.chromium.chrome.browser.signin.SigninActivityLauncher;
 import org.chromium.chrome.browser.signin.SigninPromoController;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.util.BookmarkTestUtil;
 import org.chromium.components.signin.AccountManagerFacade;
 import org.chromium.components.signin.ProfileDataSource;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
@@ -48,16 +51,11 @@ import org.chromium.components.signin.test.util.FakeAccountManagerDelegate;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.ui.test.util.UiDisableIf;
 
-import java.io.Closeable;
-import java.util.ArrayList;
-import java.util.List;
-
 /**
  * Tests for the personalized signin promo on the Bookmarks page.
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
-@RetryOnFailure(message = "crbug.com/789531")
 public class BookmarkPersonalizedSigninPromoTest {
     private static final String TEST_ACCOUNT_NAME = "test@gmail.com";
     private static final String TEST_FULL_NAME = "Test Account";
@@ -70,11 +68,23 @@ public class BookmarkPersonalizedSigninPromoTest {
     public final AccountManagerTestRule mAccountManagerTestRule =
             new AccountManagerTestRule(FakeAccountManagerDelegate.ENABLE_PROFILE_DATA_SOURCE);
 
+    private final SigninActivityLauncher mMockSigninActivityLauncher =
+            mock(SigninActivityLauncher.class);
+
     @Before
     public void setUp() {
+        BookmarkPromoHeader.forcePromoStateForTests(
+                BookmarkPromoHeader.PromoState.PROMO_SIGNIN_PERSONALIZED);
+        SigninActivityLauncher.setLauncherForTest(mMockSigninActivityLauncher);
         mActivityTestRule.startMainActivityFromLauncher();
     }
 
+    @After
+    public void tearDown() {
+        SigninActivityLauncher.setLauncherForTest(null);
+        BookmarkPromoHeader.forcePromoStateForTests(null);
+    }
+
     @Test
     @MediumTest
     @DisabledTest(message = "crbug.com/789531")
@@ -102,72 +112,54 @@ public class BookmarkPersonalizedSigninPromoTest {
 
     @Test
     @MediumTest
-    @DisabledTest(message = "crbug.com/789531")
     public void testSigninButtonDefaultAccount() {
+        doNothing()
+                .when(SigninActivityLauncher.get())
+                .launchActivityForPromoDefaultFlow(any(Context.class), anyInt(), anyString());
         addTestAccount();
-        openBookmarkManager();
-        onView(withId(R.id.signin_promo_view_container)).check(matches(isDisplayed()));
-
-        final List<Intent> startedIntents;
-        try (IntentCallbackHelper helper = new IntentCallbackHelper()) {
+        openBookmarkManagerAndCheckSigninPromoIsDisplayed();
         onView(withId(R.id.signin_promo_signin_button)).perform(click());
-            startedIntents = helper.getStartedIntents();
-        }
-
-        assertEquals("Choosing to sign in with the default account should fire an intent!", 1,
-                startedIntents.size());
-        Intent expectedIntent =
-                SigninActivity.createIntentForPromoDefaultFlow(mActivityTestRule.getActivity(),
-                        SigninAccessPoint.BOOKMARK_MANAGER, TEST_ACCOUNT_NAME);
-        assertTrue(expectedIntent.filterEquals(startedIntents.get(0)));
+        verify(mMockSigninActivityLauncher)
+                .launchActivityForPromoDefaultFlow(any(BookmarkActivity.class),
+                        eq(SigninAccessPoint.BOOKMARK_MANAGER), eq(TEST_ACCOUNT_NAME));
     }
 
     @Test
     @MediumTest
-    @DisabledTest(message = "crbug.com/789531")
     public void testSigninButtonNotDefaultAccount() {
+        doNothing()
+                .when(SigninActivityLauncher.get())
+                .launchActivityForPromoChooseAccountFlow(any(Context.class), anyInt(), anyString());
         addTestAccount();
-        openBookmarkManager();
-        onView(withId(R.id.signin_promo_view_container)).check(matches(isDisplayed()));
-
-        final List<Intent> startedIntents;
-        try (IntentCallbackHelper helper = new IntentCallbackHelper()) {
+        openBookmarkManagerAndCheckSigninPromoIsDisplayed();
         onView(withId(R.id.signin_promo_choose_account_button)).perform(click());
-            startedIntents = helper.getStartedIntents();
-        }
-
-        assertEquals("Choosing to sign in with another account should fire an intent!", 1,
-                startedIntents.size());
-        Intent expectedIntent = SigninActivity.createIntent(
-                mActivityTestRule.getActivity(), SigninAccessPoint.BOOKMARK_MANAGER);
-        assertTrue(expectedIntent.filterEquals(startedIntents.get(0)));
+        verify(mMockSigninActivityLauncher)
+                .launchActivityForPromoChooseAccountFlow(any(BookmarkActivity.class),
+                        eq(SigninAccessPoint.BOOKMARK_MANAGER), eq(TEST_ACCOUNT_NAME));
     }
 
     @Test
     @MediumTest
-    @DisabledTest(message = "crbug.com/789531")
     public void testSigninButtonNewAccount() {
-        openBookmarkManager();
-        onView(withId(R.id.signin_promo_view_container)).check(matches(isDisplayed()));
-
-        final List<Intent> startedIntents;
-        try (IntentCallbackHelper helper = new IntentCallbackHelper()) {
+        doNothing()
+                .when(SigninActivityLauncher.get())
+                .launchActivityForPromoAddAccountFlow(any(Context.class), anyInt());
+        openBookmarkManagerAndCheckSigninPromoIsDisplayed();
         onView(withId(R.id.signin_promo_signin_button)).perform(click());
-            startedIntents = helper.getStartedIntents();
+        verify(mMockSigninActivityLauncher)
+                .launchActivityForPromoAddAccountFlow(
+                        any(BookmarkActivity.class), eq(SigninAccessPoint.BOOKMARK_MANAGER));
     }
 
-        assertFalse(
-                "Adding a new account should fire at least one intent!", startedIntents.isEmpty());
-        Intent expectedIntent = SigninActivity.createIntentForPromoAddAccountFlow(
-                mActivityTestRule.getActivity(), SigninAccessPoint.BOOKMARK_MANAGER);
-        // Comparing only the first intent as SigninActivity will fire an intent after
-        // starting the flow to add an account.
-        assertTrue(expectedIntent.filterEquals(startedIntents.get(0)));
+    private void openBookmarkManagerAndCheckSigninPromoIsDisplayed() {
+        openBookmarkManager();
+        onView(withId(R.id.signin_promo_view_container)).check(matches(isDisplayed()));
     }
 
     private void openBookmarkManager() {
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> BookmarkUtils.showBookmarkManager(mActivityTestRule.getActivity()));
+        BookmarkTestUtil.waitForBookmarkModelLoaded();
     }
 
     private void addTestAccount() {
@@ -177,26 +169,4 @@ public class BookmarkPersonalizedSigninPromoTest {
                 new ProfileDataSource.ProfileData(TEST_ACCOUNT_NAME, null, TEST_FULL_NAME, null);
         mAccountManagerTestRule.addAccount(accountHolder.build(), profileData);
     }
-
-    private static class IntentCallbackHelper implements IntentCallback, Closeable {
-        private final List<Intent> mStartedIntents = new ArrayList<>();
-
-        public IntentCallbackHelper() {
-            IntentMonitorRegistry.getInstance().addIntentCallback(this);
-        }
-
-        @Override
-        public void onIntentSent(Intent intent) {
-            mStartedIntents.add(intent);
-        }
-
-        @Override
-        public void close() {
-            IntentMonitorRegistry.getInstance().removeIntentCallback(this);
-        }
-
-        public List<Intent> getStartedIntents() {
-            return mStartedIntents;
-        }
-    }
 }