 Chromium Code Reviews
 Chromium Code Reviews Issue 2382443007:
  Clean up NativeTheme (particularly CommonTheme).  (Closed)
    
  
    Issue 2382443007:
  Clean up NativeTheme (particularly CommonTheme).  (Closed) 
  | Index: ui/native_theme/common_theme.cc | 
| diff --git a/ui/native_theme/common_theme.cc b/ui/native_theme/common_theme.cc | 
| index ea18adb1fdf6a4dff183f7010f424401630297ed..2bcf53e98356f76d040e77b21808aac404a476be 100644 | 
| --- a/ui/native_theme/common_theme.cc | 
| +++ b/ui/native_theme/common_theme.cc | 
| @@ -50,122 +50,25 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| gfx::kDisabledControlAlpha); | 
| default: | 
| - break; | 
| - } | 
| - } | 
| - | 
| - // MD colors. | 
| - if (ui::MaterialDesignController::IsModeMaterial()) { | 
| - // Dialogs: | 
| - static const SkColor kDialogBackgroundColorMd = SK_ColorWHITE; | 
| - // Buttons: | 
| - static const SkColor kButtonEnabledColorMd = gfx::kChromeIconGrey; | 
| - // MenuItem: | 
| - static const SkColor kMenuHighlightBackgroundColorMd = | 
| - SkColorSetARGB(0x14, 0x00, 0x00, 0x00); | 
| - static const SkColor kSelectedMenuItemForegroundColorMd = SK_ColorBLACK; | 
| - // Link: | 
| - static const SkColor kLinkEnabledColorMd = gfx::kGoogleBlue700; | 
| - // Results tables: | 
| - static const SkColor kResultsTableTextMd = SK_ColorBLACK; | 
| - static const SkColor kResultsTableDimmedTextMd = | 
| - SkColorSetRGB(0x64, 0x64, 0x64); | 
| - // Tooltip | 
| - static const SkColor kTooltipBackgroundMd = | 
| - SkColorSetA(SK_ColorBLACK, 0xCC); | 
| - static const SkColor kTooltipTextColorMd = SkColorSetA(SK_ColorWHITE, 0xDE); | 
| - | 
| - switch (color_id) { | 
| - // Dialogs | 
| - case NativeTheme::kColorId_DialogBackground: | 
| - case NativeTheme::kColorId_BubbleBackground: | 
| - return kDialogBackgroundColorMd; | 
| - | 
| - // Buttons | 
| - case NativeTheme::kColorId_ButtonEnabledColor: | 
| - case NativeTheme::kColorId_ButtonHoverColor: | 
| - return kButtonEnabledColorMd; | 
| - | 
| - // MenuItem | 
| - case NativeTheme::kColorId_FocusedMenuItemBackgroundColor: | 
| - return kMenuHighlightBackgroundColorMd; | 
| - case NativeTheme::kColorId_SelectedMenuItemForegroundColor: | 
| - return kSelectedMenuItemForegroundColorMd; | 
| - | 
| - // Link | 
| - case NativeTheme::kColorId_LinkEnabled: | 
| - case NativeTheme::kColorId_LinkPressed: | 
| - // Normal and pressed share a color. | 
| - return kLinkEnabledColorMd; | 
| - | 
| - // FocusableBorder | 
| - case NativeTheme::kColorId_FocusedBorderColor: | 
| - return gfx::kGoogleBlue500; | 
| - // TODO(estade): I'm not sure why this one is here but it should be | 
| - // removed in favor of the value in the SecondaryUiMaterial block. | 
| - case NativeTheme::kColorId_UnfocusedBorderColor: | 
| - return SkColorSetA(SK_ColorBLACK, 0x66); | 
| - | 
| - // Results Tables | 
| - case NativeTheme::kColorId_ResultsTableHoveredBackground: | 
| - return SkColorSetA(base_theme->GetSystemColor( | 
| - NativeTheme::kColorId_ResultsTableNormalText), | 
| - 0x0D); | 
| - case NativeTheme::kColorId_ResultsTableSelectedBackground: | 
| - return SkColorSetA(base_theme->GetSystemColor( | 
| - NativeTheme::kColorId_ResultsTableNormalText), | 
| - 0x14); | 
| - case NativeTheme::kColorId_ResultsTableNormalText: | 
| - case NativeTheme::kColorId_ResultsTableHoveredText: | 
| - case NativeTheme::kColorId_ResultsTableSelectedText: | 
| - return kResultsTableTextMd; | 
| - case NativeTheme::kColorId_ResultsTableNormalDimmedText: | 
| - case NativeTheme::kColorId_ResultsTableHoveredDimmedText: | 
| - case NativeTheme::kColorId_ResultsTableSelectedDimmedText: | 
| - return kResultsTableDimmedTextMd; | 
| - case NativeTheme::kColorId_ResultsTableNormalUrl: | 
| - case NativeTheme::kColorId_ResultsTableHoveredUrl: | 
| - case NativeTheme::kColorId_ResultsTableSelectedUrl: | 
| - return base_theme->GetSystemColor(NativeTheme::kColorId_LinkEnabled); | 
| - | 
| - // Tooltip | 
| - case NativeTheme::kColorId_TooltipBackground: | 
| - return kTooltipBackgroundMd; | 
| - case NativeTheme::kColorId_TooltipText: | 
| - return kTooltipTextColorMd; | 
| - | 
| - default: | 
| break; | 
| } | 
| } | 
| - // Pre-MD colors. | 
| - // Windows: | 
| - static const SkColor kWindowBackgroundColor = SK_ColorWHITE; | 
| // Dialogs: | 
| - static const SkColor kDialogBackgroundColor = SkColorSetRGB(251, 251, 251); | 
| - // FocusableBorder: | 
| - static const SkColor kFocusedBorderColor = SkColorSetRGB(0x4D, 0x90, 0xFE); | 
| - static const SkColor kUnfocusedBorderColor = SkColorSetRGB(0xD9, 0xD9, 0xD9); | 
| - // Button: | 
| - static const SkColor kButtonBackgroundColor = SkColorSetRGB(0xDE, 0xDE, 0xDE); | 
| - static const SkColor kButtonEnabledColor = SkColorSetRGB(0x22, 0x22, 0x22); | 
| - static const SkColor kButtonHighlightColor = SkColorSetRGB(0, 0, 0); | 
| - static const SkColor kButtonHoverColor = kButtonEnabledColor; | 
| - static const SkColor kButtonHoverBackgroundColor = | 
| - SkColorSetRGB(0xEA, 0xEA, 0xEA); | 
| - static const SkColor kBlueButtonEnabledColor = SK_ColorWHITE; | 
| - static const SkColor kBlueButtonDisabledColor = SK_ColorWHITE; | 
| - static const SkColor kBlueButtonPressedColor = SK_ColorWHITE; | 
| - static const SkColor kBlueButtonHoverColor = SK_ColorWHITE; | 
| - static const SkColor kBlueButtonShadowColor = SkColorSetRGB(0x53, 0x8C, 0xEA); | 
| + static const SkColor kDialogBackgroundColor = SK_ColorWHITE; | 
| + // Buttons: | 
| + static const SkColor kButtonEnabledColor = gfx::kChromeIconGrey; | 
| static const SkColor kProminentButtonColor = gfx::kGoogleBlue500; | 
| static const SkColor kProminentButtonTextColor = SK_ColorWHITE; | 
| - static const SkColor kButtonPressedShade = SK_ColorTRANSPARENT; | 
| + static const SkColor kBlueButtonTextColor = SK_ColorWHITE; | 
| + static const SkColor kBlueButtonShadowColor = SkColorSetRGB(0x53, 0x8C, 0xEA); | 
| // MenuItem: | 
| static const SkColor kMenuBackgroundColor = SK_ColorWHITE; | 
| static const SkColor kMenuHighlightBackgroundColor = | 
| - SkColorSetRGB(0x42, 0x81, 0xF4); | 
| + SkColorSetARGB(0x14, 0x00, 0x00, 0x00); | 
| 
tdanderson
2016/09/30 21:00:56
SkColorSetA()?
 
Evan Stade
2016/09/30 22:19:00
Done.
 | 
| + static const SkColor kSelectedMenuItemForegroundColor = SK_ColorBLACK; | 
| + static const SkColor kDisabledMenuItemForegroundColor = | 
| + SkColorSetRGB(0xA1, 0xA1, 0x92); | 
| static const SkColor kMenuBorderColor = SkColorSetRGB(0xBA, 0xBA, 0xBA); | 
| static const SkColor kEnabledMenuButtonBorderColor = | 
| SkColorSetARGB(0x24, 0x00, 0x00, 0x00); | 
| @@ -175,17 +78,8 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| SkColorSetARGB(0x48, 0x00, 0x00, 0x00); | 
| static const SkColor kMenuSeparatorColor = SkColorSetRGB(0xE9, 0xE9, 0xE9); | 
| static const SkColor kEnabledMenuItemForegroundColor = SK_ColorBLACK; | 
| - static const SkColor kDisabledMenuItemForegroundColor = | 
| - SkColorSetRGB(0xA1, 0xA1, 0x92); | 
| - static const SkColor kHoverMenuItemBackgroundColor = | 
| - SkColorSetARGB(0xCC, 0xFF, 0xFF, 0xFF); | 
| - // Label: | 
| - static const SkColor kLabelEnabledColor = kButtonEnabledColor; | 
| - static const SkColor kLabelBackgroundColor = SK_ColorWHITE; | 
| // Link: | 
| - static const SkColor kLinkDisabledColor = SK_ColorBLACK; | 
| - static const SkColor kLinkEnabledColor = SkColorSetRGB(0, 51, 153); | 
| - static const SkColor kLinkPressedColor = SK_ColorRED; | 
| + static const SkColor kLinkEnabledColor = gfx::kGoogleBlue700; | 
| // Textfield: | 
| static const SkColor kTextfieldDefaultColor = SK_ColorBLACK; | 
| static const SkColor kTextfieldDefaultBackground = SK_ColorWHITE; | 
| @@ -195,45 +89,12 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| SkColorSetARGB(0x54, 0x60, 0xA8, 0xEB); | 
| static const SkColor kTextfieldSelectionColor = color_utils::AlphaBlend( | 
| SK_ColorBLACK, kTextfieldSelectionBackgroundFocused, 0xdd); | 
| - // Tooltip | 
| - static const SkColor kTooltipBackground = 0xFFFFFFCC; | 
| - static const SkColor kTooltipTextColor = kLabelEnabledColor; | 
| - // Tree | 
| - static const SkColor kTreeBackground = SK_ColorWHITE; | 
| - static const SkColor kTreeTextColor = SK_ColorBLACK; | 
| - static const SkColor kTreeSelectedTextColor = SK_ColorBLACK; | 
| - static const SkColor kTreeSelectionBackgroundColor = | 
| - SkColorSetRGB(0xEE, 0xEE, 0xEE); | 
| - static const SkColor kTreeArrowColor = SkColorSetRGB(0x7A, 0x7A, 0x7A); | 
| - // Table | 
| - static const SkColor kTableBackground = SK_ColorWHITE; | 
| - static const SkColor kTableTextColor = SK_ColorBLACK; | 
| - static const SkColor kTableSelectedTextColor = SK_ColorBLACK; | 
| - static const SkColor kTableSelectionBackgroundColor = | 
| - SkColorSetRGB(0xEE, 0xEE, 0xEE); | 
| - static const SkColor kTableGroupingIndicatorColor = | 
| - SkColorSetRGB(0xCC, 0xCC, 0xCC); | 
| - // Results Tables | 
| - static const SkColor kResultsTableSelectedBackground = | 
| - kTextfieldSelectionBackgroundFocused; | 
| - static const SkColor kResultsTableNormalText = | 
| - color_utils::AlphaBlend(SK_ColorBLACK, kTextfieldDefaultBackground, 0xDD); | 
| + // Results tables: | 
| + static const SkColor kResultsTableText = SK_ColorBLACK; | 
| + static const SkColor kResultsTableDimmedText = | 
| + SkColorSetRGB(0x64, 0x64, 0x64); | 
| static const SkColor kResultsTableHoveredBackground = color_utils::AlphaBlend( | 
| kTextfieldSelectionBackgroundFocused, kTextfieldDefaultBackground, 0x40); | 
| - static const SkColor kResultsTableHoveredText = color_utils::AlphaBlend( | 
| - SK_ColorBLACK, kResultsTableHoveredBackground, 0xDD); | 
| - static const SkColor kResultsTableSelectedText = color_utils::AlphaBlend( | 
| - SK_ColorBLACK, kTextfieldSelectionBackgroundFocused, 0xDD); | 
| - static const SkColor kResultsTableNormalDimmedText = | 
| - color_utils::AlphaBlend(SK_ColorBLACK, kTextfieldDefaultBackground, 0xBB); | 
| - static const SkColor kResultsTableHoveredDimmedText = color_utils::AlphaBlend( | 
| - SK_ColorBLACK, kResultsTableHoveredBackground, 0xBB); | 
| - static const SkColor kResultsTableSelectedDimmedText = | 
| - color_utils::AlphaBlend(SK_ColorBLACK, | 
| - kTextfieldSelectionBackgroundFocused, 0xBB); | 
| - static const SkColor kResultsTableNormalUrl = kTextfieldSelectionColor; | 
| - static const SkColor kResultsTableSelectedOrHoveredUrl = | 
| - SkColorSetARGB(0xff, 0x0b, 0x80, 0x43); | 
| const SkColor kPositiveTextColor = SkColorSetRGB(0x0b, 0x80, 0x43); | 
| const SkColor kNegativeTextColor = SkColorSetRGB(0xc5, 0x39, 0x29); | 
| static const SkColor kResultsTablePositiveText = color_utils::AlphaBlend( | 
| @@ -252,46 +113,46 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| static const SkColor kResultsTableNegativeSelectedText = | 
| color_utils::AlphaBlend(kNegativeTextColor, | 
| kTextfieldSelectionBackgroundFocused, 0xDD); | 
| - // Material spinner/throbber | 
| + // Tooltip: | 
| + static const SkColor kTooltipBackground = SkColorSetA(SK_ColorBLACK, 0xCC); | 
| + static const SkColor kTooltipTextColor = SkColorSetA(SK_ColorWHITE, 0xDE); | 
| + // Tree: | 
| + static const SkColor kTreeBackground = SK_ColorWHITE; | 
| + static const SkColor kTreeTextColor = SK_ColorBLACK; | 
| + static const SkColor kTreeSelectedTextColor = SK_ColorBLACK; | 
| + static const SkColor kTreeSelectionBackgroundColor = | 
| + SkColorSetRGB(0xEE, 0xEE, 0xEE); | 
| + static const SkColor kTreeArrowColor = SkColorSetRGB(0x7A, 0x7A, 0x7A); | 
| + // Table: | 
| + static const SkColor kTableBackground = SK_ColorWHITE; | 
| + static const SkColor kTableTextColor = SK_ColorBLACK; | 
| + static const SkColor kTableSelectedTextColor = SK_ColorBLACK; | 
| + static const SkColor kTableSelectionBackgroundColor = | 
| + SkColorSetRGB(0xEE, 0xEE, 0xEE); | 
| + static const SkColor kTableGroupingIndicatorColor = | 
| + SkColorSetRGB(0xCC, 0xCC, 0xCC); | 
| + // Material spinner/throbber: | 
| static const SkColor kThrobberSpinningColor = gfx::kGoogleBlue500; | 
| static const SkColor kThrobberWaitingColor = SkColorSetRGB(0xA6, 0xA6, 0xA6); | 
| static const SkColor kThrobberLightColor = SkColorSetRGB(0xF4, 0xF8, 0xFD); | 
| switch (color_id) { | 
| - // Windows | 
| - case NativeTheme::kColorId_WindowBackground: | 
| - return kWindowBackgroundColor; | 
| - | 
| // Dialogs | 
| + case NativeTheme::kColorId_WindowBackground: | 
| case NativeTheme::kColorId_DialogBackground: | 
| case NativeTheme::kColorId_BubbleBackground: | 
| return kDialogBackgroundColor; | 
| - // FocusableBorder | 
| - case NativeTheme::kColorId_FocusedBorderColor: | 
| - return kFocusedBorderColor; | 
| - case NativeTheme::kColorId_UnfocusedBorderColor: | 
| - return kUnfocusedBorderColor; | 
| - | 
| - // Button | 
| - case NativeTheme::kColorId_ButtonBackgroundColor: | 
| - return kButtonBackgroundColor; | 
| + // Buttons | 
| case NativeTheme::kColorId_ButtonEnabledColor: | 
| - return kButtonEnabledColor; | 
| - case NativeTheme::kColorId_ButtonHighlightColor: | 
| - return kButtonHighlightColor; | 
| case NativeTheme::kColorId_ButtonHoverColor: | 
| - return kButtonHoverColor; | 
| - case NativeTheme::kColorId_ButtonHoverBackgroundColor: | 
| - return kButtonHoverBackgroundColor; | 
| + return kButtonEnabledColor; | 
| + // TODO(estade): remove the BlueButton colors. | 
| case NativeTheme::kColorId_BlueButtonEnabledColor: | 
| - return kBlueButtonEnabledColor; | 
| case NativeTheme::kColorId_BlueButtonDisabledColor: | 
| - return kBlueButtonDisabledColor; | 
| case NativeTheme::kColorId_BlueButtonPressedColor: | 
| - return kBlueButtonPressedColor; | 
| case NativeTheme::kColorId_BlueButtonHoverColor: | 
| - return kBlueButtonHoverColor; | 
| + return kBlueButtonTextColor; | 
| case NativeTheme::kColorId_BlueButtonShadowColor: | 
| return kBlueButtonShadowColor; | 
| case NativeTheme::kColorId_ProminentButtonColor: | 
| @@ -299,9 +160,13 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| case NativeTheme::kColorId_TextOnProminentButtonColor: | 
| return kProminentButtonTextColor; | 
| case NativeTheme::kColorId_ButtonPressedShade: | 
| - return kButtonPressedShade; | 
| + return SK_ColorTRANSPARENT; | 
| + case NativeTheme::kColorId_ButtonDisabledColor: | 
| + return kDisabledMenuItemForegroundColor; | 
| // MenuItem | 
| + case NativeTheme::kColorId_SelectedMenuItemForegroundColor: | 
| + return kSelectedMenuItemForegroundColor; | 
| case NativeTheme::kColorId_MenuBorderColor: | 
| return kMenuBorderColor; | 
| case NativeTheme::kColorId_EnabledMenuButtonBorderColor: | 
| @@ -316,35 +181,27 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| return kMenuBackgroundColor; | 
| case NativeTheme::kColorId_FocusedMenuItemBackgroundColor: | 
| return kMenuHighlightBackgroundColor; | 
| - case NativeTheme::kColorId_HoverMenuItemBackgroundColor: | 
| - return kHoverMenuItemBackgroundColor; | 
| case NativeTheme::kColorId_EnabledMenuItemForegroundColor: | 
| return kEnabledMenuItemForegroundColor; | 
| case NativeTheme::kColorId_DisabledMenuItemForegroundColor: | 
| return kDisabledMenuItemForegroundColor; | 
| - case NativeTheme::kColorId_DisabledEmphasizedMenuItemForegroundColor: | 
| - return SK_ColorBLACK; | 
| - case NativeTheme::kColorId_SelectedMenuItemForegroundColor: | 
| - return SK_ColorWHITE; | 
| - case NativeTheme::kColorId_ButtonDisabledColor: | 
| - return kDisabledMenuItemForegroundColor; | 
| // Label | 
| case NativeTheme::kColorId_LabelEnabledColor: | 
| - return kLabelEnabledColor; | 
| + return kButtonEnabledColor; | 
| case NativeTheme::kColorId_LabelDisabledColor: | 
| return base_theme->GetSystemColor( | 
| NativeTheme::kColorId_ButtonDisabledColor); | 
| - case NativeTheme::kColorId_LabelBackgroundColor: | 
| - return kLabelBackgroundColor; | 
| // Link | 
| + // TODO(estade): where, if anywhere, do we use disabled links in Chrome? | 
| case NativeTheme::kColorId_LinkDisabled: | 
| - return kLinkDisabledColor; | 
| + return SK_ColorBLACK; | 
| + | 
| case NativeTheme::kColorId_LinkEnabled: | 
| - return kLinkEnabledColor; | 
| case NativeTheme::kColorId_LinkPressed: | 
| - return kLinkPressedColor; | 
| + // Normal and pressed share a color. | 
| 
tdanderson
2016/09/30 21:00:56
nit: I don't think this comment adds much value.
 
Evan Stade
2016/09/30 22:18:59
Done.
 | 
| + return kLinkEnabledColor; | 
| // Textfield | 
| case NativeTheme::kColorId_TextfieldDefaultColor: | 
| @@ -394,30 +251,36 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, | 
| case NativeTheme::kColorId_TableGroupingIndicatorColor: | 
| return kTableGroupingIndicatorColor; | 
| + // FocusableBorder | 
| + case NativeTheme::kColorId_FocusedBorderColor: | 
| + return gfx::kGoogleBlue500; | 
| + case NativeTheme::kColorId_UnfocusedBorderColor: | 
| + return SkColorSetA(SK_ColorBLACK, 0x66); | 
| + | 
| // Results Tables | 
| case NativeTheme::kColorId_ResultsTableNormalBackground: | 
| return kTextfieldDefaultBackground; | 
| + | 
| 
tdanderson
2016/09/30 21:00:56
nit: remove newline
 
Evan Stade
2016/09/30 22:18:59
Done.
 | 
| case NativeTheme::kColorId_ResultsTableHoveredBackground: | 
| - return kResultsTableHoveredBackground; | 
| + return SkColorSetA(base_theme->GetSystemColor( | 
| + NativeTheme::kColorId_ResultsTableNormalText), | 
| + 0x0D); | 
| case NativeTheme::kColorId_ResultsTableSelectedBackground: | 
| - return kResultsTableSelectedBackground; | 
| + return SkColorSetA(base_theme->GetSystemColor( | 
| + NativeTheme::kColorId_ResultsTableNormalText), | 
| + 0x14); | 
| case NativeTheme::kColorId_ResultsTableNormalText: | 
| - return kResultsTableNormalText; | 
| case NativeTheme::kColorId_ResultsTableHoveredText: | 
| - return kResultsTableHoveredText; | 
| case NativeTheme::kColorId_ResultsTableSelectedText: | 
| - return kResultsTableSelectedText; | 
| + return kResultsTableText; | 
| case NativeTheme::kColorId_ResultsTableNormalDimmedText: | 
| - return kResultsTableNormalDimmedText; | 
| case NativeTheme::kColorId_ResultsTableHoveredDimmedText: | 
| - return kResultsTableHoveredDimmedText; | 
| case NativeTheme::kColorId_ResultsTableSelectedDimmedText: | 
| - return kResultsTableSelectedDimmedText; | 
| + return kResultsTableDimmedText; | 
| case NativeTheme::kColorId_ResultsTableNormalUrl: | 
| - return kResultsTableNormalUrl; | 
| case NativeTheme::kColorId_ResultsTableHoveredUrl: | 
| case NativeTheme::kColorId_ResultsTableSelectedUrl: | 
| - return kResultsTableSelectedOrHoveredUrl; | 
| + return base_theme->GetSystemColor(NativeTheme::kColorId_LinkEnabled); | 
| case NativeTheme::kColorId_ResultsTablePositiveText: | 
| return kResultsTablePositiveText; | 
| case NativeTheme::kColorId_ResultsTablePositiveHoveredText: |