[Android][Signin] Break circular dependency between SigninUtils and SigninActivity

This CL breaks the circular dependency chain between SigninUtils and
SigninActivity by moving the method .launchActivityIfAllowed() from
SigninUtils to SigninActivityLauncherImpl.

Bug: 1163470
Change-Id: I99a98142d8a5b604b0ebf847449754f55952d03a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612858
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841980}

chrome/android/chrome_junit_test_java_sources.gni

@@ -207,10 +207,10 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/search_engines/settings/SearchEngineAdapterTest.java",
   "junit/src/org/chromium/chrome/browser/send_tab_to_self/SendTabToSelfShareActivityTest.java",
   "junit/src/org/chromium/chrome/browser/sharing/click_to_call/ClickToCallMessageHandlerTest.java",
+  "junit/src/org/chromium/chrome/browser/signin/SigninActivityLauncherImplTest.java",
   "junit/src/org/chromium/chrome/browser/signin/SigninManagerTest.java",
   "junit/src/org/chromium/chrome/browser/signin/SigninPromoUtilTest.java",
   "junit/src/org/chromium/chrome/browser/signin/SigninUtilsAccountPickerTest.java",
-  "junit/src/org/chromium/chrome/browser/signin/SigninUtilsStartActivityTest.java",
   "junit/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerDelegateTest.java",
   "junit/src/org/chromium/chrome/browser/status_indicator/StatusIndicatorMediatorTest.java",
   "junit/src/org/chromium/chrome/browser/suggestions/SuggestionsImageFetcherTest.java",

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

@@ -10,7 +10,11 @@ import android.os.Bundle;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
+import org.chromium.chrome.browser.signin.services.SigninManager;
 import org.chromium.chrome.browser.signin.ui.SigninActivityLauncher;
+import org.chromium.components.browser_ui.settings.ManagedPreferencesUtils;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
 
 /**
@@ -82,6 +86,26 @@ public class SigninActivityLauncherImpl implements SigninActivityLauncher {
         launchInternal(context, SigninFragment.createArguments(accessPoint));
     }
 
+    /**
+     * Launches the {@link SigninActivity} if signin is allowed.
+     * @param context A {@link Context} object.
+     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
+     * @return a boolean indicating if the {@link SigninActivity} is launched.
+     */
+    @Override
+    public boolean launchActivityIfAllowed(Context context, @SigninAccessPoint int accessPoint) {
+        SigninManager signinManager = IdentityServicesProvider.get().getSigninManager(
+                Profile.getLastUsedRegularProfile());
+        if (signinManager.isSignInAllowed()) {
+            launchActivity(context, accessPoint);
+            return true;
+        }
+        if (signinManager.isSigninDisabledByPolicy()) {
+            ManagedPreferencesUtils.showManagedByAdministratorToast(context);
+        }
+        return false;
+    }
+
     private void launchInternal(Context context, Bundle fragmentArgs) {
         Intent intent = SigninActivity.createIntent(context, fragmentArgs);
         context.startActivity(intent);

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

@@ -67,7 +67,8 @@ public class SigninPromoUtil {
             return false;
         }
 
-        SigninUtils.startSigninActivityIfAllowed(activity, SigninAccessPoint.SIGNIN_PROMO);
+        SigninActivityLauncherImpl.get().launchActivityIfAllowed(
+                activity, SigninAccessPoint.SIGNIN_PROMO);
         preferencesManager.setSigninPromoLastShownVersion(currentMajorVersion);
         preferencesManager.setSigninPromoLastAccountNames(accountNames);
         return true;
@@ -163,7 +164,7 @@ public class SigninPromoUtil {
     private static void openSigninActivityForPromo(WindowAndroid window, int accessPoint) {
         Activity activity = window.getActivity().get();
         if (activity != null) {
-            SigninUtils.startSigninActivityIfAllowed(activity, accessPoint);
+            SigninActivityLauncherImpl.get().launchActivityIfAllowed(activity, accessPoint);
         }
     }
 }

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

@@ -31,11 +31,9 @@ import org.chromium.chrome.browser.tabmodel.TabCreator;
 import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetControllerProvider;
-import org.chromium.components.browser_ui.settings.ManagedPreferencesUtils;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.GAIAServiceType;
 import org.chromium.components.signin.metrics.AccountConsistencyPromoAction;
-import org.chromium.components.signin.metrics.SigninAccessPoint;
 import org.chromium.ui.base.WindowAndroid;
 
 /**
@@ -122,23 +120,4 @@ public class SigninUtils {
                         new WebSigninBridge.Factory(), continueUrl),
                 regularTabModel, incognitoTabCreator, HelpAndFeedbackLauncherImpl.getInstance());
     }
-
-    /**
-     * Launches the {@link SigninActivity} if signin is allowed.
-     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
-     * @return a boolean indicating if the SigninActivity is launched.
-     */
-    public static boolean startSigninActivityIfAllowed(
-            Context context, @SigninAccessPoint int accessPoint) {
-        SigninManager signinManager = IdentityServicesProvider.get().getSigninManager(
-                Profile.getLastUsedRegularProfile());
-        if (signinManager.isSignInAllowed()) {
-            SigninActivityLauncherImpl.get().launchActivity(context, accessPoint);
-            return true;
-        }
-        if (signinManager.isSigninDisabledByPolicy()) {
-            ManagedPreferencesUtils.showManagedByAdministratorToast(context);
-        }
-        return false;
-    }
 }

chrome/android/java/src/org/chromium/chrome/browser/sync/settings/SignInPreference.java

@@ -18,7 +18,7 @@ import org.chromium.chrome.browser.firstrun.FirstRunSignInProcessor;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.chrome.browser.signin.SigninUtils;
+import org.chromium.chrome.browser.signin.SigninActivityLauncherImpl;
 import org.chromium.chrome.browser.signin.services.DisplayableProfileData;
 import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
 import org.chromium.chrome.browser.signin.services.ProfileDataCache;
@@ -193,7 +193,7 @@ public class SignInPreference
         setWidgetLayoutResource(0);
         setViewEnabled(true);
         setOnPreferenceClickListener(pref
-                -> SigninUtils.startSigninActivityIfAllowed(
+                -> SigninActivityLauncherImpl.get().launchActivityIfAllowed(
                         getContext(), SigninAccessPoint.SETTINGS));
 
         if (!mWasGenericSigninPromoDisplayed) {

chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninUtilsStartActivityTest.java → chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninActivityLauncherImplTest.java

-// Copyright 2019 The Chromium Authors. All rights reserved.
+// Copyright 2021 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 static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.notNull;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
@@ -29,14 +28,16 @@ import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
 import org.chromium.chrome.browser.signin.services.SigninManager;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
 
-/** Tests for the method startSigninActivityIfAllowed {@link SigninUtils}. */
+/**
+ * Tests {@link SigninActivityLauncherImpl}.
+ */
 @RunWith(BaseRobolectricTestRunner.class)
-public class SigninUtilsStartActivityTest {
+public class SigninActivityLauncherImplTest {
     @Mock
     private SigninManager mSigninManagerMock;
 
     @Mock
-    private SigninActivityLauncherImpl mLauncherMock;
+    private Context mContextMock;
 
     @Mock
     private Profile mProfile;
@@ -49,40 +50,34 @@ public class SigninUtilsStartActivityTest {
         IdentityServicesProvider.setInstanceForTests(mock(IdentityServicesProvider.class));
         Profile.setLastUsedProfileForTesting(mProfile);
         when(IdentityServicesProvider.get().getSigninManager(any())).thenReturn(mSigninManagerMock);
-        SigninActivityLauncherImpl.setLauncherForTest(mLauncherMock);
     }
 
     @Test
-    public void testSigninActivityLaunchedWhenSigninIsAllowed() {
+    public void testLaunchActivityIfAllowedWhenSigninIsAllowed() {
         when(mSigninManagerMock.isSignInAllowed()).thenReturn(true);
-        Assert.assertTrue(
-                SigninUtils.startSigninActivityIfAllowed(mContext, SigninAccessPoint.SETTINGS));
-        verify(SigninActivityLauncherImpl.get())
-                .launchActivity(mContext, SigninAccessPoint.SETTINGS);
+        Assert.assertTrue(SigninActivityLauncherImpl.get().launchActivityIfAllowed(
+                mContextMock, SigninAccessPoint.SETTINGS));
+        verify(mContextMock).startActivity(notNull());
     }
 
     @Test
-    public void testSigninActivityNotLaunchedWhenSigninIsNotAllowed() {
+    public void testLaunchActivityIfAllowedWhenSigninIsNotAllowed() {
         when(mSigninManagerMock.isSignInAllowed()).thenReturn(false);
         when(mSigninManagerMock.isSigninDisabledByPolicy()).thenReturn(false);
         Object toastBeforeCall = ShadowToast.getLatestToast();
-        Assert.assertFalse(
-                SigninUtils.startSigninActivityIfAllowed(mContext, SigninAccessPoint.SETTINGS));
+        Assert.assertFalse(SigninActivityLauncherImpl.get().launchActivityIfAllowed(
+                mContext, SigninAccessPoint.SETTINGS));
         Object toastAfterCall = ShadowToast.getLatestToast();
-        verify(SigninActivityLauncherImpl.get(), never())
-                .launchActivity(any(Context.class), anyInt());
         Assert.assertEquals(
                 "No new toast should be made during the call!", toastBeforeCall, toastAfterCall);
     }
 
     @Test
-    public void testToastShownWhenSigninIsDisabledByPolicy() {
+    public void testLaunchActivityIfAllowedWhenSigninIsDisabledByPolicy() {
         when(mSigninManagerMock.isSignInAllowed()).thenReturn(false);
         when(mSigninManagerMock.isSigninDisabledByPolicy()).thenReturn(true);
-        Assert.assertFalse(
-                SigninUtils.startSigninActivityIfAllowed(mContext, SigninAccessPoint.SETTINGS));
-        verify(SigninActivityLauncherImpl.get(), never())
-                .launchActivity(any(Context.class), anyInt());
+        Assert.assertFalse(SigninActivityLauncherImpl.get().launchActivityIfAllowed(
+                mContext, SigninAccessPoint.SETTINGS));
         Assert.assertTrue(ShadowToast.showedCustomToast(
                 mContext.getResources().getString(R.string.managed_by_your_organization),
                 R.id.toast_text));

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

@@ -51,4 +51,12 @@ public interface SigninActivityLauncher {
      * @param accessPoint {@link SigninAccessPoint} enum value representing.
      */
     void launchActivity(Context context, @SigninAccessPoint int accessPoint);
+
+    /**
+     * Launches the {@link SigninActivity} if signin is allowed.
+     * @param context A {@link Context} object.
+     * @param accessPoint {@link SigninAccessPoint} for starting sign-in flow.
+     * @return a boolean indicating if the {@link SigninActivity} is launched.
+     */
+    boolean launchActivityIfAllowed(Context context, @SigninAccessPoint int accessPoint);
 }