Introducing HoverButtonController and removing HoverButton inheritance

from MenuButton.

Bug: 984600

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

chrome/browser/ui/BUILD.gn

@@ -2748,6 +2748,8 @@ jumbo_split_static_library("ui") {
       "views/global_media_controls/media_toolbar_button_view.h",
       "views/hover_button.cc",
       "views/hover_button.h",
+      "views/hover_button_controller.cc",
+      "views/hover_button_controller.h",
       "views/hung_renderer_view.cc",
       "views/hung_renderer_view.h",
       "views/ime/ime_warning_bubble_view.cc",

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

@@ -9,11 +9,11 @@
 #include "build/build_config.h"
 #include "chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h"
 #include "chrome/browser/ui/ui_features.h"
+#include "chrome/browser/ui/views/hover_button_controller.h"
 #include "chrome/browser/ui/views/native_widget_factory.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "ui/events/event.h"
 #include "ui/views/controls/styled_label.h"
-#include "ui/views/test/button_test_api.h"
 
 class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
  protected:
@@ -61,10 +61,10 @@ class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
   }
 
   void TriggerNotifyClick() {
-    ui::MouseEvent click_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                               base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON, 0);
-    views::test::ButtonTestApi test_api(button_.get());
-    test_api.NotifyClick(click_event);
+    ui::MouseEvent click_event(ui::ET_MOUSE_RELEASED, gfx::Point(),
+                               gfx::Point(), base::TimeTicks(),
+                               ui::EF_LEFT_MOUSE_BUTTON, 0);
+    button_->button_controller()->OnMouseReleased(click_event);
   }
 
   base::test::ScopedFeatureList scoped_feature_list_;

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

@@ -17,6 +17,7 @@
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_container.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/hover_button_controller.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/chrome_paths.h"
@@ -24,7 +25,6 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/dns/mock_host_resolver.h"
-#include "ui/views/test/button_test_api.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/window/dialog_client_view.h"
 
@@ -83,10 +83,10 @@ class ExtensionsMenuViewBrowserTest : public DialogBrowserTest {
   void TriggerSingleExtensionButton() {
     auto buttons = GetExtensionMenuButtons();
     ASSERT_EQ(1u, buttons.size());
-    ui::MouseEvent click_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                               base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON, 0);
-    views::test::ButtonTestApi test_api(buttons[0]);
-    test_api.NotifyClick(click_event);
+    ui::MouseEvent click_event(ui::ET_MOUSE_RELEASED, gfx::Point(),
+                               gfx::Point(), base::TimeTicks(),
+                               ui::EF_LEFT_MOUSE_BUTTON, 0);
+    buttons[0]->button_controller()->OnMouseReleased(click_event);
   }
 
   base::test::ScopedFeatureList scoped_feature_list_;
@@ -181,10 +181,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
 
   ui::MouseEvent click_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
                              base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON, 0);
-  views::test::ButtonTestApi test_api(
-      ExtensionsMenuView::GetExtensionsMenuViewForTesting()
-          ->manage_extensions_button_for_testing());
-  test_api.NotifyClick(click_event);
+  ExtensionsMenuView::GetExtensionsMenuViewForTesting()
+      ->manage_extensions_button_for_testing()
+      ->button_controller()
+      ->OnMouseReleased(click_event);
 
   // Clicking the Manage Extensions button should open chrome://extensions.
   EXPECT_EQ(

chrome/browser/ui/views/hover_button.cc

@@ -11,6 +11,7 @@
 #include "chrome/app/vector_icons/vector_icons.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_controller.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/animation/ink_drop_impl.h"
@@ -20,6 +21,7 @@
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/layout/grid_layout.h"
+#include "ui/views/style/typography.h"
 #include "ui/views/view_class_properties.h"
 
 namespace {
@@ -69,7 +71,13 @@ void SetTooltipAndAccessibleName(views::Button* parent,
 
 HoverButton::HoverButton(views::ButtonListener* button_listener,
                          const base::string16& text)
-    : views::MenuButton(text, this), listener_(button_listener) {
+    : views::LabelButton(/*button_listener*/ nullptr,
+                         text,
+                         views::style::CONTEXT_BUTTON),
+      listener_(button_listener) {
+  SetButtonController(std::make_unique<HoverButtonController>(
+      this, listener_, CreateButtonControllerDelegate()));
+
   SetInstallFocusRingOnFocus(false);
   SetFocusBehavior(FocusBehavior::ALWAYS);
 
@@ -189,7 +197,7 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
         grid_layout->AddView(std::move(secondary_view), 1, num_labels);
 
     if (!resize_row_for_secondary_view) {
-      insets_ = views::MenuButton::GetInsets();
+      insets_ = views::LabelButton::GetInsets();
       auto secondary_ctl_size = secondary_view_->GetPreferredSize();
       if (secondary_ctl_size.height() > row_height) {
         // Secondary view is larger. Reduce the insets.
@@ -221,16 +229,8 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
 
 HoverButton::~HoverButton() {}
 
-bool HoverButton::OnKeyPressed(const ui::KeyEvent& event) {
-  // Unlike MenuButton, HoverButton should not be activated when the up or down
-  // arrow key is pressed.
-  if (event.key_code() == ui::VKEY_UP || event.key_code() == ui::VKEY_DOWN)
-    return false;
-  return MenuButton::OnKeyPressed(event);
-}
-
 void HoverButton::SetBorder(std::unique_ptr<views::Border> b) {
-  MenuButton::SetBorder(std::move(b));
+  LabelButton::SetBorder(std::move(b));
   // Make sure the minimum size is correct according to the layout (if any).
   if (GetLayoutManager())
     SetMinSize(GetLayoutManager()->GetPreferredSize(this));
@@ -243,7 +243,7 @@ void HoverButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
 gfx::Insets HoverButton::GetInsets() const {
   if (insets_)
     return insets_.value();
-  return views::MenuButton::GetInsets();
+  return views::LabelButton::GetInsets();
 }
 
 void HoverButton::SetSubtitleElideBehavior(gfx::ElideBehavior elide_behavior) {
@@ -275,11 +275,11 @@ views::Button::KeyClickAction HoverButton::GetKeyClickActionForEvent(
     // |PlatformStyle::kReturnClicksFocusedControl|.
     return CLICK_ON_KEY_PRESS;
   }
-  return MenuButton::GetKeyClickActionForEvent(event);
+  return LabelButton::GetKeyClickActionForEvent(event);
 }
 
 void HoverButton::StateChanged(ButtonState old_state) {
-  MenuButton::StateChanged(old_state);
+  LabelButton::StateChanged(old_state);
 
   // |HoverButtons| are designed for use in a list, so ensure only one button
   // can have a hover background at any time by requesting focus on hover.
@@ -296,7 +296,7 @@ SkColor HoverButton::GetInkDropBaseColor() const {
 }
 
 std::unique_ptr<views::InkDrop> HoverButton::CreateInkDrop() {
-  std::unique_ptr<views::InkDrop> ink_drop = MenuButton::CreateInkDrop();
+  std::unique_ptr<views::InkDrop> ink_drop = LabelButton::CreateInkDrop();
   // Turn on highlighting when the button is focused only - hovering the button
   // will request focus.
   ink_drop->SetShowHighlightOnFocus(true);
@@ -305,7 +305,7 @@ std::unique_ptr<views::InkDrop> HoverButton::CreateInkDrop() {
 }
 
 void HoverButton::Layout() {
-  MenuButton::Layout();
+  LabelButton::Layout();
 
   // Vertically center |title_| manually since it doesn't have a LayoutManager.
   if (title_) {

chrome/browser/ui/views/hover_button.h

@@ -32,7 +32,9 @@ class PageInfoBubbleViewBrowserTest;
 
 // A button taking the full width of its parent that shows a background color
 // when hovered over.
-class HoverButton : public views::MenuButton, public views::MenuButtonListener {
+// TODO (cyan): HoverButton should extend ButtonListener.
+class HoverButton : public views::LabelButton,
+                    public views::MenuButtonListener {
  public:
   enum Style { STYLE_PROMINENT, STYLE_ERROR };
 
@@ -62,8 +64,7 @@ class HoverButton : public views::MenuButton, public views::MenuButtonListener {
 
   ~HoverButton() override;
 
-  // views::MenuButton:
-  bool OnKeyPressed(const ui::KeyEvent& event) override;
+  // views::LabelButton:
   void SetBorder(std::unique_ptr<views::Border> b) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   gfx::Insets GetInsets() const override;

chrome/browser/ui/views/hover_button_controller.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/hover_button_controller.h"
+
+#include "ui/views/animation/ink_drop.h"
+#include "ui/views/controls/button/button_controller_delegate.h"
+#include "ui/views/mouse_constants.h"
+#include "ui/views/widget/root_view.h"
+#include "ui/views/widget/widget.h"
+
+HoverButtonController::HoverButtonController(
+    views::Button* button,
+    views::ButtonListener* listener,
+    std::unique_ptr<views::ButtonControllerDelegate> delegate)
+    : ButtonController(button, std::move(delegate)), listener_(listener) {
+  set_notify_action(views::ButtonController::NotifyAction::NOTIFY_ON_RELEASE);
+}
+
+HoverButtonController::~HoverButtonController() = default;
+
+bool HoverButtonController::OnKeyPressed(const ui::KeyEvent& event) {
+  // HoverButton should not be activated when the up or down arrow key is
+  // pressed.
+  if (event.key_code() == ui::VKEY_UP || event.key_code() == ui::VKEY_DOWN)
+    return false;
+  if (listener_) {
+    listener_->ButtonPressed(button(), event);
+    return true;
+  }
+  return false;
+}
+
+bool HoverButtonController::OnMousePressed(const ui::MouseEvent& event) {
+  DCHECK(notify_action() ==
+         views::ButtonController::NotifyAction::NOTIFY_ON_RELEASE);
+  if (button()->request_focus_on_press())
+    button()->RequestFocus();
+  if (listener_) {
+    button()->AnimateInkDrop(views::InkDropState::ACTION_TRIGGERED,
+                             ui::LocatedEvent::FromIfValid(&event));
+  } else {
+    button()->AnimateInkDrop(views::InkDropState::HIDDEN,
+                             ui::LocatedEvent::FromIfValid(&event));
+  }
+  return true;
+}
+
+void HoverButtonController::OnMouseReleased(const ui::MouseEvent& event) {
+  DCHECK(notify_action() ==
+         views::ButtonController::NotifyAction::NOTIFY_ON_RELEASE);
+  if (button()->state() != views::Button::STATE_DISABLED) {
+    if (listener_)
+      listener_->ButtonPressed(button(), event);
+  } else {
+    button()->AnimateInkDrop(views::InkDropState::HIDDEN, &event);
+    ButtonController::OnMouseReleased(event);
+  }
+}

chrome/browser/ui/views/hover_button_controller.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_HOVER_BUTTON_CONTROLLER_H_
+#define CHROME_BROWSER_UI_VIEWS_HOVER_BUTTON_CONTROLLER_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
+#include "ui/views/controls/button/button_controller.h"
+
+namespace views {
+class ButtonControllerDelegate;
+}  // namespace views
+
+// A controller that contains the logic for a button that's the full width of
+// its parent.
+class HoverButtonController : public views::ButtonController {
+ public:
+  HoverButtonController(
+      views::Button* button,
+      views::ButtonListener* listener,
+      std::unique_ptr<views::ButtonControllerDelegate> delegate);
+  ~HoverButtonController() override;
+
+  // views::ButtonController:
+  bool OnMousePressed(const ui::MouseEvent& event) override;
+  void OnMouseReleased(const ui::MouseEvent& event) override;
+  bool OnKeyPressed(const ui::KeyEvent& event) override;
+
+ private:
+  // Listener to be called when button is clicked.
+  views::ButtonListener* listener_;
+
+  DISALLOW_COPY_AND_ASSIGN(HoverButtonController);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_HOVER_BUTTON_CONTROLLER_H_