LayerAnimator must be prepared for its owning layer's deletion whenever it notifies observers.

ui/compositor/layer_animator.cc

Index: ui/compositor/layer_animator.cc
diff --git a/ui/compositor/layer_animator.cc b/ui/compositor/layer_animator.cc
index 5dc323053cdbd2f09a4ea7adcb116d24c94cd5df..2cf25dc5339e01742db010fe6149ff72e718917c 100644
--- a/ui/compositor/layer_animator.cc
+++ b/ui/compositor/layer_animator.cc
@@ -38,32 +38,6 @@ ui::AnimationContainer* GetAnimationContainer() {
 
 }  // namespace
 
-class LayerAnimator::DestroyedTracker
-    : public base::RefCounted<LayerAnimator::DestroyedTracker> {
- public:
-  DestroyedTracker() : is_alive_(true) {}
-
-  // Returns true if the animator is still alive.
-  bool is_alive() const { return is_alive_; }
-
-  // Invoked when the animator is destroyed.
-  void AnimatorDeleted() {
-    DCHECK(is_alive_);
-    is_alive_ = false;
-  }
-
- private:
-  friend class base::RefCounted<DestroyedTracker>;
-
-  ~DestroyedTracker() {
-    DCHECK(!is_alive_);
-  }
-
-  bool is_alive_;
-
-  DISALLOW_COPY_AND_ASSIGN(DestroyedTracker);
-};
-
 // static
 bool LayerAnimator::disable_animations_for_test_ = false;
 // static
@@ -79,15 +53,14 @@ LayerAnimator::LayerAnimator(base::TimeDelta transition_duration)
       transition_duration_(transition_duration),
       tween_type_(Tween::LINEAR),
       is_started_(false),
-      disable_timer_for_test_(false),
-      destroyed_tracker_(new DestroyedTracker) {
+      disable_timer_for_test_(false) {
 }
 
 LayerAnimator::~LayerAnimator() {
   for (size_t i = 0; i < running_animations_.size(); ++i)
     running_animations_[i].sequence->OnAnimatorDestroyed();
-  ClearAnimations();
-  destroyed_tracker_->AnimatorDeleted();
+  ClearAnimationsInternal();
+  delegate_ = NULL;
 }
 
 // static
@@ -186,11 +159,11 @@ float LayerAnimator::GetTargetGrayscale() const {
 }
 
 void LayerAnimator::SetDelegate(LayerAnimationDelegate* delegate) {
-  DCHECK(delegate);
   delegate_ = delegate;
 }
 
 void LayerAnimator::StartAnimation(LayerAnimationSequence* animation) {
+  scoped_refptr<LayerAnimator> retain(this);
   OnScheduled(animation);
   if (!StartSequenceImmediately(animation)) {
     // Attempt to preempt a running animation.
@@ -219,6 +192,7 @@ void LayerAnimator::StartAnimation(LayerAnimationSequence* animation) {
 }
 
 void LayerAnimator::ScheduleAnimation(LayerAnimationSequence* animation) {
+  scoped_refptr<LayerAnimator> retain(this);
   OnScheduled(animation);
   if (is_animating()) {
     animation_queue_.push_back(make_linked_ptr(animation));
@@ -231,6 +205,8 @@ void LayerAnimator::ScheduleAnimation(LayerAnimationSequence* animation) {
 
 void LayerAnimator::ScheduleTogether(
     const std::vector<LayerAnimationSequence*>& animations) {
+  scoped_refptr<LayerAnimator> retain(this);
+
   // Collect all the affected properties.
   LayerAnimationElement::AnimatableProperties animated_properties;
   std::vector<LayerAnimationSequence*>::const_iterator iter;
@@ -286,20 +262,19 @@ bool LayerAnimator::IsAnimatingProperty(
 
 void LayerAnimator::StopAnimatingProperty(
     LayerAnimationElement::AnimatableProperty property) {
+  scoped_refptr<LayerAnimator> retain(this);
   while (true) {
     RunningAnimation* running = GetRunningAnimation(property);
     if (!running)
       break;
-    if (FinishAnimation(running->sequence) == DESTROYED)
-      return;
+    FinishAnimation(running->sequence);
   }
 }
 
 void LayerAnimator::StopAnimating() {
-  while (is_animating()) {
-    if (FinishAnimation(running_animations_[0].sequence) == DESTROYED)
-      return;
-  }
+  scoped_refptr<LayerAnimator> retain(this);
+  while (is_animating())
+    FinishAnimation(running_animations_[0].sequence);
 }
 
 void LayerAnimator::AddObserver(LayerAnimationObserver* observer) {
@@ -318,9 +293,12 @@ void LayerAnimator::RemoveObserver(LayerAnimationObserver* observer) {
 
 // LayerAnimator protected -----------------------------------------------------
 
-bool LayerAnimator::ProgressAnimation(LayerAnimationSequence* sequence,
+void LayerAnimator::ProgressAnimation(LayerAnimationSequence* sequence,
                                       base::TimeDelta delta) {
-  return sequence->Progress(delta, delegate());
+  if (!delegate())
+    return;
+
+  sequence->Progress(delta, delegate());
 }
 
 
@@ -337,6 +315,7 @@ bool LayerAnimator::HasAnimation(LayerAnimationSequence* sequence) const {
 
 void LayerAnimator::Step(base::TimeTicks now) {
   TRACE_EVENT0("ui", "LayerAnimator::Step");
+  scoped_refptr<LayerAnimator> retain(this);
 
   last_step_time_ = now;
 
@@ -344,7 +323,6 @@ void LayerAnimator::Step(base::TimeTicks now) {
   // and finishing them may indirectly affect the collection of running
   // animations.
   RunningAnimations running_animations_copy = running_animations_;
-  bool needs_redraw = false;
   for (size_t i = 0; i < running_animations_copy.size(); ++i) {
     if (!HasAnimation(running_animations_copy[i].sequence))
       continue;
@@ -352,22 +330,10 @@ void LayerAnimator::Step(base::TimeTicks now) {
     base::TimeDelta delta = now - running_animations_copy[i].start_time;
     if (delta >= running_animations_copy[i].sequence->duration() &&
         !running_animations_copy[i].sequence->is_cyclic()) {
-      if (FinishAnimation(running_animations_copy[i].sequence) == DESTROYED)
-        return;
-      needs_redraw = true;
-    } else {
-      scoped_refptr<DestroyedTracker> tracker(destroyed_tracker_);
-      const bool progress_result =
-          ProgressAnimation(running_animations_copy[i].sequence, delta);
-      if (!tracker->is_alive())
-        return;
-      if (progress_result)
-        needs_redraw = true;
-    }
+      FinishAnimation(running_animations_copy[i].sequence);
+    } else
+      ProgressAnimation(running_animations_copy[i].sequence, delta);
   }
-
-  if (needs_redraw && delegate())
-    delegate()->ScheduleDrawForAnimation();
 }
 
 void LayerAnimator::SetStartTime(base::TimeTicks start_time) {
@@ -417,21 +383,17 @@ LayerAnimationSequence* LayerAnimator::RemoveAnimation(
   return to_return.release();
 }
 
-LayerAnimator::DestroyedType LayerAnimator::FinishAnimation(
-    LayerAnimationSequence* sequence) {
+void LayerAnimator::FinishAnimation(LayerAnimationSequence* sequence) {
+  scoped_refptr<LayerAnimator> retain(this);
   scoped_ptr<LayerAnimationSequence> removed(RemoveAnimation(sequence));
-  {
-    scoped_refptr<DestroyedTracker> tracker(destroyed_tracker_);
+  if (delegate())
     sequence->Progress(sequence->duration(), delegate());
-    if (!tracker->is_alive())
-      return DESTROYED;
-  }
   ProcessQueue();
   UpdateAnimationState();
-  return NOT_DESTROYED;
 }
 
 void LayerAnimator::FinishAnyAnimationWithZeroDuration() {
+  scoped_refptr<LayerAnimator> retain(this);
   // Special case: if we've started a 0 duration animation, just finish it now
   // and get rid of it. We need to make a copy because Progress may indirectly
   // cause new animations to start running.
@@ -441,8 +403,8 @@ void LayerAnimator::FinishAnyAnimationWithZeroDuration() {
       continue;
 
     if (running_animations_copy[i].sequence->duration() == base::TimeDelta()) {
-      running_animations_copy[i].sequence->Progress(
-          running_animations_copy[i].sequence->duration(), delegate());
+      ProgressAnimation(running_animations_copy[i].sequence,
+                        running_animations_copy[i].sequence->duration());
       scoped_ptr<LayerAnimationSequence> removed(
           RemoveAnimation(running_animations_copy[i].sequence));
     }
@@ -452,23 +414,8 @@ void LayerAnimator::FinishAnyAnimationWithZeroDuration() {
 }
 
 void LayerAnimator::ClearAnimations() {
-  // Abort should never affect the set of running animations, but just in case
-  // clients are badly behaved, we will use a copy of the running animations.
-  RunningAnimations running_animations_copy = running_animations_;
-  for (size_t i = 0; i < running_animations_copy.size(); ++i) {
-    if (!HasAnimation(running_animations_copy[i].sequence))
-      continue;
-
-    scoped_ptr<LayerAnimationSequence> removed(
-        RemoveAnimation(running_animations_copy[i].sequence));
-    if (removed.get())
-      removed->Abort();
-  }
-  // This *should* have cleared the list of running animations.
-  DCHECK(running_animations_.empty());
-  running_animations_.clear();
-  animation_queue_.clear();
-  UpdateAnimationState();
+  scoped_refptr<LayerAnimator> retain(this);
+  ClearAnimationsInternal();
 }
 
 LayerAnimator::RunningAnimation* LayerAnimator::GetRunningAnimation(
@@ -515,8 +462,8 @@ void LayerAnimator::RemoveAllAnimationsWithACommonProperty(
       if (abort)
         running_animations_copy[i].sequence->Abort();
       else
-        running_animations_copy[i].sequence->Progress(
-            running_animations_copy[i].sequence->duration(), delegate());
+        ProgressAnimation(running_animations_copy[i].sequence,
+                          running_animations_copy[i].sequence->duration());
     }
   }
 
@@ -537,7 +484,7 @@ void LayerAnimator::RemoveAllAnimationsWithACommonProperty(
       if (abort)
         sequences[i]->Abort();
       else
-        sequences[i]->Progress(sequences[i]->duration(), delegate());
+        ProgressAnimation(sequences[i], sequences[i]->duration());
     }
   }
 }
@@ -549,7 +496,7 @@ void LayerAnimator::ImmediatelySetNewTarget(LayerAnimationSequence* sequence) {
   RemoveAllAnimationsWithACommonProperty(sequence, abort);
   LayerAnimationSequence* removed = RemoveAnimation(sequence);
   DCHECK(removed == NULL || removed == sequence);
-  sequence->Progress(sequence->duration(), delegate());
+  ProgressAnimation(sequence, sequence->duration());
 }
 
 void LayerAnimator::ImmediatelyAnimateToNewTarget(
@@ -692,4 +639,24 @@ base::TimeDelta LayerAnimator::GetTransitionDuration() const {
   return transition_duration_;
 }
 
+void LayerAnimator::ClearAnimationsInternal() {
+  // Abort should never affect the set of running animations, but just in case
+  // clients are badly behaved, we will use a copy of the running animations.
+  RunningAnimations running_animations_copy = running_animations_;
+  for (size_t i = 0; i < running_animations_copy.size(); ++i) {
+    if (!HasAnimation(running_animations_copy[i].sequence))
+      continue;
+
+    scoped_ptr<LayerAnimationSequence> removed(
+        RemoveAnimation(running_animations_copy[i].sequence));
+    if (removed.get())
+      removed->Abort();
+  }
+  // This *should* have cleared the list of running animations.
+  DCHECK(running_animations_.empty());
+  running_animations_.clear();
+  animation_queue_.clear();
+  UpdateAnimationState();
+}
+
 }  // namespace ui