Move NotifyAction into ButtonController.

Bug: 901183
Change-Id: I14d43f32d571ff1a2fb23ce05c1d36e8ebd6541e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713042Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Charlene Yan <cyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683269}

ash/shelf/home_button.cc

@@ -23,6 +23,7 @@
 #include "ui/gfx/scoped_canvas.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_impl.h"
+#include "ui/views/controls/button/button_controller.h"
 
 namespace ash {
 namespace {
@@ -39,7 +40,8 @@ HomeButton::HomeButton(Shelf* shelf)
     : ShelfControlButton(shelf, this), controller_(this) {
   SetAccessibleName(
       l10n_util::GetStringUTF16(IDS_ASH_SHELF_APP_LIST_LAUNCHER_TITLE));
-  set_notify_action(Button::NOTIFY_ON_PRESS);
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::NOTIFY_ON_PRESS);
   set_has_ink_drop_action_on_click(false);
 
   SetEventTargeter(std::make_unique<views::ViewTargeter>(this));

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

@@ -13,6 +13,7 @@
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/views/controls/button/button_controller.h"
 
 ExtensionsToolbarButton::ExtensionsToolbarButton(
     Browser* browser,
@@ -21,7 +22,8 @@ ExtensionsToolbarButton::ExtensionsToolbarButton(
       browser_(browser),
       extensions_container_(extensions_container) {
   SetTooltipText(l10n_util::GetStringUTF16(IDS_TOOLTIP_EXTENSIONS_BUTTON));
-  set_notify_action(Button::NOTIFY_ON_PRESS);
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::NOTIFY_ON_PRESS);
 }
 
 void ExtensionsToolbarButton::UpdateIcon() {

chrome/browser/ui/views/global_media_controls/media_toolbar_button_view.cc

@@ -10,13 +10,15 @@
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/native_theme/native_theme.h"
+#include "ui/views/controls/button/button_controller.h"
 
 MediaToolbarButtonView::MediaToolbarButtonView(
     service_manager::Connector* connector)
     : ToolbarButton(this),
       connector_(connector),
       controller_(connector_, this) {
-  set_notify_action(Button::NOTIFY_ON_PRESS);
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::NOTIFY_ON_PRESS);
   EnableCanvasFlippingForRTLUI(false);
   ToolbarButton::Init();
 

chrome/browser/ui/views/hover_button.cc

@@ -16,6 +16,7 @@
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/button/menu_button_controller.h"
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/layout/grid_layout.h"
@@ -78,7 +79,8 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
 
   SetInkDropMode(InkDropMode::ON);
 
-  set_notify_action(Button::NOTIFY_ON_RELEASE);
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::NOTIFY_ON_RELEASE);
 }
 
 HoverButton::HoverButton(views::ButtonListener* button_listener,

chrome/browser/ui/views/media_router/cast_toolbar_button.cc

@@ -21,6 +21,7 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/native_theme/native_theme.h"
+#include "ui/views/controls/button/button_controller.h"
 
 namespace media_router {
 
@@ -54,7 +55,8 @@ CastToolbarButton::CastToolbarButton(
       browser_(browser),
       profile_(browser_->profile()),
       context_menu_(std::move(context_menu)) {
-  set_notify_action(Button::NOTIFY_ON_PRESS);
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::NOTIFY_ON_PRESS);
 
   EnableCanvasFlippingForRTLUI(false);
   SetTooltipText(l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_ICON_TOOLTIP_TEXT));

chrome/browser/ui/views/page_action/page_action_icon_view.cc

@@ -20,6 +20,7 @@
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/animation/ink_drop_mask.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
+#include "ui/views/controls/button/button_controller.h"
 #include "ui/views/controls/focus_ring.h"
 #include "ui/views/style/platform_style.h"
 
@@ -55,7 +56,8 @@ PageActionIconView::PageActionIconView(CommandUpdater* command_updater,
       GetOmniboxStateOpacity(OmniboxPartState::SELECTED));
   SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
   // Only shows bubble after mouse is released.
-  set_notify_action(NotifyAction::NOTIFY_ON_RELEASE);
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::NOTIFY_ON_RELEASE);
   UpdateBorder();
 }
 

chrome/browser/ui/views/profiles/avatar_toolbar_button.cc

@@ -39,6 +39,7 @@
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/accessibility/view_accessibility.h"
+#include "ui/views/controls/button/button_controller.h"
 #include "ui/views/controls/button/label_button_border.h"
 
 namespace {
@@ -82,7 +83,8 @@ AvatarToolbarButton::AvatarToolbarButton(Browser* browser)
 
   // Activate on press for left-mouse-button only to mimic other MenuButtons
   // without drag-drop actions (specifically the adjacent browser menu).
-  set_notify_action(Button::NOTIFY_ON_PRESS);
+  button_controller()->set_notify_action(
+      views::ButtonController::NOTIFY_ON_PRESS);
   set_triggerable_event_flags(ui::EF_LEFT_MOUSE_BUTTON);
 
   set_tag(IDC_SHOW_AVATAR_MENU);

ui/views/controls/button/button.cc

@@ -231,7 +231,10 @@ bool Button::OnMouseDragged(const ui::MouseEvent& event) {
   if (state_ != STATE_DISABLED) {
     const bool should_enter_pushed = ShouldEnterPushedState(event);
     const bool should_show_pending =
-        should_enter_pushed && notify_action_ == NOTIFY_ON_RELEASE && !InDrag();
+        should_enter_pushed &&
+        button_controller_->notify_action() ==
+            ButtonController::NotifyAction::NOTIFY_ON_RELEASE &&
+        !InDrag();
     if (HitTestPoint(event.location())) {
       SetState(should_enter_pushed ? STATE_PRESSED : STATE_HOVERED);
       if (should_show_pending && GetInkDrop()->GetTargetInkDropState() ==

ui/views/controls/button/button.h

@@ -60,12 +60,6 @@ class VIEWS_EXPORT Button : public InkDropHostView,
     STATE_COUNT,
   };
 
-  // An enum describing the events on which a button should notify its listener.
-  enum NotifyAction {
-    NOTIFY_ON_PRESS,
-    NOTIFY_ON_RELEASE,
-  };
-
   // An enum describing the events on which a button should be clicked for a
   // given key event.
   enum KeyClickAction {
@@ -133,13 +127,6 @@ class VIEWS_EXPORT Button : public InkDropHostView,
     animate_on_state_change_ = value;
   }
 
-  // Sets the event on which the button should notify its listener.
-  void set_notify_action(NotifyAction notify_action) {
-    notify_action_ = notify_action;
-  }
-
-  NotifyAction notify_action() const { return notify_action_; }
-
   void set_hide_ink_drop_when_showing_context_menu(
       bool hide_ink_drop_when_showing_context_menu) {
     hide_ink_drop_when_showing_context_menu_ =
@@ -352,9 +339,6 @@ class VIEWS_EXPORT Button : public InkDropHostView,
   // See description above setter.
   bool request_focus_on_press_ = false;
 
-  // The event on which the button should notify its listener.
-  NotifyAction notify_action_ = NOTIFY_ON_RELEASE;
-
   // True when a button click should trigger an animation action on
   // ink_drop_delegate().
   bool has_ink_drop_action_on_click_ = false;

ui/views/controls/button/button_controller.cc

@@ -28,7 +28,7 @@ bool ButtonController::OnMousePressed(const ui::MouseEvent& event) {
   }
   button_controller_delegate_->RequestFocusFromEvent();
   if (button_controller_delegate_->IsTriggerableEvent(event) &&
-      button_->notify_action() == Button::NOTIFY_ON_PRESS) {
+      notify_action_ == ButtonController::NOTIFY_ON_PRESS) {
     button_controller_delegate_->NotifyClick(event);
     // NOTE: We may be deleted at this point (by the listener's notification
     // handler).
@@ -43,7 +43,7 @@ void ButtonController::OnMouseReleased(const ui::MouseEvent& event) {
     } else {
       button_->SetState(Button::STATE_HOVERED);
       if (button_controller_delegate_->IsTriggerableEvent(event) &&
-          button_->notify_action() == Button::NOTIFY_ON_RELEASE) {
+          notify_action_ == ButtonController::NOTIFY_ON_RELEASE) {
         button_controller_delegate_->NotifyClick(event);
         // NOTE: We may be deleted at this point (by the listener's notification
         // handler).
@@ -51,7 +51,7 @@ void ButtonController::OnMouseReleased(const ui::MouseEvent& event) {
       }
     }
   }
-  if (button_->notify_action() == Button::NOTIFY_ON_RELEASE)
+  if (notify_action_ == ButtonController::NOTIFY_ON_RELEASE)
     button_controller_delegate_->OnClickCanceled(event);
 }
 

ui/views/controls/button/button_controller.h

@@ -19,8 +19,21 @@ class VIEWS_EXPORT ButtonController {
                    std::unique_ptr<ButtonControllerDelegate> delegate);
   virtual ~ButtonController();
 
+  // An enum describing the events on which a button should notify its listener.
+  enum NotifyAction {
+    NOTIFY_ON_PRESS,
+    NOTIFY_ON_RELEASE,
+  };
+
   Button* button() { return button_; }
 
+  // Sets the event on which the button's listener should be notified.
+  void set_notify_action(NotifyAction notify_action) {
+    notify_action_ = notify_action;
+  }
+
+  NotifyAction notify_action() const { return notify_action_; }
+
   // Methods that parallel View::On<Event> handlers:
   virtual bool OnMousePressed(const ui::MouseEvent& event);
   virtual void OnMouseReleased(const ui::MouseEvent& event);
@@ -49,6 +62,9 @@ class VIEWS_EXPORT ButtonController {
   // TODO(cyan): Remove |button_| and access everything via the delegate.
   std::unique_ptr<ButtonControllerDelegate> button_controller_delegate_;
 
+  // The event on which the button's listener should be notified.
+  NotifyAction notify_action_ = NOTIFY_ON_RELEASE;
+
   DISALLOW_COPY_AND_ASSIGN(ButtonController);
 };
 

ui/views/controls/button/button_unittest.cc

@@ -21,6 +21,7 @@
 #include "ui/views/animation/test/test_ink_drop.h"
 #include "ui/views/animation/test/test_ink_drop_host.h"
 #include "ui/views/context_menu_controller.h"
+#include "ui/views/controls/button/button_controller.h"
 #include "ui/views/controls/button/button_observer.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/button/image_button.h"
@@ -333,7 +334,8 @@ TEST_F(ButtonTest, NotifyAction) {
 
   // Set the notify action to its listener on mouse press.
   button()->Reset();
-  button()->set_notify_action(Button::NOTIFY_ON_PRESS);
+  button()->button_controller()->set_notify_action(
+      ButtonController::NotifyAction::NOTIFY_ON_PRESS);
   button()->OnMousePressed(ui::MouseEvent(
       ui::ET_MOUSE_PRESSED, center, center, ui::EventTimeForNow(),
       ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
@@ -367,7 +369,8 @@ TEST_F(ButtonTest, NotifyActionNoClick) {
 
   // Set the notify action to its listener on mouse press.
   button()->Reset();
-  button()->set_notify_action(Button::NOTIFY_ON_PRESS);
+  button()->button_controller()->set_notify_action(
+      ButtonController::NotifyAction::NOTIFY_ON_PRESS);
   button()->OnMousePressed(ui::MouseEvent(
       ui::ET_MOUSE_PRESSED, center, center, ui::EventTimeForNow(),
       ui::EF_RIGHT_MOUSE_BUTTON, ui::EF_RIGHT_MOUSE_BUTTON));
@@ -622,7 +625,8 @@ TEST_F(ButtonTest, InkDropShowHideOnMouseDraggedNotifyOnRelease) {
 
   TestInkDrop* ink_drop = new TestInkDrop();
   CreateButtonWithInkDrop(base::WrapUnique(ink_drop), false);
-  button()->set_notify_action(Button::NOTIFY_ON_RELEASE);
+  button()->button_controller()->set_notify_action(
+      ButtonController::NotifyAction::NOTIFY_ON_RELEASE);
 
   button()->OnMousePressed(ui::MouseEvent(
       ui::ET_MOUSE_PRESSED, center, center, ui::EventTimeForNow(),
@@ -663,7 +667,8 @@ TEST_F(ButtonTest, InkDropShowHideOnMouseDraggedNotifyOnPress) {
 
   TestInkDrop* ink_drop = new TestInkDrop();
   CreateButtonWithInkDrop(base::WrapUnique(ink_drop), true);
-  button()->set_notify_action(Button::NOTIFY_ON_PRESS);
+  button()->button_controller()->set_notify_action(
+      ButtonController::NotifyAction::NOTIFY_ON_PRESS);
 
   button()->OnMousePressed(ui::MouseEvent(
       ui::ET_MOUSE_PRESSED, center, center, ui::EventTimeForNow(),

ui/views/controls/button/menu_button_controller.cc

@@ -21,11 +21,12 @@ using base::TimeTicks;
 namespace views {
 
 namespace {
-ui::EventType NotifyActionToMouseEventType(Button::NotifyAction notify_action) {
+ui::EventType NotifyActionToMouseEventType(
+    ButtonController::NotifyAction notify_action) {
   switch (notify_action) {
-    case Button::NOTIFY_ON_PRESS:
+    case ButtonController::NOTIFY_ON_PRESS:
       return ui::ET_MOUSE_PRESSED;
-    case Button::NOTIFY_ON_RELEASE:
+    case ButtonController::NOTIFY_ON_RELEASE:
       return ui::ET_MOUSE_RELEASED;
   }
 }
@@ -80,8 +81,7 @@ MenuButtonController::MenuButtonController(
     : ButtonController(button, std::move(delegate)), listener_(listener) {
   // Triggers on button press by default, unless drag-and-drop is enabled, see
   // MenuButtonController::IsTriggerableEventType.
-  // TODO(cyan): Move NotifyAction into ButtonController.
-  button->set_notify_action(Button::NOTIFY_ON_PRESS);
+  set_notify_action(ButtonController::NOTIFY_ON_PRESS);
 }
 
 MenuButtonController::~MenuButtonController() = default;
@@ -289,7 +289,7 @@ bool MenuButtonController::IsTriggerableEventType(const ui::Event& event) {
     ui::EventType active_on =
         delegate()->GetDragOperations(mouse_event->location()) ==
                 ui::DragDropTypes::DRAG_NONE
-            ? NotifyActionToMouseEventType(button()->notify_action())
+            ? NotifyActionToMouseEventType(notify_action())
             : ui::ET_MOUSE_RELEASED;
     return event.type() == active_on;
   }

ui/views/controls/combobox/combobox.cc

@@ -24,6 +24,8 @@
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/background.h"
+#include "ui/views/controls/button/button.h"
+#include "ui/views/controls/button/button_controller.h"
 #include "ui/views/controls/combobox/combobox_listener.h"
 #include "ui/views/controls/combobox/combobox_util.h"
 #include "ui/views/controls/focus_ring.h"
@@ -58,7 +60,8 @@ class TransparentButton : public Button {
  public:
   explicit TransparentButton(ButtonListener* listener) : Button(listener) {
     SetFocusBehavior(FocusBehavior::NEVER);
-    set_notify_action(Button::NOTIFY_ON_PRESS);
+    button_controller()->set_notify_action(
+        ButtonController::NotifyAction::NOTIFY_ON_PRESS);
 
     SetInkDropMode(InkDropMode::ON);
     set_has_ink_drop_action_on_click(true);

ui/views/controls/editable_combobox/editable_combobox.cc

@@ -43,6 +43,7 @@
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/animation/ink_drop_ripple.h"
 #include "ui/views/controls/button/button.h"
+#include "ui/views/controls/button/button_controller.h"
 #include "ui/views/controls/combobox/combobox_util.h"
 #include "ui/views/controls/editable_combobox/editable_combobox_listener.h"
 #include "ui/views/controls/menu/menu_config.h"
@@ -67,7 +68,7 @@ class Arrow : public Button {
       : Button(listener), color_(color) {
     // Similar to Combobox's TransparentButton.
     SetFocusBehavior(FocusBehavior::NEVER);
-    set_notify_action(Button::NOTIFY_ON_PRESS);
+    button_controller()->set_notify_action(ButtonController::NOTIFY_ON_PRESS);
 
     SetInkDropMode(InkDropMode::ON);
     set_has_ink_drop_action_on_click(true);