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

ui/views/style/typography.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 "ui/views/style/typography.h"
 
 #include "base/logging.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/views/layout/layout_provider.h"
 #include "ui/views/style/typography_provider.h"
 
 namespace views {
 namespace style {
 namespace {
 
 void ValidateContextAndStyle(int text_context, int text_style) {
   DCHECK_GE(text_context, VIEWS_TEXT_CONTEXT_START);
   DCHECK_LT(text_context, TEXT_CONTEXT_MAX);
   DCHECK_GE(text_style, VIEWS_TEXT_STYLE_START);
 }
 
 }  // namespace
 
 const gfx::FontList& GetFont(int text_context, int text_style) {
   ValidateContextAndStyle(text_context, text_style);
   return LayoutProvider::Get()->GetTypographyProvider().GetFont(text_context,
                                                                 text_style);
 }
 
-SkColor GetColor(int text_context, int text_style) {
+SkColor GetColor(int text_context,
+                 int text_style,
+                 const ui::NativeTheme* native_theme) {
   ValidateContextAndStyle(text_context, text_style);
-  return LayoutProvider::Get()->GetTypographyProvider().GetColor(text_context,
-                                                                 text_style);
+  DCHECK(native_theme);

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

Why not just pass in by const ref then, as typography_provider.h does?

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

Then all the callsites need an extra character in front of GetNativeTheme() to
dereference it :).. 

I also considered that in future we could reasonably pass in nullptr and have
GetColor(), here, fall back to ui::NativeTheme::GetInstanceForNativeUi().
Currently those null checks are in view.cc or widget.cc, but it's possible
something will want a color when neither of those are around, like a Painter or
a Background.

(But I wanted it to be clear to the providers that they never have to consider a
null nativetheme, hence const-ref for those)

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

Hmm.  That sounds like a "maybe it would be useful to pass null someday" rather
than an "in the future I'll be changing these to pass null".

In keeping with the last sentence in
http://google.github.io/styleguide/cppguide.html#Reference_Arguments , I'd
probably pass const ref and add the '*' at all callsites.  But I will leave the
final decision to you.

[tapted, 2017/06/02 01:29:15]

I feel a '*' at callsites is more cognitive load for the callers than the issue
raised in that sentence. i.e. forcing callers to dereference requires them to
think whether they should DCHECK or check for null or provide a fallback. So..
really I think GetNativeTheme() should return const-ref rather than a pointer.
But that's a bigger refactor and raises questions on android where it currently
can return null.

+  return LayoutProvider::Get()->GetTypographyProvider().GetColor(
+      text_context, text_style, *native_theme);
 }
 
 int GetLineHeight(int text_context, int text_style) {
   ValidateContextAndStyle(text_context, text_style);
   return LayoutProvider::Get()->GetTypographyProvider().GetLineHeight(
       text_context, text_style);
 }
 
 }  // namespace style
 }  // namespace views