Add AppMenuBlocker and allow multiple classes to register as blockers

Add an AppMenuBlocker interface with a simple #canShowAppMenu method.
Split ChromeActivity#shouldShowAppMenu implementation between activity
class and RootUiCoordinator, moving UI-related checks into
RootUiCoordinator#canShowAppMenu. Register both ChromeActivity and
RootUiCoordinator as AppMenuBlocker's.

BUG=956260

Change-Id: Ic0889d349287e687dbf239e5487b51cc3e45ac90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628231
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664023}

chrome/android/chrome_java_sources.gni

@@ -71,6 +71,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/accessibility/FontSizePrefs.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenu.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java",
+  "java/src/org/chromium/chrome/browser/appmenu/AppMenuBlocker.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuButtonHelper.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuButtonHelperImpl.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuCoordinator.java",

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

@@ -57,6 +57,7 @@ import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.IntentHandler.IntentHandlerDelegate;
 import org.chromium.chrome.browser.IntentHandler.TabOpenType;
+import org.chromium.chrome.browser.appmenu.AppMenuBlocker;
 import org.chromium.chrome.browser.appmenu.AppMenuDelegate;
 import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegate;
 import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegateImpl;
@@ -200,7 +201,8 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         extends AsyncInitializationActivity
         implements TabCreatorManager, AccessibilityStateChangeListener, PolicyChangeListener,
                    ContextualSearchTabPromotionDelegate, SnackbarManageable, SceneChangeObserver,
-                   StatusBarColorController.StatusBarColorProvider, AppMenuDelegate {
+                   StatusBarColorController.StatusBarColorProvider, AppMenuDelegate,
+                   AppMenuBlocker {
     /**
      * No control container to inflate during initialization.
      */
@@ -1538,7 +1540,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
 
     @CallSuper
     @Override
-    public boolean shouldShowAppMenu() {
+    public boolean canShowAppMenu() {
         if (isActivityFinishingOrDestroyed()) return false;
 
         @ActivityState
@@ -1548,22 +1550,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
             return false;
         }
 
-        // TODO(https://crbug.com/956260): Move UI state related logic out of ChromeActivity.
-
-        // Do not show the menu if Contextual Search panel is opened.
-        if (mContextualSearchManager != null && mContextualSearchManager.isSearchPanelOpened()) {
-            return false;
-        }
-
-        if (getEphemeralTabPanel() != null && getEphemeralTabPanel().isPanelOpened()) {
-            return false;
-        }
-
-        // Do not show the menu if we are in find in page view.
-        if (mFindToolbarManager != null && mFindToolbarManager.isShowing() && !isTablet()) {
-            return false;
-        }
-
         return true;
     }
 

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

@@ -2256,7 +2256,7 @@ public class ChromeTabbedActivity
     // App Menu related code -----------------------------------------------------------------------
 
     @Override
-    public boolean shouldShowAppMenu() {
+    public boolean canShowAppMenu() {
         // The popup menu relies on the model created during the full UI initialization, so do not
         // attempt to show the menu until the UI creation has finished.
         if (!mUIWithNativeInitialized) return false;
@@ -2266,7 +2266,7 @@ public class ChromeTabbedActivity
         Tab tab = getActivityTab();
         if (tab != null && TabModalPresenter.isDialogShowing(tab)) return false;
 
-        return super.shouldShowAppMenu();
+        return super.canShowAppMenu();
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuBlocker.java

+// Copyright 2019 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.browser.appmenu;
+
+/**
+ * An interface that may be used to block the app menu from showing (e.g. when other conflicting UI
+ * is showing). To register, see {@link AppMenuCoordinator#registerAppMenuBlocker(AppMenuBlocker)}.
+ */
+public interface AppMenuBlocker {
+    /**
+     * @return Whether the app menu can be shown.
+     */
+    boolean canShowAppMenu();
+}

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuCoordinator.java

@@ -26,4 +26,15 @@ public interface AppMenuCoordinator {
      * @return The {@link AppMenuPropertiesDelegate} associated with this activity.
      */
     AppMenuPropertiesDelegate getAppMenuPropertiesDelegate();
+
+    /**
+     * Registers an {@link AppMenuBlocker} used to help determine whether the app menu can be shown.
+     * @param blocker An {@link AppMenuBlocker} to check before attempting to show the app menu.
+     */
+    void registerAppMenuBlocker(AppMenuBlocker blocker);
+
+    /**
+     * @param blocker The {@link AppMenuBlocker} to unregister.
+     */
+    void unregisterAppMenuBlocker(AppMenuBlocker blocker);
 }

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuCoordinatorImpl.java

@@ -118,7 +118,7 @@ class AppMenuCoordinatorImpl implements AppMenuCoordinator {
 
     @Override
     public void showAppMenuForKeyboardEvent() {
-        if (mAppMenuHandler == null || !mAppMenuDelegate.shouldShowAppMenu()) return;
+        if (mAppMenuHandler == null || !mAppMenuHandler.shouldShowAppMenu()) return;
 
         boolean hasPermanentMenuKey = ViewConfiguration.get(mContext).hasPermanentMenuKey();
         mAppMenuHandler.showAppMenu(
@@ -136,6 +136,16 @@ class AppMenuCoordinatorImpl implements AppMenuCoordinator {
         return mAppMenuPropertiesDelegate;
     }
 
+    @Override
+    public void registerAppMenuBlocker(AppMenuBlocker blocker) {
+        mAppMenuHandler.registerAppMenuBlocker(blocker);
+    }
+
+    @Override
+    public void unregisterAppMenuBlocker(AppMenuBlocker blocker) {
+        mAppMenuHandler.unregisterAppMenuBlocker(blocker);
+    }
+
     // Testing methods
 
     @VisibleForTesting

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuDelegate.java

@@ -23,9 +23,4 @@ public interface AppMenuDelegate {
      *         should be using.
      */
     AppMenuPropertiesDelegate createAppMenuPropertiesDelegate();
-
-    /**
-     * @return Whether the app menu should be shown.
-     */
-    boolean shouldShowAppMenu();
 }
\ No newline at end of file

chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuHandlerImpl.java

@@ -31,6 +31,7 @@ import org.chromium.chrome.browser.util.ObservableSupplier;
 import org.chromium.chrome.browser.widget.textbubble.TextBubble;
 
 import java.util.ArrayList;
+import java.util.List;
 
 /**
  * Object responsible for handling the creation, showing, hiding of the AppMenu and notifying the
@@ -42,7 +43,8 @@ class AppMenuHandlerImpl implements AppMenuHandler, StartStopWithNativeObserver,
     private AppMenu mAppMenu;
     private AppMenuDragHelper mAppMenuDragHelper;
     private Menu mMenu;
-    private final ArrayList<AppMenuObserver> mObservers;
+    private final List<AppMenuBlocker> mBlockers;
+    private final List<AppMenuObserver> mObservers;
     private final int mMenuResourceId;
     private final View mHardwareButtonMenuAnchor;
 
@@ -86,6 +88,7 @@ class AppMenuHandlerImpl implements AppMenuHandler, StartStopWithNativeObserver,
         mAppMenuDelegate = appMenuDelegate;
         mDelegate = delegate;
         mDecorView = decorView;
+        mBlockers = new ArrayList<>();
         mObservers = new ArrayList<>();
         mMenuResourceId = menuResourceId;
         mHardwareButtonMenuAnchor = mDecorView.findViewById(R.id.menu_anchor_stub);
@@ -163,7 +166,7 @@ class AppMenuHandlerImpl implements AppMenuHandler, StartStopWithNativeObserver,
     // TODO(crbug.com/635567): Fix this properly.
     @SuppressLint("ResourceType")
     boolean showAppMenu(View anchorView, boolean startDragging, boolean showFromBottom) {
-        if (!mAppMenuDelegate.shouldShowAppMenu() || isAppMenuShowing()) return false;
+        if (!shouldShowAppMenu() || isAppMenuShowing()) return false;
 
         TextBubble.dismissBubbles();
         boolean isByPermanentButton = false;
@@ -343,4 +346,26 @@ class AppMenuHandlerImpl implements AppMenuHandler, StartStopWithNativeObserver,
     void onFooterViewInflated(View view) {
         if (mDelegate != null) mDelegate.onFooterViewInflated(this, view);
     }
+
+    /**
+     * Registers an {@link AppMenuBlocker} used to help determine whether the app menu can be shown.
+     * @param blocker An {@link AppMenuBlocker} to check before attempting to show the app menu.
+     */
+    void registerAppMenuBlocker(AppMenuBlocker blocker) {
+        if (!mBlockers.contains(blocker)) mBlockers.add(blocker);
+    }
+
+    /**
+     * @param blocker The {@link AppMenuBlocker} to unregister.
+     */
+    void unregisterAppMenuBlocker(AppMenuBlocker blocker) {
+        mBlockers.remove(blocker);
+    }
+
+    boolean shouldShowAppMenu() {
+        for (int i = 0; i < mBlockers.size(); i++) {
+            if (!mBlockers.get(i).canShowAppMenu()) return false;
+        }
+        return true;
+    }
 }

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

@@ -704,10 +704,10 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent
     }
 
     @Override
-    public boolean shouldShowAppMenu() {
+    public boolean canShowAppMenu() {
         if (getActivityTab() == null || !getToolbarManager().isInitialized()) return false;
 
-        return super.shouldShowAppMenu();
+        return super.canShowAppMenu();
     }
 
     @Override

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

@@ -9,10 +9,12 @@ import android.support.annotation.Nullable;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.appmenu.AppMenuBlocker;
 import org.chromium.chrome.browser.appmenu.AppMenuCoordinator;
 import org.chromium.chrome.browser.appmenu.AppMenuCoordinatorFactory;
 import org.chromium.chrome.browser.lifecycle.Destroyable;
 import org.chromium.chrome.browser.lifecycle.InflationObserver;
+import org.chromium.ui.base.DeviceFormFactor;
 
 /**
  * The root UI coordinator. This class will eventually be responsible for inflating and managing
@@ -21,8 +23,9 @@ import org.chromium.chrome.browser.lifecycle.InflationObserver;
  * The specific things this component will manage and how it will hook into Chrome*Activity are
  * still being discussed See https://crbug.com/931496.
  */
-public class RootUiCoordinator
-        implements Destroyable, InflationObserver, ChromeActivity.MenuOrKeyboardActionHandler {
+public class RootUiCoordinator implements Destroyable, InflationObserver,
+                                          ChromeActivity.MenuOrKeyboardActionHandler,
+                                          AppMenuBlocker {
     protected ChromeActivity mActivity;
     protected @Nullable AppMenuCoordinator mAppMenuCoordinator;
 
@@ -41,7 +44,11 @@ public class RootUiCoordinator
     public void destroy() {
         mActivity.unregisterMenuOrKeyboardActionHandler(this);
         mActivity = null;
-        if (mAppMenuCoordinator != null) mAppMenuCoordinator.destroy();
+        if (mAppMenuCoordinator != null) {
+            mAppMenuCoordinator.unregisterAppMenuBlocker(this);
+            mAppMenuCoordinator.unregisterAppMenuBlocker(mActivity);
+            mAppMenuCoordinator.destroy();
+        }
     }
 
     @Override
@@ -59,6 +66,8 @@ public class RootUiCoordinator
             mActivity.getToolbarManager().onAppMenuInitialized(
                     mAppMenuCoordinator.getAppMenuHandler(),
                     mAppMenuCoordinator.getAppMenuPropertiesDelegate());
+            mAppMenuCoordinator.registerAppMenuBlocker(this);
+            mAppMenuCoordinator.registerAppMenuBlocker(mActivity);
         } else if (mActivity.getToolbarManager() != null) {
             mActivity.getToolbarManager().getToolbar().disableMenuButton();
         }
@@ -78,4 +87,30 @@ public class RootUiCoordinator
     public AppMenuCoordinator getAppMenuCoordinatorForTesting() {
         return mAppMenuCoordinator;
     }
+
+    @Override
+    public boolean canShowAppMenu() {
+        // TODO(https:crbug.com/931496): Eventually the ContextualSearchManager, EphemeralTabPanel,
+        // 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()) {
+            return false;
+        }
+
+        if (mActivity.getEphemeralTabPanel() != null
+                && mActivity.getEphemeralTabPanel().isPanelOpened()) {
+            return false;
+        }
+
+        // Do not show the menu if we are in find in page view.
+        if (mActivity.getFindToolbarManager() != null
+                && mActivity.getFindToolbarManager().isShowing()
+                && !DeviceFormFactor.isNonMultiDisplayContextOnTablet(mActivity)) {
+            return false;
+        }
+
+        return true;
+    }
 }