Fix chip styling to fully conform to specs

-Resolve issue with text overlapping close button on multiline chips
-Add top and bottom padding for multiline chips (12dp)
-Increase start and end chip padding (16dp) and padding close button padding (16dp)
-Switch to lens icon with color
-Increase size of close icon to 24dp to match lens icon
-Align text to viewStart

Screenshot light: https://drive.google.com/file/d/1kpiDc_Wr14hPvrPqF5T9ExZhhl-nW7MQ/view
Screenshot dark: https://drive.google.com/file/d/1LPfy4zJ9wYZRR9E71A6fvvgwFpPh2-b0/view

Bug: 1099982
Change-Id: I41307dd2168bd3a7e0ecb3fb8c01b0df1ee64764
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436516
Commit-Queue: Ben Goldberger <benwgold@google.com>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Sinan Sahin <sinansahin@google.com>
Cr-Commit-Position: refs/heads/master@{#812555}

chrome/android/java/res/drawable/lens_icon.xml

@@ -8,20 +8,19 @@
     android:height="24dp"
     android:viewportWidth="24"
     android:viewportHeight="24">
-
     <path
-        android:fillColor="@color/default_icon_color_secondary"
-        android:pathData="M16.2726 5.69695C17.606 5.69695 18.6969 6.78786 18.6969 8.12119V10.0606H21.1211V8.12119C21.1211 5.4424 18.9514 3.27271 16.2726 3.27271H14.3333V5.69695H16.2726Z" />
+        android:fillColor="#FBBC04"
+        android:pathData="M15.2729 5.69695C16.6062 5.69695 17.6971 6.78786 17.6971 8.12119V10.0606H20.1214V8.12119C20.1214 5.4424 17.9517 3.27271 15.2729 3.27271H13.3335V5.69695H15.2729Z" />
     <path
-        android:fillColor="@color/default_icon_color_secondary"
-        android:pathData="M6.09087 8.12119C6.09087 6.78786 7.18178 5.69695 8.51511 5.69695H10.4545V3.27271H8.51511C5.83632 3.27271 3.66663 5.4424 3.66663 8.12119V10.0606H6.09087V8.12119Z" />
+        android:fillColor="#EA4335"
+        android:pathData="M5.09099 8.12119C5.09099 6.78786 6.1819 5.69695 7.51523 5.69695H9.45463V3.27271H7.51523C4.83645 3.27271 2.66675 5.4424 2.66675 8.12119V10.0606H5.09099V8.12119Z" />
     <path
-        android:fillColor="@color/default_icon_color_secondary"
-        android:pathData="M8.51511 18.3031C7.18178 18.3031 6.09087 17.2122 6.09087 15.8788V13.9395H3.66663V15.8788C3.66663 18.5576 5.83632 20.7273 8.51511 20.7273H10.4545V18.3031H8.51511Z" />
+        android:fillColor="#4285F4"
+        android:pathData="M7.51523 18.3031C6.1819 18.3031 5.09099 17.2122 5.09099 15.8788V13.9395H2.66675V15.8788C2.66675 18.5576 4.83645 20.7273 7.51523 20.7273H9.45463V18.3031H7.51523Z" />
     <path
-        android:fillColor="@color/default_icon_color_secondary"
-        android:pathData="M12.3939 15.3938C14.2684 15.3938 15.7879 13.8743 15.7879 11.9999C15.7879 10.1255 14.2684 8.60596 12.3939 8.60596C10.5195 8.60596 9 10.1255 9 11.9999C9 13.8743 10.5195 15.3938 12.3939 15.3938Z" />
+        android:fillColor="#4285F4"
+        android:pathData="M11.3939 15.3938C13.2684 15.3938 14.7879 13.8743 14.7879 11.9999C14.7879 10.1255 13.2684 8.60596 11.3939 8.60596C9.51952 8.60596 8 10.1255 8 11.9999C8 13.8743 9.51952 15.3938 11.3939 15.3938Z" />
     <path
-        android:fillColor="@color/default_icon_color_secondary"
-        android:pathData="M18.2121 19.7575C19.2832 19.7575 20.1515 18.8892 20.1515 17.8181C20.1515 16.747 19.2832 15.8787 18.2121 15.8787C17.141 15.8787 16.2727 16.747 16.2727 17.8181C16.2727 18.8892 17.141 19.7575 18.2121 19.7575Z" />
+        android:fillColor="#34A853"
+        android:pathData="M17.2121 19.7575C18.2832 19.7575 19.1515 18.8892 19.1515 17.8181C19.1515 16.747 18.2832 15.8787 17.2121 15.8787C16.141 15.8787 15.2727 16.747 15.2727 17.8181C15.2727 18.8892 16.141 19.7575 17.2121 19.7575Z" />
 </vector>
\ No newline at end of file

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

@@ -489,8 +489,9 @@
 
     <!-- Revamped Context Menu Dimensions -->
     <!-- Prevent the chip from reaching the edges of the screen for long translations. -->
-    <dimen name="context_menu_chip_max_width">300dp</dimen>
+    <dimen name="context_menu_chip_max_width">338dp</dimen>
     <dimen name="context_menu_chip_vertical_margin">24dp</dimen>
+    <dimen name="context_menu_chip_lateral_padding">16dp</dimen>
     <!-- The chip touch target is 48dp. This reduces the chip size by 8dp to get to the desired
          40dp height (24dp for content + 4dp of padding at the top and bottom. -->
     <dimen name="context_menu_chip_vertical_inset">4dp</dimen>

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

@@ -642,7 +642,13 @@
         <item name="chipColor">@color/popup_bg_color</item>
         <item name="solidColorChip">true</item>
         <item name="allowMultipleLines">true</item>
+        <item name="textAlignStart">true</item>
+        <item name="leadingPadding">@dimen/context_menu_chip_lateral_padding</item>
+        <item name="endIconStartPadding">@dimen/context_menu_chip_lateral_padding</item>
+        <item name="endIconEndPadding">@dimen/context_menu_chip_lateral_padding</item>
         <item name="verticalInset">@dimen/context_menu_chip_vertical_inset</item>
+        <item name="endIconWidth">@dimen/context_menu_chip_icon_size</item>
+        <item name="endIconHeight">@dimen/context_menu_chip_icon_size</item>
     </style>
 
     <!-- Scrolling -->

chrome/android/java/src/org/chromium/chrome/browser/contextmenu/RevampedContextMenuChipController.java

@@ -52,14 +52,33 @@ class RevampedContextMenuChipController implements View.OnClickListener {
      * below.
      */
     int getVerticalPxNeededForChip() {
-        // TODO(benwgold): Come up with way to make height dynamic if wrapping or large text is
-        // activated.
         return 2
                 * mContext.getResources().getDimensionPixelSize(
                         R.dimen.context_menu_chip_vertical_margin)
                 + mContext.getResources().getDimensionPixelSize(R.dimen.chip_default_height);
     }
 
+    /**
+     * Derive max text view width by subtracting the width of other elements from the max chip
+     * width.
+     */
+    @VisibleForTesting
+    int getChipTextMaxWidthPx() {
+        return mContext.getResources().getDimensionPixelSize(R.dimen.context_menu_chip_max_width)
+                // Leading and trailing padding (start and end of chip and
+                // start / end of text).
+                - (3
+                        * mContext.getResources().getDimensionPixelSize(
+                                R.dimen.context_menu_chip_lateral_padding))
+                - (1
+                        * mContext.getResources().getDimensionPixelSize(
+                                R.dimen.chip_element_leading_padding))
+                // Primary and close icon width.
+                - (2
+                        * mContext.getResources().getDimensionPixelSize(
+                                R.dimen.context_menu_chip_icon_size));
+    }
+
     @VisibleForTesting
     void handleImageClassification(boolean isShoppingIntent) {
         if (isShoppingIntent) {
@@ -114,10 +133,13 @@ class RevampedContextMenuChipController implements View.OnClickListener {
         mPopupWindow.setMaxWidth(
                 mContext.getResources().getDimensionPixelSize(R.dimen.context_menu_chip_max_width));
 
-        // TODO(benwgold): Support wrapping text for longer strings.
         mChipView.getPrimaryTextView().setText(SpanApplier.removeSpanText(
                 mContext.getString(R.string.contextmenu_shop_image_with_google_lens),
                 new SpanInfo("<new>", "</new>")));
+        // TODO(benwgold): Consult with Chrome UX owners to see if Chip UI hierarchy should be
+        // refactored.
+        mChipView.getPrimaryTextView().setMaxWidth(getChipTextMaxWidthPx());
+
         mChipView.setIcon(R.drawable.lens_icon, false);
         mChipView.addRemoveIcon();
 

chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/RevampedContextMenuChipControllerTest.java

@@ -33,6 +33,14 @@ import org.chromium.ui.test.util.DummyUiActivityTestCase;
 public class RevampedContextMenuChipControllerTest extends DummyUiActivityTestCase {
     // This is the combination of the expected vertical margins and the chip height.
     private static final int EXPECTED_VERTICAL_DP = 80;
+    // Computed by taking the 338dp max width and subtracting:
+    // 16 (chip start padding)
+    // 24 (main icon width)
+    // 8 (text start padding)
+    // 16 (close button start padding)
+    // 24 (close button icon width)
+    // 16 (close button end padding)
+    private static final int EXPECTEED_CHIP_WIDTH_DP = 234;
 
     private float mMeasuredDeviceDensity;
     private View mAnchorView;
@@ -123,4 +131,14 @@ public class RevampedContextMenuChipControllerTest extends DummyUiActivityTestCa
                 (int) (EXPECTED_VERTICAL_DP * mMeasuredDeviceDensity),
                 chipController.getVerticalPxNeededForChip());
     }
+
+    @Test
+    @SmallTest
+    public void testExpectedChipTextMaxWidthPx() {
+        RevampedContextMenuChipController chipController = new RevampedContextMenuChipController(
+                getActivity(), mAnchorView, mLensAsyncManager, () -> {});
+        assertEquals("Vertical px is not matching the expectation",
+                (int) (EXPECTEED_CHIP_WIDTH_DP * mMeasuredDeviceDensity),
+                chipController.getChipTextMaxWidthPx());
+    }
 }

ui/android/java/res/values/attrs.xml

@@ -21,15 +21,20 @@
     <declare-styleable name="ChipView">
         <attr name="allowMultipleLines" format="boolean"/>
         <attr name="solidColorChip" format="boolean"/>
+        <attr name="textAlignStart" format="boolean"/>
         <attr name="chipColor" format="color"/>
         <attr name="chipStyle" format="reference"/>
         <attr name="cornerRadius" format="dimension"/>
+        <attr name="leadingPadding" format="reference|dimension"/>
+        <attr name="endPadding" format="reference|dimension"/>
         <attr name="iconWidth" format="reference|dimension"/>
         <attr name="iconHeight" format="reference|dimension"/>
         <attr name="useRoundedIcon" format="boolean"/>
         <attr name="primaryTextAppearance" format="reference"/>
         <attr name="endIconWidth" format="reference|dimension"/>
         <attr name="endIconHeight" format="reference|dimension"/>
+        <attr name="endIconStartPadding" format="reference|dimension"/>
+        <attr name="endIconEndPadding" format="reference|dimension"/>
         <attr name="secondaryTextAppearance" format="reference"/>
         <attr name="rippleColor"/>
         <attr name="verticalInset"/>

ui/android/java/res/values/dimens.xml

@@ -48,6 +48,7 @@
     <dimen name="chip_icon_size">20dp</dimen>
     <dimen name="chip_end_icon_margin_start">8dp</dimen>
     <dimen name="chip_end_padding_with_end_icon">6dp</dimen>
+    <dimen name="chip_text_multiline_vertical_padding">12dp</dimen>
 
     <!-- Dropdown default measures -->
     <dimen name="dropdown_item_height">50dp</dimen>

ui/android/java/src/org/chromium/ui/widget/ChipView.java

@@ -10,6 +10,7 @@ import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.view.ContextThemeWrapper;
 import android.view.Gravity;
+import android.view.View;
 import android.view.ViewGroup;
 import android.widget.FrameLayout;
 import android.widget.LinearLayout;
@@ -45,6 +46,8 @@ public class ChipView extends LinearLayout {
     private final @IdRes int mSecondaryTextAppearanceId;
     private final int mEndIconWidth;
     private final int mEndIconHeight;
+    private final int mEndIconStartPadding;
+    private final int mEndIconEndPadding;
 
     private ViewGroup mEndIconWrapper;
     private TextView mSecondaryText;
@@ -66,14 +69,22 @@ public class ChipView extends LinearLayout {
     private ChipView(Context context, AttributeSet attrs, @StyleRes int themeOverlay) {
         super(new ContextThemeWrapper(context, themeOverlay), attrs, R.attr.chipStyle);
 
+        TypedArray a = getContext().obtainStyledAttributes(
+                attrs, R.styleable.ChipView, R.attr.chipStyle, 0);
+
         @Px
-        int leadingElementPadding =
-                getResources().getDimensionPixelSize(R.dimen.chip_element_leading_padding);
+        int leadingElementPadding = a.getDimensionPixelSize(R.styleable.ChipView_leadingPadding,
+                getResources().getDimensionPixelSize(R.dimen.chip_element_leading_padding));
         @Px
-        int endPadding = getResources().getDimensionPixelSize(R.dimen.chip_end_padding);
+        int endPadding = a.getDimensionPixelSize(R.styleable.ChipView_endPadding,
+                getResources().getDimensionPixelSize(R.dimen.chip_end_padding));
+
+        mEndIconStartPadding = a.getDimensionPixelSize(R.styleable.ChipView_endIconStartPadding,
+                getResources().getDimensionPixelSize(R.dimen.chip_end_icon_margin_start));
+
+        mEndIconEndPadding = a.getDimensionPixelSize(R.styleable.ChipView_endIconEndPadding,
+                getResources().getDimensionPixelSize(R.dimen.chip_end_padding_with_end_icon));
 
-        TypedArray a = getContext().obtainStyledAttributes(
-                attrs, R.styleable.ChipView, R.attr.chipStyle, 0);
         boolean solidColorChip = a.getBoolean(R.styleable.ChipView_solidColorChip, false);
         int chipBorderWidthId =
                 solidColorChip ? R.dimen.chip_solid_border_width : R.dimen.chip_border_width;
@@ -100,6 +111,7 @@ public class ChipView extends LinearLayout {
         int verticalInset = a.getDimensionPixelSize(R.styleable.ChipView_verticalInset,
                 getResources().getDimensionPixelSize(R.dimen.chip_bg_vertical_inset));
         boolean allowMultipleLines = a.getBoolean(R.styleable.ChipView_allowMultipleLines, false);
+        boolean textAlignStart = a.getBoolean(R.styleable.ChipView_textAlignStart, false);
         a.recycle();
 
         mStartIcon = new ChromeImageView(getContext());
@@ -111,17 +123,30 @@ public class ChipView extends LinearLayout {
             leadingElementPadding = (chipHeight - iconHeight) / 2;
         }
 
-        // Setting this enforces 16dp padding at the end and 8dp at the start. For text, the start
-        // padding needs to be 16dp which is why a ChipTextView contributes the remaining 8dp.
+        // Setting this enforces 16dp padding at the end and 8dp at the start (unless overridden).
+        // For text, the start padding needs to be 16dp which is why a ChipTextView contributes the
+        // remaining 8dp.
         ViewCompat.setPaddingRelative(this, leadingElementPadding, 0, endPadding, 0);
 
         mPrimaryText = new TextView(new ContextThemeWrapper(getContext(), R.style.ChipTextView));
         ApiCompatibilityUtils.setTextAppearance(mPrimaryText, primaryTextAppearance);
 
         // If false fall back to single line defined in XML styles.
-        // TODO(benwgold): Fix issue where long text strings cover the close button.
         if (allowMultipleLines) {
             mPrimaryText.setMaxLines(MAX_LINES);
+            // Vertical padding must be explicitly defined for the text view to create space if text
+            // wrapping causes the chip to increase in size vertically.
+            int minMultilineVerticalTextPadding = getResources().getDimensionPixelSize(
+                    R.dimen.chip_text_multiline_vertical_padding);
+            // TODO(benwgold): Test for non multiline chips to see if 4dp vertical padding can be
+            // safely applied to all chips without affecting styling.
+            mPrimaryText.setPaddingRelative(mPrimaryText.getPaddingStart(),
+                    minMultilineVerticalTextPadding, mPrimaryText.getPaddingEnd(),
+                    minMultilineVerticalTextPadding);
+        }
+        if (textAlignStart) {
+            // Default of 'center' is defined in the ChipTextView style.
+            mPrimaryText.setTextAlignment((View.TEXT_ALIGNMENT_VIEW_START));
         }
         addView(mPrimaryText);
 
@@ -185,10 +210,8 @@ public class ChipView extends LinearLayout {
 
         FrameLayout.LayoutParams layoutParams =
                 new FrameLayout.LayoutParams(mEndIconWidth, mEndIconHeight);
-        layoutParams.setMarginStart(
-                getResources().getDimensionPixelSize(R.dimen.chip_end_icon_margin_start));
-        layoutParams.setMarginEnd(
-                getResources().getDimensionPixelSize(R.dimen.chip_end_padding_with_end_icon));
+        layoutParams.setMarginStart(mEndIconStartPadding);
+        layoutParams.setMarginEnd(mEndIconEndPadding);
         layoutParams.gravity = Gravity.CENTER_VERTICAL;
         mEndIconWrapper.addView(endIcon, layoutParams);
         addView(mEndIconWrapper,