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..0573bcc6074b432b2b4da9a0f8352b7d848399a1 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,19 @@ 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);
-    image()->SetPaintToLayer(true);
-    image()->SetFillsBoundsOpaquely(false);
-  }
-#endif  // defined(OS_CHROMEOS)
-
+  SetPaintToLayer(true);
+  // 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);
+  layer()->SetFillsBoundsOpaquely(false);
+  image()->SetPaintToLayer(true);
+  image()->SetFillsBoundsOpaquely(false);

[tdanderson, 2015/08/11 19:07:40]

Do you still want to call lines 46-48 if the MD flag is turned off?

[jonross, 2015/08/11 20:34:46]

If the factory is abstracting MD, should 46-48 possibly be triggered by the
controller?

[bruthig, 2015/08/12 15:26:40]

They don't need to be called but they didn't seem to have any visual effect so I
left them in originally.  I've wrapped them in an 'if' now.

[bruthig, 2015/08/12 15:26:40]

I don't think the factory can in all general cases set up the InkDropHost
correctly. First it wouldn't make any sense for the
InkDropHost/InkDropAnimationControllerFactory to know anything about the image()
in the ToolbarButton.  This is not something that applies to all
Views/InkDropHosts. Similarly the SetFillsBoundsOpaquely() might not apply to
all InkDropHosts either.

[jonross, 2015/08/12 18:12:56]

True, but could the InkDropHost implement a method that the controller calls?

Could help clearly define what is ink drop related, vs needed for general MD
work.

   set_context_menu_controller(this);
 }
 
@@ -86,6 +88,11 @@ gfx::Size ToolbarButton::GetPreferredSize() const {
   return size;
 }
 
+void ToolbarButton::Layout() {
+  LabelButton::Layout();
+  LayoutInkDrop();

[jonross, 2015/08/11 20:34:46]

Thoughts on having this changed to:

virtual void InkDropHost::LayoutInkDrop() {}

Then:

InkDropAnimationController calling it?

[bruthig, 2015/08/12 15:26:40]

Please see thread about this here:
https://codereview.chromium.org/1280953003/diff/40001/chrome/browser/ui/views...

Additionally, I would like to point out that the InkDropAnimationController
wouldn't be able to call LayoutInkDrop() at the appropriate times because it is
not observing layout or bounds changes anyway.

+}
+
 bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
   if (enabled() && ShouldShowMenu() &&
       IsTriggerableEvent(event) && HitTestPoint(event.location())) {
@@ -102,6 +109,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 +140,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 +163,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);
+      CHECK(event->handled()) << "event should be marked handled so that "

[tdanderson, 2015/08/11 19:07:41]

I'm still on the fence about whether this is needed / a good idea. I'm leaning
toward how you had it before (animation before call to superclass) but without
an explicit call to mark the event as handled. See what happens during ONWNERs
review.

[jonross, 2015/08/11 20:34:46]

This is just ensuring that it is handled. A valid check IMO. Otherwise there is
no guarantee that we'll see subsequent events. That could lead to partially
completed animations on controls.

+                                 "subsequent gestures are received.";
+      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 +222,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 +315,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 +329,10 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
     SetState(STATE_NORMAL);
 }
 
+void ToolbarButton::LayoutInkDrop() {
+  ink_drop_animation_controller_->SetInkDropSize(gfx::Size(width(), height()));
+}
+
 const char* ToolbarButton::GetClassName() const {
   return "ToolbarButton";
 }