[AppMenu] Create a general custom view layout xml

This CL create a general layout xml for the custom view.
Also fix the issue that "Downloads" and "Bookmarks" always show the
icon, this is not a real bug, only happen if someone manually enabled
phase 2(regroup) and nothing else.
Also set show icons by default for phase 4 & 5.

Bug:1138987

Change-Id: Ida2f697d85055ed9fcf9d3bc6e8da8010b9de3eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2467077
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817636}

chrome/android/chrome_java_resources.gni

@@ -789,7 +789,6 @@ chrome_java_resources = [
   "java/res/layout/add_to_homescreen_dialog.xml",
   "java/res/layout/add_to_menu_dialog.xml",
   "java/res/layout/add_to_menu_dialog_item.xml",
-  "java/res/layout/add_to_menu_item.xml",
   "java/res/layout/auto_sign_in_first_run_dialog.xml",
   "java/res/layout/autofill_billing_address_dropdown.xml",
   "java/res/layout/autofill_card_unmask_prompt.xml",
@@ -838,6 +837,7 @@ chrome_java_resources = [
   "java/res/layout/custom_tabs_toolbar.xml",
   "java/res/layout/custom_tabs_toolbar_button.xml",
   "java/res/layout/custom_tabs_topbar.xml",
+  "java/res/layout/custom_view_menu_item.xml",
   "java/res/layout/data_reduction_main_menu_item.xml",
   "java/res/layout/data_reduction_stats_layout.xml",
   "java/res/layout/data_usage_breakdown.xml",
@@ -877,7 +877,6 @@ chrome_java_resources = [
   "java/res/layout/homepage_editor.xml",
   "java/res/layout/incognito_description_layout.xml",
   "java/res/layout/incognito_interstitial_bottom_sheet_view.xml",
-  "java/res/layout/incognito_menu_item.xml",
   "java/res/layout/incognito_toggle_tabs.xml",
   "java/res/layout/infobar_control_url_ellipsizer.xml",
   "java/res/layout/infobar_translate_compact_content.xml",

chrome/android/java/res/layout/add_to_menu_item.xml

-<?xml version="1.0" encoding="utf-8"?>
-<!-- Copyright 2020 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.
--->
-
-<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:app="http://schemas.android.com/apk/res-auto"
-    android:layout_width="match_parent"
-    android:layout_height="?android:attr/listPreferredItemHeightSmall"
-    android:orientation="vertical" >
-
-    <org.chromium.components.browser_ui.widget.text.TextViewWithCompoundDrawables
-        android:id="@+id/title"
-        style="@style/AppMenuItemTextViewWithCompoundDrawables"
-        android:layout_width="match_parent"
-        android:layout_height="match_parent"
-        android:paddingStart="16dp"
-        android:paddingEnd="16dp"
-        android:background="?android:attr/listChoiceBackgroundIndicator"
-        android:text="@string/menu_add_to"
-        android:drawableStart="@drawable/ic_add" />
-
-</LinearLayout>
\ No newline at end of file

chrome/android/java/res/layout/incognito_menu_item.xml → chrome/android/java/res/layout/custom_view_menu_item.xml

@@ -13,21 +13,19 @@
     android:orientation="horizontal" >
 
     <org.chromium.components.browser_ui.widget.text.TextViewWithCompoundDrawables
-        android:id="@+id/menu_item_text"
+        android:id="@+id/title"
         style="@style/AppMenuItemTextViewWithCompoundDrawables"
         android:layout_weight="1"
         android:layout_width="wrap_content"
-        android:layout_height="match_parent"
-        android:text="@string/menu_new_incognito_tab" />
+        android:layout_height="match_parent" />
 
     <org.chromium.ui.widget.ChromeImageView
-        android:id="@+id/management_icon"
+        android:id="@+id/trailing_icon"
         android:layout_weight="0"
         android:layout_width="wrap_content"
         android:layout_height="match_parent"
         android:gravity="center_vertical"
         android:importantForAccessibility="no"
-        android:src="@drawable/ic_business"
         android:tint="@color/default_icon_color_secondary"
         android:visibility="gone" />
 

chrome/android/java/res/menu/main_menu_regroup.xml

@@ -95,7 +95,8 @@
         <item android:id="@+id/add_to_divider_line_id"
             android:title="@null" />
         <item android:id="@+id/add_to_menu_id"
-            android:title="@string/menu_add_to">
+            android:title="@string/menu_add_to"
+            android:icon="@drawable/ic_add" >
           <menu>
                 <item android:id="@+id/add_to_bookmarks_menu_id"
                     android:title="@string/menu_add_to_bookmarks"

chrome/android/java/src/org/chromium/chrome/browser/app/appmenu/AddToMenuItemViewBinder.java

@@ -50,7 +50,7 @@ class AddToMenuItemViewBinder extends ArrayAdapter<MenuItem>
      * @param modalDialogManager {@link ModalDialogManager} to control the dialog.
      */
     AddToMenuItemViewBinder(Context context, ModalDialogManager modalDialogManager) {
-        super(context, R.layout.add_to_menu_item);
+        super(context, R.layout.add_to_menu_dialog_item);
         mAddToMenuItems = new ArrayList<MenuItem>();
         mContext = context;
         mModalDialogManager = modalDialogManager;
@@ -86,13 +86,18 @@ class AddToMenuItemViewBinder extends ArrayAdapter<MenuItem>
         AddToMenuItemViewHolder holder;
         if (convertView == null || !(convertView.getTag() instanceof AddToMenuItemViewHolder)) {
             holder = new AddToMenuItemViewHolder();
-            convertView = inflater.inflate(R.layout.add_to_menu_item, parent, false);
+            convertView = inflater.inflate(R.layout.custom_view_menu_item, parent, false);
             holder.title = convertView.findViewById(R.id.title);
             convertView.setTag(holder);
         } else {
             holder = (AddToMenuItemViewHolder) convertView.getTag();
         }
 
+        holder.title.setCompoundDrawablesRelative(item.getIcon(), null, null, null);
+        holder.title.setText(item.getTitle());
+        // Setting |holder.title| to non-focusable will allow TalkBack highlighting the whole view
+        // of the menu item, not just title text.
+        holder.title.setFocusable(false);
         convertView.setOnClickListener(v -> showAddToDialog());
 
         // The submenu is prepared by AppMenuPropertiesDelegateImpl.

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

@@ -503,6 +503,14 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
                         && item.getItemId() != R.id.update_menu_id) {
                     item.setIcon(null);
                 }
+                // Remove icons for menu items that have submenus.
+                if (item.getItemId() == R.id.downloads_row_menu_id
+                        || item.getItemId() == R.id.all_bookmarks_row_menu_id
+                        || item.getItemId() == R.id.add_to_menu_id) {
+                    for (int j = 0; j < item.getSubMenu().size(); ++j) {
+                        item.getSubMenu().getItem(j).setIcon(null);
+                    }
+                }
             }
 
             if (item.getItemId() == R.id.new_incognito_tab_menu_id && item.isVisible()) {

chrome/android/java/src/org/chromium/chrome/browser/app/appmenu/IncognitoMenuItemViewBinder.java

@@ -46,21 +46,23 @@ class IncognitoMenuItemViewBinder implements CustomViewBinder {
         IncognitoMenuItemViewHolder holder;
         if (convertView == null || !(convertView.getTag() instanceof IncognitoMenuItemViewHolder)) {
             holder = new IncognitoMenuItemViewHolder();
-            convertView = inflater.inflate(R.layout.incognito_menu_item, parent, false);
-            holder.title = convertView.findViewById(R.id.menu_item_text);
-            holder.image = convertView.findViewById(R.id.management_icon);
+            convertView = inflater.inflate(R.layout.custom_view_menu_item, parent, false);
+            holder.title = convertView.findViewById(R.id.title);
+            holder.image = convertView.findViewById(R.id.trailing_icon);
             convertView.setTag(holder);
         } else {
             holder = (IncognitoMenuItemViewHolder) convertView.getTag();
         }
 
         holder.title.setCompoundDrawablesRelative(item.getIcon(), null, null, null);
+        holder.title.setText(item.getTitle());
         holder.title.setEnabled(item.isEnabled());
         // Setting |holder.title| to non-focusable will allow TalkBack highlighting the whole view
         // of the menu item, not just title text.
         holder.title.setFocusable(false);
         convertView.setFocusable(item.isEnabled());
         if (IncognitoUtils.isIncognitoModeManaged()) {
+            holder.image.setImageResource(R.drawable.ic_business);
             holder.image.setVisibility(View.VISIBLE);
         }
         convertView.setOnClickListener(v -> appMenuClickHandler.onItemClick(item));

chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java

@@ -96,7 +96,9 @@ public class TabbedAppMenuPropertiesDelegate extends AppMenuPropertiesDelegateIm
 
     @Override
     public boolean shouldShowIconBeforeItem() {
-        return CachedFeatureFlags.isEnabled(ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_ICONS);
+        return CachedFeatureFlags.isEnabled(ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_ICONS)
+                || CachedFeatureFlags.isEnabled(
+                        ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_THREE_BUTTON_ACTIONBAR);
     }
 
     private boolean canShowDataReductionItem(int maxMenuHeight) {

chrome/android/javatests/src/org/chromium/chrome/browser/app/appmenu/TabbedAppMenuTest.java

@@ -366,7 +366,7 @@ public class TabbedAppMenuTest {
     testActionChipViewMenuItem() throws IOException {
         LinearLayout actionBar = (LinearLayout) getListView().getChildAt(0);
         Assert.assertEquals(3, actionBar.getChildCount());
-        mRenderTestRule.render(getListView().getChildAt(0), "icon_row_three_buttons");
+        mRenderTestRule.render(getListView().getChildAt(0), "tinted_icon_row_three_buttons");
 
         int downloadRowIndex = findIndexOfMenuItemById(R.id.downloads_row_menu_id);
         Assert.assertNotEquals("No download row found.", -1, downloadRowIndex);
@@ -408,7 +408,7 @@ public class TabbedAppMenuTest {
     testDestinationChipViewMenuItem() throws IOException {
         LinearLayout actionBar = (LinearLayout) getListView().getChildAt(0);
         Assert.assertEquals(3, actionBar.getChildCount());
-        mRenderTestRule.render(getListView().getChildAt(0), "icon_row_three_buttons");
+        mRenderTestRule.render(getListView().getChildAt(0), "tinted_icon_row_three_buttons");
 
         int downloadRowIndex = findIndexOfMenuItemById(R.id.downloads_row_menu_id);
         Assert.assertNotEquals("No download row found.", -1, downloadRowIndex);
@@ -450,7 +450,7 @@ public class TabbedAppMenuTest {
     testAddToMenuItem_not_bookmarked() throws IOException {
         LinearLayout actionBar = (LinearLayout) getListView().getChildAt(0);
         Assert.assertEquals(3, actionBar.getChildCount());
-        mRenderTestRule.render(getListView().getChildAt(0), "icon_row_three_buttons");
+        mRenderTestRule.render(getListView().getChildAt(0), "tinted_icon_row_three_buttons");
 
         int addToIndex = findIndexOfMenuItemById(R.id.add_to_menu_id);
         Assert.assertNotEquals("No add to row found.", -1, addToIndex);
@@ -480,7 +480,7 @@ public class TabbedAppMenuTest {
         LinearLayout actionBar = (LinearLayout) getListView().getChildAt(0);
         Assert.assertEquals("Add to Bookmarks/Downloads/Home screen should be shown", 3,
                 actionBar.getChildCount());
-        mRenderTestRule.render(getListView().getChildAt(0), "icon_row_three_buttons");
+        mRenderTestRule.render(getListView().getChildAt(0), "tinted_icon_row_three_buttons");
 
         int addToIndex = findIndexOfMenuItemById(R.id.add_to_menu_id);
         Assert.assertNotEquals("No add to row found.", -1, addToIndex);

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

@@ -495,7 +495,9 @@ class AppMenuAdapter extends BaseAdapter {
             setupImageButton(holder.buttons[i], item.getSubMenu().getItem(i));
         }
 
-        if (CachedFeatureFlags.isEnabled(ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_ICONS)) {
+        if (CachedFeatureFlags.isEnabled(ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_ICONS)
+                || CachedFeatureFlags.isEnabled(
+                        ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_THREE_BUTTON_ACTIONBAR)) {
             // Tint action bar's background.
             convertView.setBackgroundDrawable(ApiCompatibilityUtils.getDrawable(
                     convertView.getContext().getResources(), R.drawable.menu_action_bar_bg));