compositor/layer_animator: handle delegate removal in LayerAnimation::StopAnimating

ui/wm/core/window_animations.cc

Index: ui/wm/core/window_animations.cc
diff --git a/ui/wm/core/window_animations.cc b/ui/wm/core/window_animations.cc
index 03bb1b99f421b335d3033c685e49661fc93cdc6b..3205ed41a5d1f87225359c8ecbb7b1960754bc1b 100644
--- a/ui/wm/core/window_animations.cc
+++ b/ui/wm/core/window_animations.cc
@@ -404,21 +404,34 @@ class RotateHidingWindowAnimationObserver
 
   // Destroys itself after |last_sequence| ends or is aborted. Does not take
   // ownership of |last_sequence|, which should not be NULL.
-  void SetLastSequence(ui::LayerAnimationSequence* last_sequence) {
-    last_sequence->AddObserver(this);
+  void AddSequence(ui::LayerAnimationSequence* sequence) {
+    sequences_.push_back(sequence);

[sky, 2014/12/15 17:48:54]

I think you're working around the bug. OnLayerAnimationEnded is being called
correctly and 'this' attempts to delete itself. The problem is the deletion is
triggered from a call in LayerAnimator. LayerAnimator is ref counted and isn't
destroyed. When the stack unravels LayerAnimator attempts to use its delegate(),
which is the Layer that was destroyed and we crash. The fix should be in
LayerAnimator.

+    sequence->AddObserver(this);
   }
 
   // ui::LayerAnimationObserver:
   void OnLayerAnimationEnded(ui::LayerAnimationSequence* sequence) override {
-    OnAnimationCompleted();
+    RemoveAnimation(sequence);
   }
   void OnLayerAnimationAborted(ui::LayerAnimationSequence* sequence) override {
-    OnAnimationCompleted();
+    RemoveAnimation(sequence);
   }
   void OnLayerAnimationScheduled(
       ui::LayerAnimationSequence* sequence) override {}
 
  private:
+  // Destroys itself after last sequence ends or is aborted.
+  void RemoveAnimation(ui::LayerAnimationSequence *sequence) {
+    auto it = std::remove(sequences_.begin(), sequences_.end(), sequence);
+    if (it != sequences_.end()) {
+      sequences_.erase(it);
+      if (sequences_.empty())
+        OnAnimationCompleted();
+    }
+  }
+
+  std::vector<ui::LayerAnimationSequence *> sequences_;
+
   DISALLOW_COPY_AND_ASSIGN(RotateHidingWindowAnimationObserver);
 };
 
@@ -442,8 +455,11 @@ void AddLayerAnimationsForRotate(aura::Window* window, bool show) {
           show ? kWindowAnimation_ShowOpacity : kWindowAnimation_HideOpacity,
           duration * kWindowAnimation_Rotate_OpacityDurationPercent / 100));
   opacity->set_tween_type(gfx::Tween::EASE_IN_OUT);
-  window->layer()->GetAnimator()->ScheduleAnimation(
-      new ui::LayerAnimationSequence(opacity.release()));
+  ui::LayerAnimationSequence *opacity_sequence =

[sky, 2014/12/15 17:48:54]

nit: "ui::LayerAnimationSequence *" -> "ui::LayerAnimationSequence* "

+      new ui::LayerAnimationSequence(opacity.release());
+  window->layer()->GetAnimator()->ScheduleAnimation(opacity_sequence);
+  if (observer)
+    observer->AddSequence(opacity_sequence);
 
   float xcenter = window->bounds().width() * 0.5;
 
@@ -477,12 +493,12 @@ void AddLayerAnimationsForRotate(aura::Window* window, bool show) {
   scoped_ptr<ui::LayerAnimationElement> transition(
       ui::LayerAnimationElement::CreateInterpolatedTransformElement(
           rotation.release(), duration));
-  ui::LayerAnimationSequence* last_sequence =
+  ui::LayerAnimationSequence* transition_sequence =
       new ui::LayerAnimationSequence(transition.release());
-  window->layer()->GetAnimator()->ScheduleAnimation(last_sequence);
+  window->layer()->GetAnimator()->ScheduleAnimation(transition_sequence);
 
   if (observer) {
-    observer->SetLastSequence(last_sequence);
+    observer->AddSequence(transition_sequence);
     observer->DetachAndRecreateLayers();
   }
 }