[Start] Handle the bottom sheet suppression in Start surface.

When clicking +1 button from the Tab Switcher, it launched the Start
surface via ChromeTabCreator#launchNTP(). During this transition from
the Tab Switcher mode to the Start surface mode, the onActivityChange()
event isn't triggered since tab is null in both scenarios. As a result,
the token to suppress the bottom sheet in Tab Switcher isn't released
when transiting to the Start surface.

In this CL, we make BottomSheetManager listen to the Start surface
state change, and move the logic of suppression of the bottom sheet for
Start surface + Tab switcher into the listener.

Bug: 1129746
Change-Id: I74fd545bfd6799626a57944fb97db6c9164225be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2541811Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828979}

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

@@ -542,12 +542,10 @@ class StartSurfaceMediator
         }
     }
 
-    // TODO(crbug.com/1115757): After crrev.com/c/2315823, Overview state and Startsurface state are
-    // two different things, audit the wording usage and see if we can rename this method to
-    // getStartSurfaceState.
     @VisibleForTesting
+    @Override
     @StartSurfaceState
-    public int getOverviewState() {
+    public int getStartSurfaceState() {
         return mStartSurfaceState;
     }
 
@@ -661,8 +659,9 @@ class StartSurfaceMediator
     }
 
     @Override
-    public boolean isHomePageShowing() {
-        return mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE;
+    public boolean inShowState() {
+        return mStartSurfaceState != StartSurfaceState.NOT_SHOWN
+                && mStartSurfaceState != StartSurfaceState.DISABLED;
     }
 
     // Implements TabSwitcher.OverviewModeObserver.

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

@@ -112,6 +112,7 @@ import org.chromium.chrome.test.util.ChromeApplicationTestUtils;
 import org.chromium.chrome.test.util.ChromeTabUtils;
 import org.chromium.chrome.test.util.OverviewModeBehaviorWatcher;
 import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
+import org.chromium.components.browser_ui.bottomsheet.BottomSheetTestSupport;
 import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.content_public.browser.test.util.TouchCommon;
@@ -1386,6 +1387,39 @@ public class StartSurfaceTest {
         onView(withId(R.id.voice_search_button)).check(matches(isDisplayed()));
     }
 
+    @Test
+    @MediumTest
+    @Feature({"StartSurface"})
+    // clang-format off
+    @CommandLineFlags.Add({BASE_PARAMS + "/single"})
+    public void testShow_SingleAsHomepage_BottomSheet() {
+        // clang-format on
+        if (!mImmediateReturn) {
+            onView(withId(org.chromium.chrome.tab_ui.R.id.home_button)).perform(click());
+        }
+
+        ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        BottomSheetTestSupport bottomSheetTestSupport = new BottomSheetTestSupport(
+                cta.getRootUiCoordinatorForTesting().getBottomSheetController());
+        CriteriaHelper.pollUiThread(
+                () -> cta.getLayoutManager() != null && cta.getLayoutManager().overviewVisible());
+        waitForTabModel();
+        TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
+        assertFalse(bottomSheetTestSupport.hasSuppressionTokens());
+
+        onView(withId(org.chromium.chrome.tab_ui.R.id.tab_list_view))
+                .perform(RecyclerViewActions.actionOnItemAtPosition(0, click()));
+        assertFalse(bottomSheetTestSupport.hasSuppressionTokens());
+
+        TabUiTestHelper.enterTabSwitcher(cta);
+        onViewWaiting(withId(R.id.secondary_tasks_surface_view));
+        assertTrue(bottomSheetTestSupport.hasSuppressionTokens());
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> cta.getTabCreator(false).launchNTP());
+        onViewWaiting(withId(R.id.primary_tasks_surface_view));
+        assertFalse(bottomSheetTestSupport.hasSuppressionTokens());
+    }
+
     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/StartSurface.java

@@ -158,10 +158,15 @@ public interface StartSurface {
         void enableRecordingFirstMeaningfulPaint(long activityCreateTimeMs);
 
         /**
-         * @return Whether the Start surface is currently showing with a state of
-         * {@link OverviewModeState.SHOWN_HOMEPAGE}.
+         * @return Whether the current {@link StartSurfaceState}.
          */
-        boolean isHomePageShowing();
+        @StartSurfaceState
+        int getStartSurfaceState();
+
+        /**
+         * @return Whether the Start surface or the Tab switcher is shown or showing.
+         */
+        boolean inShowState();
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/ui/BottomSheetManager.java

@@ -5,7 +5,9 @@
 package org.chromium.chrome.browser.ui;
 
 import org.chromium.base.Callback;
+import org.chromium.base.CallbackController;
 import org.chromium.base.supplier.ObservableSupplier;
+import org.chromium.base.supplier.OneshotSupplier;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.ActivityTabProvider.ActivityTabObserver;
@@ -23,6 +25,8 @@ import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
 import org.chromium.chrome.browser.vr.VrModuleProvider;
 import org.chromium.chrome.features.start_surface.StartSurface;
+import org.chromium.chrome.features.start_surface.StartSurface.StateObserver;
+import org.chromium.chrome.features.start_surface.StartSurfaceState;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.StateChangeReason;
@@ -60,8 +64,11 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
     /** A tab observer that is only attached to the active tab. */
     private final TabObserver mTabObserver;
 
+    private final CallbackController mCallbackController;
+
     /** The supplier of {@link StartSurface} instance. */
-    private final Supplier<StartSurface> mStartSurfaceSupplier;
+    private final OneshotSupplier<StartSurface> mStartSurfaceSupplier;
+    private StateObserver mStartSurfaceStateObserver;
 
     /** A browser controls manager for polling browser controls offsets. */
     private BrowserControlsVisibilityManager mBrowserControlsVisibilityManager;
@@ -119,7 +126,7 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
             TabObscuringHandler obscuringDelegate,
             ObservableSupplier<Boolean> omniboxFocusStateSupplier,
             Supplier<OverlayPanelManager> overlayManager,
-            Supplier<StartSurface> startSurfaceSupplier) {
+            OneshotSupplier<StartSurface> startSurfaceSupplier) {
         mSheetController = controller;
         mTabProvider = tabProvider;
         mBrowserControlsVisibilityManager = controlsVisibilityManager;
@@ -131,6 +138,9 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
         mOmniboxFocusStateSupplier = omniboxFocusStateSupplier;
         mOverlayPanelManager = overlayManager;
         mStartSurfaceSupplier = startSurfaceSupplier;
+        mCallbackController = new CallbackController();
+        mStartSurfaceSupplier.onAvailable(
+                mCallbackController.makeCancelable(this::addStartSurfaceStateObserver));
 
         mSheetController.addObserver(this);
         mSheetController.setAccssibilityUtil(ChromeAccessibilityUtil.get());
@@ -164,13 +174,15 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
 
             @Override
             public void onActivityTabChanged(Tab tab) {
+                if (mStartSurfaceSupplier.get() != null
+                        && mStartSurfaceSupplier.get().getController().inShowState()) {
+                    return;
+                }
+
                 // Temporarily suppress the sheet if entering a state where there is no activity
                 // tab and the Start surface homepage isn't showing.
                 if (tab == null) {
-                    if (mStartSurfaceSupplier.get() == null
-                            || !mStartSurfaceSupplier.get().getController().isHomePageShowing()) {
-                        mToken = controller.suppressSheet(StateChangeReason.COMPOSITED_UI);
-                    }
+                    mToken = controller.suppressSheet(StateChangeReason.COMPOSITED_UI);
                     return;
                 }
 
@@ -257,6 +269,31 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
         mOmniboxFocusStateSupplier.addObserver(mOmniboxFocusObserver);
     }
 
+    private void addStartSurfaceStateObserver(StartSurface startSurface) {
+        mStartSurfaceStateObserver = new StateObserver() {
+            private int mToken;
+            private int mStartSurfaceState;
+            @Override
+            public void onStateChanged(
+                    int startSurfaceState, boolean shouldShowTabSwitcherToolbar) {
+                if (mStartSurfaceState == startSurfaceState) return;
+
+                mStartSurfaceState = startSurfaceState;
+                if (!startSurface.getController().inShowState()
+                        || startSurface.getController().getStartSurfaceState()
+                                == StartSurfaceState.SHOWN_HOMEPAGE
+                        || startSurface.getController().getStartSurfaceState()
+                                == StartSurfaceState.SHOWING_HOMEPAGE) {
+                    mSheetController.unsuppressSheet(mToken);
+                } else {
+                    mToken = mSheetController.suppressSheet(StateChangeReason.COMPOSITED_UI);
+                }
+            }
+        };
+
+        startSurface.addStateChangeObserver(mStartSurfaceStateObserver);
+    }
+
     @Override
     public void onSheetOpened(int reason) {
         if (mBrowserControlsVisibilityManager.getBrowserVisibilityDelegate() != null) {
@@ -348,6 +385,7 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
 
     @Override
     public void destroy() {
+        mCallbackController.destroy();
         if (mLastActivityTab != null) mLastActivityTab.removeObserver(mTabObserver);
         mTabProvider.removeObserver(mActivityTabObserver);
         mSheetController.removeObserver(this);
@@ -355,5 +393,8 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
         mBrowserControlsVisibilityManager.removeObserver(mBrowserControlsObserver);
         mOmniboxFocusStateSupplier.removeObserver(mOmniboxFocusObserver);
         VrModuleProvider.unregisterVrModeObserver(mVrModeObserver);
+        if (mStartSurfaceSupplier.get() != null) {
+            mStartSurfaceSupplier.get().removeStateChangeObserver(mStartSurfaceStateObserver);
+        }
     }
 }

components/browser_ui/android/bottomsheet/internal/java/src/org/chromium/components/browser_ui/bottomsheet/BottomSheetControllerImpl.java

@@ -515,4 +515,9 @@ class BottomSheetControllerImpl implements ManagedBottomSheetController {
     private boolean canBottomSheetSwitchContent() {
         return !mBottomSheet.isSheetOpen();
     }
+
+    @VisibleForTesting
+    boolean hasSuppressionTokensForTesting() {
+        return mSuppressionTokens.hasTokens();
+    }
 }

components/browser_ui/android/bottomsheet/test/java/src/org/chromium/components/browser_ui/bottomsheet/BottomSheetTestSupport.java

@@ -108,4 +108,11 @@ public class BottomSheetTestSupport {
     private BottomSheet getBottomSheet() {
         return (BottomSheet) mController.getBottomSheetViewForTesting();
     }
+
+    /**
+     * @return Whether has any token to suppress the bottom sheet.
+     */
+    public boolean hasSuppressionTokens() {
+        return mController.hasSuppressionTokensForTesting();
+    }
 }