Disable animations during aura tests.

This causes all animations scheduled during a test to complete immediately.

After making this change, I noticed some code assumed that animations would not complete synchronously. Some of this code used animation observers, and I while fixing the code I have updated it to use the preferred ImplicitAnimationObserver.

BUG=None
TEST=aura_shell_unittests,aura_unittests,compositor_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=120074

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=120092

Review URL: https://chromiumcodereview.appspot.com/9222018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@120147 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 218 insertions, 123 deletions

diff --git a/ui/aura/root_window.cc b/ui/aura/root_window.cc
index b2f380e..0b4baa3 100644
--- a/ui/aura/root_window.cc
+++ b/ui/aura/root_window.cc
@@ -676,16 +676,16 @@ internal::FocusManager* RootWindow::GetFocusManager() {
 }
 
 void RootWindow::OnLayerAnimationEnded(
-    const ui::LayerAnimationSequence* animation) {
+    ui::LayerAnimationSequence* animation) {
   OnHostResized(host_->GetSize());
 }
 
 void RootWindow::OnLayerAnimationScheduled(
-    const ui::LayerAnimationSequence* animation) {
+    ui::LayerAnimationSequence* animation) {
 }
 
 void RootWindow::OnLayerAnimationAborted(
-    const ui::LayerAnimationSequence* animation) {
+    ui::LayerAnimationSequence* animation) {
 }
 
 void RootWindow::SetFocusedWindow(Window* focused_window) {
diff --git a/ui/aura/root_window.h b/ui/aura/root_window.h
index ec0f09f..4f5bd4f 100644
--- a/ui/aura/root_window.h
+++ b/ui/aura/root_window.h
@@ -219,11 +219,11 @@ class AURA_EXPORT RootWindow : public ui::CompositorDelegate,
 
   // Overridden from ui::LayerAnimationObserver:
   virtual void OnLayerAnimationEnded(
-      const ui::LayerAnimationSequence* animation) OVERRIDE;
+      ui::LayerAnimationSequence* animation) OVERRIDE;
   virtual void OnLayerAnimationScheduled(
-      const ui::LayerAnimationSequence* animation) OVERRIDE;
+      ui::LayerAnimationSequence* animation) OVERRIDE;
   virtual void OnLayerAnimationAborted(
-      const ui::LayerAnimationSequence* animation) OVERRIDE;
+      ui::LayerAnimationSequence* animation) OVERRIDE;
 
   // Overridden from FocusManager:
   virtual void SetFocusedWindow(Window* window) OVERRIDE;
diff --git a/ui/aura/test/aura_test_base.cc b/ui/aura/test/aura_test_base.cc
index 70d33f5..3e1e95f 100644
--- a/ui/aura/test/aura_test_base.cc
+++ b/ui/aura/test/aura_test_base.cc
@@ -9,6 +9,7 @@
 #endif
 
 #include "ui/aura/root_window.h"
+#include "ui/gfx/compositor/layer_animator.h"
 
 namespace aura {
 namespace test {
@@ -22,6 +23,9 @@ AuraTestBase::AuraTestBase()
 
   RootWindow::GetInstance()->Show();
   RootWindow::GetInstance()->SetHostSize(gfx::Size(600, 600));
+
+  // Disable animations during tests.
+  ui::LayerAnimator::set_disable_animations_for_test(true);
 }
 
 AuraTestBase::~AuraTestBase() {
diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
index 1e59c2d..78b9142 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -933,6 +933,9 @@ TEST_F(WindowTest, Property) {
 }
 
 TEST_F(WindowTest, SetBoundsInternalShouldCheckTargetBounds) {
+  // We cannot short-circuit animations in this test.
+  ui::LayerAnimator::set_disable_animations_for_test(false);
+
   scoped_ptr<Window> w1(
       CreateTestWindowWithBounds(gfx::Rect(0, 0, 100, 100), NULL));
 
@@ -966,7 +969,7 @@ TEST_F(WindowTest, SetBoundsInternalShouldCheckTargetBounds) {
 
   // Confirm that the target bounds are reached.
   base::TimeTicks start_time =
-      w1->layer()->GetAnimator()->get_last_step_time_for_test();
+      w1->layer()->GetAnimator()->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
diff --git a/ui/gfx/compositor/layer_animation_observer.cc b/ui/gfx/compositor/layer_animation_observer.cc
index 23217a5..a477bde 100644
--- a/ui/gfx/compositor/layer_animation_observer.cc
+++ b/ui/gfx/compositor/layer_animation_observer.cc
@@ -11,14 +11,26 @@ namespace ui {
 ////////////////////////////////////////////////////////////////////////////////
 // LayerAnimationObserver
 
+LayerAnimationObserver::LayerAnimationObserver() {
+}
+
+LayerAnimationObserver::~LayerAnimationObserver() {
+  StopObserving();
+}
+
 bool LayerAnimationObserver::RequiresNotificationWhenAnimatorDestroyed() const {
   return false;
 }
 
-LayerAnimationObserver::LayerAnimationObserver() {
+void LayerAnimationObserver::OnAttachedToSequence(
+    LayerAnimationSequence* sequence) {
 }
 
-LayerAnimationObserver::~LayerAnimationObserver() {
+void LayerAnimationObserver::OnDetachedFromSequence(
+    LayerAnimationSequence* sequence) {
+}
+
+void LayerAnimationObserver::StopObserving() {
   while (!attached_sequences_.empty()) {
     LayerAnimationSequence* sequence = *attached_sequences_.begin();
     sequence->RemoveObserver(this);
@@ -29,20 +41,21 @@ void LayerAnimationObserver::AttachedToSequence(
     LayerAnimationSequence* sequence) {
   DCHECK(attached_sequences_.find(sequence) == attached_sequences_.end());
   attached_sequences_.insert(sequence);
+  OnAttachedToSequence(sequence);
 }
 
 void LayerAnimationObserver::DetachedFromSequence(
     LayerAnimationSequence* sequence) {
   if (attached_sequences_.find(sequence) != attached_sequences_.end())
     attached_sequences_.erase(sequence);
+  OnDetachedFromSequence(sequence);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ImplicitAnimationObserver
 
 ImplicitAnimationObserver::ImplicitAnimationObserver()
-    : active_(false),
-      animation_count_(0) {
+    : active_(false) {
 }
 
 ImplicitAnimationObserver::~ImplicitAnimationObserver() {}
@@ -52,26 +65,44 @@ void ImplicitAnimationObserver::SetActive(bool active) {
   CheckCompleted();
 }
 
+void ImplicitAnimationObserver::StopObservingImplicitAnimations() {
+  SetActive(false);
+  StopObserving();
+}
+
 void ImplicitAnimationObserver::OnLayerAnimationEnded(
-    const LayerAnimationSequence* sequence) {
-  animation_count_--;
+    LayerAnimationSequence* sequence) {
+  sequence->RemoveObserver(this);
+  DCHECK(attached_sequences().find(sequence) == attached_sequences().end());
   CheckCompleted();
 }
 
 void ImplicitAnimationObserver::OnLayerAnimationAborted(
-    const LayerAnimationSequence* sequence) {
-  animation_count_--;
+    LayerAnimationSequence* sequence) {
+  sequence->RemoveObserver(this);
+  DCHECK(attached_sequences().find(sequence) == attached_sequences().end());
   CheckCompleted();
 }
 
 void ImplicitAnimationObserver::OnLayerAnimationScheduled(
-      const LayerAnimationSequence* sequence) {
-  animation_count_++;
+    LayerAnimationSequence* sequence) {
+}
+
+void ImplicitAnimationObserver::OnAttachedToSequence(
+    LayerAnimationSequence* sequence) {
+}
+
+void ImplicitAnimationObserver::OnDetachedFromSequence(
+    LayerAnimationSequence* sequence) {
+  DCHECK(attached_sequences().find(sequence) == attached_sequences().end());
+  CheckCompleted();
 }
 
 void ImplicitAnimationObserver::CheckCompleted() {
-  if (active_ && animation_count_ == 0)
+  if (active_ && attached_sequences().empty()) {
     OnImplicitAnimationsCompleted();
+    active_ = false;
+  }
 }
 
 }  // namespace ui
diff --git a/ui/gfx/compositor/layer_animation_observer.h b/ui/gfx/compositor/layer_animation_observer.h
index c35dc60..a2eb911 100644
--- a/ui/gfx/compositor/layer_animation_observer.h
+++ b/ui/gfx/compositor/layer_animation_observer.h
@@ -16,22 +16,29 @@ namespace ui {
 
 class LayerAnimationSequence;
 class ScopedLayerAnimationSettings;
+class ImplicitAnimationObserver;
 
 // LayerAnimationObservers are notified when animations complete.
 class COMPOSITOR_EXPORT LayerAnimationObserver  {
  public:
   // Called when the |sequence| ends. Not called if |sequence| is aborted.
   virtual void OnLayerAnimationEnded(
-      const LayerAnimationSequence* sequence) = 0;
+      LayerAnimationSequence* sequence) = 0;
 
   // Called if |sequence| is aborted for any reason. Should never do anything
   // that may cause another animation to be started.
   virtual void OnLayerAnimationAborted(
-      const LayerAnimationSequence* sequence) = 0;
+      LayerAnimationSequence* sequence) = 0;
 
   // Called when the animation is scheduled.
   virtual void OnLayerAnimationScheduled(
-      const LayerAnimationSequence* sequence) = 0;
+      LayerAnimationSequence* sequence) = 0;
+
+ protected:
+  typedef std::set<LayerAnimationSequence*> AttachedSequences;
+
+  LayerAnimationObserver();
+  virtual ~LayerAnimationObserver();
 
   // If the animator is destroyed during an animation, the animations are
   // aborted. The resulting NotifyAborted notifications will NOT be sent to
@@ -40,9 +47,18 @@ class COMPOSITOR_EXPORT LayerAnimationObserver  {
   // OBSERVER WHEN YOU ARE DESTROYED.
   virtual bool RequiresNotificationWhenAnimatorDestroyed() const;
 
- protected:
-  LayerAnimationObserver();
-  virtual ~LayerAnimationObserver();
+  // Called when |this| is added to |sequence|'s observer list.
+  virtual void OnAttachedToSequence(LayerAnimationSequence* sequence);
+
+  // Called when |this| is removed to |sequence|'s observer list.
+  virtual void OnDetachedFromSequence(LayerAnimationSequence* sequence);
+
+  // Detaches this observer from all sequences it is currently observing.
+  void StopObserving();
+
+  const AttachedSequences& attached_sequences() const {
+    return attached_sequences_;
+  }
 
  private:
   friend class LayerAnimationSequence;
@@ -53,7 +69,7 @@ class COMPOSITOR_EXPORT LayerAnimationObserver  {
   // Called when |this| is removed to |sequence|'s observer list.
   void DetachedFromSequence(LayerAnimationSequence* sequence);
 
-  std::set<LayerAnimationSequence*> attached_sequences_;
+  AttachedSequences attached_sequences_;
 };
 
 // An implicit animation observer is intended to be used in conjunction with a
@@ -67,29 +83,33 @@ class COMPOSITOR_EXPORT ImplicitAnimationObserver
 
   virtual void OnImplicitAnimationsCompleted() = 0;
 
+ protected:
+  // Deactivates the observer and clears the collection of animations it is
+  // waiting for.
+  void StopObservingImplicitAnimations();
+
  private:
   friend class ScopedLayerAnimationSettings;
 
-  // OnImplicitAnimationsCompleted is not fired unless the observer is active.
-  bool active() const { return active_; }
-  void SetActive(bool active);
-
   // LayerAnimationObserver implementation
   virtual void OnLayerAnimationEnded(
-      const LayerAnimationSequence* sequence) OVERRIDE;
+      LayerAnimationSequence* sequence) OVERRIDE;
   virtual void OnLayerAnimationAborted(
-      const LayerAnimationSequence* sequence) OVERRIDE;
+      LayerAnimationSequence* sequence) OVERRIDE;
   virtual void OnLayerAnimationScheduled(
-      const LayerAnimationSequence* sequence) OVERRIDE;
+      LayerAnimationSequence* sequence) OVERRIDE;
+  virtual void OnAttachedToSequence(
+      LayerAnimationSequence* sequence) OVERRIDE;
+  virtual void OnDetachedFromSequence(
+      LayerAnimationSequence* sequence) OVERRIDE;
+
+  // OnImplicitAnimationsCompleted is not fired unless the observer is active.
+  bool active() const { return active_; }
+  void SetActive(bool active);
 
   void CheckCompleted();
 
   bool active_;
-
-  // This tracks the number of scheduled animations that have yet to complete.
-  // If this value is zero, and the observer is active, then
-  // OnImplicitAnimationsCompleted is fired.
-  size_t animation_count_;
 };
 
 }  // namespace ui
diff --git a/ui/gfx/compositor/layer_animator.cc b/ui/gfx/compositor/layer_animator.cc
index 82aa949..f98904c 100644
--- a/ui/gfx/compositor/layer_animator.cc
+++ b/ui/gfx/compositor/layer_animator.cc
@@ -45,6 +45,9 @@ LayerAnimator::~LayerAnimator() {
 }
 
 // static
+bool LayerAnimator::disable_animations_for_test_ = false;
+
+// static
 LayerAnimator* LayerAnimator::CreateDefaultAnimator() {
   return new LayerAnimator(base::TimeDelta::FromMilliseconds(0));
 }
@@ -55,9 +58,12 @@ LayerAnimator* LayerAnimator::CreateImplicitAnimator() {
 }
 
 void LayerAnimator::SetTransform(const Transform& transform) {
+  base::TimeDelta duration = transition_duration_;
+  if (disable_animations_for_test_)
+    duration = base::TimeDelta();
   StartAnimation(new LayerAnimationSequence(
       LayerAnimationElement::CreateTransformElement(
-          transform, transition_duration_)));
+          transform, duration)));
 }
 
 Transform LayerAnimator::GetTargetTransform() const {
@@ -67,9 +73,12 @@ Transform LayerAnimator::GetTargetTransform() const {
 }
 
 void LayerAnimator::SetBounds(const gfx::Rect& bounds) {
+  base::TimeDelta duration = transition_duration_;
+  if (disable_animations_for_test_)
+    duration = base::TimeDelta();
   StartAnimation(new LayerAnimationSequence(
       LayerAnimationElement::CreateBoundsElement(
-          bounds, transition_duration_)));
+          bounds, duration)));
 }
 
 gfx::Rect LayerAnimator::GetTargetBounds() const {
@@ -79,9 +88,12 @@ gfx::Rect LayerAnimator::GetTargetBounds() const {
 }
 
 void LayerAnimator::SetOpacity(float opacity) {
+  base::TimeDelta duration = transition_duration_;
+  if (disable_animations_for_test_)
+    duration = base::TimeDelta();
   StartAnimation(new LayerAnimationSequence(
       LayerAnimationElement::CreateOpacityElement(
-          opacity, transition_duration_)));
+          opacity, duration)));
 }
 
 float LayerAnimator::GetTargetOpacity() const {
diff --git a/ui/gfx/compositor/layer_animator.h b/ui/gfx/compositor/layer_animator.h
index 60ee25a..2231f57 100644
--- a/ui/gfx/compositor/layer_animator.h
+++ b/ui/gfx/compositor/layer_animator.h
@@ -33,6 +33,8 @@ class Transform;
 
 // When a property of layer needs to be changed it is set by way of
 // LayerAnimator. This enables LayerAnimator to animate property changes.
+// NB: during many tests, set_disable_animations_for_test is used and causes
+// all animations to complete immediately.
 class COMPOSITOR_EXPORT LayerAnimator : public AnimationContainerElement {
  public:
   enum PreemptionStrategy {
@@ -108,17 +110,22 @@ class COMPOSITOR_EXPORT LayerAnimator : public AnimationContainerElement {
   // Stops all animation and clears any queued animations.
   void StopAnimating();
 
-  // For testing purposes only.
-  void set_disable_timer_for_test(bool enabled) {
-    disable_timer_for_test_ = enabled;
-  }
-  base::TimeTicks get_last_step_time_for_test() { return last_step_time_; }
-
   // These functions are used for adding or removing observers from the observer
   // list. The observers are notified when animations end.
   void AddObserver(LayerAnimationObserver* observer);
   void RemoveObserver(LayerAnimationObserver* observer);
 
+  // For testing purposes only.
+  void set_disable_timer_for_test(bool disable_timer) {
+    disable_timer_for_test_ = disable_timer;
+  }
+  base::TimeTicks last_step_time() const { return last_step_time_; }
+
+  // When set to true, all animations complete immediately.
+  static void set_disable_animations_for_test(bool disable_animations) {
+    disable_animations_for_test_ = disable_animations;
+  }
+
  protected:
   LayerAnimationDelegate* delegate() { return delegate_; }
   const LayerAnimationDelegate* delegate() const { return delegate_; }
@@ -233,6 +240,9 @@ class COMPOSITOR_EXPORT LayerAnimator : public AnimationContainerElement {
   // and allows for manual stepping.
   bool disable_timer_for_test_;
 
+  // This causes all animations to complete immediately.
+  static bool disable_animations_for_test_;
+
   // Observers are notified when layer animations end, are scheduled or are
   // aborted.
   ObserverList<LayerAnimationObserver> observers_;
diff --git a/ui/gfx/compositor/layer_animator_unittest.cc b/ui/gfx/compositor/layer_animator_unittest.cc
index cad4a6b..09d1c66 100644
--- a/ui/gfx/compositor/layer_animator_unittest.cc
+++ b/ui/gfx/compositor/layer_animator_unittest.cc
@@ -133,7 +133,7 @@ TEST(LayerAnimatorTest, ScheduleAnimationThatCanRunImmediately) {
   EXPECT_TRUE(animator->is_animating());
   EXPECT_FLOAT_EQ(delegate.GetOpacityForAnimation(), start_opacity);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -181,7 +181,7 @@ TEST(LayerAnimatorTest, ScheduleTwoAnimationsThatCanRunImmediately) {
   EXPECT_FLOAT_EQ(delegate.GetOpacityForAnimation(), start_opacity);
   CheckApproximatelyEqual(delegate.GetBoundsForAnimation(), start_bounds);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -224,7 +224,7 @@ TEST(LayerAnimatorTest, ScheduleTwoAnimationsOnSameProperty) {
   EXPECT_TRUE(animator->is_animating());
   EXPECT_FLOAT_EQ(delegate.GetOpacityForAnimation(), start_opacity);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -292,7 +292,7 @@ TEST(LayerAnimatorTest, ScheduleBlockedAnimation) {
   EXPECT_FLOAT_EQ(delegate.GetOpacityForAnimation(), start_opacity);
   CheckApproximatelyEqual(delegate.GetBoundsForAnimation(), start_bounds);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -364,7 +364,7 @@ TEST(LayerAnimatorTest, ScheduleTogether) {
   EXPECT_FLOAT_EQ(delegate.GetOpacityForAnimation(), start_opacity);
   CheckApproximatelyEqual(delegate.GetBoundsForAnimation(), start_bounds);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
@@ -403,7 +403,7 @@ TEST(LayerAnimatorTest, StartAnimationThatCanRunImmediately) {
   EXPECT_TRUE(animator->is_animating());
   EXPECT_FLOAT_EQ(delegate.GetOpacityForAnimation(), start_opacity);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -467,7 +467,7 @@ TEST(LayerAnimatorTest, PreemptByImmediatelyAnimatingToNewTarget) {
       new LayerAnimationSequence(
           LayerAnimationElement::CreateOpacityElement(target_opacity, delta)));
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -518,7 +518,7 @@ TEST(LayerAnimatorTest, PreemptEnqueueNewAnimation) {
       new LayerAnimationSequence(
           LayerAnimationElement::CreateOpacityElement(target_opacity, delta)));
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -571,7 +571,7 @@ TEST(LayerAnimatorTest, PreemptyByReplacingQueuedAnimations) {
       new LayerAnimationSequence(
           LayerAnimationElement::CreateOpacityElement(target_opacity, delta)));
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -630,7 +630,7 @@ TEST(LayerAnimatorTest, CyclicSequences) {
 
   animator->StartAnimation(sequence.release());
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
@@ -687,7 +687,7 @@ TEST(LayerAnimatorTest, AddObserverExplicit) {
 
   EXPECT_EQ(observer.last_scheduled_sequence(), sequence);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
@@ -704,45 +704,6 @@ TEST(LayerAnimatorTest, AddObserverExplicit) {
   EXPECT_EQ(observer.last_aborted_sequence(), sequence);
 }
 
-TEST(LayerAnimatorTest, AddObserverImplicit) {
-  scoped_ptr<LayerAnimator> animator(LayerAnimator::CreateDefaultAnimator());
-  AnimationContainerElement* element = animator.get();
-  animator->set_disable_timer_for_test(true);
-  TestLayerAnimationObserver observer;
-  TestLayerAnimationDelegate delegate;
-  animator->SetDelegate(&delegate);
-  animator->AddObserver(&observer);
-
-  // Should end a sequence with the default animator.
-  EXPECT_TRUE(!observer.last_ended_sequence());
-  animator->SetOpacity(1.0f);
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
-  EXPECT_TRUE(observer.last_ended_sequence());
-
-  TestLayerAnimationObserver scoped_observer;
-  {
-    ScopedLayerAnimationSettings settings(animator.get());
-    settings.AddObserver(&scoped_observer);
-    for (int i = 0; i < 2; ++i) {
-      // reset the observer
-      scoped_observer = TestLayerAnimationObserver();
-      EXPECT_TRUE(!scoped_observer.last_ended_sequence());
-      animator->SetOpacity(1.0f);
-      start_time = animator->get_last_step_time_for_test();
-      element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
-      EXPECT_FALSE(!scoped_observer.last_ended_sequence());
-    }
-  }
-
-  scoped_observer = TestLayerAnimationObserver();
-  EXPECT_TRUE(!scoped_observer.last_ended_sequence());
-  animator->SetOpacity(1.0f);
-  start_time = base::TimeTicks::Now();
-  element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
-  EXPECT_TRUE(!scoped_observer.last_ended_sequence());
-}
-
 // Tests that an observer added to a scoped settings object is still notified
 // when the object goes out of scope.
 TEST(LayerAnimatorTest, ImplicitAnimationObservers) {
@@ -758,12 +719,12 @@ TEST(LayerAnimatorTest, ImplicitAnimationObservers) {
 
   {
     ScopedLayerAnimationSettings settings(animator.get());
-    settings.AddImplicitObserver(&observer);
+    settings.AddObserver(&observer);
     animator->SetOpacity(0.0f);
   }
 
   EXPECT_FALSE(observer.animations_completed());
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
   EXPECT_TRUE(observer.animations_completed());
   EXPECT_FLOAT_EQ(0.0f, delegate.GetOpacityForAnimation());
@@ -783,7 +744,7 @@ TEST(LayerAnimatorTest, InterruptedImplicitAnimationObservers) {
 
   {
     ScopedLayerAnimationSettings settings(animator.get());
-    settings.AddImplicitObserver(&observer);
+    settings.AddObserver(&observer);
     animator->SetOpacity(0.0f);
   }
 
@@ -822,7 +783,7 @@ TEST(LayerAnimatorTest, RemoveObserverShouldRemoveFromSequences) {
   // This should stop the observer from observing sequence.
   animator->RemoveObserver(&removed_observer);
 
-  base::TimeTicks start_time = animator->get_last_step_time_for_test();
+  base::TimeTicks start_time = animator->last_step_time();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
@@ -858,6 +819,70 @@ TEST(LayerAnimatorTest, ObserverReleasedBeforeAnimationSequenceEnds) {
   EXPECT_EQ(static_cast<size_t>(0), sequence->observers_.size());
 }
 
+TEST(LayerAnimatorTest, ObserverAttachedAfterAnimationStarted) {
+  scoped_ptr<LayerAnimator> animator(LayerAnimator::CreateDefaultAnimator());
+  AnimationContainerElement* element = animator.get();
+  animator->set_disable_timer_for_test(true);
+
+  TestImplicitAnimationObserver observer;
+  TestLayerAnimationDelegate delegate;
+  animator->SetDelegate(&delegate);
+
+  delegate.SetOpacityFromAnimation(0.0f);
+
+  {
+    ScopedLayerAnimationSettings setter(animator.get());
+
+    base::TimeDelta delta = base::TimeDelta::FromSeconds(1);
+    LayerAnimationSequence* sequence = new LayerAnimationSequence(
+        LayerAnimationElement::CreateOpacityElement(1.0f, delta));
+
+    animator->StartAnimation(sequence);
+    base::TimeTicks start_time = animator->last_step_time();
+    element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
+
+    setter.AddObserver(&observer);
+
+    // Start observing an in-flight animation.
+    sequence->AddObserver(&observer);
+
+    element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
+  }
+
+  EXPECT_TRUE(observer.animations_completed());
+}
+
+TEST(LayerAnimatorTest, ObserverDetachedBeforeAnimationFinished) {
+  scoped_ptr<LayerAnimator> animator(LayerAnimator::CreateDefaultAnimator());
+  AnimationContainerElement* element = animator.get();
+  animator->set_disable_timer_for_test(true);
+
+  TestImplicitAnimationObserver observer;
+  TestLayerAnimationDelegate delegate;
+  animator->SetDelegate(&delegate);
+
+  delegate.SetOpacityFromAnimation(0.0f);
+  base::TimeDelta delta = base::TimeDelta::FromSeconds(1);
+  LayerAnimationSequence* sequence = new LayerAnimationSequence(
+      LayerAnimationElement::CreateOpacityElement(1.0f, delta));
+
+  {
+    ScopedLayerAnimationSettings setter(animator.get());
+    setter.AddObserver(&observer);
+
+    animator->StartAnimation(sequence);
+    base::TimeTicks start_time = animator->last_step_time();
+    element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
+  }
+
+  EXPECT_FALSE(observer.animations_completed());
+
+  // Stop observing an in-flight animation.
+  sequence->RemoveObserver(&observer);
+
+  EXPECT_TRUE(observer.animations_completed());
+}
+
 // Check that setting a property during an animation with a default animator
 // cancels the original animation.
 TEST(LayerAnimatorTest, SettingPropertyDuringAnAnimation) {
diff --git a/ui/gfx/compositor/scoped_layer_animation_settings.cc b/ui/gfx/compositor/scoped_layer_animation_settings.cc
index 874b927..7a7dbc1 100644
--- a/ui/gfx/compositor/scoped_layer_animation_settings.cc
+++ b/ui/gfx/compositor/scoped_layer_animation_settings.cc
@@ -26,27 +26,19 @@ ScopedLayerAnimationSettings::ScopedLayerAnimationSettings(
 ScopedLayerAnimationSettings::~ScopedLayerAnimationSettings() {
   animator_->transition_duration_ = old_transition_duration_;
 
-  for (std::set<LayerAnimationObserver*>::const_iterator i =
-       observers_.begin(); i != observers_.end(); ++i)
-      animator_->observers_.RemoveObserver(*i);
-
   for (std::set<ImplicitAnimationObserver*>::const_iterator i =
-       implicit_observers_.begin(); i != implicit_observers_.end(); ++i)
-      (*i)->SetActive(true);
+       observers_.begin(); i != observers_.end(); ++i) {
+    animator_->observers_.RemoveObserver(*i);
+    (*i)->SetActive(true);
+  }
 }
 
 void ScopedLayerAnimationSettings::AddObserver(
-    LayerAnimationObserver* observer) {
+    ImplicitAnimationObserver* observer) {
   observers_.insert(observer);
   animator_->AddObserver(observer);
 }
 
-void ScopedLayerAnimationSettings::AddImplicitObserver(
-    ImplicitAnimationObserver* observer) {
-  implicit_observers_.insert(observer);
-  AddObserver(observer);
-}
-
 void ScopedLayerAnimationSettings::SetTransitionDuration(
     base::TimeDelta duration) {
   animator_->transition_duration_ = duration;
diff --git a/ui/gfx/compositor/scoped_layer_animation_settings.h b/ui/gfx/compositor/scoped_layer_animation_settings.h
index 2f90055..dce64ef 100644
--- a/ui/gfx/compositor/scoped_layer_animation_settings.h
+++ b/ui/gfx/compositor/scoped_layer_animation_settings.h
@@ -27,15 +27,13 @@ class COMPOSITOR_EXPORT ScopedLayerAnimationSettings {
   explicit ScopedLayerAnimationSettings(LayerAnimator* animator);
   virtual ~ScopedLayerAnimationSettings();
 
-  void AddObserver(LayerAnimationObserver* observer);
-  void AddImplicitObserver(ImplicitAnimationObserver* observer);
+  void AddObserver(ImplicitAnimationObserver* observer);
   void SetTransitionDuration(base::TimeDelta duration);
 
  private:
   LayerAnimator* animator_;
   base::TimeDelta old_transition_duration_;
-  std::set<LayerAnimationObserver*> observers_;
-  std::set<ImplicitAnimationObserver*> implicit_observers_;
+  std::set<ImplicitAnimationObserver*> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedLayerAnimationSettings);
 };
diff --git a/ui/gfx/compositor/test/test_layer_animation_observer.cc b/ui/gfx/compositor/test/test_layer_animation_observer.cc
index fe515d3..2afcb1b 100644
--- a/ui/gfx/compositor/test/test_layer_animation_observer.cc
+++ b/ui/gfx/compositor/test/test_layer_animation_observer.cc
@@ -19,17 +19,17 @@ TestLayerAnimationObserver::~TestLayerAnimationObserver() {
 }
 
 void TestLayerAnimationObserver::OnLayerAnimationEnded(
-    const LayerAnimationSequence* sequence) {
+    LayerAnimationSequence* sequence) {
   last_ended_sequence_ = sequence;
 }
 
 void TestLayerAnimationObserver::OnLayerAnimationAborted(
-    const LayerAnimationSequence* sequence) {
+    LayerAnimationSequence* sequence) {
   last_aborted_sequence_ = sequence;
 }
 
 void TestLayerAnimationObserver::OnLayerAnimationScheduled(
-    const LayerAnimationSequence* sequence) {
+    LayerAnimationSequence* sequence) {
   last_scheduled_sequence_ = sequence;
 }
 
diff --git a/ui/gfx/compositor/test/test_layer_animation_observer.h b/ui/gfx/compositor/test/test_layer_animation_observer.h
index b90a9dc..44a7119 100644
--- a/ui/gfx/compositor/test/test_layer_animation_observer.h
+++ b/ui/gfx/compositor/test/test_layer_animation_observer.h
@@ -21,13 +21,13 @@ class TestLayerAnimationObserver : public LayerAnimationObserver {
   virtual ~TestLayerAnimationObserver();
 
   virtual void OnLayerAnimationEnded(
-      const LayerAnimationSequence* sequence) OVERRIDE;
+      LayerAnimationSequence* sequence) OVERRIDE;
 
   virtual void OnLayerAnimationAborted(
-      const LayerAnimationSequence* sequence) OVERRIDE;
+      LayerAnimationSequence* sequence) OVERRIDE;
 
   virtual void OnLayerAnimationScheduled(
-      const LayerAnimationSequence* sequence) OVERRIDE;
+      LayerAnimationSequence* sequence) OVERRIDE;
 
   virtual bool RequiresNotificationWhenAnimatorDestroyed() const OVERRIDE;