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

content/renderer/gpu/render_widget_compositor.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/gpu/render_widget_compositor.h"
 
 #include <limits>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 56 matching lines @@
       double_value >= min_value && double_value <= max_value) {
     *result = static_cast<float>(double_value);
     return true;
   } else {
     LOG(WARNING) << "Failed to parse switch " << switch_string  << ": " <<
         string_value;
     return false;
   }
 }
 
+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
 
 // static
 scoped_ptr<RenderWidgetCompositor> RenderWidgetCompositor::Create(
     RenderWidget* widget,
     bool threaded) {
   scoped_ptr<RenderWidgetCompositor> compositor(
       new RenderWidgetCompositor(widget, threaded));
 
@@ skipping 168 matching lines @@
       settings.max_unused_resource_memory_percentage =
           max_unused_resource_memory_percentage;
     }
   }
 
   settings.strict_layer_property_change_checking =
       cmd->HasSwitch(cc::switches::kStrictLayerPropertyChangeChecking);
 
   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
 
   if (!compositor->initialize(settings))
     return scoped_ptr<RenderWidgetCompositor>();
 
   return compositor.Pass();
 }
 
 RenderWidgetCompositor::RenderWidgetCompositor(RenderWidget* widget,
@@ skipping 287 matching lines @@
   return RenderThreadImpl::current()->OffscreenContextProviderForMainThread();
 }
 
 scoped_refptr<cc::ContextProvider>
 RenderWidgetCompositor::OffscreenContextProviderForCompositorThread() {
   return RenderThreadImpl::current()->
       OffscreenContextProviderForCompositorThread();
 }
 
 }  // namespace content