Cleanup: Remove Menu Subtitles/Sublabels

No one seems to use subtitles or sublabels.

BUG=865318

Change-Id: Ia850adabeab5aa280b2730e2a4177ac7a9f8d0cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764883
Auto-Submit: Robert Liao <robliao@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689998}

chrome/browser/renderer_context_menu/mock_render_view_context_menu.cc

@@ -157,7 +157,7 @@ void MockRenderViewContextMenu::AppendSubMenuItems(ui::MenuModel* model) {
     sub_item.enabled = model->IsEnabledAt(i);
     sub_item.checked = model->IsItemCheckedAt(i);
     sub_item.hidden = false;
-    sub_item.title = model->GetSublabelAt(i);
+    sub_item.title = model->GetLabelAt(i);
     items_.push_back(sub_item);
   }
 }

chrome/browser/status_icons/status_icon_menu_model.cc

@@ -20,7 +20,6 @@ struct StatusIconMenuModel::ItemState {
   bool is_dynamic;
   ui::Accelerator accelerator;
   base::string16 label;
-  base::string16 sublabel;
   gfx::Image icon;
 };
 
@@ -62,13 +61,6 @@ void StatusIconMenuModel::ChangeLabelForCommandId(int command_id,
   NotifyMenuStateChanged();
 }
 
-void StatusIconMenuModel::ChangeSublabelForCommandId(
-    int command_id, const base::string16& sublabel) {
-  item_states_[command_id].is_dynamic = true;
-  item_states_[command_id].sublabel = sublabel;
-  NotifyMenuStateChanged();
-}
-
 void StatusIconMenuModel::ChangeIconForCommandId(
     int command_id, const gfx::Image& icon) {
   item_states_[command_id].is_dynamic = true;
@@ -130,14 +122,6 @@ base::string16 StatusIconMenuModel::GetLabelForCommandId(int command_id) const {
   return base::string16();
 }
 
-base::string16 StatusIconMenuModel::GetSublabelForCommandId(
-    int command_id) const {
-  auto iter = item_states_.find(command_id);
-  if (iter != item_states_.end())
-    return iter->second.sublabel;
-  return base::string16();
-}
-
 bool StatusIconMenuModel::GetIconForCommandId(int command_id,
                                               gfx::Image* image_skia) const {
   auto iter = item_states_.find(command_id);

chrome/browser/status_icons/status_icon_menu_model.h

@@ -61,11 +61,9 @@ class StatusIconMenuModel
 
   // Calling any of these "change" methods will mark the menu item as "dynamic"
   // (see menu_model.h:IsItemDynamicAt) which many platforms take as a cue to
-  // refresh the label, sublabel and icon of the menu item each time the menu is
+  // refresh the label and icon of the menu item each time the menu is
   // shown.
   void ChangeLabelForCommandId(int command_id, const base::string16& label);
-  void ChangeSublabelForCommandId(
-      int command_id, const base::string16& sublabel);
   void ChangeIconForCommandId(int command_id, const gfx::Image& icon);
 
   void AddObserver(Observer* observer);
@@ -79,7 +77,6 @@ class StatusIconMenuModel
                                   ui::Accelerator* accelerator) const override;
   bool IsItemForCommandIdDynamic(int command_id) const override;
   base::string16 GetLabelForCommandId(int command_id) const override;
-  base::string16 GetSublabelForCommandId(int command_id) const override;
   bool GetIconForCommandId(int command_id, gfx::Image* icon) const override;
 
  protected:

chrome/browser/status_icons/status_icon_menu_model_unittest.cc

@@ -90,10 +90,6 @@ TEST_F(StatusIconMenuModelTest, SetProperties) {
   menu_model()->ChangeLabelForCommandId(0, ASCIIToUTF16("label2"));
   EXPECT_EQ(ASCIIToUTF16("label2"), menu_model()->GetLabelForCommandId(0));
 
-  // Set the sublabel and icon image for the second menu item.
-  menu_model()->ChangeSublabelForCommandId(1, ASCIIToUTF16("sublabel"));
-  EXPECT_EQ(ASCIIToUTF16("sublabel"), menu_model()->GetSublabelForCommandId(1));
-
   // Try setting icon image and changing it.
   menu_model()->ChangeIconForCommandId(1, test_image1);
   EXPECT_TRUE(menu_model()->GetIconForCommandId(1, &image_arg));
@@ -102,9 +98,8 @@ TEST_F(StatusIconMenuModelTest, SetProperties) {
   // Ensure changes to one menu item does not affect the other menu item.
   EXPECT_FALSE(menu_model()->GetAcceleratorForCommandId(1, &accel_arg));
   EXPECT_EQ(base::string16(), menu_model()->GetLabelForCommandId(1));
-  EXPECT_EQ(base::string16(), menu_model()->GetSublabelForCommandId(0));
   EXPECT_FALSE(menu_model()->GetIconForCommandId(0, &image_arg));
 
-  // Menu state should have changed 7 times in this test.
-  EXPECT_EQ(7, changed_count());
+  // Menu state should have changed 6 times in this test.
+  EXPECT_EQ(6, changed_count());
 }

chrome/browser/ui/views/menu_item_view_interactive_uitest.cc

@@ -91,8 +91,8 @@ class MenuItemViewTestInsert : public MenuTestBase {
 
     inserted_item_ = menu()->AddMenuItemAt(
         INSERT_INDEX, 1000, ASCIIToUTF16("inserted item"), base::string16(),
-        base::string16(), nullptr, gfx::ImageSkia(), nullptr,
-        views::MenuItemView::NORMAL, ui::NORMAL_SEPARATOR);
+        nullptr, gfx::ImageSkia(), nullptr, views::MenuItemView::NORMAL,
+        ui::NORMAL_SEPARATOR);
     ASSERT_TRUE(inserted_item_);
     menu()->ChildrenChanged();
 
@@ -186,8 +186,8 @@ class MenuItemViewTestInsertWithSubmenu : public MenuTestBase {
   void Step2() {
     inserted_item_ = menu()->AddMenuItemAt(
         INSERT_INDEX, 1000, ASCIIToUTF16("inserted item"), base::string16(),
-        base::string16(), nullptr, gfx::ImageSkia(), nullptr,
-        views::MenuItemView::NORMAL, ui::NORMAL_SEPARATOR);
+        nullptr, gfx::ImageSkia(), nullptr, views::MenuItemView::NORMAL,
+        ui::NORMAL_SEPARATOR);
     ASSERT_TRUE(inserted_item_);
     menu()->ChildrenChanged();
 

chrome/browser/ui/views/status_icons/concat_menu_model.cc

@@ -33,10 +33,6 @@ base::string16 ConcatMenuModel::GetLabelAt(int index) const {
   return GetterImpl(&ui::MenuModel::GetLabelAt, index);
 }
 
-base::string16 ConcatMenuModel::GetSublabelAt(int index) const {
-  return GetterImpl(&ui::MenuModel::GetSublabelAt, index);
-}
-
 base::string16 ConcatMenuModel::GetMinorTextAt(int index) const {
   return GetterImpl(&ui::MenuModel::GetMinorTextAt, index);
 }

chrome/browser/ui/views/status_icons/concat_menu_model.h

@@ -23,7 +23,6 @@ class ConcatMenuModel : public ui::MenuModel {
   ui::MenuSeparatorType GetSeparatorTypeAt(int index) const override;
   int GetCommandIdAt(int index) const override;
   base::string16 GetLabelAt(int index) const override;
-  base::string16 GetSublabelAt(int index) const override;
   base::string16 GetMinorTextAt(int index) const override;
   const gfx::VectorIcon* GetMinorIconAt(int index) const override;
   bool IsItemDynamicAt(int index) const override;

ui/base/models/menu_model.cc

@@ -46,10 +46,6 @@ bool MenuModel::GetModelAndIndexForCommandId(int command_id,
   return false;
 }
 
-base::string16 MenuModel::GetSublabelAt(int index) const {
-  return base::string16();
-}
-
 base::string16 MenuModel::GetMinorTextAt(int index) const {
   return base::string16();
 }

ui/base/models/menu_model.h

@@ -64,10 +64,6 @@ class UI_BASE_EXPORT MenuModel {
   // Returns the label of the item at the specified index.
   virtual base::string16 GetLabelAt(int index) const = 0;
 
-  // Returns the sublabel of the item at the specified index. The sublabel
-  // is rendered beneath the label and using the font GetLabelFontAt().
-  virtual base::string16 GetSublabelAt(int index) const;
-
   // Returns the minor text of the item at the specified index. The minor text
   // is rendered to the right of the label and using the font GetLabelFontAt().
   virtual base::string16 GetMinorTextAt(int index) const;

ui/base/models/simple_menu_model.cc

@@ -42,11 +42,6 @@ base::string16 SimpleMenuModel::Delegate::GetLabelForCommandId(
   return base::string16();
 }
 
-base::string16 SimpleMenuModel::Delegate::GetSublabelForCommandId(
-    int command_id) const {
-  return base::string16();
-}
-
 base::string16 SimpleMenuModel::Delegate::GetMinorTextForCommandId(
     int command_id) const {
   return base::string16();
@@ -329,11 +324,6 @@ void SimpleMenuModel::SetLabel(int index, const base::string16& label) {
   MenuItemsChanged();
 }
 
-void SimpleMenuModel::SetSublabel(int index, const base::string16& sublabel) {
-  items_[ValidateItemIndex(index)].sublabel = sublabel;
-  MenuItemsChanged();
-}
-
 void SimpleMenuModel::SetMinorText(int index,
                                    const base::string16& minor_text) {
   items_[ValidateItemIndex(index)].minor_text = minor_text;
@@ -408,12 +398,6 @@ base::string16 SimpleMenuModel::GetLabelAt(int index) const {
   return items_[ValidateItemIndex(index)].label;
 }
 
-base::string16 SimpleMenuModel::GetSublabelAt(int index) const {
-  if (IsItemDynamicAt(index))
-    return delegate_->GetSublabelForCommandId(GetCommandIdAt(index));
-  return items_[ValidateItemIndex(index)].sublabel;
-}
-
 base::string16 SimpleMenuModel::GetMinorTextAt(int index) const {
   if (IsItemDynamicAt(index))
     return delegate_->GetMinorTextForCommandId(GetCommandIdAt(index));

ui/base/models/simple_menu_model.h

@@ -44,11 +44,10 @@ class UI_BASE_EXPORT SimpleMenuModel : public MenuModel {
     // Delegate should return true if |command_id| should be visible.
     virtual bool IsCommandIdVisible(int command_id) const;
 
-    // Some command ids have labels, sublabels, minor text and icons that change
-    // over time.
+    // Some command ids have labels, minor text and icons that change over
+    // time.
     virtual bool IsItemForCommandIdDynamic(int command_id) const;
     virtual base::string16 GetLabelForCommandId(int command_id) const;
-    virtual base::string16 GetSublabelForCommandId(int command_id) const;
     virtual base::string16 GetMinorTextForCommandId(int command_id) const;
     // Gets the icon for the item with the specified id, returning true if there
     // is an icon, false otherwise.
@@ -176,9 +175,6 @@ class UI_BASE_EXPORT SimpleMenuModel : public MenuModel {
   // Sets the label for the item at |index|.
   void SetLabel(int index, const base::string16& label);
 
-  // Sets the sublabel for the item at |index|.
-  void SetSublabel(int index, const base::string16& sublabel);
-
   // Sets the minor text for the item at |index|.
   void SetMinorText(int index, const base::string16& minor_text);
 
@@ -205,7 +201,6 @@ class UI_BASE_EXPORT SimpleMenuModel : public MenuModel {
   ui::MenuSeparatorType GetSeparatorTypeAt(int index) const override;
   int GetCommandIdAt(int index) const override;
   base::string16 GetLabelAt(int index) const override;
-  base::string16 GetSublabelAt(int index) const override;
   base::string16 GetMinorTextAt(int index) const override;
   const gfx::VectorIcon* GetMinorIconAt(int index) const override;
   bool IsItemDynamicAt(int index) const override;
@@ -241,7 +236,6 @@ class UI_BASE_EXPORT SimpleMenuModel : public MenuModel {
     int command_id = 0;
     ItemType type = TYPE_COMMAND;
     base::string16 label;
-    base::string16 sublabel;
     base::string16 minor_text;
     const gfx::VectorIcon* minor_icon = nullptr;
     gfx::Image icon;

ui/views/controls/menu/menu_item_view.cc

@@ -275,7 +275,6 @@ MenuItemView* MenuItemView::AddMenuItemAt(
     int index,
     int item_id,
     const base::string16& label,
-    const base::string16& sublabel,
     const base::string16& minor_text,
     const gfx::VectorIcon* minor_icon,
     const gfx::ImageSkia& icon,
@@ -296,7 +295,6 @@ MenuItemView* MenuItemView::AddMenuItemAt(
     item->SetTitle(GetDelegate()->GetLabel(item_id));
   else
     item->SetTitle(label);
-  item->SetSubtitle(sublabel);
   item->SetMinorText(minor_text);
   item->SetMinorIcon(minor_icon);
   if (vector_icon) {
@@ -338,23 +336,21 @@ void MenuItemView::RemoveAllMenuItems() {
 MenuItemView* MenuItemView::AppendMenuItem(int item_id,
                                            const base::string16& label,
                                            Type type) {
-  return AppendMenuItemImpl(item_id, label, base::string16(), base::string16(),
-                            nullptr, gfx::ImageSkia(), type,
-                            ui::NORMAL_SEPARATOR);
+  return AppendMenuItemImpl(item_id, label, base::string16(), nullptr,
+                            gfx::ImageSkia(), type, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendSubMenu(int item_id,
                                           const base::string16& label) {
-  return AppendMenuItemImpl(item_id, label, base::string16(), base::string16(),
-                            nullptr, gfx::ImageSkia(), SUBMENU,
-                            ui::NORMAL_SEPARATOR);
+  return AppendMenuItemImpl(item_id, label, base::string16(), nullptr,
+                            gfx::ImageSkia(), SUBMENU, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendSubMenuWithIcon(int item_id,
                                                   const base::string16& label,
                                                   const gfx::ImageSkia& icon) {
-  return AppendMenuItemImpl(item_id, label, base::string16(), base::string16(),
-                            nullptr, icon, SUBMENU, ui::NORMAL_SEPARATOR);
+  return AppendMenuItemImpl(item_id, label, base::string16(), nullptr, icon,
+                            SUBMENU, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendMenuItemWithLabel(
@@ -368,14 +364,12 @@ MenuItemView* MenuItemView::AppendDelegateMenuItem(int item_id) {
 }
 
 void MenuItemView::AppendSeparator() {
-  AppendMenuItemImpl(0, base::string16(), base::string16(), base::string16(),
-                     nullptr, gfx::ImageSkia(), SEPARATOR,
-                     ui::NORMAL_SEPARATOR);
+  AppendMenuItemImpl(0, base::string16(), base::string16(), nullptr,
+                     gfx::ImageSkia(), SEPARATOR, ui::NORMAL_SEPARATOR);
 }
 
 void MenuItemView::AddSeparatorAt(int index) {
   AddMenuItemAt(index, /*item_id=*/0, /*label=*/base::string16(),
-                /*sub_label=*/base::string16(),
                 /*minor_text=*/base::string16(), /*minor_icon=*/nullptr,
                 /*icon=*/gfx::ImageSkia(), /*vector_icon=*/nullptr,
                 /*type=*/SEPARATOR,
@@ -385,22 +379,21 @@ void MenuItemView::AddSeparatorAt(int index) {
 MenuItemView* MenuItemView::AppendMenuItemWithIcon(int item_id,
                                                    const base::string16& label,
                                                    const gfx::ImageSkia& icon) {
-  return AppendMenuItemImpl(item_id, label, base::string16(), base::string16(),
-                            nullptr, icon, NORMAL, ui::NORMAL_SEPARATOR);
+  return AppendMenuItemImpl(item_id, label, base::string16(), nullptr, icon,
+                            NORMAL, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendMenuItemImpl(
     int item_id,
     const base::string16& label,
-    const base::string16& sublabel,
     const base::string16& minor_text,
     const gfx::VectorIcon* minor_icon,
     const gfx::ImageSkia& icon,
     Type type,
     ui::MenuSeparatorType separator_style) {
   const int index = submenu_ ? int{submenu_->children().size()} : 0;
-  return AddMenuItemAt(index, item_id, label, sublabel, minor_text, minor_icon,
-                       icon, nullptr, type, separator_style);
+  return AddMenuItemAt(index, item_id, label, minor_text, minor_icon, icon,
+                       nullptr, type, separator_style);
 }
 
 SubmenuView* MenuItemView::CreateSubmenu() {
@@ -432,11 +425,6 @@ void MenuItemView::SetTitle(const base::string16& title) {
   invalidate_dimensions();  // Triggers preferred size recalculation.
 }
 
-void MenuItemView::SetSubtitle(const base::string16& subtitle) {
-  subtitle_ = subtitle;
-  invalidate_dimensions();  // Triggers preferred size recalculation.
-}
-
 void MenuItemView::SetMinorText(const base::string16& minor_text) {
   minor_text_ = minor_text;
   invalidate_dimensions();  // Triggers preferred size recalculation.
@@ -1026,23 +1014,13 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) {
       (!delegate ||
        delegate->ShouldReserveSpaceForSubmenuIndicator() ?
            item_right_margin_ : config.arrow_to_edge_padding);
-  gfx::Rect text_bounds(label_start, top_margin, width,
-                        subtitle_.empty() ? available_height
-                                          : available_height / 2);
+  gfx::Rect text_bounds(label_start, top_margin, width, available_height);
   text_bounds.set_x(GetMirroredXForRect(text_bounds));
   int flags = GetDrawStringFlags();
   if (mode == PB_FOR_DRAG)
     flags |= gfx::Canvas::NO_SUBPIXEL_RENDERING;
   canvas->DrawStringRectWithFlags(title(), style.font_list, style.foreground,
                                   text_bounds, flags);
-  if (!subtitle_.empty()) {
-    canvas->DrawStringRectWithFlags(
-        subtitle_, style.font_list,
-        GetNativeTheme()->GetSystemColor(
-            ui::NativeTheme::kColorId_MenuItemMinorTextColor),
-        text_bounds + gfx::Vector2d(0, style.font_list.GetHeight()), flags);
-  }
-
   PaintMinorIconAndText(canvas, style);
 
   // Set the submenu indicator (arrow) image and color.
@@ -1295,11 +1273,6 @@ MenuItemView::MenuItemDimensions MenuItemView::CalculateDimensions() const {
 
   // Determine the length of the label text.
   int string_width = gfx::GetStringWidth(title_, style.font_list);
-  if (!subtitle_.empty()) {
-    string_width =
-        std::max(string_width, gfx::GetStringWidth(subtitle_, style.font_list));
-  }
-
   dimensions.standard_width = string_width + label_start +
       item_right_margin_;
   // Determine the length of the right-side text.
@@ -1307,10 +1280,9 @@ MenuItemView::MenuItemDimensions MenuItemView::CalculateDimensions() const {
       minor_text.empty() ? 0 : gfx::GetStringWidth(minor_text, style.font_list);
 
   // Determine the height to use.
-  dimensions.height = std::max(
-      dimensions.height, (subtitle_.empty() ? 0 : style.font_list.GetHeight()) +
-                             style.font_list.GetHeight() + GetBottomMargin() +
-                             GetTopMargin());
+  dimensions.height =
+      std::max(dimensions.height, style.font_list.GetHeight() +
+                                      GetBottomMargin() + GetTopMargin());
   dimensions.height =
       std::max(dimensions.height, MenuConfig::instance().item_min_height);
 

ui/views/controls/menu/menu_item_view.h

@@ -151,7 +151,6 @@ class VIEWS_EXPORT MenuItemView : public View {
   MenuItemView* AddMenuItemAt(int index,
                               int item_id,
                               const base::string16& label,
-                              const base::string16& sublabel,
                               const base::string16& minor_text,
                               const gfx::VectorIcon* minor_icon,
                               const gfx::ImageSkia& icon,
@@ -215,7 +214,6 @@ class VIEWS_EXPORT MenuItemView : public View {
   // All the AppendXXX methods funnel into this.
   MenuItemView* AppendMenuItemImpl(int item_id,
                                    const base::string16& label,
-                                   const base::string16& sublabel,
                                    const base::string16& minor_text,
                                    const gfx::VectorIcon* minor_icon,
                                    const gfx::ImageSkia& icon,
@@ -243,9 +241,6 @@ class VIEWS_EXPORT MenuItemView : public View {
   void SetTitle(const base::string16& title);
   const base::string16& title() const { return title_; }
 
-  // Sets the subtitle.
-  void SetSubtitle(const base::string16& subtitle);
-
   // Sets the minor text.
   void SetMinorText(const base::string16& minor_text);
 
@@ -456,7 +451,7 @@ class VIEWS_EXPORT MenuItemView : public View {
   void DestroyAllMenuHosts();
 
   // Returns the text that should be displayed on the end (right) of the menu
-  // item. This will be the accelerator (if one exists), otherwise |subtitle_|.
+  // item. This will be the accelerator (if one exists).
   base::string16 GetMinorText() const;
 
   // Returns the icon that should be displayed to the left of the minor text.
@@ -547,9 +542,6 @@ class VIEWS_EXPORT MenuItemView : public View {
   // Title.
   base::string16 title_;
 
-  // Subtitle/sublabel.
-  base::string16 subtitle_;
-
   // Minor text.
   base::string16 minor_text_;
 

ui/views/controls/menu/menu_item_view_unittest.cc

@@ -322,10 +322,10 @@ class MenuItemViewPaintUnitTest : public ViewsTestBase {
 // Provides assertion coverage for painting minor text and icons.
 TEST_F(MenuItemViewPaintUnitTest, MinorTextAndIconAssertionCoverage) {
   auto AddItem = [this](auto label, auto minor_label, auto minor_icon) {
-    menu_item_view()->AddMenuItemAt(
-        0, 1000, base::ASCIIToUTF16(label), base::string16(), minor_label,
-        minor_icon, gfx::ImageSkia(), nullptr, views::MenuItemView::NORMAL,
-        ui::NORMAL_SEPARATOR);
+    menu_item_view()->AddMenuItemAt(0, 1000, base::ASCIIToUTF16(label),
+                                    minor_label, minor_icon, gfx::ImageSkia(),
+                                    nullptr, views::MenuItemView::NORMAL,
+                                    ui::NORMAL_SEPARATOR);
   };
   AddItem("No minor content", base::string16(), nullptr);
   AddItem("Minor text only", base::ASCIIToUTF16("minor text"), nullptr);

ui/views/controls/menu/menu_model_adapter.cc

@@ -96,8 +96,8 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model,
 
   if (*type == MenuItemView::SEPARATOR) {
     return menu->AddMenuItemAt(menu_index, item_id, base::string16(),
-                               base::string16(), base::string16(), nullptr,
-                               gfx::ImageSkia(), nullptr, *type,
+                               base::string16(), nullptr, gfx::ImageSkia(),
+                               nullptr, *type,
                                model->GetSeparatorTypeAt(model_index));
   }
 
@@ -105,8 +105,7 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model,
   model->GetIconAt(model_index, &icon);
   return menu->AddMenuItemAt(
       menu_index, item_id, model->GetLabelAt(model_index),
-      model->GetSublabelAt(model_index), model->GetMinorTextAt(model_index),
-      model->GetMinorIconAt(model_index),
+      model->GetMinorTextAt(model_index), model->GetMinorIconAt(model_index),
       icon.IsEmpty() ? gfx::ImageSkia() : *icon.ToImageSkia(),
       icon.IsEmpty() ? model->GetVectorIconAt(model_index) : nullptr, *type,
       ui::NORMAL_SEPARATOR);