Adds OnClickCanceled callback to views::Button

chrome/browser/ui/views/toolbar/toolbar_button.cc

Index: chrome/browser/ui/views/toolbar/toolbar_button.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_button.cc b/chrome/browser/ui/views/toolbar/toolbar_button.cc
index 33cfcd013c9dc9d0e64e3a372ebb59925614c3ab..b621bcdd663dc1672d87c1ce213a8afa43520fe6 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_button.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_button.cc
@@ -94,24 +94,21 @@ void ToolbarButton::Layout() {
 }
 
 bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
-  if (enabled() && ShouldShowMenu() &&
-      IsTriggerableEvent(event) && HitTestPoint(event.location())) {
-    // Store the y pos of the mouse coordinates so we can use them later to
-    // determine if the user dragged the mouse down (which should pop up the
-    // drag down menu immediately, instead of waiting for the timer)
-    y_position_on_lbuttondown_ = event.y();
-
-    // Schedule a task that will show the menu.
-    const int kMenuTimerDelay = 500;
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, base::Bind(&ToolbarButton::ShowDropDownMenu,
-                              show_menu_factory_.GetWeakPtr(),
-                              ui::GetMenuSourceTypeForEvent(event)),
-        base::TimeDelta::FromMilliseconds(kMenuTimerDelay));
-  }
-
-  // views::Button actions are only triggered by left and middle mouse clicks.
-  if (event.IsLeftMouseButton() || event.IsMiddleMouseButton()) {
+  if (IsTriggerableEvent(event)) {
+    if (enabled() && ShouldShowMenu() && HitTestPoint(event.location())) {
+      // Store the y pos of the mouse coordinates so we can use them later to
+      // determine if the user dragged the mouse down (which should pop up the
+      // drag down menu immediately, instead of waiting for the timer)
+      y_position_on_lbuttondown_ = event.y();
+
+      // Schedule a task that will show the menu.
+      const int kMenuTimerDelay = 500;
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+          FROM_HERE, base::Bind(&ToolbarButton::ShowDropDownMenu,
+                                show_menu_factory_.GetWeakPtr(),
+                                ui::GetMenuSourceTypeForEvent(event)),
+          base::TimeDelta::FromMilliseconds(kMenuTimerDelay));
+    }
     ink_drop_animation_controller_->AnimateToState(
         views::InkDropState::ACTION_PENDING);

[sadrul, 2015/11/06 21:46:59]

I am beginning to question if event-handling code is the best place to actually
trigger/stop the animations. Would it make sense to trigger the animations
specifically when the ButtonState changes? For example, CustomButton::SetState()
can call a virtual OnStateChanged(old_state, new_state [, event?]) method, and
ToolbarButton here will override that to make the decision of when to start/stop
the ripple. If that works, I feel like that would simplify this code.

[varkha, 2015/11/09 15:22:21]

This may work as a plan (and would be applicable to other ripple targets) but
seems tangential to this CL which is just a bug fix in the existing handling.

   }
@@ -143,9 +140,6 @@ void ToolbarButton::OnMouseReleased(const ui::MouseEvent& event) {
 
   if (IsTriggerableEvent(event))
     show_menu_factory_.InvalidateWeakPtrs();
-
-  if (!HitTestPoint(event.location()))
-    ink_drop_animation_controller_->AnimateToState(views::InkDropState::HIDDEN);
 }
 
 void ToolbarButton::OnMouseCaptureLost() {
@@ -256,6 +250,11 @@ void ToolbarButton::NotifyClick(const ui::Event& event) {
       views::InkDropState::QUICK_ACTION);
 }
 
+void ToolbarButton::OnClickCanceled(const ui::Event& event) {
+  LabelButton::OnClickCanceled(event);
+  ink_drop_animation_controller_->AnimateToState(views::InkDropState::HIDDEN);
+}
+
 bool ToolbarButton::ShouldShowMenu() {
   return model_ != nullptr;
 }