[ash-md] Enhanced LayerAnimator to allow animations to be aborted/started in OnLayerAnimationStarte…

ui/compositor/layer_animator_unittest.cc

 // Copyright (c) 2012 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/compositor/layer_animator.h"
 
 #include <memory>
+#include <string>
+#include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animation_delegate.h"
 #include "ui/compositor/layer_animation_element.h"
 #include "ui/compositor/layer_animation_sequence.h"
@@ skipping 120 matching lines @@
 
   bool animations_completed_;
   bool notify_when_animator_destructed_;
 
   DISALLOW_COPY_AND_ASSIGN(TestImplicitAnimationObserver);
 };
 
 // When notified that an animation has ended, stops all other animations.
 class DeletingLayerAnimationObserver : public LayerAnimationObserver {
  public:
-  DeletingLayerAnimationObserver(LayerAnimator* animator)
-    : animator_(animator) {
-  }
+  explicit DeletingLayerAnimationObserver(LayerAnimator* animator)
+      : animator_(animator) {}
 
   void OnLayerAnimationEnded(LayerAnimationSequence* sequence) override {
     animator_->StopAnimating();
   }
 
   void OnLayerAnimationAborted(LayerAnimationSequence* sequence) override {
     animator_->StopAnimating();
   }
 
   void OnLayerAnimationScheduled(LayerAnimationSequence* sequence) override {}
 
  private:
   LayerAnimator* animator_;
 
   DISALLOW_COPY_AND_ASSIGN(DeletingLayerAnimationObserver);
 };
 
+// When notified that an animation has started, aborts all animations.
+class AbortAnimationsOnStartedLayerAnimationObserver
+    : public LayerAnimationObserver {
+ public:
+  explicit AbortAnimationsOnStartedLayerAnimationObserver(
+      LayerAnimator* animator)
+      : animator_(animator) {}
+
+  void OnLayerAnimationStarted(LayerAnimationSequence* sequence) override {
+    animator_->AbortAllAnimations();
+  }
+
+  void OnLayerAnimationEnded(LayerAnimationSequence* sequence) override {}
+
+  void OnLayerAnimationAborted(LayerAnimationSequence* sequence) override {}
+
+  void OnLayerAnimationScheduled(LayerAnimationSequence* sequence) override {}
+
+ private:
+  LayerAnimator* animator_;
+
+  DISALLOW_COPY_AND_ASSIGN(AbortAnimationsOnStartedLayerAnimationObserver);
+};
+
 class LayerAnimatorDestructionObserver {
  public:
   LayerAnimatorDestructionObserver() : animator_deleted_(false) {}
   virtual ~LayerAnimatorDestructionObserver() {}
 
   void NotifyAnimatorDeleted() {
     animator_deleted_ = true;
   }
 
   bool IsAnimatorDeleted() {
@@ skipping 47 matching lines @@
   }
 
   ~TestLayerAnimationSequence() override { (*num_live_instances_)--; }
 
  private:
   int* num_live_instances_;
 
   DISALLOW_COPY_AND_ASSIGN(TestLayerAnimationSequence);
 };
 
-} // namespace
+}  // namespace
+
+namespace test {
+
+// Tracks the number of calls to the LayerAnimationObserver overrides and fails
+// CHECKS if the counts are inconsistent, e.g. the number of started sequences
+// is greater than the number of attached sequences.
+class CountCheckingLayerAnimationObserver : public LayerAnimationObserver {
+ public:
+  explicit CountCheckingLayerAnimationObserver(LayerAnimationObserver* observer)
+      : observer_(observer) {}
+
+  void OnLayerAnimationStarted(LayerAnimationSequence* sequence) override {
+    ++started_count_;
+    CHECK_LE(started_count_, attached_sequence_count_);

[ajuma, 2016/11/04 22:49:40]

Given that this is happening inside a test, should these be EXPECTs rather than
CHECKs?

[bruthig, 2016/11/04 22:58:20]

Done.

+    CHECK_LE(started_count_, sequences_scheduled_);
+    if (observer_)
+      observer_->OnLayerAnimationStarted(sequence);
+  }
+
+  void OnLayerAnimationEnded(LayerAnimationSequence* sequence) override {
+    ++successful_count_;
+    CHECK_LE(GetNumSequencesCompleted(), sequences_scheduled_)
+        << " aborted_count_=" << aborted_count_
+        << " successful_count_=" << successful_count_;
+    CHECK_LE(GetNumSequencesCompleted(), attached_sequence_count_)
+        << " aborted_count_=" << aborted_count_
+        << " successful_count_=" << successful_count_;
+    if (observer_)
+      observer_->OnLayerAnimationEnded(sequence);
+  }
+
+  void OnLayerAnimationAborted(LayerAnimationSequence* sequence) override {
+    ++aborted_count_;
+    CHECK_LE(GetNumSequencesCompleted(), sequences_scheduled_)
+        << " aborted_count_=" << aborted_count_
+        << " successful_count_=" << successful_count_;
+    CHECK_LE(GetNumSequencesCompleted(), attached_sequence_count_)
+        << " aborted_count_=" << aborted_count_
+        << " successful_count_=" << successful_count_;
+    if (observer_)
+      observer_->OnLayerAnimationAborted(sequence);
+  }
+
+  void OnLayerAnimationScheduled(LayerAnimationSequence* sequence) override {
+    ++sequences_scheduled_;
+    CHECK_LE(sequences_scheduled_, attached_sequence_count_);
+    if (observer_)
+      observer_->OnLayerAnimationScheduled(sequence);
+  }
+
+  int GetNumSequencesCompleted() { return aborted_count_ + successful_count_; }
+
+ protected:
+  void OnAttachedToSequence(LayerAnimationSequence* sequence) override {
+    ++attached_sequence_count_;
+    if (observer_)
+      observer_->OnAttachedToSequence(sequence);
+  }
+
+  void OnDetachedFromSequence(LayerAnimationSequence* sequence) override {
+    ++detached_sequence_count_;
+    CHECK_LE(detached_sequence_count_, attached_sequence_count_);
+    CHECK_LE(detached_sequence_count_, GetNumSequencesCompleted())
+        << " aborted_count_=" << aborted_count_
+        << " successful_count_=" << successful_count_;
+    if (observer_)
+      observer_->OnDetachedFromSequence(sequence);
+  }
+
+ private:
+  // Observer to delegate calls to.

[ajuma, 2016/11/04 22:49:40]

nit: "the delegate"

[bruthig, 2016/11/04 22:58:20]

Not exactly sure what you are suggesting.  I've updated it to hopefully be more
clear.

+  LayerAnimationObserver* observer_;
+
+  // The total number of animation sequences that have been attached.
+  int attached_sequence_count_ = 0;
+
+  // The total number of animation sequences that have been detached.
+  int detached_sequence_count_ = 0;
+
+  // The total number of animation sequences that have been scheduled.
+  int sequences_scheduled_ = 0;
+
+  // The number of animation sequences that have been started.
+  int started_count_ = 0;
+
+  // The number of animation sequences that were aborted.
+  int aborted_count_ = 0;
+
+  // The number of animation sequences that completed successfully.
+  int successful_count_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(CountCheckingLayerAnimationObserver);
+};
+
+}  // namespace test
 
 // Checks that setting a property on an implicit animator causes an animation to
 // happen.
 TEST(LayerAnimatorTest, ImplicitAnimation) {
   TestLayerAnimationDelegate delegate;
   scoped_refptr<LayerAnimator> animator(CreateImplicitTestAnimator(&delegate));
   base::TimeTicks now = base::TimeTicks::Now();
   animator->SetBrightness(0.5);
   EXPECT_TRUE(animator->is_animating());
   animator->Step(now + base::TimeDelta::FromSeconds(1));
@@ skipping 1890 matching lines @@
   animator->SchedulePauseForProperties(
       base::TimeDelta::FromMilliseconds(100),
       LayerAnimationElement::TRANSFORM | LayerAnimationElement::BOUNDS);
   EXPECT_TRUE(animator->IsAnimatingProperty(LayerAnimationElement::TRANSFORM));
   EXPECT_TRUE(animator->IsAnimatingProperty(LayerAnimationElement::BOUNDS));
   EXPECT_FALSE(animator->IsAnimatingProperty(LayerAnimationElement::OPACITY));
 }
 
 
 class AnimatorOwner {
-public:
- AnimatorOwner() : animator_(CreateDefaultTestAnimator()) {}
+ public:
+  AnimatorOwner() : animator_(CreateDefaultTestAnimator()) {}
 
   LayerAnimator* animator() { return animator_.get(); }
 
-private:
+ private:
   scoped_refptr<LayerAnimator> animator_;
 
   DISALLOW_COPY_AND_ASSIGN(AnimatorOwner);
 };
 
 class DeletingObserver : public LayerAnimationObserver {
-public:
-  DeletingObserver(bool* was_deleted)
+ public:
+  explicit DeletingObserver(bool* was_deleted)
       : animator_owner_(new AnimatorOwner),
         delete_on_animation_ended_(false),
         delete_on_animation_aborted_(false),
         delete_on_animation_scheduled_(false),
         was_deleted_(was_deleted) {
     animator()->AddObserver(this);
   }
 
   ~DeletingObserver() override {
     animator()->RemoveObserver(this);
@@ skipping 32 matching lines @@
   void OnLayerAnimationAborted(LayerAnimationSequence* sequence) override {
     if (delete_on_animation_aborted_)
       delete this;
   }
 
   void OnLayerAnimationScheduled(LayerAnimationSequence* sequence) override {
     if (delete_on_animation_scheduled_)
       delete this;
   }
 
-private:
- std::unique_ptr<AnimatorOwner> animator_owner_;
- bool delete_on_animation_ended_;
- bool delete_on_animation_aborted_;
- bool delete_on_animation_scheduled_;
- bool* was_deleted_;
+ private:
+  std::unique_ptr<AnimatorOwner> animator_owner_;
+  bool delete_on_animation_ended_;
+  bool delete_on_animation_aborted_;
+  bool delete_on_animation_scheduled_;
+  bool* was_deleted_;
 
- DISALLOW_COPY_AND_ASSIGN(DeletingObserver);
+  DISALLOW_COPY_AND_ASSIGN(DeletingObserver);
 };
 
 TEST(LayerAnimatorTest, ObserverDeletesAnimatorAfterFinishingAnimation) {
   bool observer_was_deleted = false;
   DeletingObserver* observer = new DeletingObserver(&observer_was_deleted);
   observer->set_delete_on_animation_ended(true);
   observer->set_delete_on_animation_aborted(true);
   LayerAnimator* animator = observer->animator();
   TestLayerAnimationDelegate delegate;
   animator->SetDelegate(&delegate);
@@ skipping 175 matching lines @@
   collection->StartAnimator(layer.GetAnimator());
   EXPECT_TRUE(collection->HasActiveAnimators());
   EXPECT_FALSE(collection->last_tick_time().is_null());
 
   collection->StopAnimator(layer.GetAnimator());
   EXPECT_FALSE(collection->HasActiveAnimators());
 
   animator->SetDelegate(nullptr);
 }
 
+// Verifies that sequences are removed from the animation queue prior to be
+// notified of being started. This will allow observers to abort animations in
+// the OnLayerAnimationStarted() callback without triggering a sequence to be
+// started a second time.
+TEST(LayerAnimatorTest,
+     SequencesAreRemovedFromQueueBeforeBeingNotifiedOfStarted) {
+  TestLayerAnimationDelegate delegate;
+  scoped_refptr<LayerAnimator> animator(CreateDefaultTestAnimator(&delegate));
+
+  AbortAnimationsOnStartedLayerAnimationObserver aborting_observer(
+      animator.get());
+  test::CountCheckingLayerAnimationObserver counting_observer(
+      &aborting_observer);
+
+  const base::TimeDelta sequence_duration = base::TimeDelta::FromSeconds(1);
+
+  animator->ScheduleAnimation(new LayerAnimationSequence(
+      LayerAnimationElement::CreateBrightnessElement(1.f, sequence_duration)));
+
+  LayerAnimationSequence* observed_sequence = new LayerAnimationSequence(
+      LayerAnimationElement::CreateBrightnessElement(0.f, sequence_duration));
+  observed_sequence->AddObserver(&counting_observer);
+  animator->ScheduleAnimation(observed_sequence);
+
+  EXPECT_TRUE(animator->is_animating());
+  const base::TimeTicks start_time = animator->last_step_time();
+
+  animator->Step(start_time + sequence_duration * 3);

[ajuma, 2016/11/04 22:49:40]

Please add an EXPECT_FALSE(animator->is_animating()) here.

[bruthig, 2016/11/04 22:58:20]

Done.

+}
+
 TEST(LayerAnimatorTest, AnimatorStartedCorrectly) {
   Layer layer;
   LayerAnimatorTestController test_controller(layer.GetAnimator());
   LayerAnimator* animator = test_controller.animator();
   ASSERT_FALSE(animator->is_started_);
 
   TestLayerAnimationDelegate test_delegate;
   animator->SetDelegate(&test_delegate);
   double target_opacity = 1.0;
   base::TimeDelta time_delta = base::TimeDelta::FromSeconds(1);
@@ skipping 107 matching lines @@
 
   root.Add(&layer);
   EXPECT_TRUE(layer.cc_layer_for_testing()->HasActiveAnimationForTesting());
 
   host.reset();
   TerminateContextFactoryForTests();
 }
 
 class LayerOwnerAnimationObserver : public LayerAnimationObserver {
  public:
-  LayerOwnerAnimationObserver(LayerAnimator* animator)
+  explicit LayerOwnerAnimationObserver(LayerAnimator* animator)
       : animator_layer_(new Layer(LAYER_TEXTURED)) {
     animator_layer_->SetAnimator(animator);
   }
 
   ~LayerOwnerAnimationObserver() override {}
 
   void OnLayerAnimationEnded(LayerAnimationSequence* sequence) override {
     ASSERT_TRUE(sequence);
     animator_layer_.reset();
   }
@@ skipping 241 matching lines @@
   EXPECT_EQ(observer.last_ended_sequence(), nullptr);
   EXPECT_EQ(observer.last_detached_sequence(), first_sequence);
 
   EXPECT_TRUE(observer.AbortedEpochIsBeforeDetachedEpoch());
   EXPECT_TRUE(observer.AbortedEpochIsBeforeStartedEpoch());
   EXPECT_TRUE(observer.AttachedEpochIsBeforeScheduledEpoch());
   EXPECT_TRUE(observer.ScheduledEpochIsBeforeStartedEpoch());
 }
 
 }  // namespace ui