Aura overlay scrollbars adjust color for dark backgrounds

ui/native_theme/native_theme_aura.cc

Index: ui/native_theme/native_theme_aura.cc
diff --git a/ui/native_theme/native_theme_aura.cc b/ui/native_theme/native_theme_aura.cc
index d03a0d3211edc0f216a157fa5ff93b7ce22cab68..8a074cbee44912ab09e65ae6a5c4acd2aabc4d49 100644
--- a/ui/native_theme/native_theme_aura.cc
+++ b/ui/native_theme/native_theme_aura.cc
@@ -35,8 +35,11 @@ constexpr int kOverlayScrollbarMinimumLength = 12;
 constexpr SkAlpha kOverlayScrollbarAlphaNormal = 0x4D;
 constexpr SkAlpha kOverlayScrollbarAlphaHovered = 0x80;
 constexpr SkAlpha kOverlayScrollbarAlphaPressed = 0x80;
-constexpr SkColor kOverlayScrollbarThumbColor = SK_ColorBLACK;
-constexpr SkColor kOverlayScrollbarStrokeColor = SK_ColorWHITE;
+constexpr SkColor kOverlayScrollbarThumbDefaultColor = SK_ColorBLACK;
+constexpr SkColor kOverlayScrollbarThumbLightColor = SK_ColorWHITE;
+constexpr SkColor kOverlayScrollbarStrokeDefaultColor = SK_ColorWHITE;
+constexpr SkColor kOverlayScrollbarStrokeLightColor = SK_ColorBLACK;
+
 
 SkAlpha ThumbAlphaForState(NativeTheme::State state) {
   bool overlay = IsOverlayScrollbarEnabled();
@@ -177,15 +180,18 @@ void NativeThemeAura::PaintScrollbarTrack(
   canvas->drawIRect(gfx::RectToSkIRect(rect), paint);
 }
 
-void NativeThemeAura::PaintScrollbarThumb(SkCanvas* canvas,
-                                          Part part,
-                                          State state,
-                                          const gfx::Rect& rect) const {
+void NativeThemeAura::PaintScrollbarThumb(
+    SkCanvas* canvas,
+    Part part,
+    State state,
+    const gfx::Rect& rect,
+    ScrollbarOverlayColorTheme theme) const {
   // Do not paint if state is disabled.
   if (state == kDisabled)
     return;
 
-  PaintScrollbarThumbStateTransition(canvas, part, state, state, 1.0, rect);
+  PaintScrollbarThumbStateTransition(canvas, part, state, state, 1.0, rect,
+                                     theme);
 }
 
 void NativeThemeAura::PaintScrollbarThumbStateTransition(
@@ -194,18 +200,29 @@ void NativeThemeAura::PaintScrollbarThumbStateTransition(
     State start_state,
     State end_state,
     double progress,
-    const gfx::Rect& rect) const {
+    const gfx::Rect& rect,
+    ScrollbarOverlayColorTheme theme) const {
   TRACE_EVENT0("blink", "NativeThemeAura::PaintScrollbarThumbStateTransition");
   gfx::Rect thumb_rect(rect);
   SkColor thumb_color;
   if (IsOverlayScrollbarEnabled()) {
+    SkColor stroke_color;
+     if (theme == ScrollbarOverlayColorThemeLight) {

[Peter Kasting, 2016/10/24 23:33:42]

This code has only two styles, while on Mac we have three, which Apple's
developer docs describe as "dark", "light", and "dark with a light border".  I
don't have screenshots of those (can I see some?), but it seems like the code
here merely implements "dark with a light border" and "light with a dark
border".

If we want the Mac meanings, then I would implement three distinct states here;
make the colors be reasonable facsimiles of the Mac colors (probably not just
white/black based, but using shades for the dark/light cases; again, screenshots
would help); and rename the enums to not use "default" but explicitly describe
each case (e.g. "dark thumb light border").

If we want to have two types of thumbs based on contrasting with light or dark
backgrounds, then I would change the enums to only have two states, and make the
behavior consistent across Mac/views.

[bokan, 2016/10/25 18:45:52]

Taking a look, the Mac Dark/Default seems to produce exactly the same scrollbar
(I changed the painting code in ScrollbarThemeMac to pass Dark or Default
unconditionally), here's screen shots:

http://bokan.ca/light.png
http://bokan.ca/dark.png
http://bokan.ca/default.png

In any case, that's moot since we start with "Default" and if the background is
a dark color use "Light" scrollbars, so we never actually instantiate "Dark"
scrollbars. Given that, I'd suggest we remove "Default" from our ColorTheme enum
and just have "Light" and "Dark", defaulting to Dark as we do right now.
(ScrollbarThemeMac can use NSScrollerKnobStyleDefault for our
ScrollbarOverlayColorThemeDark).

Regarding matching colors, I think we already do. Although the the color
specified in the Aura painting code is White/Black, the shading comes from the
opacity so they're quite similar. See the spec sheet:
https://drive.google.com/a/google.com/file/d/0B6x6iYCtKinEbVBwQVNtN2tWTXc/vie...

[Peter Kasting, 2016/10/25 19:40:48]

SGTM
Sure you don't want to use "dark" instead of "default" for this?  (In case the
two change in the future, e.g. Mac for some reason decided to start defaulting
to a light-on-dark system theme.)
OK.

+      thumb_color = kOverlayScrollbarThumbLightColor;
+      stroke_color = kOverlayScrollbarStrokeLightColor;
+    } else {
+      thumb_color = kOverlayScrollbarThumbDefaultColor;
+      stroke_color = kOverlayScrollbarStrokeDefaultColor;
+    }
+
     // In overlay mode, draw a stroke (border).
     constexpr int kStrokeWidth = kOverlayScrollbarStrokeWidth;
     SkAlpha stroke_alpha = gfx::Tween::IntValueBetween(
         progress, ThumbAlphaForState(start_state),
         ThumbAlphaForState(end_state));
+
     SkPaint paint;
-    paint.setColor(SkColorSetA(kOverlayScrollbarStrokeColor, stroke_alpha));
+    paint.setColor(SkColorSetA(stroke_color, stroke_alpha));
     paint.setStyle(SkPaint::kStroke_Style);
     paint.setStrokeWidth(kStrokeWidth);
 
@@ -216,7 +233,7 @@ void NativeThemeAura::PaintScrollbarThumbStateTransition(
 
     // Inset the all the edges edges so we fill-in the stroke below.
     thumb_rect.Inset(kStrokeWidth, kStrokeWidth);
-    thumb_color = kOverlayScrollbarThumbColor;
+
   } else {
     // If there are no scrollbuttons then provide some padding so that the thumb
     // doesn't touch the top of the track.