Revert 120092 - Reland 120074 -- 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

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

TBR=vollick@google.com
Review URL: https://chromiumcodereview.appspot.com/9320018

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

13 files changed, 123 insertions, 218 deletions

diff --git a/ui/aura/root_window.cc b/ui/aura/root_window.cc
index 0b4baa3..b2f380e 100644
--- a/ui/aura/root_window.cc
+++ b/ui/aura/root_window.cc
@@ -676,16 +676,16 @@ internal::FocusManager* RootWindow::GetFocusManager() {
 }
 
 void RootWindow::OnLayerAnimationEnded(
-    ui::LayerAnimationSequence* animation) {
+    const ui::LayerAnimationSequence* animation) {
   OnHostResized(host_->GetSize());
 }
 
 void RootWindow::OnLayerAnimationScheduled(
-    ui::LayerAnimationSequence* animation) {
+    const ui::LayerAnimationSequence* animation) {
 }
 
 void RootWindow::OnLayerAnimationAborted(
-    ui::LayerAnimationSequence* animation) {
+    const ui::LayerAnimationSequence* animation) {
 }
 
 void RootWindow::SetFocusedWindow(Window* focused_window) {
diff --git a/ui/aura/root_window.h b/ui/aura/root_window.h
index 4f5bd4f..ec0f09f 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(
-      ui::LayerAnimationSequence* animation) OVERRIDE;
+      const ui::LayerAnimationSequence* animation) OVERRIDE;
   virtual void OnLayerAnimationScheduled(
-      ui::LayerAnimationSequence* animation) OVERRIDE;
+      const ui::LayerAnimationSequence* animation) OVERRIDE;
   virtual void OnLayerAnimationAborted(
-      ui::LayerAnimationSequence* animation) OVERRIDE;
+      const 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 3e1e95f..70d33f5 100644
--- a/ui/aura/test/aura_test_base.cc
+++ b/ui/aura/test/aura_test_base.cc
@@ -9,7 +9,6 @@
 #endif
 
 #include "ui/aura/root_window.h"
-#include "ui/gfx/compositor/layer_animator.h"
 
 namespace aura {
 namespace test {
@@ -23,9 +22,6 @@ 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 78b9142..1e59c2d 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -933,9 +933,6 @@ 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));
 
@@ -969,7 +966,7 @@ TEST_F(WindowTest, SetBoundsInternalShouldCheckTargetBounds) {
 
   // Confirm that the target bounds are reached.
   base::TimeTicks start_time =
-      w1->layer()->GetAnimator()->last_step_time();
+      w1->layer()->GetAnimator()->get_last_step_time_for_test();
 
   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 a477bde..23217a5 100644
--- a/ui/gfx/compositor/layer_animation_observer.cc
+++ b/ui/gfx/compositor/layer_animation_observer.cc
@@ -11,26 +11,14 @@ namespace ui {
 ////////////////////////////////////////////////////////////////////////////////
 // LayerAnimationObserver
 
-LayerAnimationObserver::LayerAnimationObserver() {
-}
-
-LayerAnimationObserver::~LayerAnimationObserver() {
-  StopObserving();
-}
-
 bool LayerAnimationObserver::RequiresNotificationWhenAnimatorDestroyed() const {
   return false;
 }
 
-void LayerAnimationObserver::OnAttachedToSequence(
-    LayerAnimationSequence* sequence) {
-}
-
-void LayerAnimationObserver::OnDetachedFromSequence(
-    LayerAnimationSequence* sequence) {
+LayerAnimationObserver::LayerAnimationObserver() {
 }
 
-void LayerAnimationObserver::StopObserving() {
+LayerAnimationObserver::~LayerAnimationObserver() {
   while (!attached_sequences_.empty()) {
     LayerAnimationSequence* sequence = *attached_sequences_.begin();
     sequence->RemoveObserver(this);
@@ -41,21 +29,20 @@ 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) {
+    : active_(false),
+      animation_count_(0) {
 }
 
 ImplicitAnimationObserver::~ImplicitAnimationObserver() {}
@@ -65,44 +52,26 @@ void ImplicitAnimationObserver::SetActive(bool active) {
   CheckCompleted();
 }
 
-void ImplicitAnimationObserver::StopObservingImplicitAnimations() {
-  SetActive(false);
-  StopObserving();
-}
-
 void ImplicitAnimationObserver::OnLayerAnimationEnded(
-    LayerAnimationSequence* sequence) {
-  sequence->RemoveObserver(this);
-  DCHECK(attached_sequences().find(sequence) == attached_sequences().end());
+    const LayerAnimationSequence* sequence) {
+  animation_count_--;
   CheckCompleted();
 }
 
 void ImplicitAnimationObserver::OnLayerAnimationAborted(
-    LayerAnimationSequence* sequence) {
-  sequence->RemoveObserver(this);
-  DCHECK(attached_sequences().find(sequence) == attached_sequences().end());
+    const LayerAnimationSequence* sequence) {
+  animation_count_--;
   CheckCompleted();
 }
 
 void ImplicitAnimationObserver::OnLayerAnimationScheduled(
-    LayerAnimationSequence* sequence) {
-}
-
-void ImplicitAnimationObserver::OnAttachedToSequence(
-    LayerAnimationSequence* sequence) {
-}
-
-void ImplicitAnimationObserver::OnDetachedFromSequence(
-    LayerAnimationSequence* sequence) {
-  DCHECK(attached_sequences().find(sequence) == attached_sequences().end());
-  CheckCompleted();
+      const LayerAnimationSequence* sequence) {
+  animation_count_++;
 }
 
 void ImplicitAnimationObserver::CheckCompleted() {
-  if (active_ && attached_sequences().empty()) {
+  if (active_ && animation_count_ == 0)
     OnImplicitAnimationsCompleted();
-    active_ = false;
-  }
 }
 
 }  // namespace ui
diff --git a/ui/gfx/compositor/layer_animation_observer.h b/ui/gfx/compositor/layer_animation_observer.h
index a2eb911..c35dc60 100644
--- a/ui/gfx/compositor/layer_animation_observer.h
+++ b/ui/gfx/compositor/layer_animation_observer.h
@@ -16,29 +16,22 @@ 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(
-      LayerAnimationSequence* sequence) = 0;
+      const 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(
-      LayerAnimationSequence* sequence) = 0;
+      const LayerAnimationSequence* sequence) = 0;
 
   // Called when the animation is scheduled.
   virtual void OnLayerAnimationScheduled(
-      LayerAnimationSequence* sequence) = 0;
-
- protected:
-  typedef std::set<LayerAnimationSequence*> AttachedSequences;
-
-  LayerAnimationObserver();
-  virtual ~LayerAnimationObserver();
+      const LayerAnimationSequence* sequence) = 0;
 
   // If the animator is destroyed during an animation, the animations are
   // aborted. The resulting NotifyAborted notifications will NOT be sent to
@@ -47,18 +40,9 @@ class COMPOSITOR_EXPORT LayerAnimationObserver  {
   // OBSERVER WHEN YOU ARE DESTROYED.
   virtual bool RequiresNotificationWhenAnimatorDestroyed() const;
 
-  // 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_;
-  }
+ protected:
+  LayerAnimationObserver();
+  virtual ~LayerAnimationObserver();
 
  private:
   friend class LayerAnimationSequence;
@@ -69,7 +53,7 @@ class COMPOSITOR_EXPORT LayerAnimationObserver  {
   // Called when |this| is removed to |sequence|'s observer list.
   void DetachedFromSequence(LayerAnimationSequence* sequence);
 
-  AttachedSequences attached_sequences_;
+  std::set<LayerAnimationSequence*> attached_sequences_;
 };
 
 // An implicit animation observer is intended to be used in conjunction with a
@@ -83,33 +67,29 @@ 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(
-      LayerAnimationSequence* sequence) OVERRIDE;
+      const LayerAnimationSequence* sequence) OVERRIDE;
   virtual void OnLayerAnimationAborted(
-      LayerAnimationSequence* sequence) OVERRIDE;
+      const LayerAnimationSequence* sequence) OVERRIDE;
   virtual void OnLayerAnimationScheduled(
-      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);
+      const LayerAnimationSequence* sequence) OVERRIDE;
 
   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 f98904c..82aa949 100644
--- a/ui/gfx/compositor/layer_animator.cc
+++ b/ui/gfx/compositor/layer_animator.cc
@@ -45,9 +45,6 @@ LayerAnimator::~LayerAnimator() {
 }
 
 // static
-bool LayerAnimator::disable_animations_for_test_ = false;
-
-// static
 LayerAnimator* LayerAnimator::CreateDefaultAnimator() {
   return new LayerAnimator(base::TimeDelta::FromMilliseconds(0));
 }
@@ -58,12 +55,9 @@ 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, duration)));
+          transform, transition_duration_)));
 }
 
 Transform LayerAnimator::GetTargetTransform() const {
@@ -73,12 +67,9 @@ 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, duration)));
+          bounds, transition_duration_)));
 }
 
 gfx::Rect LayerAnimator::GetTargetBounds() const {
@@ -88,12 +79,9 @@ 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, duration)));
+          opacity, transition_duration_)));
 }
 
 float LayerAnimator::GetTargetOpacity() const {
diff --git a/ui/gfx/compositor/layer_animator.h b/ui/gfx/compositor/layer_animator.h
index 2231f57..60ee25a 100644
--- a/ui/gfx/compositor/layer_animator.h
+++ b/ui/gfx/compositor/layer_animator.h
@@ -33,8 +33,6 @@ 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 {
@@ -110,22 +108,17 @@ 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_; }
@@ -240,9 +233,6 @@ 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 09d1c66..cad4a6b 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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(500));
 
@@ -630,7 +630,7 @@ TEST(LayerAnimatorTest, CyclicSequences) {
 
   animator->StartAnimation(sequence.release());
 
-  base::TimeTicks start_time = animator->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   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->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
@@ -704,6 +704,45 @@ 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) {
@@ -719,12 +758,12 @@ TEST(LayerAnimatorTest, ImplicitAnimationObservers) {
 
   {
     ScopedLayerAnimationSettings settings(animator.get());
-    settings.AddObserver(&observer);
+    settings.AddImplicitObserver(&observer);
     animator->SetOpacity(0.0f);
   }
 
   EXPECT_FALSE(observer.animations_completed());
-  base::TimeTicks start_time = animator->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
   EXPECT_TRUE(observer.animations_completed());
   EXPECT_FLOAT_EQ(0.0f, delegate.GetOpacityForAnimation());
@@ -744,7 +783,7 @@ TEST(LayerAnimatorTest, InterruptedImplicitAnimationObservers) {
 
   {
     ScopedLayerAnimationSettings settings(animator.get());
-    settings.AddObserver(&observer);
+    settings.AddImplicitObserver(&observer);
     animator->SetOpacity(0.0f);
   }
 
@@ -783,7 +822,7 @@ TEST(LayerAnimatorTest, RemoveObserverShouldRemoveFromSequences) {
   // This should stop the observer from observing sequence.
   animator->RemoveObserver(&removed_observer);
 
-  base::TimeTicks start_time = animator->last_step_time();
+  base::TimeTicks start_time = animator->get_last_step_time_for_test();
 
   element->Step(start_time + base::TimeDelta::FromMilliseconds(1000));
 
@@ -819,70 +858,6 @@ 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 7a7dbc1..874b927 100644
--- a/ui/gfx/compositor/scoped_layer_animation_settings.cc
+++ b/ui/gfx/compositor/scoped_layer_animation_settings.cc
@@ -26,19 +26,27 @@ 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 =
-       observers_.begin(); i != observers_.end(); ++i) {
-    animator_->observers_.RemoveObserver(*i);
-    (*i)->SetActive(true);
-  }
+       implicit_observers_.begin(); i != implicit_observers_.end(); ++i)
+      (*i)->SetActive(true);
 }
 
 void ScopedLayerAnimationSettings::AddObserver(
-    ImplicitAnimationObserver* observer) {
+    LayerAnimationObserver* 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 dce64ef..2f90055 100644
--- a/ui/gfx/compositor/scoped_layer_animation_settings.h
+++ b/ui/gfx/compositor/scoped_layer_animation_settings.h
@@ -27,13 +27,15 @@ class COMPOSITOR_EXPORT ScopedLayerAnimationSettings {
   explicit ScopedLayerAnimationSettings(LayerAnimator* animator);
   virtual ~ScopedLayerAnimationSettings();
 
-  void AddObserver(ImplicitAnimationObserver* observer);
+  void AddObserver(LayerAnimationObserver* observer);
+  void AddImplicitObserver(ImplicitAnimationObserver* observer);
   void SetTransitionDuration(base::TimeDelta duration);
 
  private:
   LayerAnimator* animator_;
   base::TimeDelta old_transition_duration_;
-  std::set<ImplicitAnimationObserver*> observers_;
+  std::set<LayerAnimationObserver*> observers_;
+  std::set<ImplicitAnimationObserver*> implicit_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 2afcb1b..fe515d3 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(
-    LayerAnimationSequence* sequence) {
+    const LayerAnimationSequence* sequence) {
   last_ended_sequence_ = sequence;
 }
 
 void TestLayerAnimationObserver::OnLayerAnimationAborted(
-    LayerAnimationSequence* sequence) {
+    const LayerAnimationSequence* sequence) {
   last_aborted_sequence_ = sequence;
 }
 
 void TestLayerAnimationObserver::OnLayerAnimationScheduled(
-    LayerAnimationSequence* sequence) {
+    const 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 44a7119..b90a9dc 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(
-      LayerAnimationSequence* sequence) OVERRIDE;
+      const LayerAnimationSequence* sequence) OVERRIDE;
 
   virtual void OnLayerAnimationAborted(
-      LayerAnimationSequence* sequence) OVERRIDE;
+      const LayerAnimationSequence* sequence) OVERRIDE;
 
   virtual void OnLayerAnimationScheduled(
-      LayerAnimationSequence* sequence) OVERRIDE;
+      const LayerAnimationSequence* sequence) OVERRIDE;
 
   virtual bool RequiresNotificationWhenAnimatorDestroyed() const OVERRIDE;