Enhance the material design ripple API so the ripple's state can be controlled by it's owning View.

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 45e429b658cf89c1f67c7609cd6fd1629cc0b303..26054ac417daca1cc158dca78c1869bd280ce747 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_button.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_button.cc
@@ -19,6 +19,7 @@
 #include "ui/gfx/screen.h"
 #include "ui/strings/grit/ui_strings.h"
 #include "ui/views/animation/ink_drop_animation_controller.h"
+#include "ui/views/animation/ink_drop_animation_controller_factory.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_model_adapter.h"
@@ -32,18 +33,21 @@ ToolbarButton::ToolbarButton(views::ButtonListener* listener,
       menu_showing_(false),
       y_position_on_lbuttondown_(0),
       show_menu_factory_(this) {
-#if defined(OS_CHROMEOS)
-  // The ink drop animation is only targeted at ChromeOS because there is
-  // concern it will conflict with OS level touch feedback in a bad way.
   if (ui::MaterialDesignController::IsModeMaterial()) {
-    ink_drop_animation_controller_.reset(
-        new views::InkDropAnimationController(this));
-    layer()->SetFillsBoundsOpaquely(false);
+    SetPaintToLayer(true);
     image()->SetPaintToLayer(true);
     image()->SetFillsBoundsOpaquely(false);
   }
-#endif  // defined(OS_CHROMEOS)
-
+  // TODO(bruthig): |ink_drop_animation_controller_| has to be initialized after
+  // SetPaintToLayer(true) becuase the InkDropAnimationControllerImpl
+  // constructor calls back in to AddInkDropLayer() to add the ink drop layer to
+  // layer(). This will be reworked so the InkDropAnimationController will call
+  // AddInkDropLayer() right before the ink drop becomes visible and then
+  // |ink_drop_animation_controller_| can be initialized in the initializer
+  // list.
+
+  ink_drop_animation_controller_ = views::InkDropAnimationControllerFactory::
+      CreateInkDropAnimationController(this);
   set_context_menu_controller(this);
 }
 
@@ -86,6 +90,11 @@ gfx::Size ToolbarButton::GetPreferredSize() const {
   return size;
 }
 
+void ToolbarButton::Layout() {
+  LabelButton::Layout();
+  LayoutInkDrop();
+}
+
 bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
   if (enabled() && ShouldShowMenu() &&
       IsTriggerableEvent(event) && HitTestPoint(event.location())) {
@@ -102,6 +111,10 @@ bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
                               ui::GetMenuSourceTypeForEvent(event)),
         base::TimeDelta::FromMilliseconds(kMenuTimerDelay));
   }
+
+  ink_drop_animation_controller_->AnimateToState(
+      views::InkDropState::ACTION_PENDING);
+
   return LabelButton::OnMousePressed(event);
 }
 
@@ -129,6 +142,8 @@ void ToolbarButton::OnMouseReleased(const ui::MouseEvent& event) {
 
   if (IsTriggerableEvent(event))
     show_menu_factory_.InvalidateWeakPtrs();
+
+  ink_drop_animation_controller_->AnimateToState(views::InkDropState::HIDDEN);
 }
 
 void ToolbarButton::OnMouseCaptureLost() {
@@ -150,6 +165,30 @@ void ToolbarButton::OnGestureEvent(ui::GestureEvent* event) {
   }
 
   LabelButton::OnGestureEvent(event);
+
+  switch (event->type()) {
+    case ui::ET_GESTURE_TAP_DOWN:
+      ink_drop_animation_controller_->AnimateToState(
+          views::InkDropState::ACTION_PENDING);

[Peter Kasting, 2015/08/12 20:04:31]

Nit: Since all these arms call AnimateToState(), this would be simpler if you
can remove the CHECK (see next comment):

  ...
  views::InkDropState state = HIDDEN;
  switch (event->type()) {
    case ui::ET_GESTURE_TAP_DOWN:
      state = views::InkDropState::ACTION_PENDING;
      break;
    case ui::ET_GESTURE_LONG_PRESS:
      state = views::InkDropState::SLOW_ACTION;
      break;
    case ui::ET_GESTURE_TAP:
      state = views::InkDropState::QUICK_ACTION;
      break;
    case ui::ET_GESTURE_END:
    case ui::ET_GESTURE_TAP_CANCEL:
      break;  // Use HIDDEN.
    default:
      return;
  }
  ink_drop_animation_controller_->AnimateToState(state);
}

[bruthig, 2015/08/13 15:15:45]

I don't think your suggestion will work here anyhow, i.e. the ink drop will be
hidden on an ET_GESTURE_SCROLL_BEGIN which is not what we want in this case.
Keep in mind this code is going to be updated as the actual animation states are
flushed out in a subsequent review.

[Peter Kasting, 2015/08/13 18:34:08]

No it won't; see how the "default" case returns rather than allowing fallthrough
to the animate call.
Are you saying you'll be adding more functionality beyond the function call? 
Will it be happening after the function call?  If those aren't both "yes" this
suggestion will still work in the future.  (But see comment below)

[bruthig, 2015/08/14 14:17:39]

Whoops, I missed that.  Fixed.

+      CHECK(event->handled()) << "event should be marked handled so that "
+                                 "subsequent gestures are received.";

[Peter Kasting, 2015/08/12 20:04:32]

Are you trying to catch a particular bug with this CHECK?  Normally we avoid
CHECKs in the codebase unless there's a security issue or it's a short-term
thing to track down some kind of bug.

[bruthig, 2015/08/13 15:15:45]

Originally this was suppposed to detect breaking changes to parent classes (and
probably should have been a DCHECK as opposed to a CHECK), however, tdanderson@,
jonross@, and I discussed offline and decided to explicitly set the event as
handled.  This will ensure that the subsequent events in the pipeline are
properly handled and not be dependent on the ancestor classes.

[Peter Kasting, 2015/08/13 18:34:08]

Why not SetHandled() on the other events too?  In any case this deserves a
comment about what it's doing, and especially one to explain why this case
differs from the others if necessary.

[bruthig, 2015/08/14 14:17:39]

Done.

+      break;
+    case ui::ET_GESTURE_LONG_PRESS:
+      ink_drop_animation_controller_->AnimateToState(
+          views::InkDropState::SLOW_ACTION);
+      break;
+    case ui::ET_GESTURE_TAP:
+      ink_drop_animation_controller_->AnimateToState(
+          views::InkDropState::QUICK_ACTION);
+      break;
+    case ui::ET_GESTURE_END:
+    case ui::ET_GESTURE_TAP_CANCEL:
+      ink_drop_animation_controller_->AnimateToState(
+          views::InkDropState::HIDDEN);
+      break;
+    default:
+      break;
+  }
 }
 
 void ToolbarButton::GetAccessibleState(ui::AXViewState* state) {
@@ -185,6 +224,15 @@ void ToolbarButton::ShowContextMenuForView(View* source,
   ShowDropDownMenu(source_type);
 }
 
+void ToolbarButton::AddInkDropLayer(ui::Layer* ink_drop_layer) {
+  layer()->Add(ink_drop_layer);
+  layer()->StackAtBottom(ink_drop_layer);
+}
+
+void ToolbarButton::RemoveInkDropLayer(ui::Layer* ink_drop_layer) {
+  layer()->Remove(ink_drop_layer);
+}
+
 bool ToolbarButton::ShouldEnterPushedState(const ui::Event& event) {
   // Enter PUSHED state on press with Left or Right mouse button or on taps.
   // Remain in this state while the context menu is open.
@@ -269,6 +317,8 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
       return;
   }
 
+  ink_drop_animation_controller_->AnimateToState(views::InkDropState::HIDDEN);
+
   menu_showing_ = false;
 
   // Need to explicitly clear mouse handler so that events get sent
@@ -281,6 +331,10 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
     SetState(STATE_NORMAL);
 }
 
+void ToolbarButton::LayoutInkDrop() {
+  ink_drop_animation_controller_->SetInkDropSize(gfx::Size(width(), height()));

[Peter Kasting, 2015/08/12 20:04:32]

Nit: Just pass size()

[bruthig, 2015/08/13 15:15:45]

Done.

+}
+
 const char* ToolbarButton::GetClassName() const {
   return "ToolbarButton";
 }