Type conversion fixes, ui/gfx/ edition.

ui/gfx/color_utils.cc

Index: ui/gfx/color_utils.cc
diff --git a/ui/gfx/color_utils.cc b/ui/gfx/color_utils.cc
index 37cfe302d5a3c8b72df24b052acbaa1c50d78e44..f0642810c72343a7e80526eac2db9c1c32712d44 100644
--- a/ui/gfx/color_utils.cc
+++ b/ui/gfx/color_utils.cc
@@ -13,6 +13,7 @@
 
 #include "base/basictypes.h"
 #include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
 #include "build/build_config.h"
 #if defined(OS_WIN)
 #include "skia/ext/skia_utils_win.h"
@@ -77,10 +78,10 @@ double ContrastRatio(double foreground_luminance, double background_luminance) {
 // ----------------------------------------------------------------------------
 
 unsigned char GetLuminanceForColor(SkColor color) {
-  int luma = static_cast<int>((0.3 * SkColorGetR(color)) +
-                              (0.59 * SkColorGetG(color)) +
-                              (0.11 * SkColorGetB(color)));
-  return std::max(std::min(luma, 255), 0);
+  return base::saturated_cast<unsigned char>(

[danakj, 2014/10/18 18:40:06]

nice change here

+      (0.3 * SkColorGetR(color)) +
+      (0.59 * SkColorGetG(color)) +
+      (0.11 * SkColorGetB(color)));
 }
 
 double RelativeLuminance(SkColor color) {
@@ -136,7 +137,7 @@ SkColor HSLToSkColor(const HSL& hsl, SkAlpha alpha) {
     else if (lightness >= 1.0)
       light = 255;
     else
-      light = SkDoubleToFixed(lightness) >> 8;
+      light = static_cast<uint8>(SkDoubleToFixed(lightness) >> 8);
 
     return SkColorSetARGB(alpha, light, light, light);
   }
@@ -198,7 +199,7 @@ bool IsWithinHSLRange(const HSL& hsl,
 
 SkColor HSLShift(SkColor color, const HSL& shift) {
   HSL hsl;
-  int alpha = SkColorGetA(color);
+  SkAlpha alpha = SkColorGetA(color);

[danakj, 2014/10/18 18:40:06]

why not uint8?

[Peter Kasting, 2014/10/20 23:38:56]

SkAlpha is typedefed to uint8, so they should be the same; using an SkAlpha to
hold an alpha value felt semantically clearer to me.  I'll change it if there's
some strong reason to.

[danakj, 2014/10/23 15:35:44]

uint8 is a familiar type and we all know what it is. SkAlpha is familiar to less
folks so I like using it less if it's the same thing.

[Peter Kasting, 2014/10/23 17:29:45]

Sorry, I forgot the main rationale here when I replied before, or I would have
mentioned it then.  This value is passed to HSLToSkColor() below, which takes an
SkAlpha, not a uint8.  I think we should ensure those types match.

(Also, if someone is writing Skia code, and dealing with color alpha values, and
yet is confused by SkAlpha, I feel like something is wrong...)

[danakj, 2014/10/23 17:43:22]

I think we rely on SkColorGet[ARGB]() to return a byte in many places in the
code base so it's not something we could just change. 


HSLToSkColor eventually ends up in SkColorSetARGBInline() which takes a uint8_t
for the alpha value, so it's just converting the SkAlpha back. I think
HSLToSkColor should really use a uint8_t also, that'd be simpler to
read/understand.

But I don't feel strongly really it's my preference here so go with what you
like, I don't mean to block the rest of the changes over this.

   SkColorToHSL(color, &hsl);
 
   // Replace the hue with the tint's hue.