Change Aura overlay scrollbars from solid color to painted scrollbars.

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 b6f47d402f4dc490dfe68bef799182ec07f9ac7e..a2628e2805d067a6f11ce3bd5b92ea2e098e855e 100644
--- a/ui/native_theme/native_theme_aura.cc
+++ b/ui/native_theme/native_theme_aura.cc
@@ -21,40 +21,33 @@
 #include "ui/gfx/skia_util.h"
 #include "ui/native_theme/common_theme.h"
 #include "ui/native_theme/native_theme_switches.h"
+#include "ui/native_theme/overlay_scrollbar_constants_aura.h"
 
 namespace ui {
 
 namespace {
 
+// Constants for painting overlay scrollbars. Other properties needed outside
+// this painting code are defined in overlay_scrollbar_constants_aura.h.
+constexpr int kOverlayScrollbarStrokeWidth = 1;
+constexpr int kOverlayScrollbarMinimumLength = 12;
+constexpr SkAlpha kOverlayScrollbarAlphaNormal = 0x4D;
+constexpr SkAlpha kOverlayScrollbarAlphaHovered = 0x80;
+constexpr SkAlpha kOverlayScrollbarAlphaPressed = 0x80;
+const SkColor kOverlayScrollbarThumbColor = SkColorSetRGB(0x0, 0x0, 0x0);

[Peter Kasting, 2016/10/18 00:43:36]

Nit: Use SK_ColorBLACK (and SK_ColorWHITE just below), and then these can be
constexpr as well.

[bokan, 2016/10/18 20:34:49]

Done.

+const SkColor kOverlayScrollbarStrokeColor = SkColorSetRGB(0xFF, 0xFF, 0xFF);
+
 SkAlpha ThumbAlphaForState(NativeTheme::State state) {
   bool overlay = IsOverlayScrollbarEnabled();
   switch (state) {
     case NativeTheme::kDisabled:
       return 0x00;
     case NativeTheme::kHovered:
-      return overlay ? 0xB2 : 0x4D;
+      return overlay ? kOverlayScrollbarAlphaHovered : 0x4D;
     case NativeTheme::kNormal:
-      return overlay ? 0x8C : 0x33;
-    case NativeTheme::kPressed:
-      return overlay ? 0xB2 : 0x80;
-    case NativeTheme::kNumStates:
-      break;
-  }
-
-  NOTREACHED();
-  return 0xFF;
-}
-
-SkAlpha ThumbStrokeAlphaForState(NativeTheme::State state) {
-  DCHECK(IsOverlayScrollbarEnabled());
-  switch (state) {
-    case NativeTheme::kDisabled:
-      return 0x00;
-    case NativeTheme::kHovered:
+      return overlay ? kOverlayScrollbarAlphaNormal : 0x33;
     case NativeTheme::kPressed:
-      return 0x33;
-    case NativeTheme::kNormal:
-      return 0x26;
+      return overlay ? kOverlayScrollbarAlphaPressed : 0x80;
     case NativeTheme::kNumStates:
       break;
   }
@@ -84,6 +77,11 @@ NativeThemeAura::NativeThemeAura() {
   set_scrollbar_button_length(0);
 #endif
 
+  if (IsOverlayScrollbarEnabled()) {
+    scrollbar_width_ =
+        kOverlayScrollbarThumbWidthPressed + kOverlayScrollbarStrokeWidth * 2;
+  }
+
   // Images and alphas declarations assume the following order.
   static_assert(kDisabled == 0, "states unexpectedly changed");
   static_assert(kHovered == 1, "states unexpectedly changed");
@@ -197,19 +195,27 @@ void NativeThemeAura::PaintScrollbarThumbStateTransition(
     double progress,
     const gfx::Rect& rect) const {
   gfx::Rect thumb_rect(rect);
+  SkColor thumb_color;
   if (IsOverlayScrollbarEnabled()) {
     // In overlay mode, draw a stroke (border).
-    const int kStrokeWidth = 1;
+    constexpr int kStrokeWidth = kOverlayScrollbarStrokeWidth;
     SkAlpha stroke_alpha = gfx::Tween::IntValueBetween(
-        progress, ThumbStrokeAlphaForState(start_state),
-        ThumbStrokeAlphaForState(end_state));
+        progress, ThumbAlphaForState(start_state),
+        ThumbAlphaForState(end_state));
     SkPaint paint;
-    paint.setColor(SkColorSetA(SK_ColorWHITE, stroke_alpha));
+    paint.setColor(SkColorSetA(kOverlayScrollbarStrokeColor, stroke_alpha));
     paint.setStyle(SkPaint::kStroke_Style);
     paint.setStrokeWidth(kStrokeWidth);
-    canvas->drawIRect(gfx::RectToSkIRect(thumb_rect), paint);
 
+    gfx::RectF stroke_rect(thumb_rect);
+    constexpr float kHalfStrokeWidth = kStrokeWidth / 2.f;
+    stroke_rect.Inset(kHalfStrokeWidth, kHalfStrokeWidth);
+    canvas->drawRect(gfx::RectFToSkRect(stroke_rect), paint);

[aelias_OOO_until_Jul13, 2016/10/18 20:37:00]

As I mentioned on http://crbug.com/304869#c36, I think this is moving in the
wrong direction.  You're switching Aura to perform raster on a non-raster
thread, and also causing CC to perform a texture upload and block activation on
scrollbars, and all this cost (and it's real cost, painted scrollbars were a
constant jank culprit in traces for the ~6 months Clank tried to use them) at no
actual value-added except for "code unification".  But you won't even get the
unification because Android is not interested in this bad approach.

I'm on board with the principle of unification but I'd like to do it the
following way instead:
1) Merge Android, Aura, and Mac overlay scrollbar code, without making them more
entangled with Blink/raster or otherwise sacrificing significant efficiency
(except that I am willing to tolerate switching Android to a singleton shared
ninepatch UIResource which would allow Aura hollow scrollbars and Mac
rounded-corner scrollbars, which I consider a negligible cost if done properly).
2) Relegate "Painted Scrollbars" to only be used for OS-system-call-mystery-meat
scrollbars and -webkit custom scrollbars, which is what their raster-heavy
design is for in the first place.

I don't think 1 and 2 have enough in common to be merged, but there is a lot to
be merged within category "1", which you are currently *un*-merging.

+
+    // Inset the top and left edges for filling in the stroke below.

[Peter Kasting, 2016/10/18 00:43:36]

Nit: Comment is inaccurate.

[bokan, 2016/10/18 20:34:49]

Done.

     thumb_rect.Inset(kStrokeWidth, kStrokeWidth, kStrokeWidth, kStrokeWidth);

[Peter Kasting, 2016/10/18 00:43:36]

Nit: Just the two-arg version will work.

[bokan, 2016/10/18 20:34:49]

Done.

+
+    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.
@@ -220,12 +226,14 @@ void NativeThemeAura::PaintScrollbarThumbStateTransition(
       thumb_rect.Inset(kThumbPadding, extra_padding);
     else
       thumb_rect.Inset(extra_padding, kThumbPadding);
+
+    thumb_color = SK_ColorBLACK;
   }
 
   SkPaint paint;
   SkAlpha alpha = gfx::Tween::IntValueBetween(
       progress, ThumbAlphaForState(start_state), ThumbAlphaForState(end_state));
-  paint.setColor(SkColorSetA(SK_ColorBLACK, alpha));
+  paint.setColor(SkColorSetA(thumb_color, alpha));
   canvas->drawIRect(gfx::RectToSkIRect(thumb_rect), paint);
 }
 
@@ -239,4 +247,36 @@ void NativeThemeAura::PaintScrollbarCorner(SkCanvas* canvas,
   canvas->drawIRect(RectToSkIRect(rect), paint);
 }
 
+gfx::Size NativeThemeAura::GetPartSize(Part part,
+                                       State state,
+                                       const ExtraParams& extra) const {
+  if (IsOverlayScrollbarEnabled()) {
+    constexpr int minimum_length =
+        kOverlayScrollbarMinimumLength + 2 * kOverlayScrollbarStrokeWidth;
+
+    // Aura overlay scrollbars need a slight tweak from the base sizes.
+    switch (part) {
+      case kScrollbarDownArrow:
+      case kScrollbarUpArrow:
+      case kScrollbarLeftArrow:
+      case kScrollbarRightArrow:
+      case kScrollbarHorizontalTrack:
+      case kScrollbarVerticalTrack:
+      case kScrollbarHorizontalGripper:
+      case kScrollbarVerticalGripper:
+        // Aura overlay scrollbars only have a thumb.
+        NOTREACHED();
+        break;
+      case kScrollbarHorizontalThumb:
+        return gfx::Size(minimum_length, scrollbar_width_);
+      case kScrollbarVerticalThumb:
+        return gfx::Size(scrollbar_width_, minimum_length);
+      default:
+        break;
+    }
+  }
+
+  return NativeThemeBase::GetPartSize(part, state, extra);
+}
+
 }  // namespace ui