Color Pipeline: Eliminate hard-coded SK_ColorXXXX constants in ScrollView.

TBR=jamescook@chromium.og

Bug: 1044687
Change-Id: Ib31fbf481e215eb6eb7a7af43369a343e322faf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095566
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748887}

ash/app_list/views/search_result_page_view.cc

@@ -23,6 +23,7 @@
 #include "ash/public/cpp/view_shadow.h"
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
+#include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/chromeos/search_box/search_box_constants.h"
@@ -199,7 +200,7 @@ SearchResultPageView::SearchResultPageView(AppListViewDelegate* view_delegate,
   scroller->ClipHeightTo(0, 0);
   scroller->SetVerticalScrollBar(
       std::make_unique<ZeroWidthVerticalScrollBar>());
-  scroller->SetBackgroundColor(SK_ColorTRANSPARENT);
+  scroller->SetBackgroundColor(base::nullopt);
   AddChildView(std::move(scroller));
 
   SetLayoutManager(std::make_unique<views::FillLayout>());

ash/assistant/ui/base/assistant_scroll_view.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/optional.h"
 #include "ui/views/controls/scrollbar/overlay_scroll_bar.h"
 
 namespace ash {
@@ -96,7 +97,7 @@ void AssistantScrollView::OnViewPreferredSizeChanged(views::View* view) {
 }
 
 void AssistantScrollView::InitLayout() {
-  SetBackgroundColor(SK_ColorTRANSPARENT);
+  SetBackgroundColor(base::nullopt);
   SetDrawOverflowIndicator(false);
 
   // Content view.

ash/login/ui/lock_debug_view.cc

@@ -753,7 +753,7 @@ LockDebugView::LockDebugView(mojom::TrayActionState initial_note_action_state,
         views::ScrollView::CreateScrollViewWithBorder();
     scroll->SetPreferredSize(gfx::Size(600, height));
     scroll->SetContents(base::WrapUnique(content));
-    scroll->SetBackgroundColor(SK_ColorTRANSPARENT);
+    scroll->SetBackgroundColor(base::nullopt);
     scroll->SetVerticalScrollBar(
         std::make_unique<views::OverlayScrollBar>(false));
     scroll->SetHorizontalScrollBar(

ash/login/ui/scrollable_users_list_view.cc

@@ -16,6 +16,7 @@
 #include "ash/wallpaper/wallpaper_controller_impl.h"
 #include "base/bind.h"
 #include "base/numerics/ranges.h"
+#include "base/optional.h"
 #include "base/timer/timer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/display/screen.h"
@@ -331,7 +332,7 @@ ScrollableUsersListView::ScrollableUsersListView(
       ->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kCenter);
   ensure_min_height->AddChildView(user_view_host_);
   SetContents(std::move(ensure_min_height));
-  SetBackgroundColor(SK_ColorTRANSPARENT);
+  SetBackgroundColor(base::nullopt);
   SetDrawOverflowIndicator(false);
 
   SetVerticalScrollBar(std::make_unique<UsersListScrollBar>(false));

ash/system/message_center/notifier_settings_view.cc

@@ -22,6 +22,7 @@
 #include "ash/system/message_center/message_center_style.h"
 #include "ash/system/tray/tray_popup_utils.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "skia/ext/image_operations.h"
@@ -496,7 +497,7 @@ NotifierSettingsView::NotifierSettingsView() {
   header_view_ = AddChildView(std::move(header_view));
 
   auto scroller = std::make_unique<views::ScrollView>();
-  scroller->SetBackgroundColor(SK_ColorTRANSPARENT);
+  scroller->SetBackgroundColor(base::nullopt);
   scroll_bar_ = scroller->SetVerticalScrollBar(
       std::make_unique<views::OverlayScrollBar>(/*horizontal=*/false));
   scroller->SetDrawOverflowIndicator(false);

ash/system/message_center/unified_message_center_view.cc

@@ -21,6 +21,7 @@
 #include "ash/system/unified/unified_system_tray_view.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/user_metrics.h"
+#include "base/optional.h"
 #include "ui/gfx/animation/linear_animation.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/views/message_view.h"
@@ -93,7 +94,7 @@ UnifiedMessageCenterView::UnifiedMessageCenterView(
   // set the default opaque background color.
   scroller_->SetContents(
       std::make_unique<ScrollerContentsView>(message_list_view_));
-  scroller_->SetBackgroundColor(SK_ColorTRANSPARENT);
+  scroller_->SetBackgroundColor(base::nullopt);
   scroller_->SetVerticalScrollBar(base::WrapUnique(scroll_bar_));
   scroller_->SetDrawOverflowIndicator(false);
   AddChildView(scroller_);

chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/optional.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ -37,7 +38,7 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(
 
   // Use a transparent background so that the menu's background shows through.
   // None of the children use layers, so this should be ok.
-  SetBackgroundColor(SK_ColorTRANSPARENT);
+  SetBackgroundColor(base::nullopt);
   BrowserActionsContainer* main =
       toolbar_button_provider->GetBrowserActionsContainer();
   auto container = std::make_unique<BrowserActionsContainer>(browser_, main,

ui/views/controls/scroll_view.cc

@@ -188,7 +188,7 @@ ScrollView::ScrollView()
   more_content_bottom_->SetVisible(false);
 
   if (scroll_with_layers_enabled_)
-    EnableViewPortLayer();
+    EnableViewportLayer();
 
   // If we're scrolling with layers, paint the overflow indicators to the layer.
   if (ScrollsWithLayers()) {
@@ -237,10 +237,12 @@ void ScrollView::SetContentsImpl(std::unique_ptr<View> a_view) {
       // play it safe here to avoid graphical glitches
       // (https://crbug.com/826472). If there's no solid background, mark the
       // view as not filling its bounds opaquely.
-      if (GetBackgroundColor() != SK_ColorTRANSPARENT)
-        a_view->SetBackground(CreateSolidBackground(GetBackgroundColor()));
-      else
+      if (GetBackgroundColor()) {
+        a_view->SetBackground(
+            CreateSolidBackground(GetBackgroundColor().value()));
+      } else {
         fills_opaquely = false;
+      }
     }
     a_view->SetPaintToLayer();
     a_view->layer()->SetDidScrollCallback(base::BindRepeating(
@@ -263,19 +265,23 @@ void ScrollView::SetHeader(std::nullptr_t) {
   SetHeaderImpl(nullptr);
 }
 
-void ScrollView::SetBackgroundColor(SkColor color) {
-  if (background_color_data_.color == color)
+void ScrollView::SetBackgroundColor(const base::Optional<SkColor>& color) {
+  if (background_color_ == color && !background_color_id_)
     return;
-  background_color_data_.color = color;
-  use_color_id_ = false;
+  background_color_ = color;
+  background_color_id_ = base::nullopt;
   UpdateBackground();
-  OnPropertyChanged(&background_color_data_, kPropertyEffectsPaint);
+  OnPropertyChanged(&background_color_, kPropertyEffectsPaint);
 }
 
-void ScrollView::SetBackgroundThemeColorId(ui::NativeTheme::ColorId color_id) {
-  background_color_data_.color_id = color_id;
-  use_color_id_ = true;
+void ScrollView::SetBackgroundThemeColorId(
+    const base::Optional<ui::NativeTheme::ColorId>& color_id) {
+  if (background_color_id_ == color_id && !background_color_)
+    return;
+  background_color_id_ = color_id;
+  background_color_ = base::nullopt;
   UpdateBackground();
+  OnPropertyChanged(&background_color_id_, kPropertyEffectsPaint);
 }
 
 gfx::Rect ScrollView::GetVisibleRect() const {
@@ -628,7 +634,7 @@ void ScrollView::OnGestureEvent(ui::GestureEvent* event) {
 
 void ScrollView::OnThemeChanged() {
   UpdateBorder();
-  if (use_color_id_)
+  if (background_color_id_)
     UpdateBackground();
 }
 
@@ -681,7 +687,7 @@ void ScrollView::UpdateViewportLayerForClipping() {
   if (has_layer == needs_layer)
     return;
   if (needs_layer)
-    EnableViewPortLayer();
+    EnableViewportLayer();
   else
     contents_viewport_->DestroyLayer();
 }
@@ -837,7 +843,7 @@ bool ScrollView::ScrollsWithLayers() const {
   return contents_viewport_->layer() != nullptr;
 }
 
-void ScrollView::EnableViewPortLayer() {
+void ScrollView::EnableViewportLayer() {
   if (DoesViewportOrScrollViewHaveLayer())
     return;
 
@@ -884,26 +890,35 @@ void ScrollView::UpdateBorder() {
 }
 
 void ScrollView::UpdateBackground() {
-  const SkColor background_color = GetBackgroundColor();
+  const base::Optional<SkColor> background_color = GetBackgroundColor();
+
+  auto create_background = [background_color]() {
+    return background_color ? CreateSolidBackground(background_color.value())
+                            : nullptr;
+  };
 
-  SetBackground(CreateSolidBackground(background_color));
+  SetBackground(create_background());
   // In addition to setting the background of |this|, set the background on
   // the viewport as well. This way if the viewport has a layer
   // SetFillsBoundsOpaquely() is honored.
-  contents_viewport_->SetBackground(CreateSolidBackground(background_color));
+  contents_viewport_->SetBackground(create_background());
   if (contents_ && ScrollsWithLayers())
-    contents_->SetBackground(CreateSolidBackground(background_color));
+    contents_->SetBackground(create_background());
   if (contents_viewport_->layer()) {
-    contents_viewport_->layer()->SetFillsBoundsOpaquely(background_color !=
-                                                        SK_ColorTRANSPARENT);
+    contents_viewport_->layer()->SetFillsBoundsOpaquely(!!background_color);
   }
   SchedulePaint();
 }
 
-SkColor ScrollView::GetBackgroundColor() const {
-  return use_color_id_
-             ? GetNativeTheme()->GetSystemColor(background_color_data_.color_id)
-             : background_color_data_.color;
+base::Optional<SkColor> ScrollView::GetBackgroundColor() const {
+  return background_color_id_
+             ? GetNativeTheme()->GetSystemColor(background_color_id_.value())
+             : background_color_;
+}
+
+base::Optional<ui::NativeTheme::ColorId> ScrollView::GetBackgroundThemeColorId()
+    const {
+  return background_color_id_;
 }
 
 void ScrollView::PositionOverflowIndicators() {
@@ -943,7 +958,10 @@ BEGIN_METADATA(ScrollView)
 METADATA_PARENT_CLASS(View)
 ADD_READONLY_PROPERTY_METADATA(ScrollView, int, MinHeight)
 ADD_READONLY_PROPERTY_METADATA(ScrollView, int, MaxHeight)
-ADD_PROPERTY_METADATA(ScrollView, SkColor, BackgroundColor)
+ADD_PROPERTY_METADATA(ScrollView, base::Optional<SkColor>, BackgroundColor)
+ADD_PROPERTY_METADATA(ScrollView,
+                      base::Optional<ui::NativeTheme::ColorId>,
+                      BackgroundThemeColorId)
 ADD_PROPERTY_METADATA(ScrollView, bool, DrawOverflowIndicator)
 ADD_PROPERTY_METADATA(ScrollView, bool, HasFocusIndicator)
 ADD_PROPERTY_METADATA(ScrollView, bool, HideHorizontalScrollBar)

ui/views/controls/scroll_view.h

@@ -12,6 +12,7 @@
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/controls/focus_ring.h"
 #include "ui/views/controls/scrollbar/scroll_bar.h"
@@ -87,17 +88,19 @@ class VIEWS_EXPORT ScrollView : public View, public ScrollBarController {
   //   called the background color comes from the theme (and changes if the
   //   theme changes).
   // . By way of setting an explicit color, i.e. SetBackgroundColor(). Use
-  //   SK_ColorTRANSPARENT if you don't want any color, but be warned this
+  //   base::nullopt if you don't want any color, but be warned this
   //   produces awful results when layers are used with subpixel rendering.
-  SkColor GetBackgroundColor() const;
-  void SetBackgroundColor(SkColor color);
+  base::Optional<SkColor> GetBackgroundColor() const;
+  void SetBackgroundColor(const base::Optional<SkColor>& color);
 
-  void SetBackgroundThemeColorId(ui::NativeTheme::ColorId color_id);
+  base::Optional<ui::NativeTheme::ColorId> GetBackgroundThemeColorId() const;
+  void SetBackgroundThemeColorId(
+      const base::Optional<ui::NativeTheme::ColorId>& color_id);
 
   // Returns the visible region of the content View.
   gfx::Rect GetVisibleRect() const;
 
-  bool GetUseColorId() const { return use_color_id_; }
+  bool GetUseColorId() const { return !!background_color_id_; }
 
   bool GetHideHorizontalScrollBar() const { return hide_horizontal_scrollbar_; }
   void SetHideHorizontalScrollBar(bool visible);
@@ -152,13 +155,8 @@ class VIEWS_EXPORT ScrollView : public View, public ScrollBarController {
 
   class Viewport;
 
-  union BackgroundColorData {
-    SkColor color;
-    ui::NativeTheme::ColorId color_id;
-  };
-
   // Forces |contents_viewport_| to have a Layer (assuming it doesn't already).
-  void EnableViewPortLayer();
+  void EnableViewportLayer();
 
   // Returns true if this or the viewport has a layer.
   bool DoesViewportOrScrollViewHaveLayer() const;
@@ -257,9 +255,9 @@ class VIEWS_EXPORT ScrollView : public View, public ScrollBarController {
   int max_height_ = -1;
 
   // See description of SetBackgroundColor() for details.
-  BackgroundColorData background_color_data_ = {
-      ui::NativeTheme::kColorId_DialogBackground};
-  bool use_color_id_ = true;
+  base::Optional<SkColor> background_color_;
+  base::Optional<ui::NativeTheme::ColorId> background_color_id_ =
+      ui::NativeTheme::kColorId_DialogBackground;
 
   // If true, never show the horizontal scrollbar (even if the contents is wider
   // than the viewport).

ui/views/controls/scroll_view_unittest.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/icu_test_util.h"
 #include "base/test/scoped_feature_list.h"
@@ -945,8 +946,8 @@ TEST_F(ScrollViewTest, ChildWithLayerTest) {
   // should be true.
   EXPECT_TRUE(test_api.contents_viewport()->layer()->fills_bounds_opaquely());
 
-  // Setting a transparent color should make fills opaquely false.
-  scroll_view_->SetBackgroundColor(SK_ColorTRANSPARENT);
+  // Setting a base::nullopt color should make fills opaquely false.
+  scroll_view_->SetBackgroundColor(base::nullopt);
   EXPECT_FALSE(test_api.contents_viewport()->layer()->fills_bounds_opaquely());
 
   child->DestroyLayer();

ui/views/metadata/type_conversion.cc

@@ -13,6 +13,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "ui/base/ime/text_input_type.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/native_theme/native_theme.h"
 
 namespace views {
 namespace metadata {
@@ -313,3 +314,8 @@ DEFINE_ENUM_CONVERTERS(ui::TextInputType,
                         base::ASCIIToUTF16("TEXT_INPUT_TYPE_DATE_TIME_FIELD")},
                        {ui::TextInputType::TEXT_INPUT_TYPE_MAX,
                         base::ASCIIToUTF16("TEXT_INPUT_TYPE_MAX")})
+
+#define OP(enum_name) \
+  { ui::NativeTheme::enum_name, base::ASCIIToUTF16(#enum_name) }
+DEFINE_ENUM_CONVERTERS(ui::NativeTheme::ColorId, NATIVE_THEME_COLOR_IDS)
+#undef OP