Added the ripple effect animation to find bar buttons.

chrome/browser/ui/views/find_bar_view.cc

Index: chrome/browser/ui/views/find_bar_view.cc
diff --git a/chrome/browser/ui/views/find_bar_view.cc b/chrome/browser/ui/views/find_bar_view.cc
index 026e3f75e6f0e8617514a2fd238ea9dd928c8643..dcca6e731ef7f4e2e8719a8a20b9e787519c9e49 100644
--- a/chrome/browser/ui/views/find_bar_view.cc
+++ b/chrome/browser/ui/views/find_bar_view.cc
@@ -36,6 +36,9 @@
 #include "ui/native_theme/common_theme.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/resources/grit/ui_resources.h"
+#include "ui/views/animation/ink_drop_animation_controller.h"
+#include "ui/views/animation/ink_drop_animation_controller_factory.h"
+#include "ui/views/animation/ink_drop_host.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/bubble/bubble_border.h"
@@ -97,6 +100,106 @@ const SkColor kSeparatorColor = SkColorSetARGB(0x26, 0, 0, 0);
 // number brings the width on a "regular fonts" system to about 300px.
 const int kDefaultCharWidth = 43;
 
+class FindBarButton : public views::ImageButton, public views::InkDropHost {
+ public:
+  FindBarButton(views::ButtonListener* listener)

[Peter Kasting, 2015/10/09 22:09:30]

Do not inline any of these methods.  Define these out of line just as you would
if the class declaration was in a header file.

[Hadi, 2015/10/13 18:48:17]

Done.

+      : views::ImageButton(listener),
+        ink_drop_animation_controller_(
+            views::InkDropAnimationControllerFactory::
+                CreateInkDropAnimationController(this)) {
+    // Sizes for ink drop animation.
+    const int kInkDropLargeSize = 24;
+    const int kInkDropLargeCornerRadius = 4;
+    const int kInkDropSmallSize = 18;
+    const int kInkDropSmallCornerRadius = 2;
+
+    ink_drop_animation_controller_->SetInkDropSize(
+        gfx::Size(kInkDropLargeSize, kInkDropLargeSize),
+        kInkDropLargeCornerRadius,
+        gfx::Size(kInkDropSmallSize, kInkDropSmallSize),
+        kInkDropSmallCornerRadius);
+  }
+
+  ~FindBarButton() override {}
+
+ private:
+  void Layout() override {
+    ImageButton::Layout();
+
+    ink_drop_animation_controller_->SetInkDropCenter(
+        GetLocalBounds().CenterPoint());
+  }
+
+  void AddInkDropLayer(ui::Layer* ink_drop_layer) override {
+    SetPaintToLayer(true);
+    SetFillsBoundsOpaquely(false);
+    layer()->Add(ink_drop_layer);
+    layer()->StackAtBottom(ink_drop_layer);
+  }
+
+  void RemoveInkDropLayer(ui::Layer* ink_drop_layer) override {
+    layer()->Remove(ink_drop_layer);
+    SetFillsBoundsOpaquely(true);
+    SetPaintToLayer(false);
+  }
+
+  bool OnMousePressed(const ui::MouseEvent& event) override {
+    if (IsTriggerableEvent(event)) {
+      ink_drop_animation_controller_->AnimateToState(
+          views::InkDropState::ACTION_PENDING);
+    }
+
+    return ImageButton::OnMousePressed(event);
+  }
+
+  void OnGestureEvent(ui::GestureEvent* event) override {
+    ImageButton::OnGestureEvent(event);

[Peter Kasting, 2015/10/09 22:09:30]

Why is it that for mouse pressed we do our ink drop handling first and then call
the superclass, but for these next functions it's the opposite?  This seems
inconsistent.

[Hadi, 2015/10/13 18:48:17]

Done.

+
+    views::InkDropState ink_drop_state = views::InkDropState::HIDDEN;
+    switch (event->type()) {
+      case ui::ET_GESTURE_TAP_DOWN:
+        ink_drop_state = views::InkDropState::ACTION_PENDING;

[Peter Kasting, 2015/10/09 22:09:30]

It kinda surprises me we haven't needed a mapping like this in other places
already and created a common function for it.

I wonder if it would make sense to do some sort of array of pairs of (source,
target) enum values and just loop over them.  Maybe that wouldn't be shorter.

[Hadi, 2015/10/13 18:48:17]

bruthig is going to comment on this.

[bruthig, 2015/10/13 18:54:29]

Ideally we would be able to capture this behavior in a common function
somewhere, however it is not as common as one might think.  After taking a look
at some of the different surfaces that the ripple would be applied to it became
clear that each surface had very unique state transitions and we couldn't
capture this 'common' behavior in a PreTargetHandler.  (A doc outlining this can
be found here:
https://docs.google.com/document/d/1U4AdR4wZVsZ8g0y95EblY5YiYVbVXcBTJ59XJBF147M).
 So we decided to apply it directly to each of the surfaces for now.  My plan is
to clean it up once a good pattern emerges.

Having said that, I am more than open to suggestions for possible solutions.

+        // The ui::ET_GESTURE_TAP_DOWN event needs to be marked as handled so
+        // that subsequent events for the gesture are sent to |this|.
+        event->SetHandled();
+        break;
+      case ui::ET_GESTURE_LONG_PRESS:
+        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;
+        break;
+      default:
+        return;
+    }
+    ink_drop_animation_controller_->AnimateToState(ink_drop_state);
+  }
+
+  void OnMouseReleased(const ui::MouseEvent& event) override {
+    ImageButton::OnMouseReleased(event);
+
+    if (!HitTestPoint(event.location()))
+      ink_drop_animation_controller_->AnimateToState(
+          views::InkDropState::HIDDEN);
+  }
+
+  void NotifyClick(const ui::Event& event) override {
+    ImageButton::NotifyClick(event);
+    ink_drop_animation_controller_->AnimateToState(
+        views::InkDropState::QUICK_ACTION);
+  }
+
+  // Animation controller for the ink drop ripple effect.
+  scoped_ptr<views::InkDropAnimationController> ink_drop_animation_controller_;
+};

[Peter Kasting, 2015/10/09 22:09:30]

DISALLOW_COPY_AND_ASSIGN?

[Hadi, 2015/10/13 18:48:17]

Done.

+
 // The match count label is like a normal label, but can process events (which
 // makes it easier to forward events to the text input --- see
 // FindBarView::TargetForRect).
@@ -133,7 +236,7 @@ FindBarView::FindBarView(FindBarHost* host)
   find_text_->SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF);
   AddChildView(find_text_);
 
-  find_previous_button_ = new views::ImageButton(this);
+  find_previous_button_ = new FindBarButton(this);
   find_previous_button_->set_tag(FIND_PREVIOUS_TAG);
   find_previous_button_->SetFocusable(true);
   find_previous_button_->SetTooltipText(
@@ -142,7 +245,7 @@ FindBarView::FindBarView(FindBarHost* host)
       l10n_util::GetStringUTF16(IDS_ACCNAME_PREVIOUS));
   AddChildView(find_previous_button_);
 
-  find_next_button_ = new views::ImageButton(this);
+  find_next_button_ = new FindBarButton(this);
   find_next_button_->set_tag(FIND_NEXT_TAG);
   find_next_button_->SetFocusable(true);
   find_next_button_->SetTooltipText(
@@ -151,7 +254,7 @@ FindBarView::FindBarView(FindBarHost* host)
       l10n_util::GetStringUTF16(IDS_ACCNAME_NEXT));
   AddChildView(find_next_button_);
 
-  close_button_ = new views::ImageButton(this);
+  close_button_ = new FindBarButton(this);
   close_button_->set_tag(CLOSE_TAG);
   close_button_->SetFocusable(true);
   close_button_->SetTooltipText(