Allow tab closing in SingleTabModel and fix more test for touchless

When we get asked to close a tab, actually close the tab before
finishing the Activity for tests that test that a tab actually closes
(and proactively defend against tabs that were closed sticking around
after relaunching).

Bug: 983743
Change-Id: I61771961edd8524b851bffc741a9b37e28e524e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758629
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688294}

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/SingleTabModel.java

@@ -39,20 +39,25 @@ public class SingleTabModel implements TabModel {
     void setTab(Tab tab) {
         Tab oldTab = mTab;
         mTab = tab;
-        assert mTab.isIncognito() == mIsIncognito;
-        if (mBlockNewWindows) nativePermanentlyBlockAllNewWindows(mTab);
+        if (tab != null) {
+            assert mTab.isIncognito() == mIsIncognito;
+            if (mBlockNewWindows) nativePermanentlyBlockAllNewWindows(mTab);
 
-        for (TabModelObserver observer : mObservers) {
-            observer.didAddTab(tab, TabLaunchType.FROM_LINK);
-            observer.didSelectTab(tab, TabSelectionType.FROM_USER, Tab.INVALID_TAB_ID);
-        }
+            for (TabModelObserver observer : mObservers) {
+                observer.didAddTab(tab, TabLaunchType.FROM_LINK);
+                observer.didSelectTab(tab, TabSelectionType.FROM_USER, Tab.INVALID_TAB_ID);
+            }
 
-        int state = ApplicationStatus.getStateForActivity(mActivity);
-        if (state == ActivityState.CREATED || state == ActivityState.STARTED
-                || state == ActivityState.RESUMED) {
-            mTab.show(TabSelectionType.FROM_USER);
+            int state = ApplicationStatus.getStateForActivity(mActivity);
+            if (state == ActivityState.CREATED || state == ActivityState.STARTED
+                    || state == ActivityState.RESUMED) {
+                mTab.show(TabSelectionType.FROM_USER);
+            }
         }
         if (oldTab != null && oldTab.isInitialized()) {
+            for (TabModelObserver observer : mObservers) {
+                observer.willCloseTab(oldTab, false);
+            }
             for (TabModelObserver observer : mObservers) {
                 observer.didCloseTab(oldTab.getId(), oldTab.isIncognito());
             }
@@ -93,11 +98,9 @@ public class SingleTabModel implements TabModel {
 
     @Override
     public boolean closeTab(Tab tab, boolean animate, boolean uponExit, boolean canUndo) {
-        if (mTab != null && mTab.getId() == tab.getId()) {
-            completeActivity();
-            return true;
-        }
-        return false;
+        if (mTab == null || mTab.getId() != tab.getId()) return false;
+        closeTabAndFinish();
+        return true;
     }
 
     @Override
@@ -111,7 +114,8 @@ public class SingleTabModel implements TabModel {
      * finishes and removes from recents. We use mBlockNewWindows flag to distinguish the user
      * of this model.
      */
-    private void completeActivity() {
+    private void closeTabAndFinish() {
+        setTab(null);
         if (mBlockNewWindows) {
             mActivity.finish();
         } else {
@@ -121,7 +125,13 @@ public class SingleTabModel implements TabModel {
 
     @Override
     public void closeMultipleTabs(List<Tab> tabs, boolean canUndo) {
-        completeActivity();
+        if (mTab == null) return;
+        for (Tab tab : tabs) {
+            if (tab.getId() == mTab.getId()) {
+                closeTabAndFinish();
+                return;
+            }
+        }
     }
 
     @Override
@@ -131,7 +141,7 @@ public class SingleTabModel implements TabModel {
 
     @Override
     public void closeAllTabs(boolean allowDelegation, boolean uponExit) {
-        completeActivity();
+        closeTabAndFinish();
     }
 
     // Tab retrieval functions.

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/SingleTabModelSelector.java

@@ -16,6 +16,19 @@ public class SingleTabModelSelector extends TabModelSelectorBase {
             boolean incognito, boolean blockNewWindows) {
         super(tabCreatorManager);
         initialize(incognito, new SingleTabModel(activity, incognito, blockNewWindows));
+
+        TabModelObserver tabModelObserver = new EmptyTabModelObserver() {
+            @Override
+            public void didCloseTab(int tabId, boolean incognito) {
+                // TabModelSelectorImpl handles the equivalent case of closing the last tab in
+                // TabModelSelectorImpl#requestToShowTab, which we don't have for this
+                // TabModelSelector, so we do it here instead.
+                if (getCurrentModel().getTabAt(0) == null) notifyChanged();
+            }
+        };
+        for (TabModel model : getModels()) {
+            model.addObserver(tabModelObserver);
+        }
     }
 
     public void setTab(Tab tab) {

chrome/android/javatests/src/org/chromium/chrome/browser/JavaScriptEvalChromeTest.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser;
 
+import android.content.Intent;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.LargeTest;
 
@@ -17,9 +18,12 @@ import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.base.test.util.UrlUtils;
+import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule;
+import org.chromium.chrome.browser.customtabs.CustomTabsTestUtils;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.util.FeatureUtilities;
+import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
-import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.ChromeTabUtils;
 import org.chromium.content_public.browser.test.util.JavaScriptUtils;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
@@ -33,7 +37,11 @@ import java.util.concurrent.TimeoutException;
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
 public class JavaScriptEvalChromeTest {
     @Rule
-    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+    public ChromeActivityTestRule<? extends ChromeActivity> mActivityTestRule =
+            ChromeActivityTestRule.forMainActivity();
+
+    @Rule
+    public CustomTabActivityTestRule mCustomTabActivityTestRule = new CustomTabActivityTestRule();
 
     private static final String JSTEST_URL = UrlUtils.encodeHtmlDataUri(
             "<html><head><script>"
@@ -59,12 +67,28 @@ public class JavaScriptEvalChromeTest {
     public void testJavaScriptEvalIsCorrectlyOrderedWithinOneTab()
             throws InterruptedException, TimeoutException {
         Tab tab1 = mActivityTestRule.getActivity().getActivityTab();
-        ChromeTabUtils.newTabFromMenu(
-                InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
-        Tab tab2 = mActivityTestRule.getActivity().getActivityTab();
-        mActivityTestRule.loadUrl(JSTEST_URL);
+        Tab tab2;
+        if (mActivityTestRule.getActivity() instanceof ChromeTabbedActivity) {
+            ChromeTabUtils.newTabFromMenu(
+                    InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
+            tab2 = mActivityTestRule.getActivity().getActivityTab();
+            mActivityTestRule.loadUrl(JSTEST_URL);
+            ChromeTabUtils.switchTabInCurrentTabModel(
+                    mActivityTestRule.getActivity(), tab1.getId());
+        } else {
+            // For now, only NoTouchMode should hit this path.
+            // In NoTouchMode, multiple tabs are only supported though CCT, so use a CCT instead of
+            // a second tab.
+            Assert.assertTrue(FeatureUtilities.isNoTouchModeEnabled());
+            Intent intent = CustomTabsTestUtils.createMinimalCustomTabIntent(
+                    InstrumentationRegistry.getTargetContext(), "about:blank");
+            // NoTouchMode only allows CCT for 1p use-cases.
+            IntentHandler.addTrustedIntentExtras(intent);
+            mCustomTabActivityTestRule.startCustomTabActivityWithIntent(intent);
+            tab2 = mCustomTabActivityTestRule.getActivity().getActivityTab();
+            mCustomTabActivityTestRule.loadUrl(JSTEST_URL);
+        }
 
-        ChromeTabUtils.switchTabInCurrentTabModel(mActivityTestRule.getActivity(), tab1.getId());
         Assert.assertFalse("Tab didn't open", tab1 == tab2);
 
         JavaScriptUtils.executeJavaScriptAndWaitForResult(

chrome/android/javatests/src/org/chromium/chrome/browser/directactions/CloseTabDirectActionHandlerTest.java

@@ -21,11 +21,14 @@ import org.junit.runner.RunWith;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.MinAndroidSdkLevel;
+import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tabmodel.SingleTabModelSelector;
+import org.chromium.chrome.browser.tabmodel.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
+import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
-import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.util.ArrayList;
@@ -37,7 +40,8 @@ import java.util.List;
 @MinAndroidSdkLevel(Build.VERSION_CODES.N)
 public class CloseTabDirectActionHandlerTest {
     @Rule
-    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+    public ChromeActivityTestRule<? extends ChromeActivity> mActivityTestRule =
+            ChromeActivityTestRule.forMainActivity();
 
     private TabModelSelector mSelector;
     private CloseTabDirectActionHandler mHandler;
@@ -46,7 +50,8 @@ public class CloseTabDirectActionHandlerTest {
     public void setUp() throws Exception {
         // Setup an activity with two blank tabs.
         mActivityTestRule.startMainActivityOnBlankPage();
-        mActivityTestRule.loadUrlInNewTab("about:blank");
+        mActivityTestRule.loadUrlInNewTab(
+                "about:blank", false /* incognito */, TabLaunchType.FROM_CHROME_UI);
 
         mSelector = mActivityTestRule.getActivity().getTabModelSelector();
         mHandler = new CloseTabDirectActionHandler(mSelector);
@@ -62,12 +67,16 @@ public class CloseTabDirectActionHandlerTest {
         // Close current tab
         Tab initiallyCurrent = mSelector.getCurrentTab();
         performAction("close_tab");
-        assertEquals(1, mSelector.getTotalTabCount());
         assertThat(
                 mSelector.getCurrentTab(), Matchers.not(Matchers.sameInstance(initiallyCurrent)));
 
-        // Close last tab
-        performAction("close_tab");
+        if (!(mSelector instanceof SingleTabModelSelector)) {
+            assertEquals(1, mSelector.getTotalTabCount());
+            // Close last tab
+            performAction("close_tab");
+        } else {
+            assertEquals(0, mSelector.getTotalTabCount());
+        }
 
         // No tabs are left, so actions aren't available anymore.
         assertThat(getDirectActions(), Matchers.empty());

chrome/android/javatests/src/org/chromium/chrome/browser/login/ChromeHttpAuthHandlerTest.java

@@ -4,10 +4,12 @@
 
 package org.chromium.chrome.browser.login;
 
+import android.content.Intent;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -17,11 +19,18 @@ import org.chromium.base.Callback;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Restriction;
+import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.ChromeTabbedActivity;
+import org.chromium.chrome.browser.IntentHandler;
+import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule;
+import org.chromium.chrome.browser.customtabs.CustomTabsTestUtils;
 import org.chromium.chrome.browser.tab.SadTab;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tabmodel.TabLaunchType;
+import org.chromium.chrome.browser.util.FeatureUtilities;
+import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
-import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.ChromeTabUtils;
 import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
@@ -38,7 +47,12 @@ import java.util.concurrent.atomic.AtomicReference;
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
 public class ChromeHttpAuthHandlerTest {
     @Rule
-    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+    public ChromeActivityTestRule<? extends ChromeActivity> mActivityTestRule =
+            ChromeActivityTestRule.forMainActivity();
+
+    @Rule
+    public CustomTabActivityTestRule mCustomTabActivityTestRule = new CustomTabActivityTestRule();
+
     private EmbeddedTestServer mTestServer;
 
     @Before
@@ -74,8 +88,10 @@ public class ChromeHttpAuthHandlerTest {
     public void authDialogDismissOnTabSwitched() throws Exception {
         ChromeHttpAuthHandler handler = triggerAuth();
         verifyAuthDialogVisibility(handler, true);
-        ChromeTabUtils.newTabFromMenu(
-                InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
+        TestThreadUtils.runOnUiThreadBlocking(
+                ()
+                        -> mActivityTestRule.getActivity().getTabCreator(false).launchUrl(
+                                "about:blank", TabLaunchType.FROM_CHROME_UI));
         verifyAuthDialogVisibility(handler, false);
     }
 
@@ -94,8 +110,20 @@ public class ChromeHttpAuthHandlerTest {
     @Restriction(Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE)
     public void authDialogSuppressedOnBackgroundTab() throws Exception {
         Tab firstTab = mActivityTestRule.getActivity().getActivityTab();
-        ChromeTabUtils.newTabFromMenu(
-                InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
+        if (mActivityTestRule.getActivity() instanceof ChromeTabbedActivity) {
+            ChromeTabUtils.newTabFromMenu(
+                    InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
+        } else {
+            // For now, only NoTouchMode should hit this path.
+            // In NoTouchMode, multiple tabs are only supported though CCT, so use a CCT instead of
+            // a second tab.
+            Assert.assertTrue(FeatureUtilities.isNoTouchModeEnabled());
+            Intent intent = CustomTabsTestUtils.createMinimalCustomTabIntent(
+                    InstrumentationRegistry.getTargetContext(), "about:blank");
+            // NoTouchMode only allows CCT for 1p use-cases.
+            IntentHandler.addTrustedIntentExtras(intent);
+            mCustomTabActivityTestRule.startCustomTabActivityWithIntent(intent);
+        }
         // If the first tab was closed due to OOM, then just exit the test.
         if (TestThreadUtils.runOnUiThreadBlocking(
                     () -> firstTab.isClosing() || SadTab.isShowing(firstTab))) {

chrome/test/android/javatests/src/org/chromium/chrome/test/util/ChromeTabUtils.java

@@ -370,8 +370,7 @@ public class ChromeTabUtils {
      * Returns when the tab has been created and has finished navigating.
      */
     public static void newTabFromMenu(Instrumentation instrumentation,
-            final ChromeTabbedActivity activity)
-            throws InterruptedException {
+            final ChromeActivity activity) throws InterruptedException {
         newTabFromMenu(instrumentation, activity, false, true);
     }
 
@@ -381,7 +380,7 @@ public class ChromeTabUtils {
      * Returns when the tab has been created and has finished navigating.
      */
     public static void newTabFromMenu(Instrumentation instrumentation,
-            final ChromeTabbedActivity activity, boolean incognito, boolean waitForNtpLoad)
+            final ChromeActivity activity, boolean incognito, boolean waitForNtpLoad)
             throws InterruptedException {
         final CallbackHelper createdCallback = new CallbackHelper();
         final CallbackHelper selectedCallback = new CallbackHelper();
@@ -488,8 +487,7 @@ public class ChromeTabUtils {
      * Returns after the tab has been closed.
      */
     public static void closeCurrentTab(final Instrumentation instrumentation,
-            final ChromeTabbedActivity activity)
-            throws InterruptedException {
+            final ChromeActivity activity) throws InterruptedException {
         closeTabWithAction(instrumentation, activity, new Runnable() {
             @Override
             public void run() {
@@ -507,7 +505,7 @@ public class ChromeTabUtils {
      * Closes a tab with the given action and waits for a tab closure to be observed.
      */
     public static void closeTabWithAction(Instrumentation instrumentation,
-            final ChromeTabbedActivity activity, Runnable action) throws InterruptedException {
+            final ChromeActivity activity, Runnable action) throws InterruptedException {
         final CallbackHelper closeCallback = new CallbackHelper();
         final TabModelObserver observer = new EmptyTabModelObserver() {
             @Override