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

This reverts commit e83d1f12.

Reason for revert: https://crbug.com/857437

Essentially, as well as adding if statements to all usages of mMenuButton within the class, you need to add if statements to all other classes calling 'getMenuButton()' (and ToolbarManager.getMenuButton() transitively).

Unfortunately this isn't so trivial for all the cases (eg, I'm guessing we still want to show the data saver when this feature is enabled, or the download text bubble?), so I'm reverting instead of attempting and TBRing a fix myself. Sorry!

Original change's description:
> Don't show top toolbar items when bottom toolbar is enabled
> 
> Remove the top toolbar buttons (home, tab switcher, and menu buttons)
> from their parent view and set them to null. Also add a bunch of
> null checks since now things can be null.
> 
> Bug: 852117
> Change-Id: I620f35f81388d1f4ac8791a9eb45576566474604
> Reviewed-on: https://chromium-review.googlesource.com/1105569
> Commit-Queue: Pedro Amaral <amaralp@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570578}

TBR=tedchoc@chromium.org,amaralp@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 852117
Change-Id: I2c16533f8dec11c5d11b920693157735e7438e89
Reviewed-on: https://chromium-review.googlesource.com/1124579
Commit-Queue: Peter Conn <peconn@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572193}

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

@@ -23,7 +23,6 @@ import android.util.AttributeSet;
 import android.view.InputDevice;
 import android.view.MotionEvent;
 import android.view.View;
-import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.widget.FrameLayout;
 import android.widget.ImageView;
@@ -42,7 +41,6 @@ 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;
@@ -157,12 +155,6 @@ 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()) {
-            if (mMenuButtonWrapper != null) UiUtils.removeViewFromParent(mMenuButtonWrapper);
-            mMenuButtonWrapper = null;
-            mMenuButton = null;
-            mMenuBadge = null;
-        }
 
         // Initialize the provider to an empty version to avoid null checking everywhere.
         mToolbarDataProvider = new ToolbarDataProvider() {
@@ -279,11 +271,9 @@ 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() {}
@@ -828,7 +818,6 @@ 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);
@@ -887,7 +876,6 @@ 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);
@@ -932,7 +920,6 @@ 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);
     }
@@ -944,7 +931,7 @@ public abstract class ToolbarLayout extends FrameLayout implements Toolbar {
      */
     protected void setMenuButtonHighlightDrawable(boolean highlighting) {
         // Return if onFinishInflate didn't finish
-        if (mMenuButtonWrapper == null || mMenuButton == null) return;
+        if (mMenuButtonWrapper == null) return;
 
         if (highlighting) {
             if (mHighlightDrawable == null) {
@@ -966,7 +953,6 @@ 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

@@ -1261,10 +1261,8 @@ 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,7 +83,6 @@ 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;
 
@@ -376,10 +375,6 @@ 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);
 
@@ -394,7 +389,7 @@ public class ToolbarPhone extends ToolbarLayout
 
             setLayoutTransition(null);
 
-            if (getMenuButtonWrapper() != null) getMenuButtonWrapper().setVisibility(View.VISIBLE);
+            getMenuButtonWrapper().setVisibility(View.VISIBLE);
             inflateTabSwitchingResources();
 
             setWillNotDraw(false);
@@ -469,19 +464,14 @@ public class ToolbarPhone extends ToolbarLayout
 
     private void inflateTabSwitchingResources() {
         mToggleTabStackButton = (ImageView) findViewById(R.id.tab_switcher_button);
-        if (FeatureUtilities.isBottomToolbarEnabled()) {
-            UiUtils.removeViewFromParent(mToggleTabStackButton);
-            mToggleTabStackButton = null;
-        } else {
+        mNewTabButton = (NewTabButton) findViewById(R.id.new_tab_button);
+
         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
@@ -501,7 +491,6 @@ public class ToolbarPhone extends ToolbarLayout
     }
 
     private void enableTabSwitchingResources() {
-        if (mToggleTabStackButton != null) {
         mToggleTabStackButton.setOnClickListener(this);
         mToggleTabStackButton.setOnLongClickListener(this);
         mToggleTabStackButton.setOnKeyListener(new KeyboardNavigationListener() {
@@ -520,7 +509,6 @@ public class ToolbarPhone extends ToolbarLayout
                 return findViewById(R.id.url_bar);
             }
         });
-        }
         mNewTabButton.setOnClickListener(this);
         mNewTabButton.setOnLongClickListener(this);
     }
@@ -547,7 +535,6 @@ public class ToolbarPhone extends ToolbarLayout
             if (FeatureUtilities.isNewTabPageButtonEnabled()) changeIconToNTPIcon(mHomeButton);
         }
 
-        if (getMenuButton() != null)
         getMenuButton().setOnKeyListener(new KeyboardNavigationListener() {
             @Override
             public View getNextFocusForward() {
@@ -1412,9 +1399,9 @@ public class ToolbarPhone extends ToolbarLayout
         }
 
         // Draw the menu button if necessary.
-        final TintedImageButton menuButton = getMenuButton();
-        if (menuButton != null && !mShowMenuBadge && mTabSwitcherAnimationMenuDrawable != null
+        if (!mShowMenuBadge && mTabSwitcherAnimationMenuDrawable != null
                 && mUrlExpansionPercent != 1f) {
+            final TintedImageButton menuButton = getMenuButton();
             mTabSwitcherAnimationMenuDrawable.setBounds(menuButton.getPaddingLeft(),
                     menuButton.getPaddingTop(),
                     menuButton.getWidth() - menuButton.getPaddingRight(),
@@ -1432,10 +1419,8 @@ 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());
@@ -1806,9 +1791,7 @@ 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 != null && !mToggleTabStackButton.isEnabled()) {
-                    requestLayout();
-                }
+                if (!mToggleTabStackButton.isEnabled()) requestLayout();
             }
         });
 
@@ -1992,7 +1975,6 @@ 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));
@@ -2119,19 +2101,16 @@ 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(
-                    menuButtonWrapper, TRANSLATION_X, toolbarButtonTranslationX);
+                getMenuButtonWrapper(), TRANSLATION_X, toolbarButtonTranslationX);
         animator.setDuration(URL_FOCUS_TOOLBAR_BUTTONS_DURATION_MS);
         animator.setInterpolator(BakedBezierInterpolator.FADE_OUT_CURVE);
         animators.add(animator);
 
-            animator = ObjectAnimator.ofFloat(menuButtonWrapper, ALPHA, 0);
+        animator = ObjectAnimator.ofFloat(getMenuButtonWrapper(), 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(
@@ -2160,20 +2139,17 @@ public class ToolbarPhone extends ToolbarLayout
         animator.setInterpolator(BakedBezierInterpolator.TRANSFORM_CURVE);
         animators.add(animator);
 
-        final View menuButtonWrapper = getMenuButtonWrapper();
-        if (menuButtonWrapper != null) {
-            animator = ObjectAnimator.ofFloat(menuButtonWrapper, TRANSLATION_X, 0);
+        animator = ObjectAnimator.ofFloat(getMenuButtonWrapper(), 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(menuButtonWrapper, ALPHA, 1);
+        animator = ObjectAnimator.ofFloat(getMenuButtonWrapper(), 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);
@@ -2668,10 +2644,7 @@ public class ToolbarPhone extends ToolbarLayout
             }
         }
 
-        if (getMenuButton() != null) {
         getMenuButton().setTint(mUseLightToolbarDrawables ? mLightModeTint : mDarkModeTint);
-        }
-
         if (mLocationBar.useModernDesign()) {
             updateModernLocationBarColor(getLocationBarColorForToolbarColor(currentPrimaryColor));
         }
@@ -2714,9 +2687,7 @@ public class ToolbarPhone extends ToolbarLayout
             mNewTabButton.setContentDescription(newTabContentDescription);
         }
 
-        if (getMenuButtonWrapper() != null) {
         getMenuButtonWrapper().setVisibility(View.VISIBLE);
-        }
 
         if (mLocationBar.useModernDesign()) {
             DrawableCompat.setTint(mLocationBarBackground,
@@ -2752,7 +2723,6 @@ public class ToolbarPhone extends ToolbarLayout
 
     @Override
     public void showAppMenuUpdateBadge() {
-        if (getMenuBadge() == null) return;
         super.showAppMenuUpdateBadge();
 
         // Set up variables.
@@ -2775,7 +2745,6 @@ public class ToolbarPhone extends ToolbarLayout
 
     @Override
     public void removeAppMenuUpdateBadge(boolean animate) {
-        if (getMenuBadge() == null) return;
         super.removeAppMenuUpdateBadge(animate);
 
         if (mBrowsingModeViews.contains(getMenuBadge())) {