[ToolbarMVC] Remove ToolbarLayout#getMenuButton

As a consequence of removing getMenuButton, this CL also reduces direct usage of MenuButton in ToolbarPhone/ToolbarTablet, although a few exceptions (getNextFocusForward, animations) remain.

Bug: 1086676
Change-Id: Ic1f13733b48a06b34d9ec2484ca2971f79f2f19f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412702Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807638}

chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java

@@ -761,7 +761,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
 
     @Override
     public @Nullable View getMenuButtonView() {
-        return mToolbar.getMenuButton();
+        return mMenuButtonCoordinator.getMenuButton().getImageButton();
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/toolbar/menu_button/MenuButton.java

@@ -20,11 +20,9 @@ import android.view.View;
 import android.widget.FrameLayout;
 import android.widget.ImageButton;
 import android.widget.ImageView;
-
 import androidx.annotation.DrawableRes;
 import androidx.annotation.VisibleForTesting;
 import androidx.core.view.ViewCompat;
-
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ThemeColorProvider;
@@ -83,6 +81,12 @@ public class MenuButton extends FrameLayout implements TintObserver {
         return mMenuImageButton;
     }
 
+    @Override
+    public void setOnKeyListener(OnKeyListener onKeyListener) {
+        if (mMenuImageButton == null) return;
+        mMenuImageButton.setOnKeyListener(onKeyListener);
+    }
+
     /**
      * Sets the update badge to visible.
      *

chrome/android/java/src/org/chromium/chrome/browser/toolbar/menu_button/MenuButtonCoordinator.java

@@ -6,11 +6,10 @@ package org.chromium.chrome.browser.toolbar.menu_button;
 
 import android.app.Activity;
 import android.content.res.ColorStateList;
-
+import android.view.View.OnKeyListener;
 import androidx.annotation.IdRes;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
-
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.Callback;
 import org.chromium.base.metrics.RecordUserAction;
@@ -133,6 +132,22 @@ public class MenuButtonCoordinator implements AppMenuObserver {
         mMenuButton.setThemeColorProvider(mThemeColorProvider);
     }
 
+    /**
+     * Handle the key press event on the menu button.
+     * @return Whether the app menu was shown as a result of this action.
+     */
+    public boolean onEnterKeyPress() {
+        if (mAppMenuButtonHelper == null || mMenuButton == null) return false;
+        return mAppMenuButtonHelper.onEnterKeyPress(mMenuButton.getImageButton());
+    }
+
+    /**
+     * @return Whether the menu button is present and visible.
+     */
+    public boolean isShown() {
+        return mMenuButton != null && mMenuButton.isShown();
+    }
+
     /**
      * Get the underlying MenuButton view. Present for legacy reasons only; don't add new usages.
      */
@@ -161,6 +176,15 @@ public class MenuButtonCoordinator implements AppMenuObserver {
         mMenuButton.setClickable(isClickable);
     }
 
+    /**
+     * Sets the on key listener for the underlying menu button.
+     * @param onKeyListener Listener for key events.
+     */
+    public void setOnKeyListener(OnKeyListener onKeyListener) {
+        if (mMenuButton == null) return;
+        mMenuButton.setOnKeyListener(onKeyListener);
+    }
+
     public void destroy() {
         if (mAppMenuButtonHelper != null) {
             mAppMenuHandler.removeObserver(this);

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarLayout.java

@@ -16,14 +16,11 @@ import android.view.MotionEvent;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.FrameLayout;
-import android.widget.ImageButton;
 import android.widget.ProgressBar;
-
 import androidx.annotation.CallSuper;
 import androidx.annotation.ColorRes;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
-
 import org.chromium.base.ObserverList;
 import org.chromium.base.TraceEvent;
 import org.chromium.chrome.R;
@@ -359,14 +356,6 @@ public abstract class ToolbarLayout
         mMenuButtonCoordinator = menuButtonCoordinator;
     }
 
-    /**
-     * @return The {@link ImageButton} containing the menu button.
-     */
-    ImageButton getMenuButton() {
-        if (mMenuButtonCoordinator.getMenuButton() == null) return null;
-        return mMenuButtonCoordinator.getMenuButton().getImageButton();
-    }
-
     /**
      * @return The {@link ProgressBar} this layout uses.
      */

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java

@@ -436,9 +436,8 @@ public class ToolbarPhone extends ToolbarLayout implements Invalidator.Client, O
         mToggleTabStackButton.setOnKeyListener(new KeyboardNavigationListener() {
             @Override
             public View getNextFocusForward() {
-                final ImageButton menuButton = getMenuButton();
-                if (menuButton != null && menuButton.isShown()) {
-                    return menuButton;
+                if (getMenuButtonCoordinator().isShown()) {
+                    return getMenuButtonCoordinator().getMenuButton();
                 } else {
                     return getCurrentTabView();
                 }
@@ -466,24 +465,23 @@ public class ToolbarPhone extends ToolbarLayout implements Invalidator.Client, O
             mHomeButton.setOnClickListener(this);
         }
 
-        if (getMenuButton() != null) {
-            getMenuButton().setOnKeyListener(new KeyboardNavigationListener() {
-                @Override
-                public View getNextFocusForward() {
-                    return getCurrentTabView();
-                }
+        getMenuButtonCoordinator().setOnKeyListener(new KeyboardNavigationListener() {
+            @Override
+            public View getNextFocusForward() {
+                return getCurrentTabView();
+            }
 
-                @Override
-                public View getNextFocusBackward() {
-                    return mToggleTabStackButton;
-                }
+            @Override
+            public View getNextFocusBackward() {
+                return mToggleTabStackButton;
+            }
+
+            @Override
+            protected boolean handleEnterKeyPress() {
+                return getMenuButtonCoordinator().onEnterKeyPress();
+            }
+        });
 
-                @Override
-                protected boolean handleEnterKeyPress() {
-                    return getMenuButtonHelper().onEnterKeyPress(getMenuButton());
-                }
-            });
-        }
         onHomeButtonUpdate(HomepageManager.isHomepageEnabled());
 
         updateVisualsForLocationBarState();
@@ -1880,9 +1878,7 @@ public class ToolbarPhone extends ToolbarLayout implements Invalidator.Client, O
             mToggleTabStackButton.setVisibility(isGone ? GONE : VISIBLE);
         }
 
-        if (getMenuButton() != null) {
-            getMenuButton().setVisibility(inTabSwitcherMode ? GONE : VISIBLE);
-        }
+        getMenuButtonCoordinator().setVisibility(inTabSwitcherMode ? GONE : VISIBLE);
 
         triggerUrlFocusAnimation(inTabSwitcherMode && !urlHasFocus());
 
@@ -2600,9 +2596,7 @@ public class ToolbarPhone extends ToolbarLayout implements Invalidator.Client, O
      * buttons besides the optional button.
      */
     private boolean isMenuButtonPresent() {
-        final ImageButton menuButton = getMenuButton();
-        if (menuButton == null) return false;
-        return menuButton.isShown();
+        return getMenuButtonCoordinator().isShown();
     }
 
     private void requestLayoutHostUpdateForOptionalButton() {

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarTablet.java

@@ -248,7 +248,7 @@ public class ToolbarTablet extends ToolbarLayout
         mBookmarkButton.setOnClickListener(this);
         mBookmarkButton.setOnLongClickListener(this);
 
-        getMenuButton().setOnKeyListener(new KeyboardNavigationListener() {
+        getMenuButtonCoordinator().setOnKeyListener(new KeyboardNavigationListener() {
             @Override
             public View getNextFocusForward() {
                 return getCurrentTabView();
@@ -261,7 +261,7 @@ public class ToolbarTablet extends ToolbarLayout
 
             @Override
             protected boolean handleEnterKeyPress() {
-                return getMenuButtonHelper().onEnterKeyPress(getMenuButton());
+                return getMenuButtonCoordinator().onEnterKeyPress();
             }
         });
         if (HomepageManager.isHomepageEnabled()) {

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/TopToolbarCoordinator.java

@@ -10,7 +10,6 @@ import android.graphics.drawable.Drawable;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.View.OnLongClickListener;
-import android.widget.ImageButton;
 
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
@@ -266,13 +265,6 @@ public class TopToolbarCoordinator implements Toolbar {
         return mMenuButtonCoordinator.getMenuButton();
     }
 
-    /**
-     * @return The {@link ImageButton} containing the menu button.
-     */
-    public @Nullable ImageButton getMenuButton() {
-        return mToolbarLayout.getMenuButton();
-    }
-
     @Override
     public ToolbarProgressBar getProgressBar() {
         return mToolbarLayout.getProgressBar();