Refactor cc scrollbar layers to separate solid-color vs desktop.

content/renderer/gpu/render_widget_compositor.cc

Index: content/renderer/gpu/render_widget_compositor.cc
diff --git a/content/renderer/gpu/render_widget_compositor.cc b/content/renderer/gpu/render_widget_compositor.cc
index c3becc8fe33556d0334937f6f5f18bffa367e25b..09ea25facbf6c4e50f84982996e04f021ae197fb 100644
--- a/content/renderer/gpu/render_widget_compositor.cc
+++ b/content/renderer/gpu/render_widget_compositor.cc
@@ -74,6 +74,21 @@ bool GetSwitchValueAsFloat(
   }
 }
 
+bool StringToSkColor(const std::string& str, SkColor& color) {

[enne (OOO), 2013/08/15 21:29:26]

Why does scrollbar color need to move to be a command line flag and not just a
LayerTreeSetting as it was before? The previous code seemed more straightforward
than parsing hex colors.

[wjmaclean, 2013/08/16 13:34:16]

Actually, it will remain in LayerTreeSettings, the question is just a matter of
how to initialise that value. At present only Android uses these scrollbars, but
that's about to change. The current initialisation is done inside an #if
defined(OS_ANDROID). Adding a flag for setting the color gives us a way to
customise the color to be used across platforms, and at runtime (so we can get
rid of another flag, --hide-scrollbars, used by android-WebView. And, it removes
a few lines of code from the #if block.

I'm open to alternative ways of doing this, but this idea had positive feedback
in the previous (now abandoned) IsSolidColor() patch
(https://codereview.chromium.org/17550008/).

+  std::string prefix = str.substr(0, 2);
+  int offset = (prefix == "0x" || prefix == "0X") ? 2 : 0;
+
+  std::vector<uint8> values;
+  if (base::HexStringToBytes(str.substr(offset), &values)) {
+    if (values.size() != 4)
+      return false;
+
+    color = SkColorSetARGB(values[0], values[1], values[2], values[3]);
+    return true;
+  }
+
+  return false;
+}
 
 }  // namespace
 
@@ -262,27 +277,28 @@ scoped_ptr<RenderWidgetCompositor> RenderWidgetCompositor::Create(
 
   settings.use_map_image = cmd->HasSwitch(cc::switches::kUseMapImage);
 
+  if (cmd->HasSwitch(cc::switches::kSolidColorScrollbarColor)) {
+    std::string str =
+        cmd->GetSwitchValueASCII(cc::switches::kSolidColorScrollbarColor);
+
+    SkColor scrollbar_color;
+    if (StringToSkColor(str, scrollbar_color))
+      settings.solid_color_scrollbar_color = scrollbar_color;
+  }
+
 #if defined(OS_ANDROID)
   // TODO(danakj): Move these to the android code.
   settings.can_use_lcd_text = false;
   settings.max_partial_texture_updates = 0;
-  settings.use_linear_fade_scrollbar_animator = true;
-  settings.solid_color_scrollbars = true;
-  settings.solid_color_scrollbar_color =
-      cmd->HasSwitch(switches::kHideScrollbars)
-          ? SK_ColorTRANSPARENT
-          : SkColorSetARGB(128, 128, 128, 128);
-  settings.solid_color_scrollbar_thickness_dip = 3;
   settings.highp_threshold_min = 2048;
   // Android WebView handles root layer flings itself.
   settings.ignore_root_layer_flings =
       widget->UsingSynchronousRendererCompositor();
-#elif !defined(OS_MACOSX)
-  if (cmd->HasSwitch(switches::kEnableOverlayScrollbars)) {
+#endif
+#if !defined(OS_MACOSX)

[jamesr, 2013/08/15 22:34:24]

this change doesn't seem right. the logic before this patch was:

If on android
  set pile of flags
otherwise, if not on mac (i.e. on windows/cros/linux)
  if have overlay scrollbar flags
    set different set of flags

The new logic will check for the kEnableOverlayScrollbars even on android, which
doesn't make a lot of sense - android always uses overlay scrollbars so it
shouldn't be conditional on a switch

[wjmaclean, 2013/08/16 13:34:16]

OK, fair enough. I knew this was a trade off, but thought I would propose it to
see what people thought. I can remove the setting of the switch from Androids
startup flags, and add the code back into the #if defined(OS_ANDROID) block.

Done.

+  if (cmd->HasSwitch(cc::switches::kEnableOverlayScrollbars)) {
     settings.use_linear_fade_scrollbar_animator = true;
     settings.solid_color_scrollbars = true;
-    settings.solid_color_scrollbar_color = SkColorSetARGB(128, 128, 128, 128);
-    settings.solid_color_scrollbar_thickness_dip = 3;
   }
 #endif