Implemented prototype for new ink drop specs.

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 835ccf2aa79112641295f60337ad902f7a8b45f1..f298a10148ad25be1ca803664c3fc1a6fdf83dc0 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_button.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_button.cc
@@ -26,6 +26,12 @@
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/widget/widget.h"
 
+// Sizes for the the ink drop.
+const int kInkDropLargeSize = 32;

[Peter Kasting, 2015/09/03 21:53:06]

Nit: Move all these to LayoutInkDrop() since that's the only consumer.

[bruthig, 2015/09/09 18:00:35]

Done.

+const int kInkDropLargeCornerRadius = 5;
+const int kInkDropSmallSize = 24;
+const int kInkDropSmallCornerRadius = 2;
+
 ToolbarButton::ToolbarButton(views::ButtonListener* listener,
                              ui::MenuModel* model)
     : views::LabelButton(listener, base::string16()),
@@ -99,8 +105,11 @@ bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
         base::TimeDelta::FromMilliseconds(kMenuTimerDelay));
   }
 
-  ink_drop_animation_controller_->AnimateToState(
-      views::InkDropState::ACTION_PENDING);
+  // Button actions are only triggered by left and middle mouse clicks.

[Peter Kasting, 2015/09/03 21:53:06]

Today in Chrome I can right-click the back button and immediately get a dropdown
of my session history.  Does that falsify this statement?

[bruthig, 2015/09/09 18:00:35]

No it does not, the menu shown due to a right-click is handled by
ToolbarButton::OnMousePressed()->ToolbarButton::ShowDropDownMenu().  I've tried
to reduce the ambiguity by adding the 'views::' prefix.

+  if (event.IsLeftMouseButton() || event.IsMiddleMouseButton()) {
+    ink_drop_animation_controller_->AnimateToState(
+        views::InkDropState::ACTION_PENDING);
+  }
 
   return LabelButton::OnMousePressed(event);
 }
@@ -162,11 +171,14 @@ void ToolbarButton::OnGestureEvent(ui::GestureEvent* event) {
       event->SetHandled();
       break;
     case ui::ET_GESTURE_LONG_PRESS:
-      ink_drop_state = views::InkDropState::SLOW_ACTION;
+      ink_drop_state = views::InkDropState::SLOW_ACTION_PENDING;
       break;
     case ui::ET_GESTURE_TAP:
       ink_drop_state = views::InkDropState::QUICK_ACTION;
       break;
+    case ui::ET_GESTURE_LONG_TAP:
+      ink_drop_state = views::InkDropState::SLOW_ACTION;
+      break;
     case ui::ET_GESTURE_END:
     case ui::ET_GESTURE_TAP_CANCEL:
       ink_drop_state = views::InkDropState::HIDDEN;
@@ -237,7 +249,7 @@ bool ToolbarButton::ShouldEnterPushedState(const ui::Event& event) {
 }
 
 bool ToolbarButton::ShouldShowMenu() {
-  return model_ != NULL;
+  return model_ != nullptr;
 }
 
 void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
@@ -282,43 +294,40 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
 
   menu_showing_ = true;
 
+  views::MenuRunner::RunResult result = views::MenuRunner::MENU_DELETED;
+  ink_drop_animation_controller_->AnimateToState(
+      views::InkDropState::ACTIVATED);
+
   // Create and run menu.  Display an empty menu if model is NULL.
   if (model_.get()) {
     views::MenuModelAdapter menu_delegate(model_.get());
     menu_delegate.set_triggerable_event_flags(triggerable_event_flags());
     menu_runner_.reset(new views::MenuRunner(menu_delegate.CreateMenu(),
                                              views::MenuRunner::HAS_MNEMONICS));
-    views::MenuRunner::RunResult result =
-        menu_runner_->RunMenuAt(GetWidget(),
-                                NULL,
-                                gfx::Rect(menu_position, gfx::Size(0, 0)),
-                                views::MENU_ANCHOR_TOPLEFT,
-                                source_type);
-    if (result == views::MenuRunner::MENU_DELETED)
-      return;
+    result = menu_runner_->RunMenuAt(GetWidget(), nullptr,

[Peter Kasting, 2015/09/03 21:53:06]

Is it safe to move this block below the conditional, since both arms do the same
thing?  Or does RunMenuAt() need to be called while |menu_delegate| is alive?

[bruthig, 2015/09/09 18:00:35]

Negative, as you suspect |menu_delegate| must be alive when RunMenuAt() is
called.

+                                     gfx::Rect(menu_position, gfx::Size(0, 0)),
+                                     views::MENU_ANCHOR_TOPLEFT, source_type);
   } else {
     views::MenuDelegate menu_delegate;
     views::MenuItemView* menu = new views::MenuItemView(&menu_delegate);
     menu_runner_.reset(
         new views::MenuRunner(menu, views::MenuRunner::HAS_MNEMONICS));
-    views::MenuRunner::RunResult result =
-        menu_runner_->RunMenuAt(GetWidget(),
-                                NULL,
-                                gfx::Rect(menu_position, gfx::Size(0, 0)),
-                                views::MENU_ANCHOR_TOPLEFT,
-                                source_type);
-    if (result == views::MenuRunner::MENU_DELETED)
-      return;
+    result = menu_runner_->RunMenuAt(GetWidget(), nullptr,
+                                     gfx::Rect(menu_position, gfx::Size(0, 0)),
+                                     views::MENU_ANCHOR_TOPLEFT, source_type);
   }
 
-  ink_drop_animation_controller_->AnimateToState(views::InkDropState::HIDDEN);
+  ink_drop_animation_controller_->AnimateToState(

[Peter Kasting, 2015/09/03 21:53:06]

This isn't safe if the result is MENU_DELETED, in that case |this| is null.  You
must return before calling this.

[bruthig, 2015/09/09 18:00:35]

Done. I've also added a comment to try and make this more obvious.

+      views::InkDropState::DEACTIVATED);
+  if (result == views::MenuRunner::MENU_DELETED)
+    return;
 
   menu_showing_ = false;
 
   // Need to explicitly clear mouse handler so that events get sent
   // properly after the menu finishes running. If we don't do this, then
   // the first click to other parts of the UI is eaten.
-  SetMouseHandler(NULL);
+  SetMouseHandler(nullptr);
 
   // Set the state back to normal after the drop down menu is closed.
   if (state_ != STATE_DISABLED)
@@ -326,7 +335,19 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
 }
 
 void ToolbarButton::LayoutInkDrop() {
-  ink_drop_animation_controller_->SetInkDropSize(size());
+  gfx::Size ink_drop_size = gfx::Size(kInkDropLargeSize, kInkDropLargeSize);

[Peter Kasting, 2015/09/03 21:53:06]

Nit: I don't see why you made a temp for this instead of doing it inline like is
done for the small size.

[bruthig, 2015/09/09 18:00:35]

Done.  Oversight on my part.

+  ink_drop_animation_controller_->SetInkDropSize(
+      ink_drop_size, kInkDropLargeCornerRadius,
+      gfx::Size(kInkDropSmallSize, kInkDropSmallSize),
+      kInkDropSmallCornerRadius);
+  ink_drop_animation_controller_->SetInkDropCenter(
+      gfx::Point(width() / 2, height() / 2));
+}
+
+void ToolbarButton::NotifyClick(const ui::Event& event) {
+  ink_drop_animation_controller_->AnimateToState(
+      views::InkDropState::QUICK_ACTION);
+  LabelButton::NotifyClick(event);
 }
 
 const char* ToolbarButton::GetClassName() const {