Adjust common theme menu colors.

Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=1024091#c6

In addition to the color value changes, this eliminates the touch-specific IDs
added in https://chromium-review.googlesource.com/c/chromium/src/+/1008841/ .
These colors were to follow the spec atop
https://bugs.chromium.org/p/chromium/issues/detail?id=826907 , but rather than
change non-touch menu colors the same way, the original change added new IDs to
allow variance.  This change instead unifies the behavior very close to the
current touch spec, except using a Google grey directly for the separator
instead of the roughly-equivalent alpha blend.  (In theory, it also supports
dark mode and other menu text styles, e.g. disabled items; in practice I don't
believe either of these is used today, and I don't think the original intent
was to prevent touch menus from using alternate styles, it was simply not
necessary.

Bug: 1024091
Change-Id: Ib3addd16ed0915720b64dfda2b629f459b2313e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1915659
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715281}

chrome/browser/ui/libgtkui/native_theme_gtk.cc

@@ -119,9 +119,6 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
             "GtkMenu#menu GtkSeparator#separator.horizontal");
       }
       return GetFgColor("GtkMenu#menu GtkMenuItem#menuitem.separator");
-    case ui::NativeTheme::kColorId_TouchableMenuItemLabelColor:
-    case ui::NativeTheme::kColorId_ActionableSubmenuVerticalSeparatorColor:
-      return gfx::kPlaceholderColor;
     // Fallback to the same colors as Aura.
     case ui::NativeTheme::kColorId_HighlightedMenuItemBackgroundColor:
     case ui::NativeTheme::kColorId_HighlightedMenuItemForegroundColor:

chrome/browser/ui/views/chrome_typography_provider.cc

@@ -90,10 +90,6 @@ SkColor GetHarmonyTextColorForNonStandardNativeTheme(
     case views::style::CONTEXT_TEXTFIELD:
       color_id = ui::NativeTheme::kColorId_TextfieldDefaultColor;
       break;
-    case views::style::CONTEXT_MENU:
-    case views::style::CONTEXT_TOUCH_MENU:
-      color_id = ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor;
-      break;
   }
 
   switch (style) {

ui/native_theme/common_theme.cc

@@ -78,19 +78,15 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id,
       case NativeTheme::kColorId_SelectedMenuItemForegroundColor:
       case NativeTheme::kColorId_HighlightedMenuItemForegroundColor:
         return gfx::kGoogleGrey200;
-      case NativeTheme::kColorId_FocusedMenuItemBackgroundColor:
-        return SkColorSetA(SK_ColorWHITE, 0x29);
-      case NativeTheme::kColorId_MenuBorderColor:
-        return gfx::kGoogleGrey800;
       case NativeTheme::kColorId_MenuSeparatorColor:
-        return SkColorSetA(gfx::kGoogleGrey800, 0xCC);
+        return gfx::kGoogleGrey800;
       case NativeTheme::kColorId_MenuBackgroundColor:
         return color_utils::AlphaBlend(SK_ColorWHITE, gfx::kGoogleGrey900,
                                        0.04f);
       case NativeTheme::kColorId_HighlightedMenuItemBackgroundColor:
         return SkColorSetRGB(0x32, 0x36, 0x39);
       case NativeTheme::kColorId_MenuItemAlertBackgroundColor:
-        return gfx::kGoogleGrey100;
+        return gfx::kGoogleBlue300;
 
       // Label
       case NativeTheme::kColorId_LabelEnabledColor:
@@ -206,29 +202,32 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id,
       return gfx::kGoogleGrey300;
 
     // MenuItem
-    case NativeTheme::kColorId_TouchableMenuItemLabelColor:
-      return kPrimaryTextColor;
-    case NativeTheme::kColorId_ActionableSubmenuVerticalSeparatorColor:
-      return SkColorSetA(gfx::kGoogleGrey900, 0x24);
-    case NativeTheme::kColorId_SelectedMenuItemForegroundColor:
     case NativeTheme::kColorId_EnabledMenuItemForegroundColor:
-      return SK_ColorBLACK;
-    case NativeTheme::kColorId_MenuBorderColor:
-      return gfx::kGoogleGrey300;
+    case NativeTheme::kColorId_SelectedMenuItemForegroundColor:
+    case NativeTheme::kColorId_HighlightedMenuItemForegroundColor:
+      return kPrimaryTextColor;
+    case NativeTheme::kColorId_FocusedMenuItemBackgroundColor:
+    case NativeTheme::kColorId_MenuBorderColor: {
+      const SkColor bg = base_theme->GetSystemColor(
+          NativeTheme::kColorId_MenuBackgroundColor, color_scheme);
+      return color_utils::BlendForMinContrast(bg, bg, base::nullopt, 1.67f)
+          .color;
+    }
     case NativeTheme::kColorId_MenuSeparatorColor:
-      return gfx::kGoogleGrey200;
+      return gfx::kGoogleGrey300;
     case NativeTheme::kColorId_MenuBackgroundColor:
       return SK_ColorWHITE;
-    case NativeTheme::kColorId_FocusedMenuItemBackgroundColor:
-      return gfx::kGoogleGrey300;
     case NativeTheme::kColorId_DisabledMenuItemForegroundColor:
       return kDisabledTextColor;
-    case NativeTheme::kColorId_MenuItemMinorTextColor:
-      return SkColorSetA(SK_ColorBLACK, 0x89);
+    case NativeTheme::kColorId_MenuItemMinorTextColor: {
+      const SkColor bg = base_theme->GetSystemColor(
+          NativeTheme::kColorId_MenuBackgroundColor, color_scheme);
+      const SkColor fg = base_theme->GetSystemColor(
+          NativeTheme::kColorId_EnabledMenuItemForegroundColor, color_scheme);
+      return color_utils::BlendForMinContrast(kDisabledTextColor, bg, fg).color;
+    }
     case NativeTheme::kColorId_HighlightedMenuItemBackgroundColor:
       return gfx::kGoogleGrey050;
-    case NativeTheme::kColorId_HighlightedMenuItemForegroundColor:
-      return kPrimaryTextColor;
     case NativeTheme::kColorId_MenuItemAlertBackgroundColor:
       return gfx::kGoogleBlue600;
 

ui/native_theme/native_theme.h

@@ -333,8 +333,6 @@ class NATIVE_THEME_EXPORT NativeTheme {
     kColorId_TextOnProminentButtonColor,
     kColorId_ButtonBorderColor,
     // MenuItem
-    kColorId_TouchableMenuItemLabelColor,
-    kColorId_ActionableSubmenuVerticalSeparatorColor,
     kColorId_EnabledMenuItemForegroundColor,
     kColorId_DisabledMenuItemForegroundColor,
     kColorId_SelectedMenuItemForegroundColor,

ui/native_theme/native_theme_win.cc

@@ -732,12 +732,10 @@ SkColor NativeThemeWin::GetPlatformHighContrastColor(ColorId color_id) const {
       return system_colors_[SystemThemeColor::kButtonFace];
 
     // Button Text Foreground
-    case kColorId_TouchableMenuItemLabelColor:
     case kColorId_EnabledMenuItemForegroundColor:
     case kColorId_MenuItemMinorTextColor:
     case kColorId_MenuBorderColor:
     case kColorId_MenuSeparatorColor:
-    case kColorId_ActionableSubmenuVerticalSeparatorColor:
     case kColorId_SeparatorColor:
     case kColorId_TextfieldDefaultColor:
     case kColorId_ButtonEnabledColor:

ui/views/controls/menu/menu_item_view.cc

@@ -862,7 +862,7 @@ void MenuItemView::Init(MenuItemView* parent,
     vertical_separator_->SetFocusBehavior(FocusBehavior::NEVER);
     const MenuConfig& config = MenuConfig::instance();
     vertical_separator_->SetColor(GetNativeTheme()->GetSystemColor(
-        ui::NativeTheme::kColorId_ActionableSubmenuVerticalSeparatorColor));
+        ui::NativeTheme::kColorId_MenuSeparatorColor));
     vertical_separator_->SetPreferredSize(
         gfx::Size(config.actionable_submenu_vertical_separator_width,
                   config.actionable_submenu_vertical_separator_height));

ui/views/examples/native_theme_example.cc

@@ -86,10 +86,6 @@ std::unique_ptr<View> CreateAllColorsView() {
                  COLOR_LABEL_ARGS(kColorId_ProminentButtonDisabledColor));
   InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_TextOnProminentButtonColor));
   InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonBorderColor));
-  InsertColorRow(layout,
-                 COLOR_LABEL_ARGS(kColorId_TouchableMenuItemLabelColor));
-  InsertColorRow(layout, COLOR_LABEL_ARGS(
-                             kColorId_ActionableSubmenuVerticalSeparatorColor));
   InsertColorRow(layout,
                  COLOR_LABEL_ARGS(kColorId_EnabledMenuItemForegroundColor));
   InsertColorRow(layout,

ui/views/style/typography_provider.cc

@@ -101,7 +101,8 @@ 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) {
+  } else if ((context == style::CONTEXT_MENU) ||
+             (context == style::CONTEXT_TOUCH_MENU)) {
     switch (style) {
       case views::style::STYLE_PRIMARY:
         color_id = ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor;
@@ -119,11 +120,6 @@ SkColor TypographyProvider::GetColor(const views::View& view,
         color_id = ui::NativeTheme::kColorId_HighlightedMenuItemForegroundColor;
         break;
     }
-  } else if (context == style::CONTEXT_TOUCH_MENU) {
-    color_id =
-        style == views::style::STYLE_HIGHLIGHTED
-            ? ui::NativeTheme::kColorId_HighlightedMenuItemForegroundColor
-            : ui::NativeTheme::kColorId_TouchableMenuItemLabelColor;
   } else if (style == style::STYLE_DISABLED) {
     color_id = ui::NativeTheme::kColorId_LabelDisabledColor;
   }