Create factory methods for instantiating bubble-menu items.

Bug: 901183
Change-Id: Iadae6a197d4e2842b285d4ea7ce7e632a771bd08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867179
Commit-Queue: Charlene Yan <cyan@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708868}

chrome/browser/ui/BUILD.gn

@@ -2661,6 +2661,8 @@ jumbo_static_library("ui") {
       "views/browser_dialogs_views.cc",
       "views/bubble_anchor_util_views.cc",
       "views/bubble_anchor_util_views.h",
+      "views/bubble_menu_item_factory.cc",
+      "views/bubble_menu_item_factory.h",
       "views/bulleted_label_list_view.cc",
       "views/bulleted_label_list_view.h",
       "views/certificate_selector.cc",

chrome/browser/ui/views/bubble_menu_item_factory.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/bubble_menu_item_factory.h"
+
+#include "chrome/browser/ui/views/hover_button.h"
+#include "chrome/browser/ui/views/hover_button_controller.h"
+#include "ui/views/animation/ink_drop_host_view.h"
+#include "ui/views/border.h"
+#include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/button/image_button_factory.h"
+#include "ui/views/style/typography.h"
+#include "ui/views/view.h"
+
+namespace {
+constexpr gfx::Insets kDefaultBorderInsets = gfx::Insets(12);
+
+void ConfigureBubbleMenuItem(views::Button* button, int button_id) {
+  // Items within a menu should not show focus rings.
+  button->SetInstallFocusRingOnFocus(false);
+  button->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
+  button->set_ink_drop_base_color(HoverButton::GetInkDropColor(button));
+  button->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+  button->SetID(button_id);
+}
+
+}  // namespace
+
+std::unique_ptr<views::LabelButton> CreateBubbleMenuItem(
+    int button_id,
+    const base::string16& name,
+    views::ButtonListener* listener) {
+  auto button = std::make_unique<views::LabelButton>(
+      listener, name, views::style::CONTEXT_BUTTON);
+
+  ConfigureBubbleMenuItem(button.get(), button_id);
+
+  button->SetButtonController(std::make_unique<HoverButtonController>(
+      button.get(), listener,
+      std::make_unique<views::Button::DefaultButtonControllerDelegate>(
+          button.get())));
+  button->SetBorder(views::CreateEmptyBorder(kDefaultBorderInsets));
+
+  return button;
+}
+
+std::unique_ptr<views::ImageButton> CreateBubbleMenuItem(
+    int button_id,
+    views::ButtonListener* listener) {
+  auto button = views::CreateVectorImageButton(listener);
+  ConfigureBubbleMenuItem(button.get(), button_id);
+  return button;
+}

chrome/browser/ui/views/bubble_menu_item_factory.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_BUBBLE_MENU_ITEM_FACTORY_H_
+#define CHROME_BROWSER_UI_VIEWS_BUBBLE_MENU_ITEM_FACTORY_H_
+
+#include <string>
+
+#include "base/strings/string16.h"
+
+namespace views {
+class ButtonListener;
+class ImageButton;
+class LabelButton;
+}  // namespace views
+
+// Convience method for creating a menu item used inside a bubble that can then
+// be futher configured to hold an image and text.
+std::unique_ptr<views::LabelButton> CreateBubbleMenuItem(
+    int button_id,
+    const base::string16& name,
+    views::ButtonListener* listener);
+
+// Convience method for creating a menu item used inside a bubble with an image.
+std::unique_ptr<views::ImageButton> CreateBubbleMenuItem(
+    int id,
+    views::ButtonListener* listener);
+
+#endif  // CHROME_BROWSER_UI_VIEWS_BUBBLE_MENU_ITEM_FACTORY_H_

chrome/browser/ui/views/extensions/extensions_menu_item_view.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
+#include "chrome/browser/ui/views/bubble_menu_item_factory.h"
 #include "chrome/browser/ui/views/extensions/extension_context_menu_controller.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_button.h"
 #include "chrome/grit/generated_resources.h"
@@ -25,18 +26,6 @@
 namespace {
 constexpr int EXTENSION_CONTEXT_MENU = 13;
 constexpr int EXTENSION_PINNING = 14;
-
-std::unique_ptr<views::ImageButton> CreateButton(
-    int id,
-    views::ButtonListener* listener) {
-  auto button = views::CreateVectorImageButton(listener);
-
-  // Items within a menu should not show focus rings.
-  button->SetInstallFocusRingOnFocus(false);
-  button->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
-  button->SetID(id);
-  return button;
-}
 }  // namespace
 
 ExtensionsMenuItemView::ExtensionsMenuItemView(
@@ -68,13 +57,14 @@ ExtensionsMenuItemView::ExtensionsMenuItemView(
       ui::NativeTheme::GetInstanceForNativeUi()->GetSystemColor(
           ui::NativeTheme::kColorId_DefaultIconColor);
 
-  auto pin_button = CreateButton(EXTENSION_PINNING, this);
+  auto pin_button = CreateBubbleMenuItem(EXTENSION_PINNING, this);
   pin_button->set_ink_drop_base_color(icon_color);
 
   pin_button_ = pin_button.get();
   AddChildView(std::move(pin_button));
 
-  auto context_menu_button = CreateButton(EXTENSION_CONTEXT_MENU, nullptr);
+  auto context_menu_button =
+      CreateBubbleMenuItem(EXTENSION_CONTEXT_MENU, nullptr);
   views::SetImageFromVectorIcon(context_menu_button.get(), kBrowserToolsIcon,
                                 kSecondaryIconSizeDp, icon_color);
   context_menu_button->SetTooltipText(

chrome/browser/ui/views/extensions/extensions_menu_view.cc

@@ -9,19 +9,16 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
+#include "chrome/browser/ui/views/bubble_menu_item_factory.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/chrome_typography.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_item_view.h"
-#include "chrome/browser/ui/views/hover_button.h"
-#include "chrome/browser/ui/views/hover_button_controller.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/vector_icons/vector_icons.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/animation/ink_drop_host_view.h"
-#include "ui/views/controls/button/image_button.h"
-#include "ui/views/controls/button/image_button_factory.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/scroll_view.h"
@@ -108,29 +105,16 @@ void ExtensionsMenuView::Repopulate() {
 
   AddChildView(std::make_unique<views::Separator>());
 
-  constexpr gfx::Insets kDefaultBorderInsets = gfx::Insets(12);
-  auto footer = std::make_unique<views::LabelButton>(
-      &button_listener_, l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION),
-      views::style::CONTEXT_BUTTON);
-  footer->SetButtonController(std::make_unique<HoverButtonController>(
-      footer.get(), &button_listener_,
-      std::make_unique<views::Button::DefaultButtonControllerDelegate>(
-          footer.get())));
+  auto footer = CreateBubbleMenuItem(
+      EXTENSIONS_SETTINGS_ID, l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION),
+      &button_listener_);
   footer->SetImage(
       views::Button::STATE_NORMAL,
       gfx::CreateVectorIcon(vector_icons::kSettingsIcon, 16,
                             GetNativeTheme()->GetSystemColor(
                                 ui::NativeTheme::kColorId_DefaultIconColor)));
-
-  // Items within a menu should not show focus rings.
-  footer->SetInstallFocusRingOnFocus(false);
-  footer->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
-  footer->SetBorder(views::CreateEmptyBorder(kDefaultBorderInsets));
-  footer->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
-  footer->set_ink_drop_base_color(HoverButton::GetInkDropColor(footer.get()));
   footer->SetImageLabelSpacing(ChromeLayoutProvider::Get()->GetDistanceMetric(
       views::DISTANCE_BUTTON_HORIZONTAL_PADDING));
-  footer->SetID(EXTENSIONS_SETTINGS_ID);
   manage_extensions_button_for_testing_ = footer.get();
   AddChildView(std::move(footer));
 }

chrome/browser/ui/views/tabs/tab_group_editor_bubble_view.cc

@@ -16,10 +16,9 @@
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/tabs/tab_group_visual_data.h"
+#include "chrome/browser/ui/views/bubble_menu_item_factory.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/chrome_typography.h"
-#include "chrome/browser/ui/views/hover_button.h"
-#include "chrome/browser/ui/views/hover_button_controller.h"
 #include "chrome/browser/ui/views/tabs/color_picker_view.h"
 #include "chrome/browser/ui/views/tabs/tab_controller.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
@@ -43,8 +42,6 @@ constexpr int TAB_GROUP_HEADER_CXMENU_NEW_TAB_IN_GROUP = 13;
 constexpr int TAB_GROUP_HEADER_CXMENU_UNGROUP = 14;
 constexpr int TAB_GROUP_HEADER_CXMENU_CLOSE_GROUP = 15;
 
-constexpr gfx::Insets kDefaultBorderInsets = gfx::Insets(12);
-
 // Returns our hard-coded set of colors.
 const std::vector<std::pair<SkColor, base::string16>>& GetColorPickerList() {
   static const base::NoDestructor<
@@ -59,28 +56,6 @@ const std::vector<std::pair<SkColor, base::string16>>& GetColorPickerList() {
             {gfx::kGoogleCyan600, base::ASCIIToUTF16("Cyan")}});
   return *list;
 }
-
-std::unique_ptr<views::LabelButton> CreateMenuItem(
-    int button_id,
-    const base::string16& name,
-    views::ButtonListener* listener) {
-  auto button = std::make_unique<views::LabelButton>(
-      listener, name, views::style::CONTEXT_BUTTON);
-  button->SetID(button_id);
-  button->SetButtonController(std::make_unique<HoverButtonController>(
-      button.get(), listener,
-      std::make_unique<views::Button::DefaultButtonControllerDelegate>(
-          button.get())));
-
-  // Items within a menu should not show focus rings.
-  button->SetInstallFocusRingOnFocus(false);
-  button->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
-
-  button->SetBorder(views::CreateEmptyBorder(kDefaultBorderInsets));
-  button->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
-  button->set_ink_drop_base_color(HoverButton::GetInkDropColor(button.get()));
-  return button;
-}
 }  // namespace
 
 // static
@@ -147,7 +122,8 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
   views::View* group_modifier_container =
       AddChildView(std::make_unique<views::View>());
   group_modifier_container->SetBorder(
-      views::CreateEmptyBorder(kDefaultBorderInsets));
+      views::CreateEmptyBorder(gfx::Insets(layout_provider->GetDistanceMetric(
+          views::DISTANCE_RELATED_CONTROL_VERTICAL))));
   group_modifier_container->SetLayoutManager(std::move(container_layout));
 
   // Add the text field for editing the title.
@@ -173,19 +149,19 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
 
   AddChildView(std::make_unique<views::Separator>());
 
-  std::unique_ptr<views::LabelButton> new_tab_menu_item = CreateMenuItem(
+  std::unique_ptr<views::LabelButton> new_tab_menu_item = CreateBubbleMenuItem(
       TAB_GROUP_HEADER_CXMENU_NEW_TAB_IN_GROUP,
       l10n_util::GetStringUTF16(IDS_TAB_GROUP_HEADER_CXMENU_NEW_TAB_IN_GROUP),
       &menu_button_listener_);
   AddChildView(std::move(new_tab_menu_item));
 
-  std::unique_ptr<views::LabelButton> ungroup_menu_item = CreateMenuItem(
+  std::unique_ptr<views::LabelButton> ungroup_menu_item = CreateBubbleMenuItem(
       TAB_GROUP_HEADER_CXMENU_UNGROUP,
       l10n_util::GetStringUTF16(IDS_TAB_GROUP_HEADER_CXMENU_UNGROUP),
       &menu_button_listener_);
   AddChildView(std::move(ungroup_menu_item));
 
-  std::unique_ptr<views::LabelButton> close_menu_item = CreateMenuItem(
+  std::unique_ptr<views::LabelButton> close_menu_item = CreateBubbleMenuItem(
       TAB_GROUP_HEADER_CXMENU_CLOSE_GROUP,
       l10n_util::GetStringUTF16(IDS_TAB_GROUP_HEADER_CXMENU_CLOSE_GROUP),
       &menu_button_listener_);