Aura overlay scrollbars adjust color for dark backgrounds

content/child/webthemeengine_impl_default.h

Index: content/child/webthemeengine_impl_default.h
diff --git a/content/child/webthemeengine_impl_default.h b/content/child/webthemeengine_impl_default.h
index 36ddb6bf7f569f0b8b67eeea0580f7b9b12d0834..a80e16a568d1f9b403160bf4234f8a6d6331ae02 100644
--- a/content/child/webthemeengine_impl_default.h
+++ b/content/child/webthemeengine_impl_default.h
@@ -9,6 +9,7 @@
 
 #include "build/build_config.h"
 #include "third_party/WebKit/public/platform/WebThemeEngine.h"
+#include "ui/native_theme/native_theme.h"

[Charlie Reis, 2016/10/24 19:22:19]

This declares ScrollbarOverlayColorTheme, but that doesn't appear to be used in
this file.  Maybe this include should stay in the .cc file and this file should
include WebScrollbarOverlayColorTheme.h instead?

 
 namespace content {
 
@@ -21,12 +22,14 @@ class WebThemeEngineImpl : public blink::WebThemeEngine {
              blink::WebThemeEngine::State state,
              const blink::WebRect& rect,
              const blink::WebThemeEngine::ExtraParams* extra_params) override;
-  virtual void paintStateTransition(blink::WebCanvas* canvas,
-                                    blink::WebThemeEngine::Part part,
-                                    blink::WebThemeEngine::State startState,
-                                    blink::WebThemeEngine::State endState,
-                                    double progress,
-                                    const blink::WebRect& rect);
+  virtual void paintStateTransition(

[Charlie Reis, 2016/10/24 19:22:19]

I'm a bit confused about this method-- it's listed with the overrides of
WebThemeEngine methods, but it's not an override (and there's no matching method
in WebThemeEngine).  I also don't see any call sites to it in code search.

When is it called?  Was it meant to be an override?  If not, we should add a
blank line to move it to a separate block from the // WebThemeEngine methods.

[chaopeng, 2016/10/24 19:51:29]

This method looks never get called and no override. Should I delete it?

[Charlie Reis, 2016/10/24 20:30:29]

That's a good question-- I might have to defer to someone who knows that code a
little more.

It looks like it was introduced in https://codereview.chromium.org/238723004/
and a call was added in ScrollbarThemeOverlay in
https://codereview.chromium.org/246293006.  That call site is no longer there,
so maybe it's dead code?

If so, it looks like this is the only place that calls
NativeTheme::PaintStateTransition, so maybe that can be removed as well?

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

I believe the original intention was for the painting code to animate transition
states (e.g. changing the opacity from idle to hovered state). The current
requirements don't call for animated opacity and any animated properties are
done on the compositor for performance reasons so I think we can get rid of
this. Similarly, NativeThemeAura has a "transition" version of the
PaintScrollbarThumb that can be merged and removed as well.

+      blink::WebCanvas* canvas,
+      blink::WebThemeEngine::Part part,
+      blink::WebThemeEngine::State startState,
+      blink::WebThemeEngine::State endState,
+      double progress,
+      const blink::WebRect& rect,
+      blink::WebScrollbarOverlayColorTheme theme);
 #if defined(OS_WIN)
   // Caches the scrollbar metrics. These are retrieved in the browser and passed
   // to the renderer in RendererPreferences because the required Windows system