Remove kChromeIconGrey from notification_control_buttons_view.cc.

Bug: 1056761
Change-Id: I3c95e75da56be6ce6fe55c8e4bf88974cf770a6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3527415
Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983651}

chrome/browser/browsing_data/cookies_tree_model.cc

@@ -53,10 +53,9 @@
 #include "net/cookies/cookie_util.h"
 #include "third_party/blink/public/common/storage_key/storage_key.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/models/image_model.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/paint_vector_icon.h"
+#include "ui/color/color_id.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -1372,12 +1371,12 @@ int CookiesTreeModel::GetSendForMessageID(const net::CanonicalCookie& cookie) {
 // TreeModel methods:
 // Returns the set of icons for the nodes in the tree. You only need override
 // this if you don't want to use the default folder icons.
-void CookiesTreeModel::GetIcons(std::vector<gfx::ImageSkia>* icons) {
-  icons->push_back(gfx::CreateVectorIcon(vector_icons::kCookieIcon, 18,
-                                         gfx::kChromeIconGrey));
-  icons->push_back(*ui::ResourceBundle::GetSharedInstance()
-                        .GetNativeImageNamed(IDR_COOKIE_STORAGE_ICON)
-                        .ToImageSkia());
+void CookiesTreeModel::GetIcons(std::vector<ui::ImageModel>* icons) {
+  icons->push_back(ui::ImageModel::FromVectorIcon(vector_icons::kCookieIcon,
+                                                  ui::kColorIcon, 18));
+  icons->push_back(ui::ImageModel::FromImage(
+      ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed(
+          IDR_COOKIE_STORAGE_ICON)));
 }
 
 // Returns the index of the icon to use for |node|. Return -1 to use the

chrome/browser/browsing_data/cookies_tree_model.h

@@ -316,7 +316,7 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
   // ui::TreeModel methods:
   // Returns the set of icons for the nodes in the tree. You only need override
   // this if you don't want to use the default folder icons.
-  void GetIcons(std::vector<gfx::ImageSkia>* icons) override;
+  void GetIcons(std::vector<ui::ImageModel>* icons) override;
 
   // Returns the index of the icon to use for |node|. Return -1 to use the
   // default icon. The index is relative to the list of icons returned from

ui/base/models/tree_model.h

@@ -10,12 +10,9 @@
 
 #include "base/component_export.h"
 
-namespace gfx {
-class ImageSkia;
-}
-
 namespace ui {
 
+class ImageModel;
 class TreeModel;
 
 // TreeModelNode --------------------------------------------------------------
@@ -90,7 +87,7 @@ class COMPONENT_EXPORT(UI_BASE) TreeModel {
 
   // Returns the set of icons for the nodes in the tree. You only need override
   // this if you don't want to use the default folder icons.
-  virtual void GetIcons(std::vector<gfx::ImageSkia>* icons) {}
+  virtual void GetIcons(std::vector<ui::ImageModel>* icons) {}
 
   // Returns the index of the icon to use for |node|. Return -1 to use the
   // default icon. The index is relative to the list of icons returned from

ui/gfx/color_palette.h

@@ -161,10 +161,7 @@ constexpr SkAlpha kGoogleGreyAlpha600 = 0x8C;  //  55%
 constexpr SkAlpha kGoogleGreyAlpha700 = 0xB5;  //  71%
 constexpr SkAlpha kGoogleGreyAlpha800 = 0xDB;  //  86%
 
-// kChromeIconGrey is subject to change in the future, kGoogleGrey700 is set in
-// stone. If you're semantically looking for "the icon color Chrome uses" then
-// use kChromeIconGrey, if you're looking for GG700 grey specifically, use the
-// Google-grey constant directly.
+// TODO(pkasting): Remove.
 constexpr SkColor kChromeIconGrey = kGoogleGrey700;
 
 // An alpha value for designating a control's disabled state. In specs this is

ui/message_center/views/notification_control_buttons_unittest.cc

@@ -14,6 +14,8 @@
 #include "ui/message_center/public/cpp/notification.h"
 #include "ui/message_center/vector_icons.h"
 #include "ui/message_center/views/message_view.h"
+#include "ui/views/test/views_test_base.h"
+#include "ui/views/widget/widget.h"
 
 namespace message_center {
 
@@ -22,20 +24,23 @@ namespace {
 class TestMessageView : public MessageView {
  public:
   explicit TestMessageView(const Notification& notification)
-      : MessageView(notification),
-        buttons_view_(std::make_unique<NotificationControlButtonsView>(this)) {}
+      : MessageView(notification) {}
 
   NotificationControlButtonsView* GetControlButtonsView() const override {
-    return buttons_view_.get();
+    return buttons_view_;
+  }
+
+  void set_control_buttons_view(NotificationControlButtonsView* buttons_view) {
+    buttons_view_ = buttons_view;
   }
 
  private:
-  std::unique_ptr<NotificationControlButtonsView> buttons_view_;
+  NotificationControlButtonsView* buttons_view_ = nullptr;
 };
 
 }  // namespace
 
-class NotificationControlButtonsTest : public testing::Test {
+class NotificationControlButtonsTest : public views::ViewsTestBase {
  public:
   NotificationControlButtonsTest() = default;
 
@@ -46,15 +51,23 @@ class NotificationControlButtonsTest : public testing::Test {
 
   ~NotificationControlButtonsTest() override = default;
 
-  // testing::Test
+  // views::ViewsTestBase:
   void SetUp() override {
-    Test::SetUp();
+    views::ViewsTestBase::SetUp();
+    widget_ = CreateTestWidget();
     Notification notification(
         NOTIFICATION_TYPE_SIMPLE, "id", u"title", u"id", ui::ImageModel(),
         std::u16string(), GURL(),
         NotifierId(NotifierType::APPLICATION, "notifier_id"),
         RichNotificationData(), nullptr);
     message_view_ = std::make_unique<TestMessageView>(notification);
+    message_view_->set_control_buttons_view(widget_->SetContentsView(
+        std::make_unique<NotificationControlButtonsView>(message_view_.get())));
+  }
+
+  void TearDown() override {
+    widget_.reset();
+    views::ViewsTestBase::TearDown();
   }
 
   NotificationControlButtonsView* buttons_view() {
@@ -79,6 +92,7 @@ class NotificationControlButtonsTest : public testing::Test {
   }
 
  private:
+  std::unique_ptr<views::Widget> widget_;
   std::unique_ptr<TestMessageView> message_view_;
 };
 
@@ -110,7 +124,7 @@ TEST_F(NotificationControlButtonsTest, IconColor_NoContrastEnforcement) {
   buttons_view()->ShowSnoozeButton(true);
 
   // Default icon color.
-  ExpectIconColor(gfx::kChromeIconGrey);
+  ExpectIconColor(buttons_view()->GetColorProvider()->GetColor(ui::kColorIcon));
 
   // Without setting a background color we won't enforce contrast ratios.
   buttons_view()->SetButtonIconColors(SK_ColorWHITE);

ui/message_center/views/notification_control_buttons_view.cc

@@ -9,6 +9,7 @@
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/color/color_provider.h"
 #include "ui/compositor/layer.h"
 #include "ui/events/event.h"
 #include "ui/gfx/color_palette.h"
@@ -26,7 +27,7 @@ namespace message_center {
 
 NotificationControlButtonsView::NotificationControlButtonsView(
     MessageView* message_view)
-    : message_view_(message_view), icon_color_(gfx::kChromeIconGrey) {
+    : message_view_(message_view) {
   auto* layout = SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kHorizontal));
   // Do not stretch buttons as that would stretch their focus indicator.
@@ -40,14 +41,22 @@ NotificationControlButtonsView::NotificationControlButtonsView(
 
 NotificationControlButtonsView::~NotificationControlButtonsView() = default;
 
+void NotificationControlButtonsView::OnThemeChanged() {
+  views::View::OnThemeChanged();
+  UpdateButtonIconColors();
+}
+
 void NotificationControlButtonsView::ShowCloseButton(bool show) {
   if (show && !close_button_) {
     close_button_ = AddChildView(std::make_unique<PaddedButton>(
         base::BindRepeating(&MessageView::OnCloseButtonPressed,
                             base::Unretained(message_view_))));
-    close_button_->SetImage(views::Button::STATE_NORMAL,
-                            gfx::CreateVectorIcon(kNotificationCloseButtonIcon,
-                                                  DetermineButtonIconColor()));
+    if (GetWidget()) {
+      close_button_->SetImage(
+          views::Button::STATE_NORMAL,
+          gfx::CreateVectorIcon(kNotificationCloseButtonIcon,
+                                DetermineButtonIconColor()));
+    }
     close_button_->SetAccessibleName(l10n_util::GetStringUTF16(
         IDS_MESSAGE_CENTER_CLOSE_NOTIFICATION_BUTTON_ACCESSIBLE_NAME));
     close_button_->SetTooltipText(l10n_util::GetStringUTF16(
@@ -71,10 +80,12 @@ void NotificationControlButtonsView::ShowSettingsButton(bool show) {
                            &MessageView::OnSettingsButtonPressed,
                            base::Unretained(message_view_))),
                        position);
-    settings_button_->SetImage(
-        views::Button::STATE_NORMAL,
-        gfx::CreateVectorIcon(kNotificationSettingsButtonIcon,
-                              DetermineButtonIconColor()));
+    if (GetWidget()) {
+      settings_button_->SetImage(
+          views::Button::STATE_NORMAL,
+          gfx::CreateVectorIcon(kNotificationSettingsButtonIcon,
+                                DetermineButtonIconColor()));
+    }
     settings_button_->SetAccessibleName(l10n_util::GetStringUTF16(
         IDS_MESSAGE_NOTIFICATION_SETTINGS_BUTTON_ACCESSIBLE_NAME));
     settings_button_->SetTooltipText(l10n_util::GetStringUTF16(
@@ -97,10 +108,12 @@ void NotificationControlButtonsView::ShowSnoozeButton(bool show) {
                            &MessageView::OnSnoozeButtonPressed,
                            base::Unretained(message_view_))),
                        0);
-    snooze_button_->SetImage(
-        views::Button::STATE_NORMAL,
-        gfx::CreateVectorIcon(kNotificationSnoozeButtonIcon,
-                              DetermineButtonIconColor()));
+    if (GetWidget()) {
+      snooze_button_->SetImage(
+          views::Button::STATE_NORMAL,
+          gfx::CreateVectorIcon(kNotificationSnoozeButtonIcon,
+                                DetermineButtonIconColor()));
+    }
     snooze_button_->SetAccessibleName(l10n_util::GetStringUTF16(
         IDS_MESSAGE_CENTER_NOTIFICATION_SNOOZE_BUTTON_TOOLTIP));
     snooze_button_->SetTooltipText(l10n_util::GetStringUTF16(
@@ -133,7 +146,8 @@ void NotificationControlButtonsView::SetButtonIconColors(SkColor color) {
   if (color == icon_color_)
     return;
   icon_color_ = color;
-  UpdateButtonIconColors();
+  if (GetWidget())
+    UpdateButtonIconColors();
 }
 
 void NotificationControlButtonsView::SetBackgroundColor(SkColor color) {
@@ -167,10 +181,12 @@ void NotificationControlButtonsView::UpdateButtonIconColors() {
 }
 
 SkColor NotificationControlButtonsView::DetermineButtonIconColor() const {
+  const SkColor icon_color =
+      icon_color_.value_or(GetColorProvider()->GetColor(ui::kColorIcon));
   if (SkColorGetA(background_color_) != SK_AlphaOPAQUE)
-    return icon_color_;
+    return icon_color;
 
-  return color_utils::BlendForMinContrast(icon_color_, background_color_).color;
+  return color_utils::BlendForMinContrast(icon_color, background_color_).color;
 }
 
 BEGIN_METADATA(NotificationControlButtonsView, views::View)

ui/message_center/views/notification_control_buttons_view.h

@@ -29,6 +29,8 @@ class MESSAGE_CENTER_EXPORT NotificationControlButtonsView
       const NotificationControlButtonsView&) = delete;
   ~NotificationControlButtonsView() override;
 
+  void OnThemeChanged() override;
+
   // Change the visibility of the close button. True to show, false to hide.
   void ShowCloseButton(bool show);
   // Change the visibility of the settings button. True to show, false to hide.
@@ -71,7 +73,7 @@ class MESSAGE_CENTER_EXPORT NotificationControlButtonsView
   raw_ptr<PaddedButton> snooze_button_ = nullptr;
 
   // The color used for the close, settings, and snooze icons.
-  SkColor icon_color_;
+  absl::optional<SkColor> icon_color_;
   // The background color for readability of the icons.
   SkColor background_color_ = SK_ColorTRANSPARENT;
 };

ui/views/controls/tree/tree_view.cc

@@ -18,6 +18,7 @@
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/base/models/image_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/color/color_id.h"
 #include "ui/color/color_provider.h"
@@ -101,20 +102,19 @@ TreeView::TreeView()
   constexpr bool kUseMdIcons = false;
 #endif
   if (kUseMdIcons) {
-    closed_icon_ = open_icon_ =
-        gfx::CreateVectorIcon(vector_icons::kFolderIcon, gfx::kChromeIconGrey);
+    closed_icon_ = open_icon_ = ui::ImageModel::FromVectorIcon(
+        vector_icons::kFolderIcon, ui::kColorIcon);
   } else {
     // TODO(ellyjones): if the pre-Harmony codepath goes away, merge
     // closed_icon_ and open_icon_.
-    closed_icon_ = *ui::ResourceBundle::GetSharedInstance()
-                        .GetImageNamed(IDR_FOLDER_CLOSED)
-                        .ToImageSkia();
-    open_icon_ = *ui::ResourceBundle::GetSharedInstance()
-                      .GetImageNamed(IDR_FOLDER_OPEN)
-                      .ToImageSkia();
+    closed_icon_ = ui::ImageModel::FromImage(
+        ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+            IDR_FOLDER_CLOSED));
+    open_icon_ = ui::ImageModel::FromImage(
+        ui::ResourceBundle::GetSharedInstance().GetImageNamed(IDR_FOLDER_OPEN));
   }
-  text_offset_ =
-      closed_icon_.width() + kImagePadding + kImagePadding + kArrowRegionSize;
+  text_offset_ = closed_icon_.Size().width() + kImagePadding + kImagePadding +
+                 kArrowRegionSize;
 }
 
 TreeView::~TreeView() {
@@ -1180,12 +1180,15 @@ void TreeView::PaintNodeIcon(gfx::Canvas* canvas,
     canvas->Translate(gfx::Vector2d(bounds.x(), 0));
     scoped_canvas.FlipIfRTL(bounds.width());
     // Now paint the icon local to that flipped region.
-    PaintRowIcon(canvas, node->is_expanded() ? open_icon_ : closed_icon_,
+    PaintRowIcon(canvas,
+                 (node->is_expanded() ? open_icon_ : closed_icon_)
+                     .Rasterize(GetColorProvider()),
                  icon_x,
                  gfx::Rect(0, bounds.y(), bounds.width(), bounds.height()));
   } else {
-    const gfx::ImageSkia& icon = icons_[icon_index];
-    icon_x += (open_icon_.width() - icon.width()) / 2;
+    const gfx::ImageSkia& icon =
+        icons_[icon_index].Rasterize(GetColorProvider());
+    icon_x += (open_icon_.Size().width() - icon.width()) / 2;
     if (base::i18n::IsRTL())
       icon_x = bounds.width() - icon_x - icon.width();
     PaintRowIcon(canvas, icon, icon_x, bounds);

ui/views/controls/tree/tree_view.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/memory/raw_ptr.h"
+#include "ui/base/models/image_model.h"
 #include "ui/base/models/tree_node_model.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/image/image_skia.h"
@@ -466,11 +467,11 @@ class VIEWS_EXPORT TreeView : public View,
   raw_ptr<ui::TreeModel> model_ = nullptr;
 
   // Default icons for closed/open.
-  gfx::ImageSkia closed_icon_;
-  gfx::ImageSkia open_icon_;
+  ui::ImageModel closed_icon_;
+  ui::ImageModel open_icon_;
 
   // Icons from the model.
-  std::vector<gfx::ImageSkia> icons_;
+  std::vector<ui::ImageModel> icons_;
 
   // The root node.
   InternalNode root_;