PreviewTab: Suppress contextual search temporarily

This CL temporarily suppresses contextual search while any BottomSheet
feature is open. RootUiCoordinator looks over ContextualSearchManager
and BottomSheetController to coordinate the suppression.

Bug: 1083396, 1077042
Change-Id: I5fa385a12105fb510a1a59d07148a6b1c5ef71fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2206562Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772452}

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

@@ -370,7 +370,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         // a recommended pattern.
         return new RootUiCoordinator(this, null, getShareDelegateSupplier(),
                 getActivityTabProvider(), mTabModelProfileSupplier, mBookmarkBridgeSupplier,
-                getOverviewModeBehaviorSupplier());
+                getOverviewModeBehaviorSupplier(), this::getContextualSearchManager);
     }
 
     private C createComponent() {

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

@@ -1539,7 +1539,7 @@ public class ChromeTabbedActivity
         return new TabbedRootUiCoordinator(this, this::onOmniboxFocusChanged, mIntentWithEffect,
                 getShareDelegateSupplier(), getActivityTabProvider(),
                 mEphemeralTabCoordinatorSupplier, mTabModelProfileSupplier, mBookmarkBridgeSupplier,
-                getOverviewModeBehaviorSupplier());
+                getOverviewModeBehaviorSupplier(), this::getContextualSearchManager);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java

@@ -197,6 +197,9 @@ public class ContextualSearchManager
     /** Whether the Accessibility Mode is enabled. */
     private boolean mIsAccessibilityModeEnabled;
 
+    /** Whether bottom sheet is visible. */
+    private boolean mIsBottomSheetVisible;
+
     /** Tap Experiments and other variable behavior. */
     private QuickAnswersHeuristic mQuickAnswersHeuristic;
 
@@ -897,6 +900,14 @@ public class ContextualSearchManager
         if (enabled) hideContextualSearch(StateChangeReason.UNKNOWN);
     }
 
+    /**
+     * Update bottom sheet visibility state.
+     */
+    public void onBottomSheetVisible(boolean visible) {
+        mIsBottomSheetVisible = visible;
+        if (visible) hideContextualSearch(StateChangeReason.RESET);
+    }
+
     /**
      * Notifies that the preference state has changed.
      * @param isEnabled Whether the feature is enabled.
@@ -1352,7 +1363,7 @@ public class ContextualSearchManager
 
     @Override
     public void handleScrollStart() {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         hideContextualSearch(StateChangeReason.BASE_PAGE_SCROLL);
     }
@@ -1366,21 +1377,21 @@ public class ContextualSearchManager
 
     @Override
     public void handleInvalidTap() {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         hideContextualSearch(StateChangeReason.BASE_PAGE_TAP);
     }
 
     @Override
     public void handleSuppressedTap() {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         hideContextualSearch(StateChangeReason.TAP_SUPPRESS);
     }
 
     @Override
     public void handleNonSuppressedTap(long tapTimeNanoseconds) {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         // If there's a wait-after-tap experiment then we may want to delay a bit longer for
         // the user to take an action like scrolling that will reset our internal state.
@@ -1428,14 +1439,14 @@ public class ContextualSearchManager
 
     @Override
     public void handleValidTap() {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         mInternalStateController.enter(InternalState.TAP_RECOGNIZED);
     }
 
     @Override
     public void handleValidResolvingLongpress() {
-        if (mIsAccessibilityModeEnabled || !mPolicy.canResolveLongpress()) return;
+        if (isSuppressed() || !mPolicy.canResolveLongpress()) return;
 
         mInternalStateController.enter(InternalState.RESOLVING_LONG_PRESS_RECOGNIZED);
     }
@@ -1448,7 +1459,7 @@ public class ContextualSearchManager
     @Override
     public void handleSelection(
             String selection, boolean selectionValid, @SelectionType int type, float x, float y) {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         if (!selection.isEmpty()) {
             ContextualSearchUma.logSelectionIsValid(selectionValid);
@@ -1473,7 +1484,7 @@ public class ContextualSearchManager
 
     @Override
     public void handleSelectionDismissal() {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         if (isSearchPanelShowing()
                 && !mIsPromotingToTab
@@ -1490,7 +1501,7 @@ public class ContextualSearchManager
     @Override
     public void handleSelectionModification(
             String selection, boolean selectionValid, float x, float y) {
-        if (mIsAccessibilityModeEnabled) return;
+        if (isSuppressed()) return;
 
         if (isSearchPanelShowing()) {
             if (selectionValid) {
@@ -1865,6 +1876,11 @@ public class ContextualSearchManager
         return mContext;
     }
 
+    @VisibleForTesting
+    public boolean isSuppressed() {
+        return mIsBottomSheetVisible || mIsAccessibilityModeEnabled;
+    }
+
     @NativeMethods
     interface Natives {
         long init(ContextualSearchManager caller);

chrome/android/java/src/org/chromium/chrome/browser/customtabs/BaseCustomTabActivity.java

@@ -144,7 +144,8 @@ public abstract class BaseCustomTabActivity<C extends BaseCustomTabActivityCompo
     protected RootUiCoordinator createRootUiCoordinator() {
         return new BaseCustomTabRootUiCoordinator(this, getShareDelegateSupplier(),
                 mToolbarCoordinator, mNavigationController, getActivityTabProvider(),
-                mTabModelProfileSupplier, mBookmarkBridgeSupplier, mOverviewModeBehaviorSupplier);
+                mTabModelProfileSupplier, mBookmarkBridgeSupplier, mOverviewModeBehaviorSupplier,
+                this::getContextualSearchManager);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/customtabs/BaseCustomTabRootUiCoordinator.java

@@ -5,10 +5,12 @@
 package org.chromium.chrome.browser.customtabs;
 
 import org.chromium.base.supplier.ObservableSupplier;
+import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
+import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityNavigationController;
 import org.chromium.chrome.browser.customtabs.features.toolbar.CustomTabToolbarCoordinator;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -28,10 +30,11 @@ public class BaseCustomTabRootUiCoordinator extends RootUiCoordinator {
             CustomTabActivityNavigationController customTabNavigationController,
             ActivityTabProvider tabProvider, ObservableSupplier<Profile> profileSupplier,
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
-            ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier) {
+            ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
+            Supplier<ContextualSearchManager> contextualSearchManagerSupplier) {
         super(activity, null, shareDelegateSupplier, tabProvider, profileSupplier,
-                bookmarkBridgeSupplier, overviewModeBehaviorSupplier);
-
+                bookmarkBridgeSupplier, overviewModeBehaviorSupplier,
+                contextualSearchManagerSupplier);
         mToolbarCoordinator = customTabToolbarCoordinator;
         mNavigationController = customTabNavigationController;
     }

chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java

@@ -11,6 +11,7 @@ import org.chromium.base.Callback;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.ObservableSupplierImpl;
+import org.chromium.base.supplier.Supplier;
 import org.chromium.base.task.PostTask;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ActivityTabProvider;
@@ -19,6 +20,7 @@ import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
 import org.chromium.chrome.browser.compositor.bottombar.ephemeraltab.EphemeralTabCoordinator;
 import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
+import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
 import org.chromium.chrome.browser.datareduction.DataReductionPromoScreen;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -78,6 +80,7 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
      * @param profileSupplier Supplier of the currently applicable profile.
      * @param bookmarkBridgeSupplier Supplier of the bookmark bridge for the current profile.
      * @param overviewModeBehaviorSupplier Supplier of the overview mode manager.
+     * @param contextualSearchManagerSupplier Supplier of the {@link ContextualSearchManager}.
      */
     public TabbedRootUiCoordinator(ChromeActivity activity,
             Callback<Boolean> onOmniboxFocusChangedListener, boolean intentWithEffect,
@@ -86,9 +89,11 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
             ObservableSupplierImpl<EphemeralTabCoordinator> ephemeralTabCoordinatorSupplier,
             ObservableSupplier<Profile> profileSupplier,
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
-            ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier) {
+            ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
+            Supplier<ContextualSearchManager> contextualSearchManagerSupplier) {
         super(activity, onOmniboxFocusChangedListener, shareDelegateSupplier, tabProvider,
-                profileSupplier, bookmarkBridgeSupplier, overviewModeBehaviorSupplier);
+                profileSupplier, bookmarkBridgeSupplier, overviewModeBehaviorSupplier,
+                contextualSearchManagerSupplier);
         mIntentWithEffect = intentWithEffect;
         mEphemeralTabCoordinatorSupplier = ephemeralTabCoordinatorSupplier;
         mCanAnimateBrowserControls = () -> {

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

@@ -31,6 +31,7 @@ import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelManager;
 import org.chromium.chrome.browser.compositor.layouts.EmptyOverviewModeObserver;
 import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
+import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
 import org.chromium.chrome.browser.directactions.DirectActionInitializer;
 import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
 import org.chromium.chrome.browser.findinpage.FindToolbarManager;
@@ -63,6 +64,9 @@ import org.chromium.chrome.browser.ui.system.StatusBarColorController;
 import org.chromium.chrome.browser.vr.VrModuleProvider;
 import org.chromium.chrome.browser.widget.ScrimView;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
+import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
+import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
+import org.chromium.chrome.browser.widget.bottomsheet.EmptyBottomSheetObserver;
 import org.chromium.components.browser_ui.widget.MenuOrKeyboardActionController;
 import org.chromium.components.browser_ui.widget.scrim.ScrimCoordinator;
 import org.chromium.components.feature_engagement.EventConstants;
@@ -134,6 +138,8 @@ public class RootUiCoordinator
             new ObservableSupplierImpl<>();
     private final ObservableSupplier<Profile> mProfileSupplier;
     private final ObservableSupplier<BookmarkBridge> mBookmarkBridgeSupplier;
+    private BottomSheetObserver mContextualSearchSuppressor;
+    private final Supplier<ContextualSearchManager> mContextualSearchManagerSupplier;
 
     /**
      * Create a new {@link RootUiCoordinator} for the given activity.
@@ -147,13 +153,15 @@ public class RootUiCoordinator
      * @param bookmarkBridgeSupplier Supplier of the bookmark bridge for the current profile.
      * @param overviewModeBehaviorSupplier Supplier of the overview mode manager for the current
      *                                     profile.
+     * @param contextualSearchManagerSupplier Supplier of the {@link ContextualSearchManager}.
      */
     public RootUiCoordinator(ChromeActivity activity,
             @Nullable Callback<Boolean> onOmniboxFocusChangedListener,
             ObservableSupplier<ShareDelegate> shareDelegateSupplier,
             ActivityTabProvider tabProvider, ObservableSupplier<Profile> profileSupplier,
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
-            ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier) {
+            ObservableSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
+            Supplier<ContextualSearchManager> contextualSearchManagerSupplier) {
         mActivity = activity;
         mOnOmniboxFocusChangedListener = onOmniboxFocusChangedListener;
         mActivity.getLifecycleDispatcher().register(this);
@@ -171,6 +179,7 @@ public class RootUiCoordinator
                 activity.getLifecycleDispatcher(), mActivityTabProvider, mTabObscuringHandler);
         mProfileSupplier = profileSupplier;
         mBookmarkBridgeSupplier = bookmarkBridgeSupplier;
+        mContextualSearchManagerSupplier = contextualSearchManagerSupplier;
 
         mOmniboxFocusStateSupplier.set(false);
 
@@ -229,7 +238,12 @@ public class RootUiCoordinator
         }
 
         if (mBottomSheetManager != null) mBottomSheetManager.destroy();
-        if (mBottomSheetController != null) mBottomSheetController.destroy();
+        if (mBottomSheetController != null) {
+            if (mContextualSearchSuppressor != null) {
+                mBottomSheetController.removeObserver(mContextualSearchSuppressor);
+            }
+            mBottomSheetController.destroy();
+        }
 
         if (mButtonDataProviders != null) {
             for (ButtonDataProvider provider : mButtonDataProviders) {
@@ -271,6 +285,7 @@ public class RootUiCoordinator
         initializeToolbar();
         initAppMenu();
         initDirectActionInitializer();
+        initContextualSearchSuppressor();
         if (mAppMenuCoordinator != null) {
             mToolbarManager.onAppMenuInitialized(mAppMenuCoordinator);
             mModalDialogManagerObserver = new ModalDialogManagerObserver() {
@@ -381,8 +396,8 @@ public class RootUiCoordinator
         // EphemeralTabCoordinator, and FindToolbarManager will all be owned by this class.
 
         // Do not show the menu if Contextual Search panel is opened.
-        if (mActivity.getContextualSearchManager() != null
-                && mActivity.getContextualSearchManager().isSearchPanelOpened()) {
+        if (mContextualSearchManagerSupplier.get() != null
+                && mContextualSearchManagerSupplier.get().isSearchPanelOpened()) {
             return false;
         }
 
@@ -576,8 +591,8 @@ public class RootUiCoordinator
      * cross-feature interaction, e.g. hide other features when this feature is shown.
      */
     protected void onFindToolbarShown() {
-        if (mActivity.getContextualSearchManager() != null) {
-            mActivity.getContextualSearchManager().hideContextualSearch(
+        if (mContextualSearchManagerSupplier.get() != null) {
+            mContextualSearchManagerSupplier.get().hideContextualSearch(
                     OverlayPanel.StateChangeReason.UNKNOWN);
         }
     }
@@ -650,6 +665,39 @@ public class RootUiCoordinator
         mActivity.getLifecycleDispatcher().register(mDirectActionInitializer);
     }
 
+    /**
+     * Initializes a glue logic that suppresses Contextual Search while a Bottom Sheet feature is
+     * in action.
+     */
+    private void initContextualSearchSuppressor() {
+        if (mBottomSheetController == null) return;
+        mContextualSearchSuppressor = new EmptyBottomSheetObserver() {
+            private boolean mOpened;
+
+            @Override
+            public void onSheetStateChanged(int newState) {
+                switch (newState) {
+                    case SheetState.PEEK:
+                    case SheetState.HALF:
+                    case SheetState.FULL:
+                        if (!mOpened) {
+                            mOpened = true;
+                            ContextualSearchManager manager =
+                                    mContextualSearchManagerSupplier.get();
+                            if (manager != null) manager.onBottomSheetVisible(true);
+                        }
+                        break;
+                    case SheetState.HIDDEN:
+                        mOpened = false;
+                        ContextualSearchManager manager = mContextualSearchManagerSupplier.get();
+                        if (manager != null) manager.onBottomSheetVisible(false);
+                        break;
+                }
+            }
+        };
+        mBottomSheetController.addObserver(mContextualSearchSuppressor);
+    }
+
     // Testing methods
 
     @VisibleForTesting

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

@@ -176,7 +176,7 @@ public class ShareIntentTest {
         RootUiCoordinator rootUiCoordinator = TestThreadUtils.runOnUiThreadBlocking(() -> {
             return new RootUiCoordinator(mockActivity, null,
                     mockActivity.getShareDelegateSupplier(), mockActivity.getActivityTabProvider(),
-                    null, null, mockActivity.getOverviewModeBehaviorSupplier());
+                    null, null, mockActivity.getOverviewModeBehaviorSupplier(), null);
         });
         ShareHelper.setLastShareComponentName(new ComponentName("test.package", "test.activity"));
         // Skips the capture of screenshot and notifies with an empty file.

chrome/android/javatests/src/org/chromium/chrome/browser/previewtab/PreviewTabTest.java

@@ -17,6 +17,7 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.compositor.bottombar.ephemeraltab.EphemeralTabCoordinator;
+import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
 import org.chromium.chrome.browser.firstrun.DisableFirstRun;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.tab.Tab;
@@ -24,6 +25,7 @@ import org.chromium.chrome.browser.tabbed_mode.TabbedRootUiCoordinator;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
+import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
 import org.chromium.chrome.test.util.browser.contextmenu.ContextMenuUtils;
 import org.chromium.content_public.browser.test.util.DOMUtils;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
@@ -35,6 +37,7 @@ import org.chromium.net.test.EmbeddedTestServerRule;
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
 @DisableFeatures({ChromeFeatureList.REVAMPED_CONTEXT_MENU})
+@EnableFeatures(ChromeFeatureList.EPHEMERAL_TAB_USING_BOTTOM_SHEET)
 public class PreviewTabTest {
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
@@ -47,6 +50,8 @@ public class PreviewTabTest {
     public DisableFirstRun mDisableFirstRunFlowRule = new DisableFirstRun();
 
     private static final String BASE_PAGE = "/chrome/test/data/android/previewtab/base_page.html";
+    private static final String PREVIEW_TAB =
+            "/chrome/test/data/android/previewtab/preview_tab.html";
     private static final String PREVIEW_TAB_DOM_ID = "previewTab";
     private static final String NEAR_BOTTOM_DOM_ID = "nearBottom";
 
@@ -65,11 +70,17 @@ public class PreviewTabTest {
     }
 
     private void endAnimations() {
-        TestThreadUtils.runOnUiThreadBlocking(()
-                                                      -> mActivityTestRule.getActivity()
+        TestThreadUtils.runOnUiThreadBlocking(
+                mActivityTestRule.getActivity()
                         .getRootUiCoordinatorForTesting()
-                                                                 .getBottomSheetController()
-                                                                 .endAnimationsForTesting());
+                        .getBottomSheetController()::endAnimationsForTesting);
+    }
+
+    private void closePreviewTab() {
+        TestThreadUtils.runOnUiThreadBlocking(mEphemeralTabCoordinator::close);
+        endAnimations();
+        Assert.assertFalse("The Preview Tab should have closed but did not indicate closed",
+                mEphemeralTabCoordinator.isOpened());
     }
 
     /**
@@ -95,12 +106,27 @@ public class PreviewTabTest {
         Assert.assertTrue("The Preview Tab did not stay open after a scroll action",
                 mEphemeralTabCoordinator.isOpened());
 
-        // Close the PT.
-        TestThreadUtils.runOnUiThreadBlocking(() -> mEphemeralTabCoordinator.close());
-        endAnimations();
-        Assert.assertFalse("The Preview Tab should have closed but did not indicate closed",
-                mEphemeralTabCoordinator.isOpened());
+        closePreviewTab();
     }
 
-    // TODO(donnd): add more tests.
+    /**
+     * Test preview tab suppresses contextual search.
+     */
+    @Test
+    @MediumTest
+    @Feature({"PreviewTab"})
+    public void testSuppressContextualSearch() throws Throwable {
+        ChromeActivity activity = mActivityTestRule.getActivity();
+        ContextualSearchManager csManager = activity.getContextualSearchManager();
+        Assert.assertFalse("Contextual Search should be active", csManager.isSuppressed());
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> mEphemeralTabCoordinator.requestOpenSheet(
+                mTestServer.getServer().getURL(PREVIEW_TAB), "PreviewTab", false));
+        endAnimations();
+        Assert.assertTrue("The Preview Tab did not open", mEphemeralTabCoordinator.isOpened());
+        Assert.assertTrue("Contextual Search should be suppressed", csManager.isSuppressed());
+
+        closePreviewTab();
+        Assert.assertFalse("Contextual Search should be active", csManager.isSuppressed());
+    }
 }