Removed unnecessary MenuItemView virtuals and other cleanup.

Bug: 648382
Change-Id: Icfb51ac00ad10173626866058d78f31515ec3b1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2119793Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753617}

ui/views/controls/menu/menu_item_view.cc

@@ -101,29 +101,10 @@ int MenuItemView::item_right_margin_;
 // static
 int MenuItemView::pref_menu_height_;
 
-MenuItemView::MenuItemView(MenuDelegate* delegate)
-    : delegate_(delegate),
-      controller_(nullptr),
-      canceled_(false),
-      parent_menu_item_(nullptr),
-      type_(Type::kSubMenu),
-      selected_(false),
-      command_(0),
-      submenu_(nullptr),
-      has_mnemonics_(false),
-      show_mnemonics_(false),
-      has_icons_(false),
-      icon_view_(nullptr),
-      top_margin_(-1),
-      bottom_margin_(-1),
-      left_icon_margin_(0),
-      right_icon_margin_(0),
-      requested_menu_position_(MenuPosition::kBestFit),
-      actual_menu_position_(requested_menu_position_),
-      use_right_margin_(true) {
+MenuItemView::MenuItemView(MenuDelegate* delegate) : delegate_(delegate) {
   // NOTE: don't check the delegate for NULL, UpdateMenuPartSizes() supplies a
   // NULL delegate.
-  Init(nullptr, 0, Type::kSubMenu, delegate);
+  Init(nullptr, 0, Type::kSubMenu);
 }
 
 void MenuItemView::ChildPreferredSizeChanged(View* child) {
@@ -291,7 +272,8 @@ MenuItemView* MenuItemView::AddMenuItemAt(
     CreateSubmenu();
   DCHECK_LE(size_t{index}, submenu_->children().size());
   if (type == Type::kSeparator) {
-    submenu_->AddChildViewAt(new MenuSeparator(separator_style), index);
+    submenu_->AddChildViewAt(std::make_unique<MenuSeparator>(separator_style),
+                             index);
     return nullptr;
   }
   MenuItemView* item = new MenuItemView(this, item_id, type);
@@ -316,8 +298,7 @@ MenuItemView* MenuItemView::AddMenuItemAt(
   }
   if (GetDelegate() && !GetDelegate()->IsCommandVisible(item_id))
     item->SetVisible(false);
-  submenu_->AddChildViewAt(item, index);
-  return item;
+  return submenu_->AddChildViewAt(item, index);
 }
 
 void MenuItemView::RemoveMenuItem(View* item) {
@@ -375,8 +356,7 @@ SubmenuView* MenuItemView::CreateSubmenu() {
     submenu_ = new SubmenuView(this);
 
     // Initialize the submenu indicator icon (arrow).
-    submenu_arrow_image_view_ = new ImageView();
-    AddChildView(submenu_arrow_image_view_);
+    submenu_arrow_image_view_ = AddChildView(std::make_unique<ImageView>());
   }
 
   return submenu_;
@@ -728,27 +708,8 @@ void MenuItemView::SetAlerted() {
 
 MenuItemView::MenuItemView(MenuItemView* parent,
                            int command,
-                           MenuItemView::Type type)
-    : delegate_(nullptr),
-      controller_(nullptr),
-      canceled_(false),
-      parent_menu_item_(parent),
-      type_(type),
-      selected_(false),
-      command_(command),
-      submenu_(nullptr),
-      has_mnemonics_(false),
-      show_mnemonics_(false),
-      has_icons_(false),
-      icon_view_(nullptr),
-      top_margin_(-1),
-      bottom_margin_(-1),
-      left_icon_margin_(0),
-      right_icon_margin_(0),
-      requested_menu_position_(MenuPosition::kBestFit),
-      actual_menu_position_(requested_menu_position_),
-      use_right_margin_(true) {
-  Init(parent, command, type, nullptr);
+                           MenuItemView::Type type) {
+  Init(parent, command, type);
 }
 
 MenuItemView::~MenuItemView() {
@@ -799,37 +760,25 @@ void MenuItemView::UpdateMenuPartSizes() {
 
 void MenuItemView::Init(MenuItemView* parent,
                         int command,
-                        MenuItemView::Type type,
-                        MenuDelegate* delegate) {
-  delegate_ = delegate;
-  controller_ = nullptr;
-  canceled_ = false;
+                        MenuItemView::Type type) {
   parent_menu_item_ = parent;
   type_ = type;
-  selected_ = false;
   command_ = command;
-  submenu_ = nullptr;
-  radio_check_image_view_ = nullptr;
-  submenu_arrow_image_view_ = nullptr;
-  vertical_separator_ = nullptr;
-  show_mnemonics_ = false;
-  corner_radius_ = 0;
   // Assign our ID, this allows SubmenuItemView to find MenuItemViews.
   SetID(kMenuItemViewID);
   has_icons_ = false;
 
   if (type_ == Type::kCheckbox || type_ == Type::kRadio) {
-    radio_check_image_view_ = new ImageView();
+    radio_check_image_view_ = AddChildView(std::make_unique<ImageView>());
     bool show_check_radio_icon =
         type_ == Type::kRadio || (type_ == Type::kCheckbox &&
                                   GetDelegate()->IsItemChecked(GetCommand()));
     radio_check_image_view_->SetVisible(show_check_radio_icon);
     radio_check_image_view_->set_can_process_events_within_subtree(false);
-    AddChildView(radio_check_image_view_);
   }
 
   if (type_ == Type::kActionableSubMenu) {
-    vertical_separator_ = new Separator();
+    vertical_separator_ = AddChildView(std::make_unique<Separator>());
     vertical_separator_->SetVisible(true);
     vertical_separator_->SetFocusBehavior(FocusBehavior::NEVER);
     const MenuConfig& config = MenuConfig::instance();
@@ -839,7 +788,6 @@ void MenuItemView::Init(MenuItemView* parent,
         gfx::Size(config.actionable_submenu_vertical_separator_width,
                   config.actionable_submenu_vertical_separator_height));
     vertical_separator_->set_can_process_events_within_subtree(false);
-    AddChildView(vertical_separator_);
   }
 
   if (submenu_arrow_image_view_)
@@ -912,7 +860,7 @@ void MenuItemView::GetLabelStyle(MenuDelegate::LabelStyle* style) const {
 void MenuItemView::AddEmptyMenus() {
   DCHECK(HasSubmenu());
   if (!submenu_->HasVisibleChildren() && !submenu_->HasEmptyMenuItemView()) {
-    submenu_->AddChildViewAt(new EmptyMenuMenuItem(this), 0);
+    submenu_->AddChildViewAt(std::make_unique<EmptyMenuMenuItem>(this), 0);
   } else {
     for (MenuItemView* item : submenu_->GetMenuItems()) {
       if (item->HasSubmenu())

ui/views/controls/menu/menu_item_view.h

@@ -199,17 +199,17 @@ class VIEWS_EXPORT MenuItemView : public View {
                                    Type type);
 
   // Returns the view that contains child menu items. If the submenu has
-  // not been creates, this creates it.
-  virtual SubmenuView* CreateSubmenu();
+  // not been created, this creates it.
+  SubmenuView* CreateSubmenu();
 
   // Returns true if this menu item has a submenu.
-  virtual bool HasSubmenu() const;
+  bool HasSubmenu() const;
 
   // Returns the view containing child menu items.
-  virtual SubmenuView* GetSubmenu() const;
+  SubmenuView* GetSubmenu() const;
 
   // Returns true if this menu item has a submenu and it is showing
-  virtual bool SubmenuIsShowing() const;
+  bool SubmenuIsShowing() const;
 
   // Returns the parent menu item.
   MenuItemView* GetParentMenuItem() { return parent_menu_item_; }
@@ -379,10 +379,7 @@ class VIEWS_EXPORT MenuItemView : public View {
   void UpdateMenuPartSizes();
 
   // Called by the two constructors to initialize this menu item.
-  void Init(MenuItemView* parent,
-            int command,
-            MenuItemView::Type type,
-            MenuDelegate* delegate);
+  void Init(MenuItemView* parent, int command, MenuItemView::Type type);
 
   // The RunXXX methods call into this to set up the necessary state before
   // running. |is_first_menu| is true if no menus are currently showing.
@@ -488,32 +485,32 @@ class VIEWS_EXPORT MenuItemView : public View {
   // The delegate. This is only valid for the root menu item. You shouldn't
   // use this directly, instead use GetDelegate() which walks the tree as
   // as necessary.
-  MenuDelegate* delegate_;
+  MenuDelegate* delegate_ = nullptr;
 
   // The controller for the run operation, or NULL if the menu isn't showing.
   base::WeakPtr<MenuController> controller_;
 
   // Used to detect when Cancel was invoked.
-  bool canceled_;
+  bool canceled_ = false;
 
   // Our parent.
-  MenuItemView* parent_menu_item_;
+  MenuItemView* parent_menu_item_ = nullptr;
 
   // Type of menu. NOTE: MenuItemView doesn't itself represent SEPARATOR,
   // that is handled by an entirely different view class.
-  Type type_;
+  Type type_ = Type::kSubMenu;
 
   // Whether we're selected.
-  bool selected_;
+  bool selected_ = false;
 
   // Whether the submenu area of an ACTIONABLE_SUBMENU is selected.
-  bool submenu_area_of_actionable_submenu_selected_;
+  bool submenu_area_of_actionable_submenu_selected_ = false;
 
   // Command id.
-  int command_;
+  int command_ = 0;
 
   // Submenu, created via CreateSubmenu.
-  SubmenuView* submenu_;
+  SubmenuView* submenu_ = nullptr;
 
   // Title.
   base::string16 title_;
@@ -529,17 +526,17 @@ class VIEWS_EXPORT MenuItemView : public View {
   const gfx::VectorIcon* vector_icon_ = nullptr;
 
   // Does the title have a mnemonic? Only useful on the root menu item.
-  bool has_mnemonics_;
+  bool has_mnemonics_ = false;
 
   // Should we show the mnemonic? Mnemonics are shown if this is true or
   // MenuConfig says mnemonics should be shown. Only used on the root menu item.
-  bool show_mnemonics_;
+  bool show_mnemonics_ = false;
 
   // Set if menu has icons or icon_views (applies to root menu item only).
-  bool has_icons_;
+  bool has_icons_ = false;
 
   // Pointer to a view with a menu icon.
-  ImageView* icon_view_;
+  ImageView* icon_view_ = nullptr;
 
   // The tooltip to show on hover for this menu item.
   base::string16 tooltip_;
@@ -564,39 +561,39 @@ class VIEWS_EXPORT MenuItemView : public View {
   std::vector<View*> removed_items_;
 
   // Margins in pixels.
-  int top_margin_;
-  int bottom_margin_;
+  int top_margin_ = -1;
+  int bottom_margin_ = -1;
 
   // Corner radius in pixels, for HIGHLIGHTED items placed at the end of a menu.
-  int corner_radius_;
+  int corner_radius_ = 0;
 
   // Horizontal icon margins in pixels, which can differ between MenuItems.
   // These values will be set in the layout process.
-  mutable int left_icon_margin_;
-  mutable int right_icon_margin_;
+  mutable int left_icon_margin_ = 0;
+  mutable int right_icon_margin_ = 0;
 
   // |menu_position_| is the requested position with respect to the bounds.
   // |actual_menu_position_| is used by the controller to cache the
   // position of the menu being shown.
-  MenuPosition requested_menu_position_;
-  MenuPosition actual_menu_position_;
+  MenuPosition requested_menu_position_ = MenuPosition::kBestFit;
+  MenuPosition actual_menu_position_ = MenuPosition::kBestFit;
 
   // If set to false, the right margin will be removed for menu lines
   // containing other elements.
-  bool use_right_margin_;
+  bool use_right_margin_ = true;
 
   // Contains an image for the checkbox or radio icon.
-  ImageView* radio_check_image_view_;
+  ImageView* radio_check_image_view_ = nullptr;
 
   // The submenu indicator arrow icon in case the menu item has a Submenu.
-  ImageView* submenu_arrow_image_view_;
+  ImageView* submenu_arrow_image_view_ = nullptr;
 
   // The forced visual selection state of this item, if any.
   base::Optional<bool> forced_visual_selection_;
 
   // The vertical separator that separates the actionable and submenu regions of
   // an ACTIONABLE_SUBMENU.
-  Separator* vertical_separator_;
+  Separator* vertical_separator_ = nullptr;
 
   // Whether this menu item is rendered differently to draw attention to it.
   bool is_alerted_ = false;