Fix double ripple on activated flood fill ripple

When switching from pending to activated state, there is no need to do
more animations as the final state of animations for these states are
the same. Otherwise, janks on slow devices might create some unwanted
effects like double ripples.

BUG=670831
TEST=manual

Review-Url: https://codereview.chromium.org/2615613003
Cr-Commit-Position: refs/heads/master@{#443366}

ui/views/animation/flood_fill_ink_drop_ripple.cc

@@ -215,11 +215,10 @@ void FloodFillInkDropRipple::AnimateStateChange(
                        GetAnimationDuration(ACTION_PENDING_FADE_IN),
                        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
                        gfx::Tween::EASE_IN, animation_observer);
-      AnimateToOpacity(visible_opacity_,
-                       GetAnimationDuration(ACTION_PENDING_TRANSFORM) -
-                           GetAnimationDuration(ACTION_PENDING_FADE_IN),
-                       ui::LayerAnimator::ENQUEUE_NEW_ANIMATION,
-                       gfx::Tween::EASE_IN, animation_observer);
+      PauseOpacityAnimation(GetAnimationDuration(ACTION_PENDING_TRANSFORM) -
+                                GetAnimationDuration(ACTION_PENDING_FADE_IN),
+                            ui::LayerAnimator::ENQUEUE_NEW_ANIMATION,
+                            animation_observer);
 
       AnimateToTransform(GetMaxSizeTargetTransform(),
                          GetAnimationDuration(ACTION_PENDING_TRANSFORM),
@@ -260,14 +259,28 @@ void FloodFillInkDropRipple::AnimateStateChange(
                        gfx::Tween::EASE_IN_OUT, animation_observer);
       break;
     case InkDropState::ACTIVATED: {
-      AnimateToOpacity(visible_opacity_,
-                       GetAnimationDuration(ACTIVATED_FADE_IN),
-                       ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
-                       gfx::Tween::EASE_IN, animation_observer);
-      AnimateToTransform(GetMaxSizeTargetTransform(),
-                         GetAnimationDuration(ACTIVATED_TRANSFORM),
+      if (old_ink_drop_state == InkDropState::ACTION_PENDING) {
+        // The final state of pending animation is the same as the final state
+        // of activated animation. We only need to enqueue a zero-length pause
+        // so that animation observers are notified in order.
+        PauseOpacityAnimation(
+            base::TimeDelta(),
+            ui::LayerAnimator::PreemptionStrategy::ENQUEUE_NEW_ANIMATION,
+            animation_observer);
+        PauseTransformAnimation(
+            base::TimeDelta(),
+            ui::LayerAnimator::PreemptionStrategy::ENQUEUE_NEW_ANIMATION,
+            animation_observer);
+      } else {
+        AnimateToOpacity(visible_opacity_,
+                         GetAnimationDuration(ACTIVATED_FADE_IN),
                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
-                         gfx::Tween::EASE_IN_OUT, animation_observer);
+                         gfx::Tween::EASE_IN, animation_observer);
+        AnimateToTransform(GetMaxSizeTargetTransform(),
+                           GetAnimationDuration(ACTIVATED_TRANSFORM),
+                           ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
+                           gfx::Tween::EASE_IN_OUT, animation_observer);
+      }
       break;
     }
     case InkDropState::DEACTIVATED:
@@ -313,6 +326,27 @@ void FloodFillInkDropRipple::AnimateToTransform(
   animator->StartAnimation(sequence);
 }
 
+void FloodFillInkDropRipple::PauseTransformAnimation(
+    base::TimeDelta duration,
+    ui::LayerAnimator::PreemptionStrategy preemption_strategy,
+    ui::LayerAnimationObserver* observer) {
+  ui::LayerAnimator* animator = painted_layer_.GetAnimator();
+  ui::ScopedLayerAnimationSettings animation(animator);
+  animation.SetPreemptionStrategy(preemption_strategy);
+
+  std::unique_ptr<ui::LayerAnimationElement> element =
+      ui::LayerAnimationElement::CreatePauseElement(
+          ui::LayerAnimationElement::TRANSFORM, duration);
+
+  ui::LayerAnimationSequence* sequence =
+      new ui::LayerAnimationSequence(std::move(element));
+
+  if (observer)
+    sequence->AddObserver(observer);
+
+  animator->StartAnimation(sequence);
+}
+
 void FloodFillInkDropRipple::SetOpacity(float opacity) {
   root_layer_.SetOpacity(opacity);
 }
@@ -338,6 +372,27 @@ void FloodFillInkDropRipple::AnimateToOpacity(
   animator->StartAnimation(animation_sequence);
 }
 
+void FloodFillInkDropRipple::PauseOpacityAnimation(
+    base::TimeDelta duration,
+    ui::LayerAnimator::PreemptionStrategy preemption_strategy,
+    ui::LayerAnimationObserver* observer) {
+  ui::LayerAnimator* animator = root_layer_.GetAnimator();
+  ui::ScopedLayerAnimationSettings animation(animator);
+  animation.SetPreemptionStrategy(preemption_strategy);
+
+  std::unique_ptr<ui::LayerAnimationElement> element =
+      ui::LayerAnimationElement::CreatePauseElement(
+          ui::LayerAnimationElement::OPACITY, duration);
+
+  ui::LayerAnimationSequence* sequence =
+      new ui::LayerAnimationSequence(std::move(element));
+
+  if (observer)
+    sequence->AddObserver(observer);
+
+  animator->StartAnimation(sequence);
+}
+
 gfx::Transform FloodFillInkDropRipple::CalculateTransform(
     float target_radius) const {
   const float target_scale = target_radius / circle_layer_delegate_.radius();

ui/views/animation/flood_fill_ink_drop_ripple.h

@@ -91,6 +91,12 @@ class VIEWS_EXPORT FloodFillInkDropRipple : public InkDropRipple {
       gfx::Tween::Type tween,
       ui::LayerAnimationObserver* observer);
 
+  // Creates a pause animation for transform property.
+  void PauseTransformAnimation(
+      base::TimeDelta duration,
+      ui::LayerAnimator::PreemptionStrategy preemption_strategy,
+      ui::LayerAnimationObserver* observer);
+
   // Sets the opacity of the ink drop. Note that this does not perform any
   // animation.
   void SetOpacity(float opacity);
@@ -106,6 +112,12 @@ class VIEWS_EXPORT FloodFillInkDropRipple : public InkDropRipple {
       gfx::Tween::Type tween,
       ui::LayerAnimationObserver* observer);
 
+  // Creates a pause animation for opacity property.
+  void PauseOpacityAnimation(
+      base::TimeDelta duration,
+      ui::LayerAnimator::PreemptionStrategy preemption_strategy,
+      ui::LayerAnimationObserver* observer);
+
   // Returns the Transform to be applied to the |painted_layer_| for the given
   // |target_radius|.
   gfx::Transform CalculateTransform(float target_radius) const;

ui/views/animation/flood_fill_ink_drop_ripple_unittest.cc

@@ -67,5 +67,45 @@ TEST(FloodFillInkDropRippleTest, MaxDistanceToCorners) {
               kAbsError);
 }
 
+// Verifies that both going directly from HIDDEN to ACTIVATED state and going
+// through PENDING to ACTIVAED state lead to the same final opacity and
+// transform values.
+TEST(FloodFillInkDropRippleTest, ActivatedFinalState) {
+  const float kAbsError = 0.01f;
+
+  const gfx::Size host_size(100, 50);
+  const gfx::Point center_point(host_size.width() / 2, host_size.height() / 2);
+  const SkColor color = SK_ColorWHITE;
+  const float visible_opacity = 0.7f;
+
+  FloodFillInkDropRipple ripple(host_size, center_point, color,
+                                visible_opacity);
+  FloodFillInkDropRippleTestApi test_api(&ripple);
+
+  // Go to ACTIVATED state directly.
+  ripple.AnimateToState(InkDropState::ACTIVATED);
+  test_api.CompleteAnimations();
+  const float activated_opacity = test_api.GetCurrentOpacity();
+  const gfx::Transform activated_transform =
+      test_api.GetPaintedLayerTransform();
+
+  // Reset state.
+  ripple.AnimateToState(InkDropState::HIDDEN);
+  test_api.CompleteAnimations();
+
+  // Go to ACTIVATED state through PENDING state.
+  ripple.AnimateToState(InkDropState::ACTION_PENDING);
+  ripple.AnimateToState(InkDropState::ACTIVATED);
+  test_api.CompleteAnimations();
+  const float pending_activated_opacity = test_api.GetCurrentOpacity();
+  const gfx::Transform pending_activated_transform =
+      test_api.GetPaintedLayerTransform();
+
+  // Compare opacity and transform values.
+  EXPECT_NEAR(activated_opacity, pending_activated_opacity, kAbsError);
+  EXPECT_TRUE(
+      activated_transform.ApproximatelyEqual(pending_activated_transform));
+}
+
 }  // namespace test
 }  // namespace views

ui/views/animation/ink_drop_highlight_unittest.cc

@@ -56,7 +56,10 @@ InkDropHighlightTest::InkDropHighlightTest() {
       gfx::Size(10, 10), 3, gfx::PointF(), SK_ColorBLACK));
 }
 
-InkDropHighlightTest::~InkDropHighlightTest() {}
+InkDropHighlightTest::~InkDropHighlightTest() {
+  // Destory highlight to make sure it is destroyed before the observer.
+  DestroyHighlight();
+}
 
 void InkDropHighlightTest::InitHighlight(
     std::unique_ptr<InkDropHighlight> new_highlight) {

ui/views/animation/ink_drop_ripple.cc

@@ -103,7 +103,8 @@ test::InkDropRippleTestApi* InkDropRipple::GetTestApi() {
 void InkDropRipple::AnimationStartedCallback(
     InkDropState ink_drop_state,
     const ui::CallbackLayerAnimationObserver& observer) {
-  observer_->AnimationStarted(ink_drop_state);
+  if (observer_)
+    observer_->AnimationStarted(ink_drop_state);
 }
 
 bool InkDropRipple::AnimationEndedCallback(
@@ -111,10 +112,11 @@ bool InkDropRipple::AnimationEndedCallback(
     const ui::CallbackLayerAnimationObserver& observer) {
   if (ink_drop_state == InkDropState::HIDDEN)
     SetStateToHidden();
-  observer_->AnimationEnded(ink_drop_state,
-                            observer.aborted_count()
-                                ? InkDropAnimationEndedReason::PRE_EMPTED
-                                : InkDropAnimationEndedReason::SUCCESS);
+  if (observer_)
+    observer_->AnimationEnded(ink_drop_state,
+                              observer.aborted_count()
+                                  ? InkDropAnimationEndedReason::PRE_EMPTED
+                                  : InkDropAnimationEndedReason::SUCCESS);
   // |this| may be deleted!
   return true;
 }

ui/views/animation/ink_drop_ripple_unittest.cc

@@ -358,5 +358,18 @@ TEST_P(InkDropRippleTest, TargetInkDropStateOnAnimationEnded) {
             observer_.target_state_at_last_animation_ended());
 }
 
+// Verifies that when an we ink drop transitions from ACTION_PENDING to
+// ACTIVATED state, animation observers are called in order.
+TEST_P(InkDropRippleTest, RipplePendingToActivatedObserverOrder) {
+  ink_drop_ripple_->AnimateToState(InkDropState::ACTION_PENDING);
+  ink_drop_ripple_->AnimateToState(InkDropState::ACTIVATED);
+  test_api_->CompleteAnimations();
+
+  EXPECT_TRUE(observer_.AnimationStartedContextsMatch(
+      {InkDropState::ACTION_PENDING, InkDropState::ACTIVATED}));
+  EXPECT_TRUE(observer_.AnimationEndedContextsMatch(
+      {InkDropState::ACTION_PENDING, InkDropState::ACTIVATED}));
+}
+
 }  // namespace test
 }  // namespace views

ui/views/animation/test/flood_fill_ink_drop_ripple_test_api.cc

@@ -37,6 +37,10 @@ float FloodFillInkDropRippleTestApi::MaxDistanceToCorners(
   return ink_drop_ripple()->MaxDistanceToCorners(point);
 }
 
+gfx::Transform FloodFillInkDropRippleTestApi::GetPaintedLayerTransform() const {
+  return ink_drop_ripple()->painted_layer_.transform();
+}
+
 float FloodFillInkDropRippleTestApi::GetCurrentOpacity() const {
   return ink_drop_ripple()->root_layer_.opacity();
 }

ui/views/animation/test/flood_fill_ink_drop_ripple_test_api.h

@@ -37,6 +37,9 @@ class FloodFillInkDropRippleTestApi : public InkDropRippleTestApi {
   // Wrapper for FloodFillInkDropRipple::MaxDistanceToCorners().
   float MaxDistanceToCorners(const gfx::Point& point) const;
 
+  // Gets the transform currently applied to the painted layer of the ripple.
+  gfx::Transform GetPaintedLayerTransform() const;
+
   // InkDropRippleTestApi:
   float GetCurrentOpacity() const override;
 

ui/views/animation/test/test_ink_drop_animation_observer_helper.h

@@ -48,12 +48,14 @@ class TestInkDropAnimationObserverHelper {
   }
 
   void OnAnimationStarted(ContextType context) {
+    animation_started_contexts_.push_back(context);
     last_animation_started_context_ = context;
     last_animation_started_ordinal_ = GetNextOrdinal();
   }
 
   void OnAnimationEnded(ContextType context,
                         InkDropAnimationEndedReason reason) {
+    animation_ended_contexts_.push_back(context);
     last_animation_ended_context_ = context;
     last_animation_ended_ordinal_ = GetNextOrdinal();
     last_animation_ended_reason_ = reason;
@@ -110,7 +112,52 @@ class TestInkDropAnimationObserverHelper {
                                        << last_animation_ended_ordinal() << ".";
   }
 
+  // Passes *_TRUE assertions when |animation_started_context_| is the same as
+  // |expected_contexts|.
+  testing::AssertionResult AnimationStartedContextsMatch(
+      const std::vector<ContextType>& expected_contexts) {
+    return ContextsMatch(expected_contexts, animation_started_contexts_);
+  }
+
+  // Passes *_TRUE assertions when |animation_ended_context_| is the same as
+  // |expected_contexts|.
+  testing::AssertionResult AnimationEndedContextsMatch(
+      const std::vector<ContextType>& expected_contexts) {
+    return ContextsMatch(expected_contexts, animation_ended_contexts_);
+  }
+
  private:
+  // Helper function that checks if |actual_contexts| is the same as
+  // |expected_contexts| returning appropriate AssertionResult.
+  testing::AssertionResult ContextsMatch(
+      const std::vector<ContextType>& expected_contexts,
+      const std::vector<ContextType>& actual_contexts) {
+    const bool match =
+        expected_contexts.size() == actual_contexts.size() &&
+        std::equal(expected_contexts.begin(), expected_contexts.end(),
+                   actual_contexts.begin());
+    testing::AssertionResult result =
+        match ? (testing::AssertionSuccess() << "Expected == Actual: {")
+              : (testing::AssertionFailure() << "Expected != Actual: {");
+    for (auto eit = expected_contexts.begin(), ait = actual_contexts.begin();
+         eit != expected_contexts.end() || ait != actual_contexts.end();) {
+      if (eit != expected_contexts.begin())
+        result << ", ";
+      const bool eexists = eit != expected_contexts.end();
+      const bool aexists = ait != actual_contexts.end();
+      const bool item_match = eexists && aexists && *eit == *ait;
+      result << (eexists ? ToString(*eit) : "<none>")
+             << (item_match ? " == " : " != ")
+             << (aexists ? ToString(*ait) : "<none>");
+      if (eexists)
+        eit++;
+      if (aexists)
+        ait++;
+    }
+    result << "}";
+    return result;
+  }
+
   // Returns the next event ordinal. The first returned ordinal will be 1.
   int GetNextOrdinal() const {
     return std::max(1, std::max(last_animation_started_ordinal_,
@@ -121,12 +168,18 @@ class TestInkDropAnimationObserverHelper {
   // The ordinal time of the last AnimationStarted() call.
   int last_animation_started_ordinal_;
 
+  // List of contexts for which animation is started.
+  std::vector<ContextType> animation_started_contexts_;
+
   // The |context| passed to the last call to AnimationStarted().
   ContextType last_animation_started_context_;
 
   // The ordinal time of the last AnimationEnded() call.
   int last_animation_ended_ordinal_;
 
+  // List of contexts for which animation is ended.
+  std::vector<ContextType> animation_ended_contexts_;
+
   // The |context| passed to the last call to AnimationEnded().
   ContextType last_animation_ended_context_;