color_utils cleanup:

ui/gfx/color_utils.h

Index: ui/gfx/color_utils.h
diff --git a/ui/gfx/color_utils.h b/ui/gfx/color_utils.h
index 44302c808c8138c6a2d2702efd17e0ed66631a5a..a13ebb31c7ac2f96f4b47d4b861513f0d6f6cee3 100644
--- a/ui/gfx/color_utils.h
+++ b/ui/gfx/color_utils.h
@@ -23,14 +23,22 @@ struct HSL {
 // This value is taken from w3c accessibility guidelines.
 const double kMinimumReadableContrastRatio = 4.5f;
 
-// Determines the contrast ratio of two colors.
-GFX_EXPORT double GetContrastRatio(SkColor color_a, SkColor color_b);
-
-GFX_EXPORT unsigned char GetLuminanceForColor(SkColor color);
-
-// Calculated according to http://www.w3.org/TR/WCAG20/#relativeluminancedef
+// Determines the contrast ratio of two colors or two relative luminance values
+// (as computed by RelativeLuminance()), calculated according to
+// http://www.w3.org/TR/WCAG20/#contrast-ratiodef .
+GFX_EXPORT double ContrastRatio(SkColor color_a, SkColor color_b);

[danakj, 2016/03/04 01:54:49]

GetContrastRatio is preferred (verbs not nouns for methods) or Compute or
ContrastRatioFromSkColors or something (bonus points for not overloading).. (see
recent blink-dev bikesheds for what goes wrong with noun methods ^_^)

Same for RelativeLuminance and Luma. These all seem like type constructors at
the callsites.

[Peter Kasting, 2016/03/04 02:10:52]

That may be true in Blink, but in Chromium Get...() is normally discouraged
unless this is some kind of an accessor, from what I've seen.  (The Chromium and
Google style guides are both silent on this issue.)
I don't know what you mean by "type constructors", since there are not distinct
luminance or luma types.

[danakj, 2016/03/04 19:05:55]

It's the other way. The WebKit style guide says to not use "get" in accessors.
Chromium/google style is to make methods verbs AFAIK. I also tried to find this
but failed. You're right Get is usually an accessor so I also brought some other
suggestions to make them verbs such as Compute or To or From.
Nouns are types, so Noun() looks like constructing a type. I realize there are
not such types but the callsites look like there are such types since they are
not a verb saying what the function does.

[Peter Kasting, 2016/03/04 20:28:57]

I can't find any such text in the Google style guide or Chromium style guide,
and the parts of Chromium I'm familiar with do both; noun function names are
fairly common, for example, from autocomplete_input.h:

FormattedStringWithEquivalentMeaning()
NumNonHostComponents()

Or from tab.h:

IconCapacity();

I mean, it's not a huge deal -- adding Get will not make the function of these
APIs less clear, it's just a bit more verbose -- so since I gather this is the
pattern you want ui/gfx/ to have, I will do that.  I just thought it read a bit
better without it.
Ah, I understand.  That actually seems sort of like a pro more than a con -- in
a sense, we are "constructing" the luma or luminance value, so reading like
we're doing so may be good?  Perhaps I am smoking crack.

[danakj, 2016/03/04 21:10:00]

I'm probably being over sensitive to this cuz all I do is rename things in Blink
right now :/

+GFX_EXPORT double ContrastRatio(double luminance_a, double luminance_b);
+
+// The relative luminance of |color|, that is, the weighted sum of the
+// linearized RGB components, normalized to 0..1, per BT.709.  See
+// http://www.w3.org/TR/WCAG20/#relativeluminancedef .
 GFX_EXPORT double RelativeLuminance(SkColor color);
 
+// The luma of |color|, that is, the weighted sum of the gamma-compressed R'G'B'
+// components, per BT.601, a.k.a. the Y' in Y'UV.  See
+// https://en.wikipedia.org/wiki/Luma_(video).
+GFX_EXPORT uint8_t Luma(SkColor color);
+
 // Note: these transformations assume sRGB as the source color space
 GFX_EXPORT void SkColorToHSL(SkColor c, HSL* hsl);
 GFX_EXPORT SkColor HSLToSkColor(const HSL& hsl, SkAlpha alpha);
@@ -50,8 +58,8 @@ GFX_EXPORT bool IsWithinHSLRange(const HSL& hsl,
                                  const HSL& upper_bound);
 
 // Makes |hsl| valid input for HSLShift(). Sets values of hue, saturation
-// and luminosity which are outside of the valid range [0, 1] to -1.
-// -1 is a special value which indicates 'no change'.
+// and lightness which are outside of the valid range [0, 1] to -1.  -1 is a
+// special value which indicates 'no change'.
 GFX_EXPORT void MakeHSLShiftValid(HSL* hsl);
 
 // HSL-Shift an SkColor. The shift values are in the range of 0-1, with the
@@ -70,8 +78,7 @@ GFX_EXPORT void MakeHSLShiftValid(HSL* hsl);
 //    1 = full lightness (make all pixels white).
 GFX_EXPORT SkColor HSLShift(SkColor color, const HSL& shift);
 
-// Builds a histogram based on the Y' of the Y'UV representation of
-// this image.
+// Builds a histogram based on the Y' of the Y'UV representation of this image.
 GFX_EXPORT void BuildLumaHistogram(const SkBitmap& bitmap, int histogram[256]);
 
 // Calculates how "boring" an image is. The boring score is the
@@ -87,25 +94,30 @@ GFX_EXPORT double CalculateBoringScore(const SkBitmap& bitmap);
 GFX_EXPORT SkColor AlphaBlend(SkColor foreground, SkColor background,
                               SkAlpha alpha);
 
-// Returns true if the luminance of |color| is closer to black than white.
+// Returns true if the luma of |color| is closer to black than white.
 GFX_EXPORT bool IsDark(SkColor color);
 
 // Makes a dark color lighter or a light color darker by blending |color| with
-// white or black depending on its current luminance.  |alpha| controls the
-// amount of white or black that will be alpha-blended into |color|.
-GFX_EXPORT SkColor BlendTowardOppositeLuminance(SkColor color, SkAlpha alpha);
-
-// Given an opaque foreground and background color, try to return a foreground
-// color that is "readable" over the background color by luma-inverting the
-// foreground color and then picking whichever foreground color has higher
-// contrast against the background color.  You should not pass colors with
-// non-255 alpha to this routine, since determining the correct behavior in such
-// cases can be impossible.
+// white or black depending on its current luma.  |alpha| controls the amount of
+// white or black that will be alpha-blended into |color|.
+GFX_EXPORT SkColor BlendTowardOppositeLuma(SkColor color, SkAlpha alpha);
+
+// Given a foreground and background color, try to return a foreground color
+// that is "readable" over the background color by luma-inverting the foreground
+// color and then using PickContrastingColor() to pick the one with greater
+// contrast.  During this process, alpha values will be ignored; the returned
+// color will have the same alpha as |foreground|.
 //
 // NOTE: This won't do anything but waste time if the supplied foreground color
 // has a luma value close to the midpoint (0.5 in the HSL representation).
 GFX_EXPORT SkColor GetReadableColor(SkColor foreground, SkColor background);
 
+// Returns whichever of |foreground1| or |foreground2| has higher contrast with
+// |background|.
+GFX_EXPORT SkColor PickContrastingColor(SkColor foreground1,
+                                        SkColor foreground2,
+                                        SkColor background);
+
 // Invert a color.
 GFX_EXPORT SkColor InvertColor(SkColor color);