Close the current regular tab after the user clicks "Continue" in the

Incognito Interstitial.

This CL adds the logic to close the regular tab where the incognito
interstitial bottom sheet is displayed once the user clicks on
"Continue" button in the incognito interstitial.

Bug: 1125960
Change-Id: Iadab65f07a1177b08d13dd601c52d38f0e5c063f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398920
Commit-Queue: Rohit Agarwal <roagarwal@chromium.org>
Reviewed-by: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807862}

chrome/android/java/src/org/chromium/chrome/browser/incognito/interstitial/IncognitoInterstitialDelegate.java

@@ -12,8 +12,11 @@ import org.chromium.base.ThreadUtils;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.TabCreator;
+import org.chromium.chrome.browser.tabmodel.TabModel;
+import org.chromium.chrome.browser.tabmodel.TabModelUtils;
 
 /**
  * This class is responsible for the providing the functionality to the "Learn More" and "Continue"
@@ -23,14 +26,24 @@ public class IncognitoInterstitialDelegate {
     private final Activity mActivity;
     private final TabCreator mIncognitoTabCreator;
     private final HelpAndFeedback mHelpAndFeedback;
-    private final String mCurrentUrl;
+    private final TabModel mRegularTabModel;
 
-    public IncognitoInterstitialDelegate(Activity activity, TabCreator incognitoTabCreator,
-            HelpAndFeedback helpAndFeedback, String currentUrl) {
+    /**
+     * @param activity The {@link Activity} which would be used to show the HelpAndFeedback page.
+     * @param regularTabModel A regular {@link TabModel} via which we would close the
+     *         current regular tab in which the interstitial is shown after the user clicked on
+     *         "Continue" in the incognito interstitial.
+     * @param incognitoTabCreator An incognito {@link TabCreator} instance which would be used to
+     *         create incognito tab after the user clicks on "Continue" in the incognito
+     *         interstitial.
+     * @param helpAndFeedback A {@link HelpAndFeedback} instance through which we will load the
+     */
+    public IncognitoInterstitialDelegate(Activity activity, TabModel regularTabModel,
+            TabCreator incognitoTabCreator, HelpAndFeedback helpAndFeedback) {
         mActivity = activity;
+        mRegularTabModel = regularTabModel;
         mIncognitoTabCreator = incognitoTabCreator;
         mHelpAndFeedback = helpAndFeedback;
-        mCurrentUrl = currentUrl;
     }
 
     /**
@@ -45,12 +58,16 @@ public class IncognitoInterstitialDelegate {
     }
 
     /**
-     * Navigates to |mCurrentUrl| in a new incognito tab.
+     * Navigates to the URL currently shown in the regular tab, in a new incognito tab and closes
+     * the current regular tab.
      */
     @MainThread
     void openCurrentUrlInIncognitoTab() {
         // TODO(https://crbug.com/1120334): Add metrics to web sign-in flow.
         ThreadUtils.assertOnUiThread();
-        mIncognitoTabCreator.launchUrl(mCurrentUrl, TabLaunchType.FROM_CHROME_UI);
+        Tab currentRegularTab = TabModelUtils.getCurrentTab(mRegularTabModel);
+        mIncognitoTabCreator.launchUrl(
+                currentRegularTab.getUrlString(), TabLaunchType.FROM_CHROME_UI);
+        assert mRegularTabModel.closeTab(currentRegularTab);
     }
 }
\ No newline at end of file

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

@@ -18,11 +18,15 @@ import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.browser.app.ChromeActivity;
+import org.chromium.chrome.browser.help.HelpAndFeedback;
+import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
 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.sync.settings.AccountManagementFragment;
+import org.chromium.chrome.browser.tabmodel.TabCreator;
+import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetControllerProvider;
 import org.chromium.components.browser_ui.settings.ManagedPreferencesUtils;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
@@ -91,11 +95,22 @@ public class SigninUtils {
             return;
         }
         ChromeActivity activity = (ChromeActivity) windowAndroid.getActivity().get();
+
+        // To close the current regular tab after the user clicks on "Continue" in the incognito
+        // interstitial.
+        TabModel regularTabModel = activity.getTabModelSelector().getModel(/*incognito=*/false);
+        // To create a new incognito tab after after the user clicks on "Continue" in the incognito
+        // interstitial.
+        TabCreator incognitoTabCreator = activity.getTabCreator(/*incognito=*/true);
+        IncognitoInterstitialDelegate incognitoInterstitialDelegate =
+                new IncognitoInterstitialDelegate(activity, regularTabModel, incognitoTabCreator,
+                        HelpAndFeedback.getInstance());
+
         AccountPickerBottomSheetCoordinator coordinator = new AccountPickerBottomSheetCoordinator(
                 windowAndroid.getContext().get(), BottomSheetControllerProvider.from(windowAndroid),
                 new AccountPickerDelegate(windowAndroid, activity.getActivityTab(),
-                        activity.getTabCreator(/*incognito=*/true), new WebSigninBridge.Factory(),
-                        continueUrl));
+                        new WebSigninBridge.Factory(), continueUrl),
+                incognitoInterstitialDelegate);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerBottomSheetCoordinator.java

@@ -11,6 +11,7 @@ import androidx.annotation.MainThread;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialCoordinator;
+import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerCoordinator.AccountPickerAccessPoint;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
@@ -42,15 +43,16 @@ public class AccountPickerBottomSheetCoordinator {
     @MainThread
     public AccountPickerBottomSheetCoordinator(Context context,
             BottomSheetController bottomSheetController,
-            AccountPickerDelegate accountPickerDelegate) {
+            AccountPickerDelegate accountPickerDelegate,
+            IncognitoInterstitialDelegate incognitoInterstitialDelegate) {
         mAccountPickerBottomSheetMediator =
                 new AccountPickerBottomSheetMediator(context, accountPickerDelegate);
         mView = new AccountPickerBottomSheetView(context, mAccountPickerBottomSheetMediator);
         mAccountPickerCoordinator = new AccountPickerCoordinator(mView.getAccountListView(),
                 mAccountPickerBottomSheetMediator, null, AccountPickerAccessPoint.WEB);
         IncognitoInterstitialCoordinator incognitoInterstitialCoordinator =
-                new IncognitoInterstitialCoordinator(mView.getIncognitoInterstitialView(),
-                        accountPickerDelegate.getIncognitoInterstitialDelegate());
+                new IncognitoInterstitialCoordinator(
+                        mView.getIncognitoInterstitialView(), incognitoInterstitialDelegate);
         mBottomSheetController = bottomSheetController;
         PropertyModelChangeProcessor.create(mAccountPickerBottomSheetMediator.getModel(), mView,
                 AccountPickerBottomSheetViewBinder::bind);

chrome/android/java/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerDelegate.java

@@ -14,15 +14,12 @@ 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.help.HelpAndFeedback;
-import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
 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.chrome.browser.tabmodel.TabCreator;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.AccountUtils;
 import org.chromium.components.signin.base.CoreAccountInfo;
@@ -40,37 +37,32 @@ public class AccountPickerDelegate implements WebSigninBridge.Listener {
     private final WindowAndroid mWindowAndroid;
     private final Activity mActivity;
     private final Tab mCurrentTab;
-    private final TabCreator mIncognitoTabCreator;
     private final WebSigninBridge.Factory mWebSigninBridgeFactory;
     private final String mContinueUrl;
     private final SigninManager mSigninManager;
     private @Nullable WebSigninBridge mWebSigninBridge;
     private Callback<GoogleServiceAuthError> mOnSignInErrorCallback;
 
+    /**
+     * @param windowAndroid The {@link WindowAndroid} instance of the {@link ChromeActivity}.
+     * @param currentTab The {@Tab} instance where the account picker dialog is displayed.
+     * @param webSigninBridgeFactory A {@link WebSigninBridge.Factory} to create {@link
+     *         WebSigninBridge} instances.
+     * @param continueUrl A string representing the URL the user would be redirected to after
+     *         sign-in succeeds.
+     */
     public AccountPickerDelegate(WindowAndroid windowAndroid, Tab currentTab,
-            TabCreator incognitoTabCreator, WebSigninBridge.Factory webSigninBridgeFactory,
-            String continueUrl) {
+            WebSigninBridge.Factory webSigninBridgeFactory, String continueUrl) {
         mWindowAndroid = windowAndroid;
         mActivity = mWindowAndroid.getActivity().get();
         assert mActivity != null : "Activity should not be null!";
         mCurrentTab = currentTab;
-        mIncognitoTabCreator = incognitoTabCreator;
         mWebSigninBridgeFactory = webSigninBridgeFactory;
         mContinueUrl = continueUrl;
         mSigninManager = IdentityServicesProvider.get().getSigninManager(
                 Profile.getLastUsedRegularProfile());
     }
 
-    /**
-     * Creates and return the {@link IncognitoInterstitialDelegate}.
-     */
-    public IncognitoInterstitialDelegate getIncognitoInterstitialDelegate() {
-        IncognitoInterstitialDelegate incognitoInterstitialDelegate =
-                new IncognitoInterstitialDelegate(mActivity, mIncognitoTabCreator,
-                        HelpAndFeedback.getInstance(), mCurrentTab.getUrlString());
-        return incognitoInterstitialDelegate;
-    }
-
     /**
      * Releases resources used by this class.
      */

chrome/android/javatests/src/org/chromium/chrome/browser/signin/AccountPickerBottomSheetRenderTest.java

@@ -38,6 +38,7 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.app.ChromeActivity;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
+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;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerDelegate;
@@ -91,6 +92,9 @@ public class AccountPickerBottomSheetRenderTest {
     @Mock
     private AccountPickerDelegate mAccountPickerDelegateMock;
 
+    @Mock
+    private IncognitoInterstitialDelegate mIncognitoInterstitialDelegateMock;
+
     private AccountPickerBottomSheetCoordinator mCoordinator;
 
     @ParameterAnnotations.UseMethodParameterBefore(NightModeTestUtils.NightModeParams.class)
@@ -293,7 +297,8 @@ public class AccountPickerBottomSheetRenderTest {
     private void buildAndShowCollapsedBottomSheet() {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             mCoordinator = new AccountPickerBottomSheetCoordinator(mActivityTestRule.getActivity(),
-                    getBottomSheetController(), mAccountPickerDelegateMock);
+                    getBottomSheetController(), mAccountPickerDelegateMock,
+                    mIncognitoInterstitialDelegateMock);
         });
         CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById(
                 R.id.account_picker_continue_as_button)::isShown);

chrome/android/javatests/src/org/chromium/chrome/browser/signin/AccountPickerBottomSheetTest.java

@@ -49,6 +49,7 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.app.ChromeActivity;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
+import org.chromium.chrome.browser.incognito.interstitial.IncognitoInterstitialDelegate;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerBottomSheetCoordinator;
 import org.chromium.chrome.browser.signin.account_picker.AccountPickerDelegate;
 import org.chromium.chrome.test.ChromeActivityTestRule;
@@ -114,6 +115,9 @@ public class AccountPickerBottomSheetTest {
     @Mock
     private AccountPickerDelegate mAccountPickerDelegateMock;
 
+    @Mock
+    private IncognitoInterstitialDelegate mIncognitoInterstitialDelegateMock;
+
     @Captor
     public ArgumentCaptor<Callback<String>> callbackArgumentCaptor;
 
@@ -528,7 +532,8 @@ public class AccountPickerBottomSheetTest {
     private void buildAndShowCollapsedBottomSheet() {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             mCoordinator = new AccountPickerBottomSheetCoordinator(sActivityTestRule.getActivity(),
-                    getBottomSheetController(), mAccountPickerDelegateMock);
+                    getBottomSheetController(), mAccountPickerDelegateMock,
+                    mIncognitoInterstitialDelegateMock);
         });
         CriteriaHelper.pollUiThread(mCoordinator.getBottomSheetViewForTesting().findViewById(
                 R.id.account_picker_continue_as_button)::isShown);

chrome/android/junit/src/org/chromium/chrome/browser/incognito/interstitial/IncognitoInterstitialDelegateTest.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.incognito.interstitial;
 
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -22,8 +23,10 @@ import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.TabCreator;
+import org.chromium.chrome.browser.tabmodel.TabModel;
 
 /**
  * Roboelectric tests class for the incognito interstitial.
@@ -31,7 +34,7 @@ import org.chromium.chrome.browser.tabmodel.TabCreator;
 @RunWith(BaseRobolectricTestRunner.class)
 public class IncognitoInterstitialDelegateTest {
     private static final String sIncognitoLearnMoreText = "dummy_chrome_incognito";
-    private static final String sContinueUrlPage = "dummy_url_string.com";
+    private static final String sCurrentUrlPage = "dummy_url_string.com";
 
     @Mock
     private HelpAndFeedback mHelpAndFeedbackMock;
@@ -45,15 +48,24 @@ public class IncognitoInterstitialDelegateTest {
     @Mock
     private TabCreator mIncognitoTabCreatorMock;
 
+    @Mock
+    private TabModel mRegularTabModelMock;
+
+    @Mock
+    private Tab mTabMock;
+
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
         when(mActivityMock.getString(R.string.help_context_incognito_learn_more))
                 .thenReturn(sIncognitoLearnMoreText);
+        when(mRegularTabModelMock.getTabAt(anyInt())).thenReturn(mTabMock);
+        when(mRegularTabModelMock.closeTab(mTabMock)).thenReturn(true);
+        when(mTabMock.getUrlString()).thenReturn(sCurrentUrlPage);
 
         Profile.setLastUsedProfileForTesting(mProfileMock);
-        mIncognitoInterstitialDelegate = new IncognitoInterstitialDelegate(
-                mActivityMock, mIncognitoTabCreatorMock, mHelpAndFeedbackMock, sContinueUrlPage);
+        mIncognitoInterstitialDelegate = new IncognitoInterstitialDelegate(mActivityMock,
+                mRegularTabModelMock, mIncognitoTabCreatorMock, mHelpAndFeedbackMock);
     }
 
     @After
@@ -76,6 +88,7 @@ public class IncognitoInterstitialDelegateTest {
     @MediumTest
     public void testOpenCurrentUrlInIncognitoTab() {
         mIncognitoInterstitialDelegate.openCurrentUrlInIncognitoTab();
-        verify(mIncognitoTabCreatorMock).launchUrl(sContinueUrlPage, TabLaunchType.FROM_CHROME_UI);
+        verify(mIncognitoTabCreatorMock).launchUrl(sCurrentUrlPage, TabLaunchType.FROM_CHROME_UI);
+        verify(mRegularTabModelMock).closeTab(mTabMock);
     }
 }

chrome/android/junit/src/org/chromium/chrome/browser/incognito/interstitial/OWNERS

+file://chrome/android/java/src/org/chromium/chrome/browser/signin/OWNERS
+roagarwal@chromium.org
+
+# TEAM: chrome-privacy-core@google.com
+# COMPONENT: Privacy>Incognito

chrome/android/junit/src/org/chromium/chrome/browser/signin/account_picker/AccountPickerDelegateTest.java

@@ -36,7 +36,6 @@ import org.chromium.chrome.browser.signin.IdentityServicesProvider;
 import org.chromium.chrome.browser.signin.SigninManager;
 import org.chromium.chrome.browser.signin.WebSigninBridge;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.tabmodel.TabCreator;
 import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
 import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.base.GoogleServiceAuthError;
@@ -75,9 +74,6 @@ public class AccountPickerDelegateTest {
     @Mock
     private Tab mTabMock;
 
-    @Mock
-    private TabCreator mTabCreatorMock;
-
     @Captor
     private ArgumentCaptor<LoadUrlParams> mLoadUrlParamsCaptor;
 
@@ -99,8 +95,8 @@ public class AccountPickerDelegateTest {
         when(IdentityServicesProvider.get().getIdentityManager(any())).thenReturn(mIdentityManager);
         when(IdentityServicesProvider.get().getSigninManager(any())).thenReturn(mSigninManagerMock);
 
-        mDelegate = new AccountPickerDelegate(mWindowAndroidMock, mTabMock, mTabCreatorMock,
-                mWebSigninBridgeFactoryMock, CONTINUE_URL);
+        mDelegate = new AccountPickerDelegate(
+                mWindowAndroidMock, mTabMock, mWebSigninBridgeFactoryMock, CONTINUE_URL);
         when(mWebSigninBridgeFactoryMock.create(eq(mProfileMock), any(), eq(mDelegate)))
                 .thenReturn(mWebSigninBridgeMock);
     }