compositor/layer_animator: handle delegate removal in LayerAnimation::StopAnimating

ui/compositor/layer_animator_unittest.cc

Index: ui/compositor/layer_animator_unittest.cc
diff --git a/ui/compositor/layer_animator_unittest.cc b/ui/compositor/layer_animator_unittest.cc
index 99d2426c0299fc6bf3c110c48729aa7ae06aa36e..b94a2f704604ae9a33d631a7d712ce94a8574ae7 100644
--- a/ui/compositor/layer_animator_unittest.cc
+++ b/ui/compositor/layer_animator_unittest.cc
@@ -2611,4 +2611,76 @@ TEST(LayerAnimatorTest, LayerMovedBetweenCompositorsDuringAnimation) {
   TerminateContextFactoryForTests();
 }
 
+class CompletionOrderAnimationObserver : public LayerAnimationObserver {
+ public:
+

[sky, 2014/12/12 18:05:15]

nit: no newline here.

+  CompletionOrderAnimationObserver() {};

[sky, 2014/12/12 18:05:15]

no ;

+
+  void OnLayerAnimationEnded(LayerAnimationSequence* sequence) override {
+    ASSERT_TRUE(sequence);
+    completion_order_.push_back(sequence);
+  }
+
+  void OnLayerAnimationAborted(LayerAnimationSequence* sequence) override {
+    ASSERT_TRUE(sequence);
+    completion_order_.push_back(sequence);
+  }
+
+  const std::vector<LayerAnimationSequence *>& GetCompletionOrder() {

[sky, 2014/12/12 18:05:15]

Make this function const too and name completion_order().

+    return completion_order_;
+  }
+
+  void OnLayerAnimationScheduled(LayerAnimationSequence* sequence) override {}

[sky, 2014/12/12 18:05:15]

Keep the LayerAnimationObserver methods together. Style guid says, "When you
derive from a base class, group any overriding functions in your header file in
one labeled section".

+
+ private:
+  std::vector<LayerAnimationSequence *> completion_order_;
+
+  DISALLOW_COPY_AND_ASSIGN(CompletionOrderAnimationObserver);
+};
+
+// This test illustrates completion order of animations interrupted by
+// LayerAnimator::StopAnimating().
+TEST(LayerAnimatorTest, StopAnimatingCompletionOrder) {

[sky, 2014/12/12 18:05:15]

This is an interesting test, but what you want is a test that fails without your
patch and passes with your patch.

+  scoped_refptr<LayerAnimator> animator(
+      LayerAnimator::CreateImplicitAnimator());
+  animator->set_disable_timer_for_test(true);
+  TestLayerAnimationDelegate delegate;
+  animator->SetDelegate(&delegate);
+  CompletionOrderAnimationObserver observer;
+  animator->AddObserver(&observer);
+
+  const gfx::Rect start_bounds(0, 0, 50, 50);
+  const gfx::Rect target_bounds(10, 10, 100, 100);
+  const double target_opacity = 1.0;
+
+  delegate.SetOpacityFromAnimation(0.0f);
+  delegate.SetBoundsFromAnimation(start_bounds);
+
+  animator->SchedulePauseForProperties(base::TimeDelta::FromMilliseconds(100),
+                                       LayerAnimationElement::OPACITY);
+
+  base::TimeDelta time_delta = base::TimeDelta::FromSeconds(1);
+  LayerAnimationSequence* opacity = new LayerAnimationSequence(
+      LayerAnimationElement::CreateOpacityElement(target_opacity, time_delta));
+  animator->ScheduleAnimation(opacity);
+  time_delta = base::TimeDelta::FromSeconds(2);
+  LayerAnimationSequence* move = new LayerAnimationSequence(
+      LayerAnimationElement::CreateBoundsElement(target_bounds, time_delta));
+  animator->ScheduleAnimation(move);
+  EXPECT_TRUE(animator->is_animating());
+  animator->StopAnimating();
+
+  auto completion_order = observer.GetCompletionOrder();
+  EXPECT_NE(
+      std::find(completion_order.begin(), completion_order.end(), opacity),
+      completion_order.end());
+  EXPECT_NE(std::find(completion_order.begin(), completion_order.end(), move),
+            completion_order.end());
+  EXPECT_GT(
+      std::find(completion_order.begin(), completion_order.end(), opacity),
+      std::find(completion_order.begin(), completion_order.end(), move));
+
+  animator->RemoveObserver(&observer);
+}
+
 }  // namespace ui