Use views::style::GetColor in MenuItemView

This change attempts to be a behavior preserving change at the views
layer while allowing the Chrome Typography provider to specify colors
for menu item text.

The mapping of Native Color IDs to styles are as follows:

kColorId_EnabledMenuItemForegroundColor     = CONTEXT_MENU + STYLE_PRIMARY
kColorId_SelectedMenuItemForegroundColor    = CONTEXT_MENU + STYLE_SELECTED
kColorId_DisabledMenuItemForegroundColor    = CONTEXT_MENU + STYLE_DISABLED
kColorId_TouchableMenuItemLabelColor        = CONTEXT_TOUCH_MENU + STYLE_PRIMARY
kColorId_MenuItemMinorTextColor             = CONTEXT_MENU + STYLE_SECONDARY
kColorId_HighlightedMenuItemForegroundColor = CONTEXT_MENU + STYLE_HIGHLIGHTED

The resultant colors are as follows, with views representing the old colors.

                                             Light Mode       Light Mode
                                             Default (Views)  Chrome
kColorId_EnabledMenuItemForegroundColor      Black            GG900
kColorId_SelectedMenuItemForegroundColor     Black            GG900
kColorId_DisabledMenuItemForegroundColor     GG600 [1]        9e9e9e
kColorId_TouchableMenuItemLabelColor         GG900            GG900
kColorId_MenuItemMinorTextColor              Black A:0x89     GG700
kColorId_HighlightedMenuItemForegroundColor  GG900            GG900
* (A:Value) means Alpha Set to Value, transparent at 0, fully opaque at 255.
[1] kDisabledTextColor

                                             Dark Mode        Dark Mode
                                             Default (Views)  Chrome
kColorId_EnabledMenuItemForegroundColor      GG200            White A:0xDD
kColorId_SelectedMenuItemForegroundColor     GG200            White A:0xDD
kColorId_DisabledMenuItemForegroundColor     GG600            GG600 [2]
kColorId_TouchableMenuItemLabelColor         GG900            White A:0xDD
kColorId_MenuItemMinorTextColor              Black A:0x89     GG500
kColorId_HighlightedMenuItemForegroundColor  GG200            White A:0xDD
* (A:Value) means Alpha Set to Value, transparent at 0, fully opaque at 255.
[2] Was GG800 by default, but changed to GG600 to maintain previous contrast.

We can consider merging the colors after this change.

BUG=865318

Change-Id: Ie0c0b566637e752e9947c702e7fb8160d51a3558
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1767083
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694284}

chrome/browser/ui/views/chrome_typography_provider.cc

@@ -187,6 +187,10 @@ SkColor ChromeTypographyProvider::GetColor(const views::View& view,
   const ui::NativeTheme* native_theme = view.GetNativeTheme();
   DCHECK(native_theme);
   if (ShouldIgnoreHarmonySpec(*native_theme)) {
+    if (context == views::style::CONTEXT_MENU ||
+        context == views::style::CONTEXT_TOUCH_MENU) {
+      return TypographyProvider::GetColor(view, context, style);
+    }
     return GetHarmonyTextColorForNonStandardNativeTheme(context, style,
                                                         *native_theme);
   }
@@ -217,9 +221,10 @@ SkColor ChromeTypographyProvider::GetColor(const views::View& view,
     case views::style::STYLE_DIALOG_BUTTON_DEFAULT:
       return SK_ColorWHITE;
     case views::style::STYLE_DISABLED:
-      return native_theme->ShouldUseDarkColors()
-                 ? gfx::kGoogleGrey800
-                 : SkColorSetRGB(0x9e, 0x9e, 0x9e);
+      if (!native_theme->ShouldUseDarkColors())
+        return SkColorSetRGB(0x9e, 0x9e, 0x9e);
+      return context == views::style::CONTEXT_MENU ? gfx::kGoogleGrey600
+                                                   : gfx::kGoogleGrey800;
     case views::style::STYLE_LINK:
       return gfx::kGoogleBlue700;
     case views::style::STYLE_SECONDARY:

ui/views/controls/menu/menu_item_view.cc

@@ -40,6 +40,7 @@
 #include "ui/views/controls/menu/menu_separator.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/controls/separator.h"
+#include "ui/views/style/typography.h"
 #include "ui/views/vector_icons.h"
 #include "ui/views/view_class_properties.h"
 #include "ui/views/widget/widget.h"
@@ -1131,23 +1132,28 @@ void MenuItemView::PaintMinorIconAndText(
 }
 
 SkColor MenuItemView::GetTextColor(bool minor, bool render_selection) const {
-  ui::NativeTheme::ColorId color_id =
-      minor ? ui::NativeTheme::kColorId_MenuItemMinorTextColor
-            : ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor;
+  style::TextContext context = style::CONTEXT_MENU;
+  style::TextStyle text_style =
+      minor ? text_style = style::STYLE_SECONDARY : style::STYLE_PRIMARY;
+
   if (GetEnabled()) {
     if (render_selection)
-      color_id = ui::NativeTheme::kColorId_SelectedMenuItemForegroundColor;
+      text_style = style::STYLE_SELECTED;
   } else {
-    color_id = ui::NativeTheme::kColorId_DisabledMenuItemForegroundColor;
+    text_style = style::STYLE_DISABLED;
   }
 
-  if (GetMenuController() && GetMenuController()->use_touchable_layout())
-    color_id = ui::NativeTheme::kColorId_TouchableMenuItemLabelColor;
+  if (GetMenuController() && GetMenuController()->use_touchable_layout()) {
+    context = style::CONTEXT_TOUCH_MENU;
+    text_style = style::STYLE_PRIMARY;
+  }
 
-  if (type_ == HIGHLIGHTED)
-    color_id = ui::NativeTheme::kColorId_HighlightedMenuItemForegroundColor;
+  if (type_ == HIGHLIGHTED) {
+    context = style::CONTEXT_MENU;
+    text_style = style::STYLE_HIGHLIGHTED;
+  }
 
-  return GetNativeTheme()->GetSystemColor(color_id);
+  return style::GetColor(*this, context, text_style);
 }
 
 void MenuItemView::DestroyAllMenuHosts() {

ui/views/style/typography.h

@@ -49,6 +49,9 @@ enum TextContext {
   // An editable text field. Usually matches CONTROL_LABEL.
   CONTEXT_TEXTFIELD,
 
+  // Text in a menu.
+  CONTEXT_MENU,
+
   // Text for the menu items that appear in the touch-selection context menu.
   CONTEXT_TOUCH_MENU,
 
@@ -74,6 +77,12 @@ enum TextStyle {
   // Secondary text: Appears near the primary text.
   STYLE_SECONDARY,
 
+  // Style for text that is displayed in a selection.
+  STYLE_SELECTED,
+
+  // Style for text is part of a static highlight.
+  STYLE_HIGHLIGHTED,
+
   // Style for the default button on a dialog.
   STYLE_DIALOG_BUTTON_DEFAULT,
 

ui/views/style/typography_provider.cc

@@ -101,6 +101,26 @@ SkColor TypographyProvider::GetColor(const views::View& view,
     color_id = style == style::STYLE_DISABLED
                    ? ui::NativeTheme::kColorId_TextfieldReadOnlyColor
                    : ui::NativeTheme::kColorId_TextfieldDefaultColor;
+  } else if (context == style::CONTEXT_MENU) {
+    switch (style) {
+      case views::style::STYLE_PRIMARY:
+        color_id = ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor;
+        break;
+      case views::style::STYLE_DISABLED:
+        color_id = ui::NativeTheme::kColorId_DisabledMenuItemForegroundColor;
+        break;
+      case views::style::STYLE_SECONDARY:
+        color_id = ui::NativeTheme::kColorId_MenuItemMinorTextColor;
+        break;
+      case views::style::STYLE_SELECTED:
+        color_id = ui::NativeTheme::kColorId_SelectedMenuItemForegroundColor;
+        break;
+      case views::style::STYLE_HIGHLIGHTED:
+        color_id = ui::NativeTheme::kColorId_HighlightedMenuItemForegroundColor;
+        break;
+    }
+  } else if (context == style::CONTEXT_TOUCH_MENU) {
+    color_id = ui::NativeTheme::kColorId_TouchableMenuItemLabelColor;
   } else if (style == style::STYLE_DISABLED) {
     color_id = ui::NativeTheme::kColorId_LabelDisabledColor;
   }