Add support for highlighting menu items

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

Index: chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
index 2518f4b0163b2ba0a8b05f7a02046d2bd50bf110..3b41c0a2d6ee5ef559a57b3cd08751e1798ede93 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuAdapter.java
@@ -8,7 +8,10 @@ import android.animation.Animator;
 import android.animation.AnimatorListenerAdapter;
 import android.animation.AnimatorSet;
 import android.animation.ObjectAnimator;
+import android.content.res.Resources;
 import android.graphics.drawable.Drawable;
+import android.graphics.drawable.LayerDrawable;
+import android.support.annotation.IdRes;
 import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.MenuItem;
@@ -21,6 +24,7 @@ import android.widget.ListView;
 import android.widget.TextView;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.ContextUtils;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
 import org.chromium.chrome.browser.widget.TintedImageButton;
@@ -97,12 +101,22 @@ class AppMenuAdapter extends BaseAdapter {
     private final LayoutInflater mInflater;
     private final List<MenuItem> mMenuItems;
     private final int mNumMenuItems;
+    @IdRes
+    private final int mHighlightedItemId;
     private final float mDpToPx;
 
-    public AppMenuAdapter(AppMenu appMenu, List<MenuItem> menuItems, LayoutInflater inflater) {
+    // Use a single PulseDrawable to spawn the other drawables so that the ConstantState gets
+    // shared.  This allows the animation to stay in step even as the views are recycled and the
+    // background gets changed.  This Drawable isn't just used directly because Drawables need to
+    // have a single owner or the internals of View can modify it's state as it gets cleared later.
+    private PulseDrawable mHighlightDrawableSource;
+
+    public AppMenuAdapter(AppMenu appMenu, List<MenuItem> menuItems, LayoutInflater inflater,
+            @IdRes int highlightedItemId) {
         mAppMenu = appMenu;
         mMenuItems = menuItems;
         mInflater = inflater;
+        mHighlightedItemId = highlightedItemId;
         mNumMenuItems = menuItems.size();
         mDpToPx = inflater.getContext().getResources().getDisplayMetrics().density;
     }
@@ -164,6 +178,8 @@ class AppMenuAdapter extends BaseAdapter {
                     convertView.setTag(holder);
                     convertView.setTag(R.id.menu_item_enter_anim_id,
                             buildStandardItemEnterAnimator(convertView, position));
+                    convertView.setTag(
+                            R.id.menu_item_original_background, convertView.getBackground());
                 } else {
                     holder = (StandardMenuItemViewHolder) convertView.getTag();
                 }
@@ -183,6 +199,8 @@ class AppMenuAdapter extends BaseAdapter {
                     convertView.setTag(holder);
                     convertView.setTag(R.id.menu_item_enter_anim_id,
                             buildStandardItemEnterAnimator(convertView, position));
+                    convertView.setTag(
+                            R.id.menu_item_original_background, convertView.getBackground());
                 } else {
                     holder = (CustomMenuItemViewHolder) convertView.getTag();
                 }
@@ -218,12 +236,15 @@ class AppMenuAdapter extends BaseAdapter {
                     convertView = mInflater.inflate(R.layout.title_button_menu_item, parent, false);
                     holder.title = (TextView) convertView.findViewById(R.id.title);
                     holder.button = (TintedImageButton) convertView.findViewById(R.id.button);
-
+                    holder.button.setTag(
+                            R.id.menu_item_original_background, holder.button.getBackground());
                     View animatedView = convertView;
 
                     convertView.setTag(holder);
                     convertView.setTag(R.id.menu_item_enter_anim_id,
                             buildStandardItemEnterAnimator(animatedView, position));
+                    convertView.setTag(
+                            R.id.menu_item_original_background, convertView.getBackground());
                 } else {
                     holder = (TitleButtonMenuItemViewHolder) convertView.getTag();
                 }
@@ -251,6 +272,9 @@ class AppMenuAdapter extends BaseAdapter {
             default:
                 assert false : "Unexpected MenuItem type";
         }
+
+        checkHighlightDrawable(convertView, false, item.getItemId());
+
         return convertView;
     }
 
@@ -275,6 +299,8 @@ class AppMenuAdapter extends BaseAdapter {
             }
         });
 
+        checkHighlightDrawable(button, true, item.getItemId());
+
         // Menu items may be hidden by command line flags before they get to this point.
         button.setVisibility(item.isVisible() ? View.VISIBLE : View.GONE);
     }
@@ -336,21 +362,22 @@ class AppMenuAdapter extends BaseAdapter {
      * This builds an {@link Animator} for the enter animation of icon row menu items.  This means
      * it will animate the alpha from 0 to 1 and translate the views from 10dp to 0dp on the x axis.
      *
-     * @param views        The list if icons in the menu item that should be animated.
-     * @return             The {@link Animator}.
+     * @param buttons The list of icons in the menu item that should be animated.
+     * @return        The {@link Animator}.
      */
-    private Animator buildIconItemEnterAnimator(final ImageView[] views) {
+    private Animator buildIconItemEnterAnimator(final ImageView[] buttons) {
         final boolean rtl = LocalizationUtils.isLayoutRtl();
         final float offsetXPx = ENTER_STANDARD_ITEM_OFFSET_X_DP * mDpToPx * (rtl ? -1.f : 1.f);
-        final int maxViewsToAnimate = views.length;
+        final int maxViewsToAnimate = buttons.length;
 
         AnimatorSet animation = new AnimatorSet();
         AnimatorSet.Builder builder = null;
         for (int i = 0; i < maxViewsToAnimate; i++) {
             final int startDelay = ENTER_ITEM_ADDL_DELAY_MS * i;
 
-            Animator alpha = ObjectAnimator.ofFloat(views[i], View.ALPHA, 0.f, 1.f);
-            Animator translate = ObjectAnimator.ofFloat(views[i], View.TRANSLATION_X, offsetXPx, 0);
+            ImageView view = buttons[i];
+            Animator alpha = ObjectAnimator.ofFloat(view, View.ALPHA, 0.f, 1.f);
+            Animator translate = ObjectAnimator.ofFloat(view, View.TRANSLATION_X, offsetXPx, 0);
             alpha.setStartDelay(startDelay);
             translate.setStartDelay(startDelay);
             alpha.setDuration(ENTER_ITEM_DURATION_MS);
@@ -370,7 +397,7 @@ class AppMenuAdapter extends BaseAdapter {
             @Override
             public void onAnimationStart(Animator animation) {
                 for (int i = 0; i < maxViewsToAnimate; i++) {
-                    views[i].setAlpha(0.f);
+                    buttons[i].setAlpha(0.f);
                 }
             }
         });
@@ -385,11 +412,15 @@ class AppMenuAdapter extends BaseAdapter {
                 || ((RowItemViewHolder) convertView.getTag()).buttons.length != numItems) {
             holder = new RowItemViewHolder(numItems);
             convertView = mInflater.inflate(R.layout.icon_row_menu_item, parent, false);
+            convertView.setTag(R.id.menu_item_original_background, convertView.getBackground());
 
             // Save references to all the buttons.
             for (int i = 0; i < numItems; i++) {
-                holder.buttons[i] =
+                TintedImageButton view =
                         (TintedImageButton) convertView.findViewById(BUTTON_IDS[i]);
+                holder.buttons[i] = view;
+                holder.buttons[i].setTag(
+                        R.id.menu_item_original_background, holder.buttons[i].getBackground());
             }
 
             // Remove unused menu items.
@@ -412,6 +443,33 @@ class AppMenuAdapter extends BaseAdapter {
         return convertView;
     }
 
+    private void checkHighlightDrawable(View view, boolean isIcon, int itemId) {

[Ted C, 2017/04/12 18:05:57]

I think maybe this should be:

highlightItemIfNecessary or something

This name is not what I expected this method would do.

[David Trainor- moved to gerrit, 2017/04/12 18:59:37]

Done.

+        Drawable background = (Drawable) view.getTag(R.id.menu_item_original_background);
+        if (background == null) return;
+
+        if (itemId != mHighlightedItemId) {
+            view.setBackground(background);
+            return;
+        }
+
+        if (mHighlightDrawableSource == null) {
+            mHighlightDrawableSource = isIcon ? PulseDrawableFactory.createCircle()
+                                              : PulseDrawableFactory.createHighlight();
+        }
+
+        Resources resources = ContextUtils.getApplicationContext().getResources();
+
+        PulseDrawable pulse =
+                (PulseDrawable) mHighlightDrawableSource.getConstantState().newDrawable(resources);
+        if (background.getConstantState() != null) {
+            background = background.getConstantState().newDrawable(resources);

[Ted C, 2017/04/12 18:05:56]

Why aren't we reusing the background as is?

[David Trainor- moved to gerrit, 2017/04/12 18:59:36]

I can probably make it work.  Here's why it's breaking:
1. LayerDrawable calls Drawable#setCallback(...) on the background Drawable as
it's added, effectively stealing ownership but not telling anyone.
2. View.setBackground(LayerDrawable) calls Drawable#setCallback(null) when it
drops it's internal reference to the old background.  Sadly this detaches the
Drawable from LayerDrawable and not the View... as the callback was already
swapped to the View.

A solution might be:

view.setBackground(null);

// build the LayerDrawable, reuse the background.

view.setBackground(drawable);

I'll test this tonight

+        }
+
+        LayerDrawable drawable = new LayerDrawable(new Drawable[] {pulse, background});
+        view.setBackground(drawable);
+        pulse.start();
+    }
+
     static class StandardMenuItemViewHolder {
         public TextView text;
         public AppMenuItemIcon image;