Introduce appmenu.CustomViewBinder interface for update menu item

Introduce a new CustomViewBinder interface that allows
AppMenuPropertiesDelegates to handle binding for custom menu items.

This interface is used to provide a custom binder for the update menu
item, which relies on UpdateMenuItemHelper methods/classes that will not
be moved to the appmenu/ package when its modularized.

Bug: 966644
Change-Id: I6c49522775c1f0fca3cd64764a0eba6a5b82f3b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872980Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710895}

chrome/android/chrome_java_sources.gni

@@ -85,8 +85,10 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuObserver.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuPropertiesDelegate.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuPropertiesDelegateImpl.java",
+  "java/src/org/chromium/chrome/browser/appmenu/CustomViewBinder.java",
   "java/src/org/chromium/chrome/browser/appmenu/MenuButtonDelegate.java",
   "java/src/org/chromium/chrome/browser/appmenu/AppMenuTestSupport.java",
+  "java/src/org/chromium/chrome/browser/appmenu/UpdateMenuItemViewBinder.java",
   "java/src/org/chromium/chrome/browser/autofill/AutofillExpirationDateFixFlowBridge.java",
   "java/src/org/chromium/chrome/browser/autofill/AutofillExpirationDateFixFlowPrompt.java",
   "java/src/org/chromium/chrome/browser/autofill/AutofillLogger.java",

chrome/android/chrome_test_java_sources.gni

@@ -46,6 +46,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/WarmupManagerTest.java",
   "javatests/src/org/chromium/chrome/browser/accessibility/FontSizePrefsTest.java",
   "javatests/src/org/chromium/chrome/browser/accessibility_tab_switcher/OverviewListLayoutTest.java",
+  "javatests/src/org/chromium/chrome/browser/appmenu/AppMenuAdapterTest.java",
   "javatests/src/org/chromium/chrome/browser/appmenu/TabbedAppMenuTest.java",
   "javatests/src/org/chromium/chrome/browser/appmenu/DataSaverAppMenuTest.java",
   "javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupTest.java",

chrome/android/java/res/values/ids.xml

@@ -115,8 +115,8 @@
 
     <!-- App menu item custom tags -->
     <item type="id" name="menu_item_enter_anim_id" />
-    <item type="id" name="menu_item_original_background" />
     <item type="id" name="menu_item_background_drawable_position" />
+    <item type="id" name="menu_item_view_type" />
 
     <!-- VR constants -->
     <item type="id" name="vr_overlay_view" />

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

@@ -39,7 +39,6 @@ import org.chromium.base.ContextUtils;
 import org.chromium.base.SysUtils;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
 import org.chromium.chrome.browser.ui.widget.highlight.ViewHighlighter;
 import org.chromium.ui.widget.Toast;
 
@@ -52,7 +51,7 @@ import java.util.List;
  *   - Only visible MenuItems are shown.
  *   - Disabled items are grayed out.
  */
-class AppMenu implements OnItemClickListener, OnKeyListener {
+class AppMenu implements OnItemClickListener, OnKeyListener, AppMenuAdapter.OnClickHandler {
     private static final float LAST_ITEM_SHOW_FRACTION = 0.5f;
 
     private final Menu mMenu;
@@ -73,7 +72,6 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
     private AnimatorSet mMenuItemEnterAnimator;
     private AnimatorListener mAnimationHistogramRecorder = AnimationFrameTimeHistogram
             .getAnimatorRecorder("WrenchMenu.OpeningAnimationFrameTimes");
-    private Runnable mAdapterInvalidator;
 
     /**
      * Creates and sets up the App Menu.
@@ -102,10 +100,6 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
                 res.getDimensionPixelSize(R.dimen.menu_negative_vertical_offset_not_top_anchored);
 
         mTempLocation = new int[2];
-
-        mAdapterInvalidator = () -> {
-            if (mAdapter != null) mAdapter.notifyDataSetChanged();
-        };
     }
 
     /**
@@ -170,15 +164,16 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
      * @param circleHighlightItem   Whether the highlighted item should use a circle highlight or
      *                              not.
      * @param showFromBottom        Whether the appearance animation should run from the bottom up.
+     * @param customViewBinders     See {@link AppMenuPropertiesDelegate#getCustomViewBinders()}.
      */
     void show(Context context, final View anchorView, boolean isByPermanentButton,
             int screenRotation, Rect visibleDisplayFrame, int screenHeight,
             @IdRes int footerResourceId, @IdRes int headerResourceId, Integer highlightedItemId,
-            boolean circleHighlightItem, boolean showFromBottom) {
+            boolean circleHighlightItem, boolean showFromBottom,
+            @Nullable List<CustomViewBinder> customViewBinders) {
         mPopup = new PopupWindow(context);
         mPopup.setFocusable(true);
         mPopup.setInputMethodMode(PopupWindow.INPUT_METHOD_NOT_NEEDED);
-        UpdateMenuItemHelper.getInstance().registerObserver(mAdapterInvalidator);
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
             // The window layout type affects the z-index of the popup window on M+.
@@ -192,7 +187,6 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
 
             if (mMenuItemEnterAnimator != null) mMenuItemEnterAnimator.cancel();
 
-            UpdateMenuItemHelper.getInstance().unregisterObserver(mAdapterInvalidator);
             mHandler.appMenuDismissed();
             mHandler.onMenuVisibilityChanged(false);
 
@@ -251,8 +245,8 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
 
         // A List adapter for visible items in the Menu. The first row is added as a header to the
         // list view.
-        mAdapter = new AppMenuAdapter(
-                this, menuItems, LayoutInflater.from(context), highlightedItemId);
+        mAdapter = new AppMenuAdapter(this, menuItems, LayoutInflater.from(context),
+                highlightedItemId, customViewBinders);
 
         ViewGroup contentView =
                 (ViewGroup) LayoutInflater.from(context).inflate(R.layout.app_menu_layout, null);
@@ -366,15 +360,9 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
         return position;
     }
 
-    /**
-     * Handles clicks on the AppMenu popup.
-     * @param menuItem The menu item in the popup that was clicked.
-     */
-    void onItemClick(MenuItem menuItem) {
+    @Override
+    public void onItemClick(MenuItem menuItem) {
         if (menuItem.isEnabled()) {
-            if (menuItem.getItemId() == R.id.update_menu_id) {
-                UpdateMenuItemHelper.getInstance().setMenuItemClicked();
-            }
             dismiss();
             if (menuItem.getItemId() == R.id.new_tab_menu_id) {
                 RecordUserAction.record("MobileMenuNewTab.AppMenu");
@@ -385,12 +373,8 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
         }
     }
 
-    /**
-     * Handles long clicks on image buttons on the AppMenu popup.
-     * @param menuItem The menu item in the popup that was long clicked.
-     * @param view The anchor view of the menu item.
-     */
-    boolean onItemLongClick(MenuItem menuItem, View view) {
+    @Override
+    public boolean onItemLongClick(MenuItem menuItem, View view) {
         if (!menuItem.isEnabled()) return false;
 
         String description = null;
@@ -476,10 +460,17 @@ class AppMenu implements OnItemClickListener, OnKeyListener {
     /**
      * @return The menu instance inside of this class.
      */
-    public Menu getMenu() {
+    Menu getMenu() {
         return mMenu;
     }
 
+    /**
+     * Invalidate the app menu data. See {@link AppMenuAdapter#notifyDataSetChanged}.
+     */
+    void invalidate() {
+        if (mAdapter != null) mAdapter.notifyDataSetChanged();
+    }
+
     private int setMenuHeight(int numMenuItems, Rect appDimensions, int screenHeight, Rect padding,
             int footerHeight, int headerHeight, View anchorView) {
         int menuHeight;

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

@@ -5,15 +5,12 @@
 package org.chromium.chrome.browser.appmenu;
 
 import android.content.Context;
-import android.view.MenuItem;
 import android.view.View;
 import android.view.ViewConfiguration;
 
 
 import org.chromium.base.VisibleForTesting;
-import org.chromium.chrome.R;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
-import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
 
 /** A UI coordinator the app menu. */
 class AppMenuCoordinatorImpl implements AppMenuCoordinator {
@@ -38,14 +35,6 @@ class AppMenuCoordinatorImpl implements AppMenuCoordinator {
                 ActivityLifecycleDispatcher activityLifecycleDispatcher);
     }
 
-    /**
-     * @param factory The {@link AppMenuHandlerFactory} for creating {@link #mAppMenuHandler}
-     */
-    @VisibleForTesting
-    static void setAppMenuHandlerFactoryForTesting(AppMenuHandlerFactory factory) {
-        sAppMenuHandlerFactory = factory;
-    }
-
     private final Context mContext;
     private final MenuButtonDelegate mButtonDelegate;
     private final AppMenuDelegate mAppMenuDelegate;
@@ -53,11 +42,6 @@ class AppMenuCoordinatorImpl implements AppMenuCoordinator {
     private AppMenuPropertiesDelegate mAppMenuPropertiesDelegate;
     private AppMenuHandlerImpl mAppMenuHandler;
 
-    private static AppMenuHandlerFactory sAppMenuHandlerFactory =
-            (delegate, appMenuDelegate, menuResourceId, decorView, activityLifecycleDispatcher)
-            -> new AppMenuHandlerImpl(delegate, appMenuDelegate, menuResourceId, decorView,
-                    activityLifecycleDispatcher);
-
     /**
      * Construct a new AppMenuCoordinatorImpl.
      * @param context The activity context.
@@ -76,27 +60,9 @@ class AppMenuCoordinatorImpl implements AppMenuCoordinator {
         mAppMenuDelegate = appMenuDelegate;
         mAppMenuPropertiesDelegate = mAppMenuDelegate.createAppMenuPropertiesDelegate();
 
-        mAppMenuHandler = sAppMenuHandlerFactory.get(mAppMenuPropertiesDelegate, mAppMenuDelegate,
+        mAppMenuHandler = new AppMenuHandlerImpl(mAppMenuPropertiesDelegate, mAppMenuDelegate,
                 mAppMenuPropertiesDelegate.getAppMenuLayoutId(), decorView,
                 activityLifecycleDispatcher);
-
-        // TODO(twellington): Move to UpdateMenuItemHelper or common UI coordinator parent?
-        mAppMenuHandler.addObserver(new AppMenuObserver() {
-            @Override
-            public void onMenuVisibilityChanged(boolean isVisible) {
-                if (isVisible) return;
-
-                mAppMenuPropertiesDelegate.onMenuDismissed();
-                MenuItem updateMenuItem =
-                        mAppMenuHandler.getAppMenu().getMenu().findItem(R.id.update_menu_id);
-                if (updateMenuItem != null && updateMenuItem.isVisible()) {
-                    UpdateMenuItemHelper.getInstance().onMenuDismissed();
-                }
-            }
-
-            @Override
-            public void onMenuHighlightChanged(boolean highlighting) {}
-        });
     }
 
     @Override

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

@@ -59,4 +59,9 @@ public interface AppMenuHandler {
      * @return A new {@link AppMenuButtonHelper} to hook up to a view containing a menu button.
      */
     AppMenuButtonHelper createAppMenuButtonHelper();
+
+    /**
+     * Call to cause a redraw when an item in the app menu changes.
+     */
+    void invalidateAppMenu();
 }
\ No newline at end of file

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

@@ -169,7 +169,7 @@ class AppMenuHandlerImpl
             tempMenu.inflate(mMenuResourceId);
             mMenu = tempMenu.getMenu();
         }
-        mDelegate.prepareMenu(mMenu);
+        mDelegate.prepareMenu(mMenu, this);
 
         ContextThemeWrapper wrapper =
                 new ContextThemeWrapper(context, R.style.OverflowMenuThemeOverlay);
@@ -210,7 +210,7 @@ class AppMenuHandlerImpl
         }
         mAppMenu.show(wrapper, anchorView, isByPermanentButton, rotation, appRect, pt.y,
                 footerResourceId, headerResourceId, mHighlightMenuId, mCircleHighlight,
-                showFromBottom);
+                showFromBottom, mDelegate.getCustomViewBinders());
         mAppMenuDragHelper.onShow(startDragging);
         clearMenuHighlight();
         RecordUserAction.record("MobileMenuShow");
@@ -219,6 +219,7 @@ class AppMenuHandlerImpl
 
     void appMenuDismissed() {
         mAppMenuDragHelper.finishDragging();
+        mDelegate.onMenuDismissed();
     }
 
     @Override
@@ -247,6 +248,11 @@ class AppMenuHandlerImpl
         return new AppMenuButtonHelperImpl(this);
     }
 
+    @Override
+    public void invalidateAppMenu() {
+        if (mAppMenu != null) mAppMenu.invalidate();
+    }
+
     @Override
     public void addObserver(AppMenuObserver observer) {
         mObservers.add(observer);

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

@@ -11,6 +11,8 @@ import android.view.View;
 
 import androidx.annotation.Nullable;
 
+import java.util.List;
+
 /**
  * App Menu helper that handles hiding and showing menu items based on activity state.
  */
@@ -25,13 +27,21 @@ public interface AppMenuPropertiesDelegate {
      */
     int getAppMenuLayoutId();
 
+    /**
+     * @return A list of {@link CustomViewBinder}s to use for binding specific menu items or null if
+     *         there are no custom binders for this delegate.
+     */
+    @Nullable
+    List<CustomViewBinder> getCustomViewBinders();
+
     /**
      * Allows the delegate to show and hide items before the App Menu is shown. It is called every
      * time the menu is shown. This assumes that the provided menu contains all the items expected
      * in the application menu (i.e. that the main menu has been inflated into it).
      * @param menu Menu that will be used as the source for the App Menu pop up.
+     * @param handler The {@link AppMenuHandler} associated with {@code menu}.
      */
-    void prepareMenu(Menu menu);
+    void prepareMenu(Menu menu, AppMenuHandler handler);
 
     /**
      * Gets an optional bundle of extra data associated with the provided MenuItem.

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

@@ -48,6 +48,9 @@ import org.chromium.components.dom_distiller.core.DomDistillerUrlUtils;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.webapk.lib.client.WebApkValidator;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Base implementation of {@link AppMenuPropertiesDelegate} that handles hiding and showing menu
  * items based on activity state.
@@ -66,9 +69,11 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
     private final ObservableSupplier<BookmarkBridge> mBookmarkBridgeSupplier;
     private @Nullable Callback<OverviewModeBehavior> mOverviewModeSupplierCallback;
     private Callback<BookmarkBridge> mBookmarkBridgeSupplierCallback;
+    private boolean mUpdateMenuItemVisible;
 
     protected @Nullable OverviewModeBehavior mOverviewModeBehavior;
     protected BookmarkBridge mBookmarkBridge;
+    protected Runnable mAppMenuInvalidator;
 
     /**
      * Construct a new {@link AppMenuPropertiesDelegateImpl}.
@@ -124,6 +129,13 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
         return R.menu.main_menu;
     }
 
+    @Override
+    public @Nullable List<CustomViewBinder> getCustomViewBinders() {
+        List<CustomViewBinder> customViewBinders = new ArrayList<>();
+        customViewBinders.add(new UpdateMenuItemViewBinder());
+        return customViewBinders;
+    }
+
     /**
      * @return Whether the app menu for a web page should be shown.
      */
@@ -140,7 +152,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
     }
 
     @Override
-    public void prepareMenu(Menu menu) {
+    public void prepareMenu(Menu menu, AppMenuHandler handler) {
         // Exactly one of these will be true, depending on the type of menu showing.
         boolean isPageMenu = shouldShowPageMenu();
         boolean isOverviewMenu;
@@ -206,8 +218,13 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
                 }
             }
 
-            menu.findItem(R.id.update_menu_id)
-                    .setVisible(UpdateMenuItemHelper.getInstance().getUiState().itemState != null);
+            mUpdateMenuItemVisible =
+                    UpdateMenuItemHelper.getInstance().getUiState().itemState != null;
+            menu.findItem(R.id.update_menu_id).setVisible(mUpdateMenuItemVisible);
+            if (mUpdateMenuItemVisible) {
+                mAppMenuInvalidator = () -> handler.invalidateAppMenu();
+                UpdateMenuItemHelper.getInstance().registerObserver(mAppMenuInvalidator);
+            }
 
             boolean hasMoreThanOneTab = mTabModelSelector.getTotalTabCount() > 1;
             menu.findItem(R.id.move_to_other_window_menu_id)
@@ -373,6 +390,12 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
     @Override
     public void onMenuDismissed() {
         mReloadMenuItem = null;
+        if (mUpdateMenuItemVisible) {
+            UpdateMenuItemHelper.getInstance().onMenuDismissed();
+            UpdateMenuItemHelper.getInstance().unregisterObserver(mAppMenuInvalidator);
+            mUpdateMenuItemVisible = false;
+            mAppMenuInvalidator = null;
+        }
     }
 
     // Set enabled to be |enable| for all MenuItems with |id| in |menu|.

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

@@ -42,6 +42,23 @@ public class AppMenuTestSupport {
                 .onOptionsItemSelected(item);
     }
 
+    /**
+     * Simulates a click on the menu item matching the provided id.
+     * @param coordinator The {@link AppMenuCoordinator} associated with the app menu being tested.
+     * @param menuItemId The id of the menu item to click.
+     */
+    public static void callOnItemClick(AppMenuCoordinator coordinator, int menuItemId) {
+        MenuItem item = ((AppMenuCoordinatorImpl) coordinator)
+                                .getAppMenuHandlerImplForTesting()
+                                .getAppMenu()
+                                .getMenu()
+                                .findItem(menuItemId);
+        ((AppMenuCoordinatorImpl) coordinator)
+                .getAppMenuHandlerImplForTesting()
+                .getAppMenu()
+                .onItemClick(item);
+    }
+
     /**
      * Show the app menu.
      * @param coordinator The {@link AppMenuCoordinator} associated with the app menu being tested.
@@ -66,23 +83,23 @@ public class AppMenuTestSupport {
 
     /**
      * @param coordinator The {@link AppMenuCoordinator} associated with the app menu being tested.
-     * @return The {@link ListView} for the app menu.
+     * @return Whether the app menu component thinks the app menu can currently be shown.
      */
-    public static ListView getListView(AppMenuCoordinator coordinator) {
+    public static boolean shouldShowAppMenu(AppMenuCoordinator coordinator) {
         return ((AppMenuCoordinatorImpl) coordinator)
                 .getAppMenuHandlerImplForTesting()
-                .getAppMenu()
-                .getListView();
+                .shouldShowAppMenu();
     }
 
     /**
      * @param coordinator The {@link AppMenuCoordinator} associated with the app menu being tested.
-     * @return Whether the app menu should be shown according to the app menu component.
+     * @return The {@link ListView} for the app menu.
      */
-    public static boolean shouldShowAppMenu(AppMenuCoordinator coordinator) {
+    public static ListView getListView(AppMenuCoordinator coordinator) {
         return ((AppMenuCoordinatorImpl) coordinator)
                 .getAppMenuHandlerImplForTesting()
-                .shouldShowAppMenu();
+                .getAppMenu()
+                .getListView();
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/appmenu/CustomViewBinder.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;
+
+import android.view.LayoutInflater;
+import android.view.MenuItem;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.Adapter;
+
+import androidx.annotation.Nullable;
+
+/**
+ * An interface for providing a custom view binder for a menu item displayed in the app menu.
+ * The binder may be used to custom layout/presentation of individual menu items. Clicks on the menu
+ * item will still be handled by the app menu.
+ */
+public interface CustomViewBinder {
+    /**
+     * Indicates that this view binder does not handle a particular menu item.
+     * See {{@link #getItemViewType(int)}}.
+     */
+    int NOT_HANDLED = -1;
+
+    /**
+     * @return The number of types of Views that will be created by
+     * {{@link #getView(MenuItem, View, ViewGroup, LayoutInflater)}}. The value returned by this
+     * method should be effectively treated as final. Once the CustomViewBinder has been
+     * retrieved by the app menu, it is expected that the item view type count remains stable.
+     */
+    int getViewTypeCount();
+
+    /**
+     * @param id The id of the menu item to check.
+     * @return Return the view type of the item matching the provided id or {@link #NOT_HANDLED} if
+     *         the item is not handled by this binder.
+     */
+    int getItemViewType(int id);
+
+    /**
+     * Get a View that displays the data for an item handled by this binder.
+     * See {@link Adapter#getView(int, View, ViewGroup)}.
+     * @param item The {@link MenuItem} for which to create and bind a view.
+     * @param convertView The old view to re-use if possible.
+     * @param parent The parent that this view will eventually be attached to.
+     * @param inflater A {@link LayoutInflater} to use when inflating new views.
+     * @return A View corresponding to the provided menu item.
+     */
+    View getView(
+            MenuItem item, @Nullable View convertView, ViewGroup parent, LayoutInflater inflater);
+
+    /**
+     * Determines whether the enter animation should be applied to the menu item matching the
+     * provided id.
+     * @param id The id of the menu item to check.
+     * @return True if the standard animation should be applied.
+     */
+    boolean supportsEnterAnimation(int id);
+}

chrome/android/java/src/org/chromium/chrome/browser/appmenu/UpdateMenuItemViewBinder.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;
+
+import android.content.res.Resources;
+import android.graphics.drawable.Drawable;
+import android.support.v4.graphics.drawable.DrawableCompat;
+import android.text.TextUtils;
+import android.view.LayoutInflater;
+import android.view.MenuItem;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.TextView;
+
+import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
+
+/**
+ * A custom binder used to bind the update menu item.
+ */
+class UpdateMenuItemViewBinder implements CustomViewBinder {
+    private static final int UPDATE_ITEM_VIEW_TYPE = 0;
+
+    @Override
+    public int getViewTypeCount() {
+        return 1;
+    }
+
+    @Override
+    public int getItemViewType(int id) {
+        return id == R.id.update_menu_id ? UPDATE_ITEM_VIEW_TYPE : CustomViewBinder.NOT_HANDLED;
+    }
+
+    @Override
+    public View getView(
+            MenuItem item, View convertView, ViewGroup parent, LayoutInflater inflater) {
+        assert item.getItemId() == R.id.update_menu_id;
+
+        UpdateMenuItemViewHolder holder;
+        if (convertView == null || !(convertView.getTag() instanceof UpdateMenuItemViewHolder)) {
+            holder = new UpdateMenuItemViewHolder();
+            convertView = inflater.inflate(R.layout.update_menu_item, parent, false);
+            holder.text = convertView.findViewById(R.id.menu_item_text);
+            holder.image = convertView.findViewById(R.id.menu_item_icon);
+            holder.summary = convertView.findViewById(R.id.menu_item_summary);
+            convertView.setTag(holder);
+        } else {
+            holder = (UpdateMenuItemViewHolder) convertView.getTag();
+        }
+
+        UpdateMenuItemHelper.MenuItemState itemState =
+                UpdateMenuItemHelper.getInstance().getUiState().itemState;
+
+        if (itemState == null) return convertView;
+
+        Resources resources = convertView.getResources();
+
+        Drawable icon = item.getIcon();
+        holder.image.setImageDrawable(icon);
+        holder.image.setVisibility(icon == null ? View.GONE : View.VISIBLE);
+
+        holder.text.setText(itemState.title);
+        holder.text.setContentDescription(resources.getString(itemState.title));
+        holder.text.setTextColor(ApiCompatibilityUtils.getColor(resources, itemState.titleColorId));
+
+        boolean isEnabled = item.isEnabled();
+        holder.text.setEnabled(isEnabled);
+
+        if (!TextUtils.isEmpty(itemState.summary)) {
+            holder.summary.setText(itemState.summary);
+            holder.summary.setVisibility(View.VISIBLE);
+        } else {
+            holder.summary.setText("");
+            holder.summary.setVisibility(View.GONE);
+        }
+
+        holder.image.setImageResource(itemState.icon);
+        if (itemState.iconTintId != 0) {
+            DrawableCompat.setTint(holder.image.getDrawable(),
+                    ApiCompatibilityUtils.getColor(resources, itemState.iconTintId));
+        }
+        convertView.setEnabled(itemState.enabled);
+
+        return convertView;
+    }
+
+    @Override
+    public boolean supportsEnterAnimation(int id) {
+        return true;
+    }
+
+    private static class UpdateMenuItemViewHolder {
+        public TextView text;
+        public AppMenuItemIcon image;
+        public TextView summary;
+    }
+}

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

@@ -22,7 +22,9 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.DefaultBrowserInfo;
+import org.chromium.chrome.browser.appmenu.AppMenuHandler;
 import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegateImpl;
+import org.chromium.chrome.browser.appmenu.CustomViewBinder;
 import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
 import org.chromium.chrome.browser.browserservices.BrowserServicesIntentDataProvider.CustomTabsUiType;
 import org.chromium.chrome.browser.download.DownloadUtils;
@@ -34,6 +36,7 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.toolbar.ToolbarManager;
 import org.chromium.chrome.browser.util.UrlConstants;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -83,7 +86,12 @@ public class CustomTabAppMenuPropertiesDelegate extends AppMenuPropertiesDelegat
     }
 
     @Override
-    public void prepareMenu(Menu menu) {
+    public @Nullable List<CustomViewBinder> getCustomViewBinders() {
+        return Collections.EMPTY_LIST;
+    }
+
+    @Override
+    public void prepareMenu(Menu menu, AppMenuHandler handler) {
         Tab currentTab = mActivityTabProvider.get();
         if (currentTab != null) {
             MenuItem forwardMenuItem = menu.findItem(R.id.forward_menu_id);

chrome/android/java/src/org/chromium/chrome/browser/omaha/UpdateMenuItemHelper.java

@@ -20,8 +20,10 @@ import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.ObserverList;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.task.PostTask;
+import org.chromium.base.task.TaskTraits;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.omaha.UpdateStatusProvider.UpdateInteractionSource;
 import org.chromium.chrome.browser.omaha.UpdateStatusProvider.UpdateState;
@@ -41,12 +43,14 @@ import org.chromium.content_public.browser.UiThreadTaskTraits;
  * For manually testing this functionality, see {@link UpdateConfigs}.
  */
 public class UpdateMenuItemHelper {
+    static final String ACTION_TAKEN_ON_MENU_OPEN_HISTOGRAM =
+            "GoogleUpdate.MenuItem.ActionTakenOnMenuOpen";
     private static final String TAG = "UpdateMenuItemHelper";
 
     // UMA constants for logging whether the menu item was clicked.
-    private static final int ITEM_NOT_CLICKED = 0;
-    private static final int ITEM_CLICKED_INTENT_LAUNCHED = 1;
-    private static final int ITEM_CLICKED_INTENT_FAILED = 2;
+    static final int ITEM_NOT_CLICKED = 0;
+    static final int ITEM_CLICKED_INTENT_LAUNCHED = 1;
+    static final int ITEM_CLICKED_INTENT_FAILED = 2;
     private static final int ITEM_CLICKED_BOUNDARY = 3;
 
     // UMA constants for logging whether Chrome was updated after the menu item was clicked.
@@ -141,6 +145,12 @@ public class UpdateMenuItemHelper {
     // Whether the menu item was clicked. This is used to log the click-through rate.
     private boolean mMenuItemClicked;
 
+    /**
+     * Whether the runnable posted when the app menu is dismissed has been executed. Tracked for
+     * testing.
+     */
+    private boolean mMenuDismissedRunnableExecuted;
+
     /** @return The {@link UpdateMenuItemHelper} instance. */
     public static UpdateMenuItemHelper getInstance() {
         synchronized (UpdateMenuItemHelper.sGetInstanceLock) {
@@ -191,6 +201,7 @@ public class UpdateMenuItemHelper {
      * @param activity The current {@code Activity}.
      */
     public void onMenuItemClicked(Activity activity) {
+        mMenuItemClicked = true;
         if (mStatus == null) return;
 
         switch (mStatus.updateState) {
@@ -237,18 +248,18 @@ public class UpdateMenuItemHelper {
         handleStateChanged();
     }
 
-    /** Should be called before the AppMenu is dismissed if the update menu item was clicked. */
-    public void setMenuItemClicked() {
-        mMenuItemClicked = true;
-    }
-
     /**
-     * Called when the {@link AppMenu} is dimissed. Logs a histogram immediately if the update menu
-     * item was not clicked. If it was clicked, logging is delayed until #onMenuItemClicked().
+     * Called when the menu containing the update menu item is dismissed.
      */
     public void onMenuDismissed() {
-        if (!mMenuItemClicked) recordItemClickedHistogram(ITEM_NOT_CLICKED);
-        mMenuItemClicked = false;
+        mMenuDismissedRunnableExecuted = false;
+        // Post a task to record the item clicked histogram. Post task is used so that the runnable
+        // executes after #onMenuItemClicked is called (if it's going to be called).
+        PostTask.postTask(TaskTraits.CHOREOGRAPHER_FRAME, () -> {
+            if (!mMenuItemClicked) recordItemClickedHistogram(ITEM_NOT_CLICKED);
+            mMenuItemClicked = false;
+            mMenuDismissedRunnableExecuted = true;
+        });
     }
 
     /**
@@ -386,8 +397,8 @@ public class UpdateMenuItemHelper {
     }
 
     private void recordItemClickedHistogram(int action) {
-        RecordHistogram.recordEnumeratedHistogram("GoogleUpdate.MenuItem.ActionTakenOnMenuOpen",
-                action, ITEM_CLICKED_BOUNDARY);
+        RecordHistogram.recordEnumeratedHistogram(
+                ACTION_TAKEN_ON_MENU_OPEN_HISTOGRAM, action, ITEM_CLICKED_BOUNDARY);
     }
 
     private void recordUpdateHistogram() {
@@ -402,5 +413,8 @@ public class UpdateMenuItemHelper {
         }
     }
 
-
+    @VisibleForTesting
+    boolean getMenuDismissedRunnableExecutedForTests() {
+        return mMenuDismissedRunnableExecuted;
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/omaha/UpdateMenuItemHelperTest.java

@@ -4,34 +4,42 @@
 
 package org.chromium.chrome.browser.omaha;
 
+import android.app.Activity;
+import android.app.Instrumentation.ActivityResult;
 import android.content.Context;
+import android.support.test.espresso.intent.Intents;
+import android.support.test.espresso.intent.matcher.IntentMatchers;
 import android.support.test.filters.MediumTest;
 
+import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import org.chromium.base.task.PostTask;
+import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Restriction;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.appmenu.AppMenuObserver;
 import org.chromium.chrome.browser.appmenu.AppMenuTestSupport;
 import org.chromium.chrome.browser.util.UrlConstants;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.OverviewModeBehaviorWatcher;
 import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
-import org.chromium.content_public.browser.UiThreadTaskTraits;
 import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.ui.test.util.UiRestriction;
 
+import java.util.concurrent.TimeoutException;
+
 /**
  * Tests for the UpdateMenuItemHelper.
  */
@@ -41,6 +49,9 @@ public class UpdateMenuItemHelperTest {
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
 
+    private static final String TEST_MARKET_URL =
+            "https://play.google.com/store/apps/details?id=com.android.chrome";
+
     private static final long MS_TIMEOUT = 2000;
     private static final long MS_INTERVAL = 500;
 
@@ -95,8 +106,26 @@ public class UpdateMenuItemHelperTest {
         }
     }
 
+    private static class TestAppMenuObserver implements AppMenuObserver {
+        CallbackHelper mMenuShownCallback = new CallbackHelper();
+        CallbackHelper mMenuHiddenCallback = new CallbackHelper();
+
+        @Override
+        public void onMenuVisibilityChanged(boolean isVisible) {
+            if (isVisible) {
+                mMenuShownCallback.notifyCalled();
+            } else {
+                mMenuHiddenCallback.notifyCalled();
+            }
+        }
+
+        @Override
+        public void onMenuHighlightChanged(boolean highlighting) {}
+    }
+
     private MockVersionNumberGetter mMockVersionNumberGetter;
     private MockMarketURLGetter mMockMarketURLGetter;
+    private TestAppMenuObserver mMenuObserver;
 
     @Before
     public void setUp() {
@@ -116,12 +145,13 @@ public class UpdateMenuItemHelperTest {
         VersionNumberGetter.setInstanceForTests(mMockVersionNumberGetter);
 
         // Report a dummy URL to Omaha.
-        mMockMarketURLGetter = new MockMarketURLGetter(
-                "https://play.google.com/store/apps/details?id=com.android.chrome");
+        mMockMarketURLGetter = new MockMarketURLGetter(TEST_MARKET_URL);
         MarketURLGetter.setInstanceForTests(mMockMarketURLGetter);
 
         // Start up main.
         mActivityTestRule.startMainActivityWithURL(UrlConstants.NTP_URL);
+        mMenuObserver = new TestAppMenuObserver();
+        mActivityTestRule.getAppMenuCoordinator().getAppMenuHandler().addObserver(mMenuObserver);
 
         // Check to make sure that the version numbers get queried.
         versionNumbersQueried();
@@ -220,33 +250,105 @@ public class UpdateMenuItemHelperTest {
                 mActivityTestRule.getMenu().findItem(R.id.update_menu_id).isVisible());
     }
 
-    private void showAppMenuAndAssertMenuShown() {
-        PostTask.runOrPostTask(UiThreadTaskTraits.DEFAULT, () -> {
+    @Test
+    @MediumTest
+    @Feature({"Omaha"})
+    @DisableFeatures("InlineUpdateFlow")
+    public void testClickUpdateMenuItem() throws Exception {
+        checkUpdateMenuItemIsShowing("0.0.0.0", "1.2.3.4");
+
+        Assert.assertEquals(
+                "Incorrect item clicked histogram count", 0, getTotalItemClickedCount());
+        Assert.assertEquals(
+                "Incorrect item not clicked histogram count", 0, getTotalItemNotClickedCount());
+
+        Intents.init();
+        ActivityResult intentResult = new ActivityResult(Activity.RESULT_OK, null);
+        Intents.intending(IntentMatchers.hasData(TEST_MARKET_URL)).respondWith(intentResult);
+
+        TestThreadUtils.runOnUiThreadBlocking(
+                ()
+                        -> AppMenuTestSupport.callOnItemClick(
+                                mActivityTestRule.getAppMenuCoordinator(), R.id.update_menu_id));
+
+        Intents.intended(Matchers.allOf(IntentMatchers.hasData(TEST_MARKET_URL)));
+
+        Assert.assertEquals("Incorrect item clicked histogram count after item clicked", 1,
+                getTotalItemClickedCount());
+        Assert.assertEquals("Incorrect item not clicked histogram count after item clicked", 0,
+                getTotalItemNotClickedCount());
+
+        mMenuObserver.mMenuHiddenCallback.waitForCallback(0);
+        waitForAppMenuDimissedRunnable();
+
+        Assert.assertEquals("Incorrect item clicked histogram count after menu dismissed", 1,
+                getTotalItemClickedCount());
+        Assert.assertEquals("Incorrect item not clicked histogram count after menu dismissed", 0,
+                getTotalItemNotClickedCount());
+
+        Intents.release();
+    }
+
+    @Test
+    @MediumTest
+    @Feature({"Omaha"})
+    @DisableFeatures("InlineUpdateFlow")
+    public void testHideMenuWithoutClicking() throws Exception {
+        checkUpdateMenuItemIsShowing("0.0.0.0", "1.2.3.4");
+
+        Assert.assertEquals(
+                "Incorrect item clicked histogram count", 0, getTotalItemClickedCount());
+        Assert.assertEquals(
+                "Incorrect item not clicked histogram count", 0, getTotalItemNotClickedCount());
+
+        hideAppMenuAndAssertMenuShown();
+        waitForAppMenuDimissedRunnable();
+
+        Assert.assertEquals("Incorrect item clicked histogram count after menu dismissed", 0,
+                getTotalItemClickedCount());
+        Assert.assertEquals("Incorrect item not clicked histogram count after menu dismissed", 1,
+                getTotalItemNotClickedCount());
+    }
+
+    private void showAppMenuAndAssertMenuShown() throws TimeoutException {
+        int currentCallCount = mMenuObserver.mMenuShownCallback.getCallCount();
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
             AppMenuTestSupport.showAppMenu(
                     mActivityTestRule.getAppMenuCoordinator(), null, false, false);
         });
-        CriteriaHelper.pollInstrumentationThread(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                return mActivityTestRule.getActivity()
-                        .getRootUiCoordinatorForTesting()
-                        .getAppMenuCoordinatorForTesting()
-                        .getAppMenuHandler()
-                        .isAppMenuShowing();
-            }
-        });
+        mMenuObserver.mMenuShownCallback.waitForCallback(currentCallCount);
     }
 
-    private void hideAppMenuAndAssertMenuShown() {
-        PostTask.runOrPostTask(UiThreadTaskTraits.DEFAULT, () -> {
-            mActivityTestRule.getAppMenuCoordinator().getAppMenuHandler().hideAppMenu();
-        });
+    private void hideAppMenuAndAssertMenuShown() throws TimeoutException {
+        int currentCallCount = mMenuObserver.mMenuHiddenCallback.getCallCount();
+
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> mActivityTestRule.getAppMenuCoordinator().getAppMenuHandler().hideAppMenu());
+
+        mMenuObserver.mMenuHiddenCallback.waitForCallback(currentCallCount);
+    }
+
+    private int getTotalItemClickedCount() {
+        return RecordHistogram.getHistogramValueCountForTesting(
+                       UpdateMenuItemHelper.ACTION_TAKEN_ON_MENU_OPEN_HISTOGRAM,
+                       UpdateMenuItemHelper.ITEM_CLICKED_INTENT_FAILED)
+                + RecordHistogram.getHistogramValueCountForTesting(
+                        UpdateMenuItemHelper.ACTION_TAKEN_ON_MENU_OPEN_HISTOGRAM,
+                        UpdateMenuItemHelper.ITEM_CLICKED_INTENT_LAUNCHED);
+    }
+
+    private int getTotalItemNotClickedCount() {
+        return RecordHistogram.getHistogramValueCountForTesting(
+                UpdateMenuItemHelper.ACTION_TAKEN_ON_MENU_OPEN_HISTOGRAM,
+                UpdateMenuItemHelper.ITEM_NOT_CLICKED);
+    }
+
+    private void waitForAppMenuDimissedRunnable() {
         CriteriaHelper.pollInstrumentationThread(new Criteria() {
             @Override
             public boolean isSatisfied() {
-                return !mActivityTestRule.getAppMenuCoordinator()
-                                .getAppMenuHandler()
-                                .isAppMenuShowing();
+                return UpdateMenuItemHelper.getInstance()
+                        .getMenuDismissedRunnableExecutedForTests();
             }
         });
     }