Enhanced the InkDropRippleImpl to create/destroy InkDropAnimations as needed.

ui/views/animation/ink_drop_animation.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/animation/ink_drop_animation.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "ui/base/ui_base_switches.h"
+#include "ui/compositor/callback_layer_animation_observer.h"
 #include "ui/compositor/layer.h"
-#include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/layer_animation_sequence.h"
 #include "ui/compositor/paint_recorder.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/transform_util.h"
+#include "ui/views/animation/ink_drop_animation_observer.h"
 #include "ui/views/view.h"
 
 namespace {
 
 // The minimum scale factor to use when scaling rectangle layers. Smaller values
 // were causing visual anomalies.
 const float kMinimumRectScale = 0.0001f;
 
 // The minimum scale factor to use when scaling circle layers. Smaller values
 // were causing visual anomalies.
@@ skipping 215 matching lines @@
       rect_layer_delegate_(
           new RectangleLayerDelegate(kInkDropColor, large_size_)),
       root_layer_(new ui::Layer(ui::LAYER_NOT_DRAWN)),
       ink_drop_state_(InkDropState::HIDDEN) {
   for (int i = 0; i < PAINTED_SHAPE_COUNT; ++i)
     AddPaintLayer(static_cast<PaintedShape>(i));
 
   root_layer_->SetMasksToBounds(false);
   root_layer_->SetBounds(gfx::Rect(large_size_));
 
-  ResetTransformsToMinSize();
-
-  SetOpacity(kHiddenOpacity);
+  SetStateToHidden();
 }
 
-InkDropAnimation::~InkDropAnimation() {}
+InkDropAnimation::~InkDropAnimation() {
+  // Explicitly aborting all the animations ensures all callbacks are invoked
+  // while this instance still exists.
+  AbortAllAnimations();
+}
+
+void InkDropAnimation::AddObserver(InkDropAnimationObserver* observer) {
+  if (!observers_.HasObserver(observer))

[varkha, 2015/10/06 20:58:38]

When is this the case? Do we need this?

[sadrul, 2015/10/08 01:06:01]

+1 Let's remove this.

[bruthig, 2015/10/08 21:42:55]

It was originally intended as a safe-guard to ensure observers aren't added
multiple times.  It appeared to be a common pattern elsewhere.
I have removed it though.

+    observers_.AddObserver(observer);
+}
+
+void InkDropAnimation::RemoveObserver(InkDropAnimationObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
 
 void InkDropAnimation::AnimateToState(InkDropState ink_drop_state) {
-  if (ink_drop_state_ == ink_drop_state)
-    return;
+  // |animation_observer| will be deleted when AnimationEndedCallback() returns
+  // true.
+  ui::CallbackLayerAnimationObserver* animation_observer =
+      new ui::CallbackLayerAnimationObserver(
+          base::Bind(&InkDropAnimation::AnimationStartedCallback,
+                     base::Unretained(this), ink_drop_state),
+          base::Bind(&InkDropAnimation::AnimationEndedCallback,
+                     base::Unretained(this), ink_drop_state));
+  AnimateToStateInternal(ink_drop_state, animation_observer);
+  animation_observer->SetActive();
+}
+
+void InkDropAnimation::AnimateToStateInternal(
+    InkDropState ink_drop_state,
+    ui::LayerAnimationObserver* animation_observer) {
+  ink_drop_state_ = ink_drop_state;
 
   if (ink_drop_state_ == InkDropState::HIDDEN) {
-    ResetTransformsToMinSize();
-    SetOpacity(kVisibleOpacity);
+    // Animating to the HIDDEN state doesn't actually use any
+    // LayerAnimationSequences so we need to explicitly abort any running ones
+    // so that observers receive an InkDropAnimationEnded() event for the
+    // running animation prior to receiving an InkDropAnimationStarted() event
+    // for the HIDDEN 'animation'.
+    AbortAllAnimations();
+    root_layer_->SetVisible(false);
+    SetStateToHidden();
+    return;
   }
 
   InkDropTransforms transforms;
-
-  // Must set the |ink_drop_state_| before handling the state change because
-  // some state changes make recursive calls to AnimateToState() and the last
-  // call should 'win'.
-  ink_drop_state_ = ink_drop_state;
+  root_layer_->SetVisible(true);
 
   switch (ink_drop_state_) {
     case InkDropState::HIDDEN:
-      GetCurrentTansforms(&transforms);
-      AnimateToTransforms(transforms, kHiddenOpacity,
-                          GetAnimationDuration(InkDropState::HIDDEN),
-                          ui::LayerAnimator::ENQUEUE_NEW_ANIMATION);
+      // This case is handled above in a short circuit return.
       break;
     case InkDropState::ACTION_PENDING:
       CalculateCircleTransforms(large_size_, &transforms);
       AnimateToTransforms(transforms, kVisibleOpacity,
                           GetAnimationDuration(InkDropState::ACTION_PENDING),
-                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+                          animation_observer);
       break;
     case InkDropState::QUICK_ACTION:
       CalculateCircleTransforms(large_size_, &transforms);
       AnimateToTransforms(transforms, kHiddenOpacity,
                           GetAnimationDuration(InkDropState::QUICK_ACTION),
-                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-      AnimateToState(InkDropState::HIDDEN);
+                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+                          animation_observer);
       break;
     case InkDropState::SLOW_ACTION_PENDING:
       CalculateRectTransforms(small_size_, small_corner_radius_, &transforms);
       AnimateToTransforms(
           transforms, kVisibleOpacity,
           GetAnimationDuration(InkDropState::SLOW_ACTION_PENDING),
-          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+          animation_observer);
       break;
     case InkDropState::SLOW_ACTION:
       CalculateRectTransforms(large_size_, large_corner_radius_, &transforms);
       AnimateToTransforms(transforms, kHiddenOpacity,
                           GetAnimationDuration(InkDropState::SLOW_ACTION),
-                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-      AnimateToState(InkDropState::HIDDEN);
+                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+                          animation_observer);
       break;
     case InkDropState::ACTIVATED:
       CalculateRectTransforms(small_size_, small_corner_radius_, &transforms);
       AnimateToTransforms(transforms, kVisibleOpacity,
                           GetAnimationDuration(InkDropState::ACTIVATED),
-                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+                          animation_observer);
       break;
     case InkDropState::DEACTIVATED:
       CalculateRectTransforms(large_size_, large_corner_radius_, &transforms);
       AnimateToTransforms(transforms, kHiddenOpacity,
                           GetAnimationDuration(InkDropState::DEACTIVATED),
-                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-      AnimateToState(InkDropState::HIDDEN);
+                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+                          animation_observer);
       break;
   }
 }
 
 void InkDropAnimation::AnimateToTransforms(
     const InkDropTransforms transforms,
     float opacity,
     base::TimeDelta duration,
-    ui::LayerAnimator::PreemptionStrategy preemption_strategy) {
+    ui::LayerAnimator::PreemptionStrategy preemption_strategy,
+    ui::LayerAnimationObserver* animation_observer) {
   ui::LayerAnimator* root_animator = root_layer_->GetAnimator();
   ui::ScopedLayerAnimationSettings root_animation(root_animator);
   root_animation.SetPreemptionStrategy(preemption_strategy);
   ui::LayerAnimationElement* root_element =
       ui::LayerAnimationElement::CreateOpacityElement(opacity, duration);
   ui::LayerAnimationSequence* root_sequence =
       new ui::LayerAnimationSequence(root_element);
+
+  if (animation_observer)
+    root_sequence->AddObserver(animation_observer);
+
   root_animator->StartAnimation(root_sequence);
 
   for (int i = 0; i < PAINTED_SHAPE_COUNT; ++i) {
     ui::LayerAnimator* animator = painted_layers_[i]->GetAnimator();
     ui::ScopedLayerAnimationSettings animation(animator);
     animation.SetPreemptionStrategy(preemption_strategy);
     ui::LayerAnimationElement* element =
         ui::LayerAnimationElement::CreateTransformElement(transforms[i],
                                                           duration);
     ui::LayerAnimationSequence* sequence =
         new ui::LayerAnimationSequence(element);
+
+    if (animation_observer)
+      sequence->AddObserver(animation_observer);
+
     animator->StartAnimation(sequence);
   }
 }
 
-void InkDropAnimation::ResetTransformsToMinSize() {
+void InkDropAnimation::SetStateToHidden() {
   InkDropTransforms transforms;
   // Using a size of 0x0 creates visual anomalies.
   CalculateCircleTransforms(gfx::Size(1, 1), &transforms);
   SetTransforms(transforms);
+  SetOpacity(kHiddenOpacity);
 }
 
 void InkDropAnimation::SetTransforms(const InkDropTransforms transforms) {
   for (int i = 0; i < PAINTED_SHAPE_COUNT; ++i)
     painted_layers_[i]->SetTransform(transforms[i]);
 }
 
 void InkDropAnimation::SetOpacity(float opacity) {
   root_layer_->SetOpacity(opacity);
 }
@@ skipping 58 matching lines @@
                (size.width() - 2.0f * corner_radius) / rect_delegate_width),
       std::max(kMinimumRectScale, size.height() / rect_delegate_height));
 }
 
 void InkDropAnimation::GetCurrentTansforms(
     InkDropTransforms* transforms_out) const {
   for (int i = 0; i < PAINTED_SHAPE_COUNT; ++i)
     (*transforms_out)[i] = painted_layers_[i]->GetTargetTransform();
 }
 
 void InkDropAnimation::SetCenterPoint(const gfx::Point& center_point) {

[varkha, 2015/10/06 20:58:37]

nit: Should probably be moved above to reflect the order in .h file.

[bruthig, 2015/10/08 21:42:55]

Done.

   gfx::Transform transform;
   transform.Translate(center_point.x(), center_point.y());
   root_layer_->SetTransform(transform);
 }
 
 void InkDropAnimation::AddPaintLayer(PaintedShape painted_shape) {
   ui::LayerDelegate* delegate = nullptr;
   switch (painted_shape) {
     case TOP_LEFT_CIRCLE:
     case TOP_RIGHT_CIRCLE:
@@ skipping 16 matching lines @@
   layer->SetBounds(gfx::Rect(large_size_));
   layer->SetFillsBoundsOpaquely(false);
   layer->set_delegate(delegate);
   layer->SetVisible(true);
   layer->SetOpacity(1.0);
   layer->SetMasksToBounds(false);
 
   painted_layers_[painted_shape].reset(layer);
 }
 
+void InkDropAnimation::AbortAllAnimations() {
+  root_layer_->GetAnimator()->AbortAllAnimations();
+  for (int i = 0; i < PAINTED_SHAPE_COUNT; ++i)

[varkha, 2015/10/06 20:58:38]

Will range syntax work here (and in SetTransforms and other similar places)?

[bruthig, 2015/10/08 21:42:55]

It appears there isn't built in support for range based for loops on enums (See
http://stackoverflow.com/questions/14965174/misunderstanding-of-range-based-f...).
There are some work arounds to make it work but you have to overload the * and
++ operator on the enum type and I don't think we have a precedent set for that
in our coding guidelines.

+    painted_layers_[i]->GetAnimator()->AbortAllAnimations();
+}
+
+void InkDropAnimation::AnimationStartedCallback(
+    InkDropState ink_drop_state,
+    const ui::CallbackLayerAnimationObserver& observer) {
+  FOR_EACH_OBSERVER(InkDropAnimationObserver, observers_,
+                    InkDropAnimationStarted(ink_drop_state));
+}
+
+bool InkDropAnimation::AnimationEndedCallback(
+    InkDropState ink_drop_state,
+    const ui::CallbackLayerAnimationObserver& observer) {
+  FOR_EACH_OBSERVER(
+      InkDropAnimationObserver, observers_,
+      InkDropAnimationEnded(ink_drop_state,
+                            observer.aborted_count()

[sadrul, 2015/10/08 01:06:01]

Is this the only reason to send the observer back with the callback? Would it
make sense to just send that information explicitly (e.g. a boolean/enum to say
everything went well, or some didn't make it) instead?

[bruthig, 2015/10/08 21:42:55]

In the current use case yes, this is the only data required by the callback,
however I figured it was more future proof to pass the observer in case the
observer is enhanced to provide more detailed information.

What would the benefits be of passing an enum or bool?

[sadrul, 2015/10/09 20:36:26]

Simplicity. It's not complex as it is now, so that's OK too.

+                                ? InkDropAnimationObserver::PRE_EMPTED
+                                : InkDropAnimationObserver::SUCCESS));
+  return true;
+}
+
 }  // namespace views