Don't show top toolbar items when bottom toolbar is enabled

The original CL for this (https://chromium-review.googlesource.com/1105569)
was reverted because of a null pointer bug. This CL does the same thing
as the reverted CL and fixes the null pointer exception by having
ToolbarManager#getMenuButton() return the bottom toolbar menu button when
the bottom toolbar is enabled.

Bug: 852117

Change-Id: Ic2668a3cac6a552bee4476d1cfb58f8e6c290bfd
Reviewed-on: https://chromium-review.googlesource.com/1135776Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575372}

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

@@ -143,6 +143,10 @@ public class BottomToolbarCoordinator {
         mMediator.setToolbarSwipeLayout(layout);
     }
 
+    public View getMenuButton() {
+        return mMenuButton.getMenuButton();
+    }
+
     /**
      * Clean up any state when the bottom toolbar is destroyed.
      */

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

@@ -54,4 +54,8 @@ class MenuButton extends FrameLayout {
     boolean isShowingAppMenuUpdateBadge() {
         return mUpdateBadgeView.getVisibility() == View.VISIBLE;
     }
+
+    View getMenuButton() {
+        return mMenuButtonView;
+    }
 }

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

@@ -41,6 +41,7 @@ import org.chromium.chrome.browser.omnibox.UrlBarData;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
+import org.chromium.chrome.browser.util.FeatureUtilities;
 import org.chromium.chrome.browser.util.ViewUtils;
 import org.chromium.chrome.browser.widget.PulseDrawable;
 import org.chromium.chrome.browser.widget.ScrimView;
@@ -155,6 +156,12 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
         mMenuButton = (TintedImageButton) findViewById(R.id.menu_button);
         mMenuBadge = (ImageView) findViewById(R.id.menu_badge);
         mMenuButtonWrapper = findViewById(R.id.menu_button_wrapper);
+        if (FeatureUtilities.isBottomToolbarEnabled()) {
+            UiUtils.removeViewFromParent(mMenuButtonWrapper);
+            mMenuButtonWrapper = null;
+            mMenuButton = null;
+            mMenuBadge = null;
+        }
 
         // Initialize the provider to an empty version to avoid null checking everywhere.
         mToolbarDataProvider = new ToolbarDataProvider() {
@@ -271,9 +278,11 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
 
         mAppMenuButtonHelper = appMenuButtonHelper;
 
+        if (mMenuButton != null) {
             mMenuButton.setOnTouchListener(mAppMenuButtonHelper);
             mMenuButton.setAccessibilityDelegate(mAppMenuButtonHelper);
         }
+    }
 
     /** Notified that the menu was shown. */
     protected void onMenuShown() {}
@@ -818,6 +827,7 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
 
     @Override
     public void removeAppMenuUpdateBadge(boolean animate) {
+        if (mMenuBadge == null) return;
         boolean wasShowingMenuBadge = mShowMenuBadge;
         mShowMenuBadge = false;
         setMenuButtonContentDescription(false);
@@ -876,6 +886,7 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
      * bitmap.
      */
     protected void setAppMenuUpdateBadgeToVisible(boolean animate) {
+        if (mMenuBadge == null || mMenuButton == null) return;
         setMenuButtonContentDescription(true);
         if (!animate || mIsMenuBadgeAnimationRunning) {
             mMenuBadge.setVisibility(View.VISIBLE);
@@ -920,6 +931,7 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
      * @param useLightDrawable Whether the light drawable should be used.
      */
     protected void setAppMenuUpdateBadgeDrawable(boolean useLightDrawable) {
+        if (mMenuBadge == null) return;
         mMenuBadge.setImageResource(useLightDrawable ? R.drawable.badge_update_light
                 : R.drawable.badge_update_dark);
     }
@@ -931,7 +943,7 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
      */
     protected void setMenuButtonHighlightDrawable(boolean highlighting) {
         // Return if onFinishInflate didn't finish
-        if (mMenuButtonWrapper == null) return;
+        if (mMenuButtonWrapper == null || mMenuButton == null) return;
 
         if (highlighting) {
             if (mHighlightDrawable == null) {
@@ -953,6 +965,7 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
      * @param isUpdateBadgeVisible Whether the update menu badge is visible.
      */
     protected void setMenuButtonContentDescription(boolean isUpdateBadgeVisible) {
+        if (mMenuButton == null) return;
         if (isUpdateBadgeVisible) {
             mMenuButton.setContentDescription(getResources().getString(
                     R.string.accessibility_toolbar_btn_menu_update));

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

@@ -866,6 +866,8 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
      * @return The view containing the pop up menu button.
      */
     public View getMenuButton() {
+        if (mBottomToolbarCoordinator != null) return mBottomToolbarCoordinator.getMenuButton();
+
         return mToolbar.getMenuButton();
     }
 
@@ -1313,8 +1315,10 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
         updateReloadState(tabCrashed);
         updateBookmarkButtonStatus();
 
+        if (mToolbar.getMenuButtonWrapper() != null) {
             mToolbar.getMenuButtonWrapper().setVisibility(View.VISIBLE);
         }
+    }
 
     private void updateBookmarkButtonStatus() {
         Tab currentTab = mToolbarModel.getTab();

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

@@ -83,6 +83,7 @@ import org.chromium.chrome.browser.widget.incognitotoggle.IncognitoToggleButton;
 import org.chromium.chrome.browser.widget.newtab.NewTabButton;
 import org.chromium.chrome.browser.widget.textbubble.TextBubble;
 import org.chromium.components.feature_engagement.EventConstants;
+import org.chromium.ui.UiUtils;
 import org.chromium.ui.base.LocalizationUtils;
 import org.chromium.ui.interpolators.BakedBezierInterpolator;
 
@@ -375,6 +376,10 @@ public class ToolbarPhone extends ToolbarLayout
             mToolbarButtonsContainer = (ViewGroup) findViewById(R.id.toolbar_buttons);
 
             mHomeButton = (TintedImageButton) findViewById(R.id.home_button);
+            if (FeatureUtilities.isBottomToolbarEnabled() && mHomeButton != null) {
+                UiUtils.removeViewFromParent(mHomeButton);
+                mHomeButton = null;
+            }
 
             mUrlBar = (TextView) findViewById(R.id.url_bar);
 
@@ -389,7 +394,7 @@ public class ToolbarPhone extends ToolbarLayout
 
             setLayoutTransition(null);
 
-            getMenuButtonWrapper().setVisibility(View.VISIBLE);
+            if (getMenuButtonWrapper() != null) getMenuButtonWrapper().setVisibility(View.VISIBLE);
             inflateTabSwitchingResources();
 
             setWillNotDraw(false);
@@ -464,14 +469,19 @@ public class ToolbarPhone extends ToolbarLayout
 
     private void inflateTabSwitchingResources() {
         mToggleTabStackButton = (ImageView) findViewById(R.id.tab_switcher_button);
-        mNewTabButton = (NewTabButton) findViewById(R.id.new_tab_button);
-
+        if (FeatureUtilities.isBottomToolbarEnabled()) {
+            UiUtils.removeViewFromParent(mToggleTabStackButton);
+            mToggleTabStackButton = null;
+        } else {
             mToggleTabStackButton.setClickable(false);
             mTabSwitcherButtonDrawable =
                     TabSwitcherDrawable.createTabSwitcherDrawable(getContext(), false);
             mTabSwitcherButtonDrawableLight =
                     TabSwitcherDrawable.createTabSwitcherDrawable(getContext(), true);
             mToggleTabStackButton.setImageDrawable(mTabSwitcherButtonDrawable);
+        }
+
+        mNewTabButton = (NewTabButton) findViewById(R.id.new_tab_button);
         mTabSwitcherModeViews.add(mNewTabButton);
 
         // Ensure that the new tab button will not draw over the toolbar buttons if the
@@ -491,6 +501,7 @@ public class ToolbarPhone extends ToolbarLayout
     }
 
     private void enableTabSwitchingResources() {
+        if (mToggleTabStackButton != null) {
             mToggleTabStackButton.setOnClickListener(this);
             mToggleTabStackButton.setOnLongClickListener(this);
             mToggleTabStackButton.setOnKeyListener(new KeyboardNavigationListener() {
@@ -509,6 +520,7 @@ public class ToolbarPhone extends ToolbarLayout
                     return findViewById(R.id.url_bar);
                 }
             });
+        }
         mNewTabButton.setOnClickListener(this);
         mNewTabButton.setOnLongClickListener(this);
     }
@@ -535,6 +547,7 @@ public class ToolbarPhone extends ToolbarLayout
             if (FeatureUtilities.isNewTabPageButtonEnabled()) changeIconToNTPIcon(mHomeButton);
         }
 
+        if (getMenuButton() != null)
             getMenuButton().setOnKeyListener(new KeyboardNavigationListener() {
                 @Override
                 public View getNextFocusForward() {
@@ -1399,9 +1412,9 @@ public class ToolbarPhone extends ToolbarLayout
         }
 
         // Draw the menu button if necessary.
-        if (!mShowMenuBadge && mTabSwitcherAnimationMenuDrawable != null
-                && mUrlExpansionPercent != 1f) {
         final TintedImageButton menuButton = getMenuButton();
+        if (menuButton != null && !mShowMenuBadge && mTabSwitcherAnimationMenuDrawable != null
+                && mUrlExpansionPercent != 1f) {
             mTabSwitcherAnimationMenuDrawable.setBounds(menuButton.getPaddingLeft(),
                     menuButton.getPaddingTop(),
                     menuButton.getWidth() - menuButton.getPaddingRight(),
@@ -1419,8 +1432,10 @@ public class ToolbarPhone extends ToolbarLayout
         Drawable badgeDrawable = mUseLightDrawablesForTextureCapture
                 ? mTabSwitcherAnimationMenuBadgeLightDrawable
                         : mTabSwitcherAnimationMenuBadgeDarkDrawable;
-        if (mShowMenuBadge && badgeDrawable != null && mUrlExpansionPercent != 1f) {
+
         final View menuBadge = getMenuBadge();
+        if (menuBadge != null && mShowMenuBadge && badgeDrawable != null
+                && mUrlExpansionPercent != 1f) {
             badgeDrawable.setBounds(menuBadge.getPaddingLeft(), menuBadge.getPaddingTop(),
                     menuBadge.getWidth() - menuBadge.getPaddingRight(),
                     menuBadge.getHeight() - menuBadge.getPaddingBottom());
@@ -1791,7 +1806,9 @@ public class ToolbarPhone extends ToolbarLayout
                 // This is to deal with the view going invisible when resuming the activity and
                 // running this animation.  The view is still there and clickable but does not
                 // render and only a layout triggers a refresh.  See crbug.com/306890.
-                if (!mToggleTabStackButton.isEnabled()) requestLayout();
+                if (mToggleTabStackButton != null && !mToggleTabStackButton.isEnabled()) {
+                    requestLayout();
+                }
             }
         });
 
@@ -1975,6 +1992,7 @@ public class ToolbarPhone extends ToolbarLayout
      * switcher mode.
      */
     private void updateTabSwitcherButtonRipple() {
+        if (mToggleTabStackButton == null) return;
         if (mTabSwitcherState == ENTERING_TAB_SWITCHER) {
             mToggleTabStackButton.setBackgroundColor(
                     ApiCompatibilityUtils.getColor(getResources(), android.R.color.transparent));
@@ -2101,16 +2119,19 @@ public class ToolbarPhone extends ToolbarLayout
         float toolbarButtonTranslationX = MathUtils.flipSignIf(
                 URL_FOCUS_TOOLBAR_BUTTONS_TRANSLATION_X_DP, isRtl) * density;
 
+        final View menuButtonWrapper = getMenuButtonWrapper();
+        if (menuButtonWrapper != null) {
             animator = ObjectAnimator.ofFloat(
-                getMenuButtonWrapper(), TRANSLATION_X, toolbarButtonTranslationX);
+                    menuButtonWrapper, TRANSLATION_X, toolbarButtonTranslationX);
             animator.setDuration(URL_FOCUS_TOOLBAR_BUTTONS_DURATION_MS);
             animator.setInterpolator(BakedBezierInterpolator.FADE_OUT_CURVE);
             animators.add(animator);
 
-        animator = ObjectAnimator.ofFloat(getMenuButtonWrapper(), ALPHA, 0);
+            animator = ObjectAnimator.ofFloat(menuButtonWrapper, ALPHA, 0);
             animator.setDuration(URL_FOCUS_TOOLBAR_BUTTONS_DURATION_MS);
             animator.setInterpolator(BakedBezierInterpolator.FADE_OUT_CURVE);
             animators.add(animator);
+        }
 
         if (mToggleTabStackButton != null) {
             animator = ObjectAnimator.ofFloat(
@@ -2139,17 +2160,20 @@ public class ToolbarPhone extends ToolbarLayout
         animator.setInterpolator(BakedBezierInterpolator.TRANSFORM_CURVE);
         animators.add(animator);
 
-        animator = ObjectAnimator.ofFloat(getMenuButtonWrapper(), TRANSLATION_X, 0);
+        final View menuButtonWrapper = getMenuButtonWrapper();
+        if (menuButtonWrapper != null) {
+            animator = ObjectAnimator.ofFloat(menuButtonWrapper, TRANSLATION_X, 0);
             animator.setDuration(URL_FOCUS_TOOLBAR_BUTTONS_DURATION_MS);
             animator.setStartDelay(URL_CLEAR_FOCUS_MENU_DELAY_MS);
             animator.setInterpolator(BakedBezierInterpolator.TRANSFORM_CURVE);
             animators.add(animator);
 
-        animator = ObjectAnimator.ofFloat(getMenuButtonWrapper(), ALPHA, 1);
+            animator = ObjectAnimator.ofFloat(menuButtonWrapper, ALPHA, 1);
             animator.setDuration(URL_FOCUS_TOOLBAR_BUTTONS_DURATION_MS);
             animator.setStartDelay(URL_CLEAR_FOCUS_MENU_DELAY_MS);
             animator.setInterpolator(BakedBezierInterpolator.TRANSFORM_CURVE);
             animators.add(animator);
+        }
 
         if (mToggleTabStackButton != null) {
             animator = ObjectAnimator.ofFloat(mToggleTabStackButton, TRANSLATION_X, 0);
@@ -2644,7 +2668,10 @@ public class ToolbarPhone extends ToolbarLayout
             }
         }
 
+        if (getMenuButton() != null) {
             getMenuButton().setTint(mUseLightToolbarDrawables ? mLightModeTint : mDarkModeTint);
+        }
+
         if (mLocationBar.useModernDesign()) {
             updateModernLocationBarColor(getLocationBarColorForToolbarColor(currentPrimaryColor));
         }
@@ -2687,7 +2714,9 @@ public class ToolbarPhone extends ToolbarLayout
             mNewTabButton.setContentDescription(newTabContentDescription);
         }
 
+        if (getMenuButtonWrapper() != null) {
             getMenuButtonWrapper().setVisibility(View.VISIBLE);
+        }
 
         if (mLocationBar.useModernDesign()) {
             DrawableCompat.setTint(mLocationBarBackground,
@@ -2723,6 +2752,7 @@ public class ToolbarPhone extends ToolbarLayout
 
     @Override
     public void showAppMenuUpdateBadge() {
+        if (getMenuBadge() == null) return;
         super.showAppMenuUpdateBadge();
 
         // Set up variables.
@@ -2745,6 +2775,7 @@ public class ToolbarPhone extends ToolbarLayout
 
     @Override
     public void removeAppMenuUpdateBadge(boolean animate) {
+        if (getMenuBadge() == null) return;
         super.removeAppMenuUpdateBadge(animate);
 
         if (mBrowsingModeViews.contains(getMenuBadge())) {