Attempt 2 at Fixes crash introduced @ 153047 (you can hit crash by maximizing a

ui/compositor/layer_animator.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 "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/base/animation/animation_container.h"
@@ skipping 10 matching lines @@
 namespace {
 
 static const base::TimeDelta kDefaultTransitionDuration =
     base::TimeDelta::FromMilliseconds(120);
 
 static const base::TimeDelta kTimerInterval =
     base::TimeDelta::FromMilliseconds(10);
 
 }  // namespace
 
+class LayerAnimator::DestroyedTracker
+    : public base::RefCounted<LayerAnimator::DestroyedTracker> {
+ public:
+  DestroyedTracker() : is_alive_(true) {}
+  virtual ~DestroyedTracker() {
+    DCHECK(!is_alive_);
+  }
+
+  // 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:
+  bool is_alive_;
+
+  DISALLOW_COPY_AND_ASSIGN(DestroyedTracker);
+};
+
 // static
 bool LayerAnimator::disable_animations_for_test_ = false;
 // static
 bool LayerAnimator::slow_animation_mode_ = false;
 // static
 int LayerAnimator::slow_animation_scale_factor_ = 4;
 
 // LayerAnimator public --------------------------------------------------------
 
 LayerAnimator::LayerAnimator(base::TimeDelta transition_duration)
     : delegate_(NULL),
       preemption_strategy_(IMMEDIATELY_SET_NEW_TARGET),
       transition_duration_(transition_duration),
       tween_type_(Tween::LINEAR),
       is_started_(false),
-      disable_timer_for_test_(false) {
+      disable_timer_for_test_(false),
+      destroyed_tracker_(new DestroyedTracker) {
 }
 
 LayerAnimator::~LayerAnimator() {
   for (size_t i = 0; i < running_animations_.size(); ++i)
     running_animations_[i].sequence->OnAnimatorDestroyed();
   ClearAnimations();
+  destroyed_tracker_->AnimatorDeleted();
 }
 
 // static
 LayerAnimator* LayerAnimator::CreateDefaultAnimator() {
   return new LayerAnimator(base::TimeDelta::FromMilliseconds(0));
 }
 
 // static
 LayerAnimator* LayerAnimator::CreateImplicitAnimator() {
   return new LayerAnimator(kDefaultTransitionDuration);
@@ skipping 182 matching lines @@
   }
   return false;
 }
 
 void LayerAnimator::StopAnimatingProperty(
     LayerAnimationElement::AnimatableProperty property) {
   while (true) {
     RunningAnimation* running = GetRunningAnimation(property);
     if (!running)
       break;
-    FinishAnimation(running->sequence);
+    if (FinishAnimation(running->sequence) == DESTROYED)
+      return;
   }
 }
 
 void LayerAnimator::StopAnimating() {
-  while (is_animating())
-    FinishAnimation(running_animations_[0].sequence);
+  while (is_animating()) {
+    if (FinishAnimation(running_animations_[0].sequence) == DESTROYED)
+      return;
+  }
 }
 
 void LayerAnimator::AddObserver(LayerAnimationObserver* observer) {
   if (!observers_.HasObserver(observer))
     observers_.AddObserver(observer);
 }
 
 void LayerAnimator::RemoveObserver(LayerAnimationObserver* observer) {
   observers_.RemoveObserver(observer);
   // Remove the observer from all sequences as well.
@@ skipping 31 matching lines @@
   // 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;
 
     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()) {
-      FinishAnimation(running_animations_copy[i].sequence);
-    } else if (ProgressAnimation(running_animations_copy[i].sequence, delta))
+      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;
+    }
   }
 
   if (needs_redraw && delegate())
     delegate()->ScheduleDrawForAnimation();
 }
 
 void LayerAnimator::SetStartTime(base::TimeTicks start_time) {
   // Do nothing.
 }
 
@@ skipping 39 matching lines @@
     if ((*queue_iter) == sequence) {
       to_return = *queue_iter;
       animation_queue_.erase(queue_iter);
       break;
     }
   }
 
   return to_return.release();
 }
 
-void LayerAnimator::FinishAnimation(LayerAnimationSequence* sequence) {
+LayerAnimator::DestroyedType LayerAnimator::FinishAnimation(
+    LayerAnimationSequence* sequence) {
   scoped_ptr<LayerAnimationSequence> removed(RemoveAnimation(sequence));
-  sequence->Progress(sequence->duration(), delegate());
+  {
+    scoped_refptr<DestroyedTracker> tracker(destroyed_tracker_);
+    sequence->Progress(sequence->duration(), delegate());
+    if (!tracker->is_alive())
+      return DESTROYED;
+  }
   ProcessQueue();
   UpdateAnimationState();
+  return NOT_DESTROYED;
 }
 
 void LayerAnimator::FinishAnyAnimationWithZeroDuration() {
   // 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.
   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;
 
     if (running_animations_copy[i].sequence->duration() == base::TimeDelta()) {
       running_animations_copy[i].sequence->Progress(
           running_animations_copy[i].sequence->duration(), delegate());

[Ian Vollick, 2012/09/06 00:57:35]

Seems like there's a chance we could delete 'this' in response to this call to
Progress. Could we do the same thing here that we do in FinishAnimation?

       scoped_ptr<LayerAnimationSequence> removed(
           RemoveAnimation(running_animations_copy[i].sequence));
     }
   }
   ProcessQueue();
   UpdateAnimationState();
 }
 
 void LayerAnimator::ClearAnimations() {
   // Abort should never affect the set of running animations, but just in case
@@ skipping 53 matching lines @@
       continue;
 
     if (running_animations_copy[i].sequence->HasCommonProperty(
             sequence->properties())) {
       scoped_ptr<LayerAnimationSequence> removed(
           RemoveAnimation(running_animations_copy[i].sequence));
       if (abort)
         running_animations_copy[i].sequence->Abort();
       else
         running_animations_copy[i].sequence->Progress(
             running_animations_copy[i].sequence->duration(), delegate());

[Ian Vollick, 2012/09/06 00:57:35]

This seems like another place we can bail early if we've been deleted.

     }
   }
 
   // Same for the queued animations that haven't been started. Again, we'll
   // need to operate on a copy.
   std::vector<LayerAnimationSequence*> sequences;
   for (AnimationQueue::iterator queue_iter = animation_queue_.begin();
        queue_iter != animation_queue_.end(); ++queue_iter)
     sequences.push_back((*queue_iter).get());
 
   for (size_t i = 0; i < sequences.size(); ++i) {
     if (!HasAnimation(sequences[i]))
       continue;
 
     if (sequences[i]->HasCommonProperty(sequence->properties())) {
       scoped_ptr<LayerAnimationSequence> removed(
           RemoveAnimation(sequences[i]));
       if (abort)
         sequences[i]->Abort();
       else
         sequences[i]->Progress(sequences[i]->duration(), delegate());

[Ian Vollick, 2012/09/06 00:57:35]

Same here.

     }
   }
 }
 
 void LayerAnimator::ImmediatelySetNewTarget(LayerAnimationSequence* sequence) {
   // Ensure that sequence is disposed of when this function completes.
   scoped_ptr<LayerAnimationSequence> to_dispose(sequence);
   const bool abort = false;
   RemoveAllAnimationsWithACommonProperty(sequence, abort);
   LayerAnimationSequence* removed = RemoveAnimation(sequence);
   DCHECK(removed == NULL || removed == sequence);
   sequence->Progress(sequence->duration(), delegate());

[Ian Vollick, 2012/09/06 00:57:35]

Same here.

 }
 
 void LayerAnimator::ImmediatelyAnimateToNewTarget(
     LayerAnimationSequence* sequence) {
   const bool abort = true;
   RemoveAllAnimationsWithACommonProperty(sequence, abort);
   AddToQueueIfNotPresent(sequence);
   StartSequenceImmediately(sequence);
 }
 
@@ skipping 116 matching lines @@
     }
   }
   sequence->OnScheduled();
 }
 
 base::TimeDelta LayerAnimator::GetTransitionDuration() const {
   return transition_duration_;
 }
 
 }  // namespace ui