Remove views::Label::SetDisabledColor(). Replace with typography colors.

chrome/browser/ui/views/harmony/harmony_typography_provider.cc

 // Copyright 2017 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.
 
 #include "chrome/browser/ui/views/harmony/harmony_typography_provider.h"
 
 #include "chrome/browser/ui/views/harmony/chrome_typography.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/gfx/platform_font.h"
+#include "ui/native_theme/native_theme.h"
 
 #if defined(USE_ASH)
 #include "ash/public/cpp/ash_typography.h"  // nogncheck
 #endif
 
 const gfx::FontList& HarmonyTypographyProvider::GetFont(int text_context,
                                                         int text_style) const {
   // "Target" font size constants from the Harmony spec.
   constexpr int kHeadlineSize = 20;
   constexpr int kTitleSize = 15;
@@ skipping 29 matching lines @@
     default:
       break;
   }
 
   // Ignore |text_style| since it only affects color in the Harmony spec.
 
   return ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(
       size_delta, gfx::Font::NORMAL, font_weight);
 }
 
-SkColor HarmonyTypographyProvider::GetColor(int text_context,
-                                            int text_style) const {
-  // TODO(tapted): Look up colors from the spec.
-  return SK_ColorBLACK;
+SkColor HarmonyTypographyProvider::GetColor(
+    int text_context,
+    int text_style,
+    const ui::NativeTheme& theme) const {
+  const SkColor foreground_color =
+      theme.GetSystemColor(ui::NativeTheme::kColorId_LabelEnabledColor);
+
+  // If the default foreground color from the native theme isn't black, the rest
+  // of the Harmony spec isn't going to work. Just use the foreground color for
+  // text, with some exceptions.
+  if (foreground_color != SK_ColorBLACK) {
+    switch (text_style) {

[Peter Kasting, 2017/06/01 04:55:58]

Seems like it might be OK to return the "red" and "green" colors as-is in this
case, since those are more physical than logical descriptors, so there's no real
"mapping"?

[tapted, 2017/06/01 11:22:18]

Done - Although I special-cased a white foreground to match the approach in
NativeThemeDarkAura (this will matter for the "Secure" text displayed in the
omnibox for incognito windows.. when we get around to migrating those labels to
typography styles).

+      case views::style::STYLE_DISABLED:
+      case STYLE_SECONDARY:
+      case STYLE_HINT:
+        return theme.GetSystemColor(
+            ui::NativeTheme::kColorId_LabelDisabledColor);
+      case views::style::STYLE_LINK:
+        return theme.GetSystemColor(ui::NativeTheme::kColorId_LinkEnabled);
+    }
+    return foreground_color;
+  }
+
+  switch (text_style) {

[Peter Kasting, 2017/06/01 04:55:58]

Did you consider trying to implement these colors using alpha blends of the
foreground and background colors with white and black?  For example, compute the
primary color initially by blending the foreground color with the default
background color, or blending the foreground color toward the opposite luma with
BlendTowardOppositeLuma().  Construct the disabled, hint, and secondary styles
by further blending the foreground or primary colors with the background color. 
Use GetReadableColor() over the background color to ensure the link color reads
properly.  Use GetReadableColor() with the foreground and
default-button's-background colors to compute the default button color.  Etc.

These sorts of routes might eliminate the need to bail out for all custom theme
cases above, plus make it clearer where these colors come from.

[tapted, 2017/06/01 11:22:18]

I didn't consider this.. But it might be overkill. There isn't much themed UI
that will pass in requests to this.

But it needs to start with something basic, otherwise the MD buttons in the
incognito downloads bar will attempt to draw black-on-black when we use
--secondary-ui-md..

[Peter Kasting, 2017/06/01 19:08:01]

Long-term, I'd like all the colors in all Chrome UI to respond to system
settings changes.  It doesn't happen in too many places currently, but I try to
push for code that accommodates it where I can.
FWIW, I'm totally fine with checking in something like what you have and having
a TODO for a more "generic" system, if any of my ideas for said system make
sense.

+    case views::style::STYLE_DIALOG_BUTTON_DEFAULT:
+      return SK_ColorWHITE;
+    case views::style::STYLE_DISABLED:
+      return SkColorSetRGB(0x9e, 0x9e, 0x9e);
+    case views::style::STYLE_LINK:
+      return gfx::kGoogleBlue700;
+    case STYLE_SECONDARY:
+    case STYLE_HINT:
+      return SkColorSetRGB(0x75, 0x75, 0x75);
+    case STYLE_RED:
+      return gfx::kGoogleRed700;
+    case STYLE_GREEN:
+      return gfx::kGoogleGreen700;
+  }
+  return SkColorSetRGB(0x21, 0x21, 0x21);  // Primary for everything else.
 }
 
 int HarmonyTypographyProvider::GetLineHeight(int text_context,
                                              int text_style) const {
   // "Target" line height constants from the Harmony spec. A default OS
   // configuration should use these heights. However, if the user overrides OS
   // defaults, then GetLineHeight() should return the height that would add the
   // same extra space between lines as the default configuration would have.
   constexpr int kHeadlineHeight = 32;
   constexpr int kTitleHeight = 22;
@@ skipping 50 matching lines @@
     case views::style::CONTEXT_DIALOG_TITLE:
       return title_height;
     case CONTEXT_BODY_TEXT_LARGE:
       return body_large_height;
     case CONTEXT_HEADLINE:
       return headline_height;
     default:
       return default_height;
   }
 }