[Start] Fix: 3 dot menu of article does not work.

With this CL, we don't suppress bottom sheet in Start Surface any more.

Bug: 1129746
Change-Id: Ie1b1b2b3e29c6a77c1f24ea651a7f1030d60171b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431545Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Mei Liang <meiliang@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@{#812963}

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

@@ -619,6 +619,11 @@ class StartSurfaceMediator
         }
     }
 
+    @Override
+    public boolean isHomePageShowing() {
+        return mOverviewModeState == OverviewModeState.SHOWN_HOMEPAGE;
+    }
+
     // Implements TabSwitcher.OverviewModeObserver.
     @Override
     public void startedShowing() {

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

@@ -103,6 +103,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Integration tests of Instant Start which requires 2-stage initialization for Clank startup.
@@ -243,6 +244,15 @@ public class InstantStartTest {
         Assert.assertTrue(LibraryLoader.getInstance().isInitialized());
     }
 
+    private StartSurfaceCoordinator getStartSurfaceFromUIThread() {
+        AtomicReference<StartSurface> startSurface = new AtomicReference<>();
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            startSurface.set(
+                    ((ChromeTabbedActivity) mActivityTestRule.getActivity()).getStartSurface());
+        });
+        return (StartSurfaceCoordinator) startSurface.get();
+    }
+
     /**
      * Test TabContentManager is able to fetch thumbnail jpeg files before native is initialized.
      */
@@ -339,9 +349,7 @@ public class InstantStartTest {
         assertThat(mActivityTestRule.getActivity().getLayoutManager().getOverviewLayout())
                 .isInstanceOf(StartSurfaceLayout.class);
 
-        StartSurfaceCoordinator startSurfaceCoordinator =
-                (StartSurfaceCoordinator) ((ChromeTabbedActivity) mActivityTestRule.getActivity())
-                        .getStartSurface();
+        StartSurfaceCoordinator startSurfaceCoordinator = getStartSurfaceFromUIThread();
         Assert.assertTrue(startSurfaceCoordinator.isInitPendingForTesting());
         Assert.assertFalse(startSurfaceCoordinator.isInitializedWithNativeForTesting());
         Assert.assertFalse(startSurfaceCoordinator.isSecondaryTaskInitPendingForTesting());

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

@@ -130,6 +130,12 @@ public interface StartSurface {
          * @param activityCreateTimeMs {@link SystemClock#elapsedRealtime} at activity creation.
          */
         void enableRecordingFirstMeaningfulPaint(long activityCreateTimeMs);
+
+        /**
+         * @return Whether the Start surface is currently showing with a state of
+         * {@link OverviewModeState.SHOWN_HOMEPAGE}.
+         */
+        boolean isHomePageShowing();
     }
 
     /**

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

@@ -256,8 +256,6 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
 
     private Runnable mShowHistoryRunnable;
 
-    private StartSurface mStartSurface;
-
     private CompositorViewHolder mCompositorViewHolder;
     private OverviewListLayout mOverviewListLayout;
     /**
@@ -294,6 +292,8 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
 
     private ObservableSupplierImpl<EphemeralTabCoordinator> mEphemeralTabCoordinatorSupplier =
             new ObservableSupplierImpl<>();
+    private final OneshotSupplierImpl<StartSurface> mStartSurfaceSupplier =
+            new OneshotSupplierImpl<>();
 
     private CallbackController mCallbackController = new CallbackController();
 
@@ -558,13 +558,15 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
                 TabManagementDelegate tabManagementDelegate =
                         TabManagementModuleProvider.getDelegate();
                 if (tabManagementDelegate != null) {
-                    mStartSurface = tabManagementDelegate.createStartSurface(this,
+                    StartSurface startSurface = tabManagementDelegate.createStartSurface(this,
                             mRootUiCoordinator.getScrimCoordinator(),
                             mRootUiCoordinator.getBottomSheetController());
+                    mStartSurfaceSupplier.set(startSurface);
                 }
             }
             mLayoutManager = new LayoutManagerChromePhone(compositorViewHolder, mContentContainer,
-                    mStartSurface, getTabContentManagerSupplier(), mOverviewModeBehaviorSupplier);
+                    mStartSurfaceSupplier.get(), getTabContentManagerSupplier(),
+                    mOverviewModeBehaviorSupplier);
             mOverviewModeController = mLayoutManager;
         }
     }
@@ -924,7 +926,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
                         TAB_COUNT_ON_RETURN, getCurrentTabModel().getCount());
             }
             if (TabUiFeatureUtilities.isGridTabSwitcherEnabled() && !isTablet()) {
-                mStartSurface.getController().enableRecordingFirstMeaningfulPaint(
+                mStartSurfaceSupplier.get().getController().enableRecordingFirstMeaningfulPaint(
                         getOnCreateTimestampMs());
             }
             mOverviewShownOnStart = true;
@@ -1421,7 +1423,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
                 mIntentWithEffectSupplier, getShareDelegateSupplier(), getActivityTabProvider(),
                 mEphemeralTabCoordinatorSupplier, mTabModelProfileSupplier, mBookmarkBridgeSupplier,
                 getOverviewModeBehaviorSupplier(), this::getContextualSearchManager,
-                mTabModelSelectorSupplier);
+                mTabModelSelectorSupplier, mStartSurfaceSupplier);
     }
 
     @Override
@@ -1462,7 +1464,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
         Supplier<Boolean> dialogVisibilitySupplier = null;
         if (TabUiFeatureUtilities.isTabGroupsAndroidEnabled()) {
             dialogVisibilitySupplier = () -> {
-                assert mStartSurface != null;
+                assert mStartSurfaceSupplier.get() != null;
                 assert getToolbarManager().getBottomControlsCoordinator() != null;
                 // Return true if dialog from either tab switcher or tab strip is visible.
                 Supplier<Boolean> tabGroupUiDialogVisibilitySupplier =
@@ -1470,7 +1472,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
                                 .getBottomControlsCoordinator()
                                 .getTabGridDialogVisibilitySupplier();
                 Supplier<Boolean> tabSwitcherDialogVisibilitySupplier =
-                        mStartSurface.getTabGridDialogVisibilitySupplier();
+                        mStartSurfaceSupplier.get().getTabGridDialogVisibilitySupplier();
                 boolean isDialogVisible = false;
                 if (tabGroupUiDialogVisibilitySupplier != null) {
                     isDialogVisible = tabGroupUiDialogVisibilitySupplier.get();
@@ -1622,8 +1624,8 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
 
             DefaultBrowserPromoUtils.maybeRecordOutcomeOnStart();
 
-            if (mStartSurface != null && mOverviewShownOnStart) {
-                mStartSurface.onOverviewShownAtLaunch(getOnCreateTimestampMs());
+            if (mStartSurfaceSupplier.get() != null && mOverviewShownOnStart) {
+                mStartSurfaceSupplier.get().onOverviewShownAtLaunch(getOnCreateTimestampMs());
             }
         });
     }
@@ -1979,7 +1981,9 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
                 || state == OverviewModeState.SHOWING_HOMEPAGE
                 || state == OverviewModeState.SHOWING_PREVIOUS
                 || state == OverviewModeState.SHOWING_START);
-        if (mStartSurface != null) mStartSurface.getController().setOverviewState(state);
+        if (mStartSurfaceSupplier.get() != null) {
+            mStartSurfaceSupplier.get().getController().setOverviewState(state);
+        }
 
         if (mOverviewModeController == null || mOverviewModeController.overviewVisible()) return;
 
@@ -2162,7 +2166,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
 
     @VisibleForTesting
     public StartSurface getStartSurface() {
-        return mStartSurface;
+        return mStartSurfaceSupplier.get();
     }
 
     private ComposedBrowserControlsVisibilityDelegate getAppBrowserControlsVisibilityDelegate() {

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

@@ -51,6 +51,7 @@ import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.ObservableSupplierImpl;
 import org.chromium.base.supplier.OneshotSupplier;
+import org.chromium.base.supplier.OneshotSupplierImpl;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.AppHooks;
@@ -390,7 +391,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         return new RootUiCoordinator(this, null, getShareDelegateSupplier(),
                 getActivityTabProvider(), mTabModelProfileSupplier, mBookmarkBridgeSupplier,
                 getOverviewModeBehaviorSupplier(), this::getContextualSearchManager,
-                mTabModelSelectorSupplier);
+                mTabModelSelectorSupplier, new OneshotSupplierImpl<>());
     }
 
     private NotificationManagerProxy getNotificationManagerProxy() {

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

@@ -157,7 +157,7 @@ public abstract class BaseCustomTabActivity extends ChromeActivity<BaseCustomTab
                         -> mNavigationController,
                 getActivityTabProvider(), mTabModelProfileSupplier, mBookmarkBridgeSupplier,
                 mOverviewModeBehaviorSupplier, this::getContextualSearchManager,
-                mTabModelSelectorSupplier);
+                mTabModelSelectorSupplier, new OneshotSupplierImpl<>());
     }
 
     @Override

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

@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.reengagement.ReengagementNotificationControll
 import org.chromium.chrome.browser.share.ShareDelegate;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.ui.RootUiCoordinator;
+import org.chromium.chrome.features.start_surface.StartSurface;
 import org.chromium.components.feature_engagement.Tracker;
 
 /**
@@ -41,10 +42,11 @@ public class BaseCustomTabRootUiCoordinator
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
             OneshotSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
             Supplier<ContextualSearchManager> contextualSearchManagerSupplier,
-            ObservableSupplier<TabModelSelector> tabModelSelectorSupplier) {
+            ObservableSupplier<TabModelSelector> tabModelSelectorSupplier,
+            OneshotSupplier<StartSurface> startSurfaceSupplier) {
         super(activity, null, shareDelegateSupplier, tabProvider, profileSupplier,
                 bookmarkBridgeSupplier, overviewModeBehaviorSupplier,
-                contextualSearchManagerSupplier, tabModelSelectorSupplier);
+                contextualSearchManagerSupplier, tabModelSelectorSupplier, startSurfaceSupplier);
         mToolbarCoordinator = customTabToolbarCoordinator;
         mNavigationController = customTabNavigationController;
     }

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

@@ -55,6 +55,7 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
 import org.chromium.chrome.browser.ui.default_browser_promo.DefaultBrowserPromoUtils;
 import org.chromium.chrome.browser.ui.tablet.emptybackground.EmptyBackgroundViewWrapper;
 import org.chromium.chrome.browser.vr.VrModuleProvider;
+import org.chromium.chrome.features.start_surface.StartSurface;
 import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
 import org.chromium.components.browser_ui.util.ComposedBrowserControlsVisibilityDelegate;
 import org.chromium.components.browser_ui.widget.scrim.ScrimCoordinator;
@@ -96,6 +97,7 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
      * @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}.
+     * @param startSurfaceSupplier Supplier of the {@link StartSurface}.
      */
     public TabbedRootUiCoordinator(ChromeActivity activity,
             Callback<Boolean> onOmniboxFocusChangedListener,
@@ -107,10 +109,11 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
             OneshotSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
             Supplier<ContextualSearchManager> contextualSearchManagerSupplier,
-            ObservableSupplier<TabModelSelector> tabModelSelectorSupplier) {
+            ObservableSupplier<TabModelSelector> tabModelSelectorSupplier,
+            OneshotSupplier<StartSurface> startSurfaceSupplier) {
         super(activity, onOmniboxFocusChangedListener, shareDelegateSupplier, tabProvider,
                 profileSupplier, bookmarkBridgeSupplier, overviewModeBehaviorSupplier,
-                contextualSearchManagerSupplier, tabModelSelectorSupplier);
+                contextualSearchManagerSupplier, tabModelSelectorSupplier, startSurfaceSupplier);
         mIntentWithEffect = intentWithEffect;
         mEphemeralTabCoordinatorSupplier = ephemeralTabCoordinatorSupplier;
         mCanAnimateBrowserControls = () -> {

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

@@ -22,6 +22,7 @@ import org.chromium.chrome.browser.tab.TabObserver;
 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.components.browser_ui.bottomsheet.BottomSheetContent;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.StateChangeReason;
@@ -59,6 +60,9 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
     /** A tab observer that is only attached to the active tab. */
     private final TabObserver mTabObserver;
 
+    /** The supplier of {@link StartSurface} instance. */
+    private final Supplier<StartSurface> mStartSurfaceSupplier;
+
     /** A browser controls manager for polling browser controls offsets. */
     private BrowserControlsVisibilityManager mBrowserControlsVisibilityManager;
 
@@ -114,7 +118,8 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
             Supplier<SnackbarManager> snackbarManagerSupplier,
             TabObscuringHandler obscuringDelegate,
             ObservableSupplier<Boolean> omniboxFocusStateSupplier,
-            Supplier<OverlayPanelManager> overlayManager) {
+            Supplier<OverlayPanelManager> overlayManager,
+            Supplier<StartSurface> startSurfaceSupplier) {
         mSheetController = controller;
         mTabProvider = tabProvider;
         mBrowserControlsVisibilityManager = controlsVisibilityManager;
@@ -125,6 +130,7 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
         mTabObscuringToken = TokenHolder.INVALID_TOKEN;
         mOmniboxFocusStateSupplier = omniboxFocusStateSupplier;
         mOverlayPanelManager = overlayManager;
+        mStartSurfaceSupplier = startSurfaceSupplier;
 
         mSheetController.addObserver(this);
         mSheetController.setAccssibilityUtil(ChromeAccessibilityUtil.get());
@@ -159,9 +165,12 @@ class BottomSheetManager extends EmptyBottomSheetObserver implements Destroyable
             @Override
             public void onActivityTabChanged(Tab tab) {
                 // Temporarily suppress the sheet if entering a state where there is no activity
-                // tab.
+                // tab and the Start surface homepage isn't showing.
                 if (tab == null) {
-                    mToken = controller.suppressSheet(StateChangeReason.COMPOSITED_UI);
+                    if (mStartSurfaceSupplier.get() == null
+                            || !mStartSurfaceSupplier.get().getController().isHomePageShowing()) {
+                        mToken = controller.suppressSheet(StateChangeReason.COMPOSITED_UI);
+                    }
                     return;
                 }
 

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

@@ -70,6 +70,7 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator;
 import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinatorFactory;
 import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.vr.VrModuleProvider;
+import org.chromium.chrome.features.start_surface.StartSurface;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.SheetState;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetControllerFactory;
@@ -154,6 +155,7 @@ public class RootUiCoordinator
     @Nullable
     private BrowserControlsManager mBrowserControlsManager;
     private ObservableSupplier<TabModelSelector> mTabModelSelectorSupplier;
+    private final OneshotSupplier<StartSurface> mStartSurfaceSupplier;
     @Nullable
     private MessageQueueManager mMessageQueueManager;
 
@@ -171,6 +173,7 @@ public class RootUiCoordinator
      *                                     profile.
      * @param contextualSearchManagerSupplier Supplier of the {@link ContextualSearchManager}.
      * @param tabModelSelectorSupplier Supplier of the {@link TabModelSelector}.
+     * @param startSurfaceSupplier Supplier of the {@link StartSurface}.
      */
     public RootUiCoordinator(ChromeActivity activity,
             @Nullable Callback<Boolean> onOmniboxFocusChangedListener,
@@ -179,7 +182,8 @@ public class RootUiCoordinator
             ObservableSupplier<BookmarkBridge> bookmarkBridgeSupplier,
             OneshotSupplier<OverviewModeBehavior> overviewModeBehaviorSupplier,
             Supplier<ContextualSearchManager> contextualSearchManagerSupplier,
-            ObservableSupplier<TabModelSelector> tabModelSelectorSupplier) {
+            ObservableSupplier<TabModelSelector> tabModelSelectorSupplier,
+            OneshotSupplier<StartSurface> startSurfaceSupplier) {
         mCallbackController = new CallbackController();
         mActivity = activity;
         mOnOmniboxFocusChangedListener = onOmniboxFocusChangedListener;
@@ -209,6 +213,7 @@ public class RootUiCoordinator
         mOverviewModeBehaviorSupplier = overviewModeBehaviorSupplier;
         mOverviewModeBehaviorSupplier.onAvailable(
                 mCallbackController.makeCancelable(this::setOverviewModeBehavior));
+        mStartSurfaceSupplier = startSurfaceSupplier;
 
         if (CachedFeatureFlags.isEnabled(ChromeFeatureList.MESSAGES_FOR_ANDROID_INFRASTRUCTURE)) {
             mMessageQueueManager =
@@ -693,7 +698,8 @@ public class RootUiCoordinator
         mBottomSheetManager = new BottomSheetManager(mBottomSheetController, mActivityTabProvider,
                 mActivity.getBrowserControlsManager(), mActivity.getFullscreenManager(),
                 mActivity::getModalDialogManager, this::getBottomSheetSnackbarManager,
-                mTabObscuringHandler, mOmniboxFocusStateSupplier, panelManagerSupplier);
+                mTabObscuringHandler, mOmniboxFocusStateSupplier, panelManagerSupplier,
+                mStartSurfaceSupplier);
     }
 
     /**

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

@@ -21,6 +21,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.supplier.ObservableSupplier;
+import org.chromium.base.supplier.OneshotSupplierImpl;
 import org.chromium.base.task.PostTask;
 import org.chromium.base.task.TaskTraits;
 import org.chromium.base.test.util.CommandLineFlags;
@@ -170,7 +171,8 @@ public class ShareIntentTest {
         RootUiCoordinator rootUiCoordinator = TestThreadUtils.runOnUiThreadBlocking(() -> {
             return new RootUiCoordinator(mockActivity, null,
                     mockActivity.getShareDelegateSupplier(), mockActivity.getActivityTabProvider(),
-                    null, null, mockActivity.getOverviewModeBehaviorSupplier(), null, null);
+                    null, null, mockActivity.getOverviewModeBehaviorSupplier(), null, null,
+                    new OneshotSupplierImpl<>());
         });
         ShareHelper.setLastShareComponentName(new ComponentName("test.package", "test.activity"));
         // Skips the capture of screenshot and notifies with an empty file.