Fix double ripple on activated flood fill ripple

ui/views/animation/flood_fill_ink_drop_ripple_unittest.cc

 // Copyright 2016 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/flood_fill_ink_drop_ripple.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/views/animation/test/flood_fill_ink_drop_ripple_test_api.h"
+#include "ui/views/animation/test/test_ink_drop_ripple_observer.h"
 
 namespace views {
 namespace test {
 
 TEST(FloodFillInkDropRippleTest, TransformedCenterPointForIrregularClipBounds) {
   const gfx::Size host_size(48, 50);
   const gfx::Insets clip_insets(9, 8);
   const gfx::Point requested_center_point(25, 24);
 
   // |expected_center_point| is in the coordinate space of ripple's clip bounds
@@ skipping 37 matching lines @@
   EXPECT_NEAR(110.79f, test_api.MaxDistanceToCorners(gfx::Point(3, 5)),
               kAbsError);
   EXPECT_NEAR(108.17f, test_api.MaxDistanceToCorners(gfx::Point(70, 10)),
               kAbsError);
   EXPECT_NEAR(103.08f, test_api.MaxDistanceToCorners(gfx::Point(75, 110)),
               kAbsError);
   EXPECT_NEAR(101.24f, test_api.MaxDistanceToCorners(gfx::Point(5, 115)),
               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;
+
+  TestInkDropRippleObserver observer;
+  FloodFillInkDropRipple ripple(host_size, center_point, color,
+                                visible_opacity);
+  ripple.set_observer(&observer);

[bruthig, 2017/01/10 17:08:03]

nit: Can you add a TODO in InkDropRipple to handle null observers? Given that
this change is targetting M56 I think it's better to keep this |observer| in the
test but longer term we probably shouldn't require this overhead.

[mohsen, 2017/01/10 22:28:16]

It's a very small change to add null check to InkDropRipple. Done.

+  FloodFillInkDropRippleTestApi test_api(&ripple);
+
+  // Go to ACTIVATED state directly.
+  ripple.AnimateToState(InkDropState::ACTIVATED);
+  test_api.CompleteAnimations();
+  float activated_opacity = test_api.GetCurrentOpacity();

[bruthig, 2017/01/10 17:08:03]

nit: Might be a good idea to mark these as const.

[mohsen, 2017/01/10 22:28:16]

Done.

+  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();
+  float pending_activated_opacity = test_api.GetCurrentOpacity();
+  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