Changing ExtensionsMenuButton to inherit from LabelButton.

ExtensionsMenuButton was a HoverButton which did not implement the logic
for adding elide. LabelButton has elide but needs additional padding to
look like what was a HoverButton.

Bug: 985382
Change-Id: I94ed473e0fadb234774fd26fa9109a9adab11e22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1788512
Commit-Queue: Charlene Yan <cyan@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Taylor Bergquist <tbergquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695819}

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

@@ -9,10 +9,12 @@
 #include "chrome/browser/ui/views/extensions/extensions_menu_view.h"
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/hover_button.h"
+#include "chrome/browser/ui/views/hover_button_controller.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "ui/views/controls/button/button.h"
-#include "ui/views/controls/button/menu_button_controller.h"
 #include "ui/views/layout/box_layout.h"
+#include "ui/views/style/typography.h"
 
 const char ExtensionsMenuButton::kClassName[] = "ExtensionsMenuButton";
 
@@ -20,17 +22,14 @@ ExtensionsMenuButton::ExtensionsMenuButton(
     Browser* browser,
     ExtensionsMenuItemView* parent,
     ToolbarActionViewController* controller)
-    : HoverButton(this,
-                  ExtensionsMenuView::CreateFixedSizeIconView(),
-                  base::string16(),
-                  base::string16(),
-                  std::make_unique<views::View>(),
-                  true,
-                  true),
+    : views::LabelButton(this, base::string16(), views::style::CONTEXT_BUTTON),
       browser_(browser),
       parent_(parent),
       controller_(controller) {
-  set_auto_compute_tooltip(false);
+  SetInkDropMode(InkDropMode::ON);
+  SetButtonController(std::make_unique<HoverButtonController>(
+      this, this,
+      std::make_unique<views::Button::DefaultButtonControllerDelegate>(this)));
   controller_->SetDelegate(this);
   UpdateState();
 }
@@ -41,6 +40,10 @@ const char* ExtensionsMenuButton::GetClassName() const {
   return kClassName;
 }
 
+SkColor ExtensionsMenuButton::GetInkDropBaseColor() const {
+  return HoverButton::GetInkDropColor(this);
+}
+
 void ExtensionsMenuButton::ButtonPressed(Button* sender,
                                          const ui::Event& event) {
   controller_->ExecuteAction(true);
@@ -66,14 +69,12 @@ content::WebContents* ExtensionsMenuButton::GetCurrentWebContents() const {
 }
 
 void ExtensionsMenuButton::UpdateState() {
-  DCHECK_EQ(views::ImageView::kViewClassName, icon_view()->GetClassName());
-  static_cast<views::ImageView*>(icon_view())
-      ->SetImage(controller_
-                     ->GetIcon(GetCurrentWebContents(),
-                               icon_view()->GetPreferredSize())
-                     .AsImageSkia());
-  SetTitleTextWithHintRange(controller_->GetActionName(),
-                            gfx::Range::InvalidRange());
+  SetImage(Button::STATE_NORMAL,
+           controller_
+               ->GetIcon(GetCurrentWebContents(),
+                         ExtensionsMenuView::kExtensionsMenuIconSize)
+               .AsImageSkia());
+  SetText(controller_->GetActionName());
   SetTooltipText(controller_->GetTooltip(GetCurrentWebContents()));
   SetEnabled(controller_->IsEnabled(GetCurrentWebContents()));
 }

chrome/browser/ui/views/extensions/extensions_menu_button.h

@@ -10,9 +10,9 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 #include "chrome/browser/ui/views/extensions/extension_context_menu_controller.h"
-#include "chrome/browser/ui/views/hover_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_action_view_delegate_views.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
+#include "ui/views/controls/button/label_button.h"
 
 class ExtensionsMenuItemView;
 
@@ -20,7 +20,7 @@ namespace views {
 class Button;
 }  // namespace views
 
-class ExtensionsMenuButton : public HoverButton,
+class ExtensionsMenuButton : public views::LabelButton,
                              public views::ButtonListener,
                              public ToolbarActionViewDelegateViews {
  public:
@@ -31,6 +31,12 @@ class ExtensionsMenuButton : public HoverButton,
 
   static const char kClassName[];
 
+  SkColor GetInkDropBaseColor() const override;
+
+  const base::string16& label_text_for_testing() const {
+    return label()->GetText();
+  }
+
  private:
   // views::ButtonListener:
   const char* GetClassName() const override;

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

@@ -74,12 +74,13 @@ class ExtensionsMenuItemViewTest : public BrowserWithTestWindowTest {
 };
 
 TEST_F(ExtensionsMenuItemViewTest, UpdatesToDisplayCorrectActionTitle) {
-  EXPECT_EQ(primary_button()->title()->GetText(), initial_extension_name_);
+  EXPECT_EQ(primary_button()->label_text_for_testing(),
+            initial_extension_name_);
 
   base::string16 extension_name = base::ASCIIToUTF16("Extension Name");
   controller_->SetActionName(extension_name);
 
-  EXPECT_EQ(primary_button()->title()->GetText(), extension_name);
+  EXPECT_EQ(primary_button()->label_text_for_testing(), extension_name);
 }
 
 TEST_F(ExtensionsMenuItemViewTest, NotifyClickExecutesAction) {

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

@@ -44,13 +44,12 @@ ExtensionsMenuItemView::ExtensionsMenuItemView(
   views::FlexLayout* layout_manager_ =
       SetLayoutManager(std::make_unique<views::FlexLayout>());
   layout_manager_->SetOrientation(views::LayoutOrientation::kHorizontal)
-      .SetCollapseMargins(true)
       .SetIgnoreDefaultMainAxisMargins(true);
 
   AddChildView(primary_action_button_);
   primary_action_button_->SetProperty(
       views::kFlexBehaviorKey, views::FlexSpecification::ForSizeRule(
-                                   views::MinimumFlexSizeRule::kPreferred,
+                                   views::MinimumFlexSizeRule::kScaleToZero,
                                    views::MaximumFlexSizeRule::kUnbounded));
 
   const SkColor icon_color =

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

@@ -31,9 +31,10 @@ namespace {
 ExtensionsMenuView* g_extensions_dialog = nullptr;
 
 constexpr int EXTENSIONS_SETTINGS_ID = 42;
-
 }  // namespace
 
+constexpr gfx::Size ExtensionsMenuView::kExtensionsMenuIconSize;
+
 ExtensionsMenuView::ExtensionsMenuView(
     views::View* anchor_view,
     Browser* browser,
@@ -82,6 +83,13 @@ bool ExtensionsMenuView::ShouldSnapFrameWidth() const {
   return true;
 }
 
+gfx::Size ExtensionsMenuView::CalculatePreferredSize() const {
+  const int width = ChromeLayoutProvider::Get()->GetDistanceMetric(
+                        DISTANCE_BUBBLE_PREFERRED_WIDTH) -
+                    margins().width();
+  return gfx::Size(width, GetHeightForWidth(width));
+}
+
 void ExtensionsMenuView::Repopulate() {
   RemoveAllChildViews(true);
 
@@ -266,7 +274,7 @@ ExtensionsMenuView::CreateFixedSizeIconView() {
   // Note that this size is larger than the 16dp extension icons as it needs to
   // accommodate 24dp click-to-script badging and surrounding shadows.
   auto image_view = std::make_unique<views::ImageView>();
-  image_view->SetPreferredSize(gfx::Size(28, 28));
+  image_view->SetPreferredSize(kExtensionsMenuIconSize);
   return image_view;
 }
 

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

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 #include "ui/views/controls/button/button.h"
 
@@ -26,6 +27,8 @@ class ExtensionsMenuView : public views::ButtonListener,
                            public views::BubbleDialogDelegateView,
                            public ToolbarActionsModel::Observer {
  public:
+  static constexpr gfx::Size kExtensionsMenuIconSize = gfx::Size(28, 28);
+
   ExtensionsMenuView(views::View* anchor_view,
                      Browser* browser,
                      ExtensionsContainer* extensions_container);
@@ -47,6 +50,10 @@ class ExtensionsMenuView : public views::ButtonListener,
   bool ShouldShowCloseButton() const override;
   int GetDialogButtons() const override;
   bool ShouldSnapFrameWidth() const override;
+  // TODO(crbug.com/1003072): This override is copied from PasswordItemsView to
+  // contrain the width. It would be nice to have a unified way of getting the
+  // preferred size to not duplicate the code.
+  gfx::Size CalculatePreferredSize() const override;
 
   // ToolbarActionsModel::Observer:
   void OnToolbarActionAdded(const ToolbarActionsModel::ActionId& item,

chrome/browser/ui/views/hover_button.cc

@@ -233,6 +233,12 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
 
 HoverButton::~HoverButton() {}
 
+// static
+SkColor HoverButton::GetInkDropColor(const views::View* view) {
+  return views::style::GetColor(*view, views::style::CONTEXT_BUTTON,
+                                views::style::STYLE_SECONDARY);
+}
+
 void HoverButton::SetBorder(std::unique_ptr<views::Border> b) {
   LabelButton::SetBorder(std::move(b));
   // Make sure the minimum size is correct according to the layout (if any).
@@ -295,8 +301,7 @@ void HoverButton::StateChanged(ButtonState old_state) {
 }
 
 SkColor HoverButton::GetInkDropBaseColor() const {
-  return views::style::GetColor(*this, views::style::CONTEXT_BUTTON,
-                                views::style::STYLE_SECONDARY);
+  return GetInkDropColor(this);
 }
 
 std::unique_ptr<views::InkDrop> HoverButton::CreateInkDrop() {

chrome/browser/ui/views/hover_button.h

@@ -64,6 +64,8 @@ class HoverButton : public views::LabelButton,
 
   ~HoverButton() override;
 
+  static SkColor GetInkDropColor(const views::View* view);
+
   // views::LabelButton:
   void SetBorder(std::unique_ptr<views::Border> b) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;