Eliminate hardcoded message_center::kNotificationDefaultAccentColor

This change eliminates hardcoded color kNotificationDefaultAccentColor
and creates a new native color ID for it.

Bug: 1056950
Change-Id: I3430d10367a7dd6b680a61ab5af95731fcbd64f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2219407Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Maria Villarreal <mavill@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#780018}

ash/system/message_center/stacked_notification_bar.cc

@@ -123,6 +123,25 @@ class StackedNotificationBar::StackedNotificationBarIcon
       layer()->GetAnimator()->StopAnimating();
   }
 
+  void OnThemeChanged() override {
+    views::ImageView::OnThemeChanged();
+
+    auto* notification =
+        message_center::MessageCenter::Get()->FindVisibleNotificationById(id_);
+    SkColor accent_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_NotificationDefaultAccentColor);
+    gfx::Image masked_small_icon = notification->GenerateMaskedSmallIcon(
+        kStackedNotificationIconSize, accent_color);
+
+    if (masked_small_icon.IsEmpty()) {
+      SetImage(gfx::CreateVectorIcon(message_center::kProductIcon,
+                                     kStackedNotificationIconSize,
+                                     accent_color));
+    } else {
+      SetImage(masked_small_icon.AsImageSkia());
+    }
+  }
+
   void AnimateIn() {
     DCHECK(!is_animating_out());
 
@@ -326,18 +345,6 @@ void StackedNotificationBar::AddNotificationIcon(
     notification_icons_container_->AddChildViewAt(icon_view, 0);
   else
     notification_icons_container_->AddChildView(icon_view);
-
-  gfx::Image masked_small_icon = notification->GenerateMaskedSmallIcon(
-      kStackedNotificationIconSize,
-      message_center::kNotificationDefaultAccentColor);
-
-  if (masked_small_icon.IsEmpty()) {
-    icon_view->SetImage(gfx::CreateVectorIcon(
-        message_center::kProductIcon, kStackedNotificationIconSize,
-        message_center::kNotificationDefaultAccentColor));
-  } else {
-    icon_view->SetImage(masked_small_icon.AsImageSkia());
-  }
 }
 
 void StackedNotificationBar::OnIconAnimatedOut(views::View* icon) {

components/media_message_center/media_notification_view_impl_unittest.cc

@@ -815,7 +815,7 @@ TEST_F(MAYBE_MediaNotificationViewImplTest, ActionButtonsToggleVisibility) {
 
 TEST_F(MAYBE_MediaNotificationViewImplTest, UpdateArtworkFromItem) {
   int title_artist_width = title_artist_row()->width();
-  const SkColor accent = header_row()->accent_color_for_testing();
+  const SkColor accent = header_row()->accent_color_for_testing().value();
   gfx::Size size = view()->size();
   EXPECT_CALL(container(), OnMediaArtworkChanged(_)).Times(2);
   EXPECT_CALL(container(), OnColorsChanged(_, _)).Times(2);
@@ -843,7 +843,9 @@ TEST_F(MAYBE_MediaNotificationViewImplTest, UpdateArtworkFromItem) {
   EXPECT_FALSE(GetArtworkImage().isNull());
   EXPECT_EQ(gfx::Size(10, 10), GetArtworkImage().size());
   EXPECT_EQ(size, view()->size());
-  EXPECT_NE(accent, header_row()->accent_color_for_testing());
+  auto accent_color = header_row()->accent_color_for_testing();
+  ASSERT_TRUE(accent_color.has_value());
+  EXPECT_NE(accent, accent_color.value());
 
   GetItem()->MediaControllerImageChanged(
       media_session::mojom::MediaSessionImageType::kArtwork, SkBitmap());
@@ -858,7 +860,9 @@ TEST_F(MAYBE_MediaNotificationViewImplTest, UpdateArtworkFromItem) {
   // affected.
   EXPECT_TRUE(GetArtworkImage().isNull());
   EXPECT_EQ(size, view()->size());
-  EXPECT_EQ(accent, header_row()->accent_color_for_testing());
+  accent_color = header_row()->accent_color_for_testing();
+  ASSERT_TRUE(accent_color.has_value());
+  EXPECT_EQ(accent, accent_color.value());
 }
 
 TEST_F(MAYBE_MediaNotificationViewImplTest, ExpandableDefaultState) {

ui/message_center/public/cpp/message_center_constants.h

@@ -69,9 +69,6 @@ constexpr SkColor kSmallImageMaskForegroundColor = SK_ColorWHITE;
 constexpr SkColor kSmallImageMaskBackgroundColor =
     SkColorSetRGB(0xa3, 0xa3, 0xa3);
 
-// Default accent color of notifications that are not generated by system.
-constexpr SkColor kNotificationDefaultAccentColor = gfx::kChromeIconGrey;
-
 // For list notifications.
 // Not used when --enabled-new-style-notification is set.
 const size_t kNotificationMaximumItems = 5;

ui/message_center/views/notification_header_view.cc

@@ -250,7 +250,6 @@ NotificationHeaderView::NotificationHeaderView(views::ButtonListener* listener)
   spacer->SetProperty(views::kFlexBehaviorKey, kSpacerFlex);
   AddChildView(spacer);
 
-  SetAccentColor(accent_color_);
   SetPreferredSize(gfx::Size(kNotificationWidth, kHeaderHeight));
 }
 
@@ -262,9 +261,8 @@ void NotificationHeaderView::SetAppIcon(const gfx::ImageSkia& img) {
 }
 
 void NotificationHeaderView::ClearAppIcon() {
-  app_icon_view_->SetImage(
-      gfx::CreateVectorIcon(kProductIcon, kSmallImageSizeMD, accent_color_));
   using_default_app_icon_ = true;
+  UpdateColors();
 }
 
 void NotificationHeaderView::SetAppName(const base::string16& name) {
@@ -309,6 +307,11 @@ void NotificationHeaderView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
     node_data->AddState(ax::mojom::State::kExpanded);
 }
 
+void NotificationHeaderView::OnThemeChanged() {
+  Button::OnThemeChanged();
+  UpdateColors();
+}
+
 void NotificationHeaderView::SetTimestamp(base::Time timestamp) {
   base::string16 relative_time;
   base::TimeDelta next_update;
@@ -343,9 +346,7 @@ void NotificationHeaderView::SetExpandButtonEnabled(bool enabled) {
 
 void NotificationHeaderView::SetExpanded(bool expanded) {
   is_expanded_ = expanded;
-  expand_button_->SetImage(gfx::CreateVectorIcon(
-      expanded ? kNotificationExpandLessIcon : kNotificationExpandMoreIcon,
-      kExpandIconSize, accent_color_));
+  UpdateColors();
   expand_button_->set_tooltip_text(l10n_util::GetStringUTF16(
       expanded ? IDS_MESSAGE_CENTER_COLLAPSE_NOTIFICATION
                : IDS_MESSAGE_CENTER_EXPAND_NOTIFICATION));
@@ -354,15 +355,7 @@ void NotificationHeaderView::SetExpanded(bool expanded) {
 
 void NotificationHeaderView::SetAccentColor(SkColor color) {
   accent_color_ = color;
-  app_name_view_->SetEnabledColor(accent_color_);
-  summary_text_view_->SetEnabledColor(accent_color_);
-  summary_text_divider_->SetEnabledColor(accent_color_);
-  SetExpanded(is_expanded_);
-
-  // If we are using the default app icon we should clear it so we refresh it
-  // with the new accent color.
-  if (using_default_app_icon_)
-    ClearAppIcon();
+  UpdateColors();
 }
 
 void NotificationHeaderView::SetBackgroundColor(SkColor color) {
@@ -406,4 +399,21 @@ void NotificationHeaderView::UpdateSummaryTextVisibility() {
   detail_views_->InvalidateLayout();
 }
 
+void NotificationHeaderView::UpdateColors() {
+  SkColor color = accent_color_.value_or(GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_NotificationDefaultAccentColor));
+  app_name_view_->SetEnabledColor(color);
+  summary_text_view_->SetEnabledColor(color);
+  summary_text_divider_->SetEnabledColor(color);
+
+  expand_button_->SetImage(gfx::CreateVectorIcon(
+      is_expanded_ ? kNotificationExpandLessIcon : kNotificationExpandMoreIcon,
+      kExpandIconSize, color));
+
+  if (using_default_app_icon_) {
+    app_icon_view_->SetImage(
+        gfx::CreateVectorIcon(kProductIcon, kSmallImageSizeMD, color));
+  }
+}
+
 }  // namespace message_center

ui/message_center/views/notification_header_view.h

@@ -41,8 +41,8 @@ class MESSAGE_CENTER_EXPORT NotificationHeaderView : public views::Button {
   void SetExpandButtonEnabled(bool enabled);
   void SetExpanded(bool expanded);
 
-  // Set the unified theme color used among the app icon, app name, and expand
-  // button.
+  // Calls UpdateColors() to set the unified theme color used among the
+  // app icon, app name, and expand button.
   void SetAccentColor(SkColor color);
 
   // Sets the background color of the notification. This is used to ensure that
@@ -57,10 +57,11 @@ class MESSAGE_CENTER_EXPORT NotificationHeaderView : public views::Button {
 
   // views::View:
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+  void OnThemeChanged() override;
 
   views::ImageView* expand_button() { return expand_button_; }
 
-  SkColor accent_color_for_testing() { return accent_color_; }
+  base::Optional<SkColor> accent_color_for_testing() { return accent_color_; }
 
   const views::Label* summary_text_for_testing() const {
     return summary_text_view_;
@@ -84,7 +85,9 @@ class MESSAGE_CENTER_EXPORT NotificationHeaderView : public views::Button {
   // Update visibility for both |summary_text_view_| and |timestamp_view_|.
   void UpdateSummaryTextVisibility();
 
-  SkColor accent_color_ = kNotificationDefaultAccentColor;
+  void UpdateColors();
+
+  base::Optional<SkColor> accent_color_;
 
   // Timer that updates the timestamp over time.
   base::OneShotTimer timestamp_update_timer_;

ui/message_center/views/notification_view_md.cc

@@ -861,9 +861,8 @@ void NotificationViewMD::OnNotificationInputSubmit(size_t index,
 
 void NotificationViewMD::CreateOrUpdateContextTitleView(
     const Notification& notification) {
-  header_row_->SetAccentColor(notification.accent_color() == SK_ColorTRANSPARENT
-                                  ? kNotificationDefaultAccentColor
-                                  : notification.accent_color());
+  if (notification.accent_color() != SK_ColorTRANSPARENT)
+    header_row_->SetAccentColor(notification.accent_color());
   header_row_->SetTimestamp(notification.timestamp());
   header_row_->SetAppNameElideBehavior(gfx::ELIDE_TAIL);
   header_row_->SetSummaryText(base::string16());
@@ -1087,9 +1086,11 @@ void NotificationViewMD::CreateOrUpdateSmallIconView(
   // TODO(knollr): figure out if this has a performance impact and
   // cache images if so. (crbug.com/768748)
   gfx::Image masked_small_icon = notification.GenerateMaskedSmallIcon(
-      kSmallImageSizeMD, notification.accent_color() == SK_ColorTRANSPARENT
-                             ? message_center::kNotificationDefaultAccentColor
-                             : notification.accent_color());
+      kSmallImageSizeMD,
+      notification.accent_color() == SK_ColorTRANSPARENT
+          ? GetNativeTheme()->GetSystemColor(
+                ui::NativeTheme::kColorId_NotificationDefaultAccentColor)
+          : notification.accent_color());
 
   if (masked_small_icon.IsEmpty()) {
     header_row_->ClearAppIcon();
@@ -1433,6 +1434,11 @@ void NotificationViewMD::OnThemeChanged() {
       inline_settings_visible
           ? ui::NativeTheme::kColorId_NotificationInlineSettingsBackground
           : ui::NativeTheme::kColorId_NotificationDefaultBackground));
+
+  auto* notification =
+      MessageCenter::Get()->FindVisibleNotificationById(notification_id());
+  if (notification)
+    CreateOrUpdateSmallIconView(*notification);
 }
 
 void NotificationViewMD::Activate() {

ui/message_center/views/notification_view_md_unittest.cc

@@ -988,10 +988,9 @@ TEST_F(NotificationViewMDTest, TestAccentColor) {
     notification_view()->ToggleExpanded();
   EXPECT_TRUE(notification_view()->actions_row_->GetVisible());
 
-  // By default, header does not have accent color (default grey), and
-  // buttons have default accent color.
-  EXPECT_EQ(kNotificationDefaultAccentColor,
-            notification_view()->header_row_->accent_color_for_testing());
+  // By default, header does not have accent color.
+  EXPECT_FALSE(
+      notification_view()->header_row_->accent_color_for_testing().has_value());
   EXPECT_EQ(
       kActionButtonTextColor,
       notification_view()->action_buttons_[0]->enabled_color_for_testing());
@@ -1003,8 +1002,10 @@ TEST_F(NotificationViewMDTest, TestAccentColor) {
   // same accent color.
   notification->set_accent_color(kCustomAccentColor);
   UpdateNotificationViews(*notification);
-  EXPECT_EQ(kCustomAccentColor,
-            notification_view()->header_row_->accent_color_for_testing());
+  auto accent_color =
+      notification_view()->header_row_->accent_color_for_testing();
+  ASSERT_TRUE(accent_color.has_value());
+  EXPECT_EQ(kCustomAccentColor, accent_color.value());
   EXPECT_EQ(
       kCustomAccentColor,
       notification_view()->action_buttons_[0]->enabled_color_for_testing());

ui/native_theme/common_theme.cc

@@ -431,6 +431,8 @@ SkColor GetDefaultColor(NativeTheme::ColorId color_id,
     case NativeTheme::kColorId_NotificationButtonBackground:
       return SkColorSetA(SK_ColorWHITE, 0.9 * 0xff);
 #endif
+    case NativeTheme::kColorId_NotificationDefaultAccentColor:
+      return gfx::kChromeIconGrey;
 
     // Scrollbar
     case NativeTheme::kColorId_OverlayScrollbarThumbBackground:

ui/native_theme/native_theme_color_id.h

@@ -91,6 +91,7 @@
   OP(kColorId_NotificationLargeImageBackground),                               \
   OP(kColorId_NotificationPlaceholderIconColor),                               \
   OP(kColorId_NotificationEmptyPlaceholderIconColor),                          \
+  OP(kColorId_NotificationDefaultAccentColor),                                 \
   /* Slider */                                                                 \
   OP(kColorId_SliderThumbDefault),                                             \
   OP(kColorId_SliderTroughDefault),                                            \