Improve Harmony compliance for desktop notifications.

This fixes an insufficient line height issue and improves text contrast.

Bug: 1138840
Change-Id: I90030ad6e63dc0642378da26f34a9012c255decb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521285
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826333}

ash/system/message_center/unified_message_center_view_unittest.cc

@@ -98,6 +98,14 @@ class UnifiedMessageCenterViewTest : public AshTestBase,
     return id;
   }
 
+  // Adds more than enough notifications to make the message center scrollable.
+  std::vector<std::string> AddManyNotifications() {
+    std::vector<std::string> ids;
+    for (int i = 0; i < 10; ++i)
+      ids.push_back(AddNotification(false));
+    return ids;
+  }
+
   std::unique_ptr<TestUnifiedMessageCenterView> CreateMessageCenterViewImpl(
       int max_height) {
     auto message_center_view =
@@ -277,8 +285,7 @@ TEST_F(UnifiedMessageCenterViewTest, AddAndRemoveNotification) {
 
 TEST_F(UnifiedMessageCenterViewTest, RemoveNotificationAtTail) {
   // Show message center with multiple notifications.
-  for (int i = 0; i < 10; ++i)
-    AddNotification(false /* pinned */);
+  AddManyNotifications();
   CreateMessageCenterView();
   EXPECT_TRUE(message_center_view()->GetVisible());
 
@@ -306,9 +313,7 @@ TEST_F(UnifiedMessageCenterViewTest, RemoveNotificationAtTail) {
 }
 
 TEST_F(UnifiedMessageCenterViewTest, ContentsRelayout) {
-  std::vector<std::string> ids;
-  for (size_t i = 0; i < 10; ++i)
-    ids.push_back(AddNotification(false /* pinned */));
+  std::vector<std::string> ids = AddManyNotifications();
   CreateMessageCenterView();
   EXPECT_TRUE(message_center_view()->GetVisible());
   // MessageCenterView is maxed out.
@@ -404,8 +409,7 @@ TEST_F(UnifiedMessageCenterViewTest, InitialPosition) {
 }
 
 TEST_F(UnifiedMessageCenterViewTest, InitialPositionMaxOut) {
-  for (size_t i = 0; i < 6; ++i)
-    AddNotification(false /* pinned */);
+  AddManyNotifications();
   CreateMessageCenterView();
   EXPECT_TRUE(message_center_view()->GetVisible());
 
@@ -417,7 +421,7 @@ TEST_F(UnifiedMessageCenterViewTest, InitialPositionMaxOut) {
 TEST_F(UnifiedMessageCenterViewTest, InitialPositionWithLargeNotification) {
   AddNotification(false /* pinned */);
   AddNotification(false /* pinned */);
-  CreateMessageCenterView(80 /* max_height */);
+  CreateMessageCenterView(60 /* max_height */);
   EXPECT_TRUE(message_center_view()->GetVisible());
 
   // MessageCenterView is shorter than the notification.
@@ -430,8 +434,7 @@ TEST_F(UnifiedMessageCenterViewTest, InitialPositionWithLargeNotification) {
 }
 
 TEST_F(UnifiedMessageCenterViewTest, ScrollPositionWhenResized) {
-  for (size_t i = 0; i < 6; ++i)
-    AddNotification(false /* pinned */);
+  AddManyNotifications();
   CreateMessageCenterView();
   EXPECT_TRUE(message_center_view()->GetVisible());
 
@@ -461,8 +464,7 @@ TEST_F(UnifiedMessageCenterViewTest, ScrollPositionWhenResized) {
 }
 
 TEST_F(UnifiedMessageCenterViewTest, StackingCounterLayout) {
-  for (size_t i = 0; i < 10; ++i)
-    AddNotification(false /* pinned */);
+  AddManyNotifications();
 
   // MessageCenterView is maxed out.
   CreateMessageCenterView();
@@ -487,8 +489,7 @@ TEST_F(UnifiedMessageCenterViewTest, StackingCounterLayout) {
 }
 
 TEST_F(UnifiedMessageCenterViewTest, StackingCounterMessageListScrolled) {
-  for (size_t i = 0; i < 10; ++i)
-    AddNotification(false /* pinned */);
+  AddManyNotifications();
   CreateMessageCenterView();
   EXPECT_TRUE(message_center_view()->GetVisible());
   EXPECT_TRUE(GetNotificationBarLabel()->GetVisible());
@@ -522,9 +523,7 @@ TEST_F(UnifiedMessageCenterViewTest, StackingCounterMessageListScrolled) {
 }
 
 TEST_F(UnifiedMessageCenterViewTest, StackingCounterNotificationRemoval) {
-  std::vector<std::string> ids;
-  for (size_t i = 0; i < 6; ++i)
-    ids.push_back(AddNotification(false /* pinned */));
+  std::vector<std::string> ids = AddManyNotifications();
   CreateMessageCenterView();
   EXPECT_TRUE(message_center_view()->GetVisible());
 
@@ -534,7 +533,7 @@ TEST_F(UnifiedMessageCenterViewTest, StackingCounterNotificationRemoval) {
 
   // Dismiss until there are 2 notifications. The bar should still be visible.
   EXPECT_TRUE(GetNotificationBar()->GetVisible());
-  for (size_t i = 0; i < 4; ++i) {
+  for (size_t i = 0; (i + 2) < ids.size(); ++i) {
     MessageCenter::Get()->RemoveNotification(ids[i], true /* by_user */);
     AnimateMessageListToEnd();
   }
@@ -549,7 +548,8 @@ TEST_F(UnifiedMessageCenterViewTest, StackingCounterNotificationRemoval) {
 
   // Dismiss until there is only 1 notification left. The bar should be
   // hidden after an animation.
-  MessageCenter::Get()->RemoveNotification(ids[4], true /* by_user */);
+  MessageCenter::Get()->RemoveNotification(ids[ids.size() - 2],
+                                           true /* by_user */);
   EXPECT_TRUE(GetNotificationBar()->GetVisible());
 
   // The HIDE_STACKING_BAR animation starts after the notification is slid out.
@@ -622,10 +622,10 @@ TEST_F(UnifiedMessageCenterViewTest, StackingCounterVisibility) {
   // notifications are hidden).
   EXPECT_FALSE(GetNotificationBar()->GetVisible());
 
-  for (size_t i = 0; i < 4; ++i)
+  for (size_t i = 0; i < 8; ++i)
     AddNotification(true /* pinned */);
 
-  // The bar should be visible with 6 pinned notifications (some of the
+  // The bar should be visible with 10 pinned notifications (some of the
   // notifications are hidden). However, clear all button should not be shown.
   EXPECT_TRUE(GetNotificationBar()->GetVisible());
   EXPECT_FALSE(GetNotificationBarClearAllButton()->GetVisible());

ui/message_center/views/notification_view_md.cc

@@ -113,29 +113,15 @@ const int kMinPixelsPerTitleCharacterMD = 4;
 constexpr size_t kMessageCharacterLimitMD =
     kNotificationWidth * kMessageExpandedLineLimit / 3;
 
-// The default is 12, so this normally come out to 13.
-constexpr int kTextFontSizeDelta = 1;
-
 // In progress notification, if both the title and the message are long, the
 // message would be prioritized and the title would be elided.
 // However, it is not preferable that we completely omit the title, so
 // the ratio of the message width is limited to this value.
 constexpr double kProgressNotificationMessageRatio = 0.7;
 
-// Line height of title and message views.
-constexpr int kLineHeightMD = 17;
-
 // This key/property allows tagging the textfield with its index.
 DEFINE_UI_CLASS_PROPERTY_KEY(int, kTextfieldIndexKey, 0U)
 
-// FontList for the texts except for the header.
-gfx::FontList GetTextFontList() {
-  gfx::Font default_font;
-  gfx::Font font = default_font.Derive(kTextFontSizeDelta, gfx::Font::NORMAL,
-                                       gfx::Font::Weight::NORMAL);
-  return gfx::FontList(font);
-}
-
 class ClickActivator : public ui::EventHandler {
  public:
   explicit ClickActivator(NotificationViewMD* owner) : owner_(owner) {}
@@ -162,20 +148,16 @@ std::unique_ptr<views::View> CreateItemView(const NotificationItem& item) {
   view->SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kHorizontal, gfx::Insets(), 0));
 
-  const gfx::FontList font_list = GetTextFontList();
-
-  auto* title = new views::Label(item.title);
-  title->SetFontList(font_list);
+  auto* title = view->AddChildView(std::make_unique<views::Label>(
+      item.title, views::style::CONTEXT_DIALOG_BODY_TEXT));
   title->SetCollapseWhenHidden(true);
   title->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  view->AddChildView(title);
 
-  views::Label* message = view->AddChildView(std::make_unique<views::Label>(
+  auto* message = view->AddChildView(std::make_unique<views::Label>(
       l10n_util::GetStringFUTF16(
           IDS_MESSAGE_CENTER_LIST_NOTIFICATION_MESSAGE_WITH_DIVIDER,
           item.message),
-      views::style::CONTEXT_LABEL, views::style::STYLE_DISABLED));
-  message->SetFontList(font_list);
+      views::style::CONTEXT_DIALOG_BODY_TEXT, views::style::STYLE_SECONDARY));
   message->SetCollapseWhenHidden(true);
   message->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   return view;
@@ -211,17 +193,13 @@ const char* CompactTitleMessageView::GetClassName() const {
 }
 
 CompactTitleMessageView::CompactTitleMessageView() {
-  const gfx::FontList& font_list = GetTextFontList();
-
-  title_ = new views::Label();
-  title_->SetFontList(font_list);
+  title_ = AddChildView(std::make_unique<views::Label>(
+      base::string16(), views::style::CONTEXT_DIALOG_BODY_TEXT));
   title_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  AddChildView(title_);
 
   message_ = AddChildView(std::make_unique<views::Label>(
-      base::string16(), views::style::CONTEXT_LABEL,
-      views::style::STYLE_DISABLED));
-  message_->SetFontList(font_list);
+      base::string16(), views::style::CONTEXT_DIALOG_BODY_TEXT,
+      views::style::STYLE_SECONDARY));
   message_->SetHorizontalAlignment(gfx::ALIGN_RIGHT);
 }
 
@@ -489,7 +467,7 @@ class InlineSettingsRadioButton : public views::RadioButton {
  public:
   explicit InlineSettingsRadioButton(const base::string16& label_text)
       : views::RadioButton(label_text, 1 /* group */) {
-    label()->SetFontList(GetTextFontList());
+    label()->SetTextContext(views::style::CONTEXT_DIALOG_BODY_TEXT);
     label()->SetSubpixelRenderingEnabled(false);
   }
 
@@ -863,12 +841,9 @@ void NotificationViewMD::CreateOrUpdateTitleView(
   base::string16 title = gfx::TruncateString(
       notification.title(), title_character_limit, gfx::WORD_BREAK);
   if (!title_view_) {
-    const gfx::FontList& font_list = GetTextFontList();
-
-    title_view_ = new views::Label(title);
-    title_view_->SetFontList(font_list);
+    title_view_ =
+        new views::Label(title, views::style::CONTEXT_DIALOG_BODY_TEXT);
     title_view_->SetHorizontalAlignment(gfx::ALIGN_TO_HEAD);
-    title_view_->SetLineHeight(kLineHeightMD);
     // TODO(knollr): multiline should not be required, but we need to set the
     // width of |title_view_| (because of crbug.com/682266), which only works in
     // multiline mode.
@@ -897,15 +872,12 @@ void NotificationViewMD::CreateOrUpdateMessageView(
       notification.message(), kMessageCharacterLimitMD, gfx::WORD_BREAK);
 
   if (!message_view_) {
-    const gfx::FontList& font_list = GetTextFontList();
-
     message_view_ = left_content_->AddChildViewAt(
-        std::make_unique<views::Label>(text, views::style::CONTEXT_LABEL,
-                                       views::style::STYLE_DISABLED),
+        std::make_unique<views::Label>(text,
+                                       views::style::CONTEXT_DIALOG_BODY_TEXT,
+                                       views::style::STYLE_SECONDARY),
         left_content_count_);
-    message_view_->SetFontList(font_list);
     message_view_->SetHorizontalAlignment(gfx::ALIGN_TO_HEAD);
-    message_view_->SetLineHeight(kLineHeightMD);
     message_view_->SetMultiLine(true);
     message_view_->SetMaxLines(kMaxLinesForMessageView);
     message_view_->SetAllowCharacterBreak(true);
@@ -979,13 +951,11 @@ void NotificationViewMD::CreateOrUpdateProgressStatusView(
   }
 
   if (!status_view_) {
-    const gfx::FontList& font_list = GetTextFontList();
     status_view_ = left_content_->AddChildViewAt(
         std::make_unique<views::Label>(base::string16(),
-                                       views::style::CONTEXT_LABEL,
-                                       views::style::STYLE_DISABLED),
+                                       views::style::CONTEXT_DIALOG_BODY_TEXT,
+                                       views::style::STYLE_SECONDARY),
         left_content_count_);
-    status_view_->SetFontList(font_list);
     status_view_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     status_view_->SetBorder(views::CreateEmptyBorder(kStatusTextPadding));
   }