Revert "[Start] Don't auto delete a tab selected from the tab switcher."

This reverts commit 387260f4.

Reason for revert: StartSurfaceTest#testShow_SingleAsHomepage_BackButton__Instant_Return is failing on builder "test-o-phone", see https://crbug.com/1163874

Original change's description:
> [Start] Don't auto delete a tab selected from the tab switcher.
>
> In this CL, we introduce a new flag "keepTab" for a Tab with launch type
> FROM_START_SURFACE. When this kind of Tab is selected from the carousel
> Tab switcher on the Start surface, or from grid tab switcher, the
> "keepTab" is set to be true. This guarantees the tab won't be auto
> deleted from the TabModel when the back button is tapped.
>
> More details see doc:
> https://docs.google.com/document/d/1mZXgQ0fi_b96vcwy1DkgWunl0FutkFwyt-7PrU-Pgjc/edit#
>
> Bug: 1155139
> Change-Id: Ib1e2ce917b0396bbd147ada2ebfc470cffd4718a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585822
> Commit-Queue: Xi Han <hanxi@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840737}

TBR=dtrainor@chromium.org,hanxi@chromium.org,wychen@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Id3a59cff24aa7568b8d6866877731e8979ffeaae
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1155139, 1163874
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612897Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841017}

chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/StartSurfaceMediator.java

@@ -637,12 +637,6 @@ class StartSurfaceMediator
             }
         }
 
-        // Pressing back button on the Start surface homepage is handled by
-        // ChromeTabbedActivity#handleBackPressed().
-        if (mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE && !mShowStackTabSwitcher) {
-            return false;
-        }
-
         if (mPropertyModel != null && mPropertyModel.get(IS_EXPLORE_SURFACE_VISIBLE)
                 && mStartSurfaceState == StartSurfaceState.SHOWN_TABSWITCHER_TWO_PANES) {
             setExploreSurfaceVisibility(false);

chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceTest.java

@@ -50,7 +50,6 @@ import android.support.test.uiautomator.UiDevice;
 import android.view.KeyEvent;
 import android.view.View;
 import android.view.ViewGroup;
-import android.widget.LinearLayout;
 
 import androidx.recyclerview.widget.RecyclerView;
 import androidx.test.espresso.UiController;
@@ -99,7 +98,6 @@ import org.chromium.chrome.browser.flags.CachedFeatureFlags;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
-import org.chromium.chrome.browser.tab.TabLaunchType;
 import org.chromium.chrome.browser.tasks.ReturnToChromeExperimentsUtil;
 import org.chromium.chrome.browser.tasks.SingleTabSwitcherMediator;
 import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache;
@@ -752,26 +750,15 @@ public class StartSurfaceTest {
                                        .getLayoutManager()
                                        .overviewVisible());
             waitForTabModel();
-            if (isInstantReturn()) {
-                // TODO(crbug.com/1076274): fix toolbar to avoid wrongly focusing on the toolbar
-                // omnibox.
-                return;
-            }
-            // Single surface is shown as homepage. Clicks "more_tabs" button to get into tab
-            // switcher.
-            try {
-                TestThreadUtils.runOnUiThreadBlocking(
-                        ()
-                                -> mActivityTestRule.getActivity()
-                                           .findViewById(org.chromium.chrome.tab_ui.R.id.more_tabs)
-                                           .performClick());
-            } catch (ExecutionException e) {
-                fail("Failed to tap 'more tabs' " + e.toString());
-            }
-        } else {
-            TabUiTestHelper.enterTabSwitcher(mActivityTestRule.getActivity());
+            // Single surface is shown as homepage. Exit in order to get into tab switcher later.
+            pressBack();
         }
-
+        if (isInstantReturn()) {
+            // TODO(crbug.com/1076274): fix toolbar to avoid wrongly focusing on the toolbar
+            // omnibox.
+            return;
+        }
+        TabUiTestHelper.enterTabSwitcher(mActivityTestRule.getActivity());
         onViewWaiting(allOf(withId(R.id.secondary_tasks_surface_view), isDisplayed()));
 
         OverviewModeBehaviorWatcher hideWatcher =
@@ -1557,132 +1544,6 @@ public class StartSurfaceTest {
         assertEquals(taskSurfaceHeader.getBottom(), taskSurfaceHeader.getHeight());
     }
 
-    @Test
-    @MediumTest
-    @Feature({"StartSurface"})
-    // clang-format off
-    @CommandLineFlags.Add({BASE_PARAMS + "/single"})
-    public void testShow_SingleAsHomepage_BackButton() throws ExecutionException {
-        // clang-format on
-        if (!mImmediateReturn) {
-            onView(withId(org.chromium.chrome.tab_ui.R.id.home_button)).perform(click());
-        }
-
-        ChromeTabbedActivity cta = mActivityTestRule.getActivity();
-        CriteriaHelper.pollUiThread(
-                () -> cta.getLayoutManager() != null && cta.getLayoutManager().overviewVisible());
-        waitForTabModel();
-        TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
-
-        // Case 1:
-        // Launches the first site in mv tiles, and press back button.
-        LinearLayout tilesLayout =
-                cta.findViewById(org.chromium.chrome.tab_ui.R.id.mv_tiles_layout);
-        onView(allOf(withId(org.chromium.chrome.tab_ui.R.id.mv_tiles_container), isDisplayed()));
-        TestThreadUtils.runOnUiThreadBlocking(() -> tilesLayout.getChildAt(0).performClick());
-        CriteriaHelper.pollUiThread(() -> !cta.getLayoutManager().overviewVisible());
-        // Verifies a new Tab is created.
-        TabUiTestHelper.verifyTabModelTabCount(cta, 2, 0);
-        pressBack();
-
-        if (isInstantReturn()
-                && (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP
-                        && Build.VERSION.SDK_INT < Build.VERSION_CODES.O)) {
-            // Fix the issue that failed to perform a single click on the back button.
-            return;
-        }
-
-        CriteriaHelper.pollUiThread(() -> cta.getLayoutManager().overviewVisible());
-        // Verifies the new Tab is deleted.
-        TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
-
-        // Case 2:
-        // Launches the first site in mv tiles, and press home button to return to the Start
-        // surface.
-        onView(allOf(withId(org.chromium.chrome.tab_ui.R.id.mv_tiles_container), isDisplayed()));
-        TestThreadUtils.runOnUiThreadBlocking(() -> tilesLayout.getChildAt(0).performClick());
-        CriteriaHelper.pollUiThread(() -> !cta.getLayoutManager().overviewVisible());
-        onViewWaiting(allOf(withId(org.chromium.chrome.R.id.home_button), isDisplayed()));
-        TestThreadUtils.runOnUiThreadBlocking(
-                () -> { cta.findViewById(org.chromium.chrome.R.id.home_button).performClick(); });
-        onViewWaiting(withId(R.id.primary_tasks_surface_view));
-        onView(allOf(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view), isDisplayed()));
-        // Verifies a new Tab is created, and can be seen in the Start surface.
-        TabUiTestHelper.verifyTabModelTabCount(cta, 2, 0);
-
-        // Launches the new tab from the carousel tab switcher, and press back button.
-        onView(allOf(withParent(withId(
-                             org.chromium.chrome.tab_ui.R.id.carousel_tab_switcher_container)),
-                       withId(org.chromium.chrome.tab_ui.R.id.tab_list_view)))
-                .perform(RecyclerViewActions.actionOnItemAtPosition(1, click()));
-        Assert.assertEquals(TabLaunchType.FROM_START_SURFACE,
-                cta.getTabModelSelector().getCurrentTab().getLaunchType());
-        CriteriaHelper.pollUiThread(() -> !cta.getLayoutManager().overviewVisible());
-        pressBack();
-        onViewWaiting(withId(R.id.primary_tasks_surface_view));
-        // Verifies the tab isn't auto deleted from the TabModel.
-        TabUiTestHelper.verifyTabModelTabCount(cta, 2, 0);
-    }
-
-    @Test
-    @MediumTest
-    @Feature({"StartSurface"})
-    // clang-format off
-    @CommandLineFlags.Add({BASE_PARAMS + "/single"})
-    public void testShow_SingleAsHomepage_BackButtonWithTabSwitcher() throws ExecutionException {
-        // clang-format on
-        if (!mImmediateReturn) {
-            onView(withId(org.chromium.chrome.tab_ui.R.id.home_button)).perform(click());
-        }
-
-        ChromeTabbedActivity cta = mActivityTestRule.getActivity();
-        CriteriaHelper.pollUiThread(
-                () -> cta.getLayoutManager() != null && cta.getLayoutManager().overviewVisible());
-        onViewWaiting(withId(org.chromium.chrome.tab_ui.R.id.mv_tiles_container));
-        TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
-
-        // Launches the first site in mv tiles.
-        LinearLayout tilesLayout =
-                cta.findViewById(org.chromium.chrome.tab_ui.R.id.mv_tiles_layout);
-        TestThreadUtils.runOnUiThreadBlocking(() -> tilesLayout.getChildAt(0).performClick());
-        CriteriaHelper.pollUiThread(() -> !cta.getLayoutManager().overviewVisible());
-        // Verifies a new Tab is created.
-        TabUiTestHelper.verifyTabModelTabCount(cta, 2, 0);
-
-        if (isInstantReturn()
-                && (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP
-                        && Build.VERSION.SDK_INT < Build.VERSION_CODES.O)) {
-            // Fix the issue that failed to perform a single click on the tab switcher button.
-            // See code below.
-            return;
-        }
-
-        // Enters the tab switcher, and choose the new tab. After the tab is opening, press back.
-        onView(allOf(withId(org.chromium.chrome.R.id.tab_switcher_button), isDisplayed()));
-        TestThreadUtils.runOnUiThreadBlocking(() -> {
-            cta.findViewById(org.chromium.chrome.R.id.tab_switcher_button).performClick();
-        });
-        CriteriaHelper.pollUiThread(() -> cta.getLayoutManager().overviewVisible());
-        onViewWaiting(withId(R.id.primary_tasks_surface_view));
-        onView(allOf(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view), isDisplayed()));
-        onView(allOf(withParent(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body)),
-                       withId(org.chromium.chrome.tab_ui.R.id.tab_list_view)))
-                .perform(RecyclerViewActions.actionOnItemAtPosition(1, click()));
-        CriteriaHelper.pollUiThread(() -> !cta.getLayoutManager().overviewVisible());
-        Assert.assertEquals(TabLaunchType.FROM_START_SURFACE,
-                cta.getTabModelSelector().getCurrentTab().getLaunchType());
-        TestThreadUtils.runOnUiThreadBlocking(
-                ()
-                        -> Assert.assertTrue(StartSurfaceUserData.getKeepTab(
-                                cta.getTabModelSelector().getCurrentTab())));
-        pressBack();
-
-        // Verifies the new Tab isn't deleted, and Start surface is shown.
-        CriteriaHelper.pollUiThread(() -> cta.getLayoutManager().overviewVisible());
-        onViewWaiting(withId(R.id.primary_tasks_surface_view));
-        TabUiTestHelper.verifyTabModelTabCount(cta, 2, 0);
-    }
-
     private static Matcher<View> isView(final View targetView) {
         return new TypeSafeMatcher<View>() {
             @Override

chrome/android/features/start_surface/public/java/src/org/chromium/chrome/features/start_surface/StartSurfaceUserData.java

-// 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.features.start_surface;
-
-import org.chromium.base.UserData;
-import org.chromium.chrome.browser.tab.Tab;
-
-/**
- * Helper class for Tabs created from the Start surface.
- */
-public class StartSurfaceUserData implements UserData {
-    private static final Class<StartSurfaceUserData> USER_DATA_KEY = StartSurfaceUserData.class;
-    private boolean mKeepTab;
-
-    /**
-     * Sets the flag of whether to keep the given tab in the TabModel without auto deleting when
-     * tapping the back button. This flag is for a tab with launchType
-     * {@link org.chromium.chrome.browser.tab.TabLaunchType.FROM_START_SURFACE}.
-     */
-    public static void setKeepTab(Tab tab, boolean keepTab) {
-        StartSurfaceUserData startSurfaceUserData = get(tab);
-        if (startSurfaceUserData == null) {
-            startSurfaceUserData = new StartSurfaceUserData();
-        }
-        startSurfaceUserData.mKeepTab = keepTab;
-        tab.getUserDataHost().setUserData(USER_DATA_KEY, startSurfaceUserData);
-    }
-
-    /**
-     * @return Whether to keep the given tab in the TabModel without auto deleting when tapping the
-     * back button. Returns false if the UserData isn't set.
-     */
-    public static boolean getKeepTab(Tab tab) {
-        StartSurfaceUserData startSurfaceUserData = get(tab);
-        return startSurfaceUserData == null ? false : startSurfaceUserData.mKeepTab;
-    }
-
-    private static StartSurfaceUserData get(Tab tab) {
-        return tab.getUserDataHost().getUserData(USER_DATA_KEY);
-    }
-}

chrome/android/features/start_surface/public/start_surface_public_java_sources.gni

@@ -6,5 +6,4 @@ start_surface_public_java_sources = [
   "//chrome/android/features/start_surface/public/java/src/org/chromium/chrome/features/start_surface/StartSurface.java",
   "//chrome/android/features/start_surface/public/java/src/org/chromium/chrome/features/start_surface/StartSurfaceConfiguration.java",
   "//chrome/android/features/start_surface/public/java/src/org/chromium/chrome/features/start_surface/StartSurfaceState.java",
-  "//chrome/android/features/start_surface/public/java/src/org/chromium/chrome/features/start_surface/StartSurfaceUserData.java",
 ]

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java

@@ -259,9 +259,8 @@ class TabListMediator {
         /**
          * Run additional actions on tab selection.
          * @param tabId The ID of selected {@link Tab}.
-         * @param fromActionButton Whether it is called from the Action button on the card.
          */
-        void onTabSelecting(int tabId, boolean fromActionButton);
+        void onTabSelecting(int tabId);
     }
 
     @IntDef({TabClosedFrom.TAB_STRIP, TabClosedFrom.GRID_TAB_SWITCHER,
@@ -323,8 +322,7 @@ class TabListMediator {
                 recordUserSwitchedTab(currentTab, newlySelectedTab);
             }
             if (mGridCardOnClickListenerProvider != null) {
-                mGridCardOnClickListenerProvider.onTabSelecting(
-                        tabId, true /* fromActiionButton */);
+                mGridCardOnClickListenerProvider.onTabSelecting(tabId);
             } else {
                 mTabModelSelector.getCurrentModel().setIndex(
                         TabModelUtils.getTabIndexById(mTabModelSelector.getCurrentModel(), tabId),

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabSwitcherMediator.java

@@ -55,7 +55,6 @@ import org.chromium.chrome.browser.tasks.pseudotab.PseudoTab;
 import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter;
 import org.chromium.chrome.browser.tasks.tab_management.TabListCoordinator.TabListMode;
 import org.chromium.chrome.features.start_surface.StartSurfaceConfiguration;
-import org.chromium.chrome.features.start_surface.StartSurfaceUserData;
 import org.chromium.chrome.tab_ui.R;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -294,7 +293,7 @@ class TabSwitcherMediator
                 }
 
                 if (mContainerViewModel.get(IS_VISIBLE)) {
-                    onTabSelecting(tab.getId(), false);
+                    onTabSelecting(tab.getId());
                 }
             }
 
@@ -726,7 +725,7 @@ class TabSwitcherMediator
         }
         if (mTabModelSelector.getCurrentTab() == null) return false;
 
-        onTabSelecting(mTabModelSelector.getCurrentTabId(), false);
+        onTabSelecting(mTabModelSelector.getCurrentTabId());
 
         return true;
     }
@@ -813,12 +812,7 @@ class TabSwitcherMediator
     }
 
     @Override
-    public void onTabSelecting(int tabId, boolean fromActionButton) {
-        if (fromActionButton && (mMode == TabListMode.CAROUSEL || mMode == TabListMode.GRID)) {
-            Tab newlySelectedTab =
-                    TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), tabId);
-            StartSurfaceUserData.setKeepTab(newlySelectedTab, true);
-        }
+    public void onTabSelecting(int tabId) {
         mIsSelectingInTabSwitcher = true;
         if (mOnTabSelectingListener != null) {
             mOnTabSelectingListener.onTabSelecting(LayoutManagerImpl.time(), tabId);

chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java

@@ -490,7 +490,7 @@ public class TabListMediatorUnitTest {
                 .run(mModel.get(1).model.get(TabProperties.TAB_ID));
 
         verify(mGridCardOnClickListenerProvider)
-                .onTabSelecting(mModel.get(1).model.get(TabProperties.TAB_ID), true);
+                .onTabSelecting(mModel.get(1).model.get(TabProperties.TAB_ID));
     }
 
     @Test
@@ -2102,7 +2102,7 @@ public class TabListMediatorUnitTest {
                 .run(mModel.get(0).model.get(TabProperties.TAB_ID));
 
         verify(mGridCardOnClickListenerProvider)
-                .onTabSelecting(mModel.get(0).model.get(TabProperties.TAB_ID), true);
+                .onTabSelecting(mModel.get(0).model.get(TabProperties.TAB_ID));
         verify(mTab1).loadUrl(
                 refEq(new LoadUrlParams(searchUrl.getSpec(), PageTransition.KEYWORD_GENERATED)));
     }
@@ -2133,7 +2133,7 @@ public class TabListMediatorUnitTest {
                 .run(mModel.get(0).model.get(TabProperties.TAB_ID));
 
         verify(mGridCardOnClickListenerProvider)
-                .onTabSelecting(mModel.get(0).model.get(TabProperties.TAB_ID), true);
+                .onTabSelecting(mModel.get(0).model.get(TabProperties.TAB_ID));
         verify(navigationController, never()).goToOffset(0);
 
         doReturn(webContents).when(mTab1).getWebContents();

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -160,7 +160,6 @@ import org.chromium.chrome.browser.vr.VrModuleProvider;
 import org.chromium.chrome.features.start_surface.StartSurface;
 import org.chromium.chrome.features.start_surface.StartSurfaceConfiguration;
 import org.chromium.chrome.features.start_surface.StartSurfaceState;
-import org.chromium.chrome.features.start_surface.StartSurfaceUserData;
 import org.chromium.components.browser_ui.util.BrowserControlsVisibilityDelegate;
 import org.chromium.components.browser_ui.util.ComposedBrowserControlsVisibilityDelegate;
 import org.chromium.components.embedder_support.util.UrlConstants;
@@ -1861,12 +1860,8 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
             return true;
         }
 
-        // If we are in the tab switcher mode (not in the Start surface homepage) and not a tablet,
-        // then leave tab switcher mode on back.
-        if (mOverviewModeController.overviewVisible() && !isTablet()
-                && (mStartSurfaceSupplier.get() == null
-                        || mStartSurfaceSupplier.get().getController().getStartSurfaceState()
-                                == StartSurfaceState.SHOWN_TABSWITCHER)) {
+        // If we are in overview mode and not a tablet, then leave overview mode on back.
+        if (mOverviewModeController.overviewVisible() && !isTablet()) {
             mOverviewModeController.hideOverview(true);
             return true;
         }
@@ -1883,22 +1878,11 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
             return true;
         }
 
-        // If we aren't in the overview mode, we handle the Tab with launchType
-        // TabLaunchType.FROM_START_SURFACE here.
-        if (!mOverviewModeController.overviewVisible()
-                && type == TabLaunchType.FROM_START_SURFACE) {
-            if (StartSurfaceUserData.getKeepTab(currentTab)) {
-                // If the current tab is created from the start surface with the keepTab property,
-                // shows the Start surface Homepage to prevent a loop between the current tab and
-                // previous overview mode. Once in the Start surface, it will close Chrome if back
-                // button is tapped again.
-                showOverview(StartSurfaceState.SHOWING_HOMEPAGE);
-            } else {
-                // Otherwise, clicking the back button should close the tab and go back to the
-                // previous overview mode.
-                getCurrentTabModel().closeTab(currentTab);
-                showOverview(StartSurfaceState.SHOWING_PREVIOUS);
-            }
+        // If the current tab is created from the start surface, then the back button should close
+        // the tab and go back to the start surface.
+        if (type == TabLaunchType.FROM_START_SURFACE) {
+            getCurrentTabModel().closeTab(currentTab);
+            showOverview(StartSurfaceState.SHOWING_PREVIOUS);
             return true;
         }