Reland "Menu Model: fix bug where menu item title was greyed out because it is disabled."

This is a reland of f2baca71

Original change's description:
> Menu Model: fix bug where menu item title was greyed out because it is disabled.
>
> The old approach of adding title as a disabled item meant that this CL:
>
> https://chromium-review.googlesource.com/c/chromium/src/+/1788198
>
> causes the menu title to be displayed in grey text instead of solid black text. This CL introduces a new menu type that, while still disabled and non-reactive, be displayed in solid black text.
>
> Bug: 1006203
> Change-Id: I9fa38132160228ebde5bd43d8db9c2f60d5f001c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841583
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Toni Baržić <tbarzic@chromium.org>
> Commit-Queue: Yulun Wu <yulunwu@chromium.org>
> Auto-Submit: Yulun Wu <yulunwu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#707147}

Bug: 1006203
Change-Id: I02ac38b96dea074f8e9c3e8c9a019a53db512513
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869475
Commit-Queue: Yulun Wu <yulunwu@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710959}

ash/shelf/shelf_application_menu_model.cc

@@ -20,7 +20,7 @@ ShelfApplicationMenuModel::ShelfApplicationMenuModel(
     Items items,
     ShelfItemDelegate* delegate)
     : ui::SimpleMenuModel(this), delegate_(delegate) {
-  AddItem(std::numeric_limits<int>::max(), title);
+  AddTitle(title);
   for (size_t i = 0; i < items.size(); i++)
     AddItemWithIcon(i, items[i].first, items[i].second);
   AddSeparator(ui::SPACING_SEPARATOR);

ash/shelf/shelf_application_menu_model_unittest.cc

@@ -51,7 +51,7 @@ TEST(ShelfApplicationMenuModelTest, VerifyContentsWithNoMenuItems) {
   ShelfApplicationMenuModel menu(title, {}, nullptr);
   // Expect the title and a separator.
   ASSERT_EQ(2, menu.GetItemCount());
-  EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(0));
+  EXPECT_EQ(ui::MenuModel::TYPE_TITLE, menu.GetTypeAt(0));
   EXPECT_EQ(title, menu.GetLabelAt(0));
   EXPECT_FALSE(menu.IsEnabledAt(0));
   EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, menu.GetTypeAt(1));
@@ -75,7 +75,7 @@ TEST(ShelfApplicationMenuModelTest, VerifyContentsWithMenuItems) {
   ASSERT_EQ(static_cast<int>(5), menu.GetItemCount());
 
   // The label title should not be enabled.
-  EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu.GetTypeAt(0));
+  EXPECT_EQ(ui::MenuModel::TYPE_TITLE, menu.GetTypeAt(0));
   EXPECT_EQ(title, menu.GetLabelAt(0));
   EXPECT_FALSE(menu.IsEnabledAt(0));
 

components/dbus/menu/menu_property_list.cc

@@ -67,6 +67,7 @@ MenuItemProperties ComputeMenuPropertiesForMenuItem(ui::MenuModel* menu,
   switch (menu->GetTypeAt(i)) {
     case ui::MenuModel::TYPE_COMMAND:
     case ui::MenuModel::TYPE_HIGHLIGHTED:
+    case ui::MenuModel::TYPE_TITLE:
       // Nothing special to do.
       break;
     case ui::MenuModel::TYPE_CHECK:

components/dbus/menu/menu_property_list_unittest.cc

@@ -149,6 +149,9 @@ class TestMenuModelBuilder {
       case ui::MenuModel::TYPE_COMMAND:
         menu->AddItem(0, label_);
         break;
+      case ui::MenuModel::TYPE_TITLE:
+        menu->AddTitle(label_);
+        break;
       case ui::MenuModel::TYPE_CHECK:
         menu->AddCheckItem(0, label_);
         break;

ui/base/models/menu_model.h

@@ -39,6 +39,8 @@ class UI_BASE_EXPORT MenuModel : public base::SupportsWeakPtr<MenuModel> {
     TYPE_HIGHLIGHTED,  // Performs an action when selected, and has a different
                        // colored background. When placed at the bottom, the
                        // background matches the menu's rounded corners.
+    TYPE_TITLE,        // Plain text that does not perform any action when
+                       // selected.
   };
 
   MenuModel();

ui/base/models/simple_menu_model.cc

@@ -17,6 +17,10 @@ namespace ui {
 
 const int kSeparatorId = -1;
 
+// TYPE_TITLE should be rendered as enabled but is non-interactive and cannot be
+// highlighted.
+const int kTitleId = -2;
+
 ////////////////////////////////////////////////////////////////////////////////
 // SimpleMenuModel::Delegate, public:
 
@@ -142,6 +146,13 @@ void SimpleMenuModel::AddHighlightedItemWithIcon(int command_id,
   AppendItem(std::move(item));
 }
 
+void SimpleMenuModel::AddTitle(const base::string16& label) {
+  // Title items are non-interactive and should not be enabled.
+  Item title_item = Item(kTitleId, TYPE_TITLE, label);
+  title_item.enabled = false;
+  AppendItem(std::move(title_item));
+}
+
 void SimpleMenuModel::AddSeparator(MenuSeparatorType separator_type) {
   if (items_.empty()) {
     if (separator_type == NORMAL_SEPARATOR) {
@@ -460,6 +471,7 @@ ButtonMenuItemModel* SimpleMenuModel::GetButtonMenuItemAt(int index) const {
 
 bool SimpleMenuModel::IsEnabledAt(int index) const {
   int command_id = GetCommandIdAt(index);
+
   if (!delegate_ || command_id == kSeparatorId || GetButtonMenuItemAt(index))
     return items_[ValidateItemIndex(index)].enabled;
 
@@ -469,8 +481,10 @@ bool SimpleMenuModel::IsEnabledAt(int index) const {
 
 bool SimpleMenuModel::IsVisibleAt(int index) const {
   int command_id = GetCommandIdAt(index);
-  if (!delegate_ || command_id == kSeparatorId || GetButtonMenuItemAt(index))
+  if (!delegate_ || command_id == kSeparatorId || command_id == kTitleId ||
+      GetButtonMenuItemAt(index)) {
     return items_[ValidateItemIndex(index)].visible;
+  }
 
   return delegate_->IsCommandIdVisible(command_id) &&
          items_[ValidateItemIndex(index)].visible;
@@ -542,13 +556,15 @@ void SimpleMenuModel::InsertItemAtIndex(Item item, int index) {
 }
 
 void SimpleMenuModel::ValidateItem(const Item& item) {
-#ifndef NDEBUG
+#if DCHECK_IS_ON()
   if (item.type == TYPE_SEPARATOR) {
     DCHECK_EQ(item.command_id, kSeparatorId);
+  } else if (item.type == TYPE_TITLE) {
+    DCHECK_EQ(item.command_id, kTitleId);
   } else {
     DCHECK_GE(item.command_id, 0);
   }
-#endif  // NDEBUG
+#endif  // DCHECK_IS_ON()
 }
 
 }  // namespace ui

ui/base/models/simple_menu_model.h

@@ -107,6 +107,7 @@ class UI_BASE_EXPORT SimpleMenuModel : public MenuModel {
   void AddHighlightedItemWithIcon(int command_id,
                                   const base::string16& label,
                                   const gfx::ImageSkia& icon);
+  void AddTitle(const base::string16& label);
 
   // Adds a separator of the specified type to the model.
   // - Adding a separator after another separator is always invalid if they

ui/views/controls/menu/menu_item_view.cc

@@ -200,6 +200,7 @@ void MenuItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
       node_data->SetCheckedState(is_checked ? ax::mojom::CheckedState::kTrue
                                             : ax::mojom::CheckedState::kFalse);
     } break;
+    case TITLE:
     case NORMAL:
     case SEPARATOR:
     case EMPTY:
@@ -1139,7 +1140,9 @@ SkColor MenuItemView::GetTextColor(bool minor, bool render_selection) const {
           : style::CONTEXT_MENU;
 
   style::TextStyle text_style = style::STYLE_PRIMARY;
-  if (type_ == HIGHLIGHTED)
+  if (type_ == Type::TITLE)
+    text_style = style::STYLE_PRIMARY;
+  else if (type_ == HIGHLIGHTED)
     text_style = style::STYLE_HIGHLIGHTED;
   else if (!GetEnabled())
     text_style = style::STYLE_DISABLED;

ui/views/controls/menu/menu_item_view.h

@@ -95,8 +95,9 @@ class VIEWS_EXPORT MenuItemView : public View {
     HIGHLIGHTED,         // Performs an action when selected, and has a
                          // different colored background that merges with the
                          // menu's rounded corners when placed at the bottom.
-    EMPTY,  // EMPTY is a special type for empty menus that is only used
-            // internally.
+    TITLE,               // Title text, does not perform any action.
+    EMPTY,               // EMPTY is a special type for empty menus that is only
+                         // used internally.
   };
 
   // Where the menu should be drawn, above or below the bounds (when

ui/views/controls/menu/menu_model_adapter.cc

@@ -70,6 +70,9 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model,
   base::Optional<MenuItemView::Type> type;
   ui::MenuModel::ItemType menu_type = model->GetTypeAt(model_index);
   switch (menu_type) {
+    case ui::MenuModel::TYPE_TITLE:
+      type = MenuItemView::TITLE;
+      break;
     case ui::MenuModel::TYPE_COMMAND:
     case ui::MenuModel::TYPE_BUTTON_ITEM:
       type = MenuItemView::NORMAL;
@@ -275,7 +278,11 @@ void MenuModelAdapter::BuildMenuImpl(MenuItemView* menu, ui::MenuModel* model) {
   for (int i = 0; i < item_count; ++i) {
     MenuItemView* item = AppendMenuItem(menu, model, i);
     if (item) {
-      item->SetEnabled(model->IsEnabledAt(i));
+      // Enabled state should be ignored for titles as they are non-interactive.
+      if (model->GetTypeAt(i) == ui::MenuModel::TYPE_TITLE)
+        item->SetEnabled(false);
+      else
+        item->SetEnabled(model->IsEnabledAt(i));
       item->SetVisible(model->IsVisibleAt(i));
     }
 

ui/views/controls/menu/menu_model_adapter_unittest.cc

@@ -213,6 +213,9 @@ void CheckSubmenu(const RootModel& model,
 
     // Check type.
     switch (model_item.type) {
+      case ui::MenuModel::TYPE_TITLE:
+        EXPECT_EQ(views::MenuItemView::TITLE, item->GetType());
+        break;
       case ui::MenuModel::TYPE_COMMAND:
         EXPECT_EQ(views::MenuItemView::NORMAL, item->GetType());
         break;
@@ -287,6 +290,9 @@ TEST_F(MenuModelAdapterTest, BasicTest) {
 
     // Check type.
     switch (model_item.type) {
+      case ui::MenuModel::TYPE_TITLE:
+        EXPECT_EQ(views::MenuItemView::TITLE, item->GetType());
+        break;
       case ui::MenuModel::TYPE_COMMAND:
         EXPECT_EQ(views::MenuItemView::NORMAL, item->GetType());
         break;