Stops animations when removing a window from its parent

ui/aura/window.cc

Index: ui/aura/window.cc
diff --git a/ui/aura/window.cc b/ui/aura/window.cc
index 899fdfdb047c14b76f8a72ce270380c75eb911c7..d577f3e4c7815770c412c16014c7582a32bc9092 100644
--- a/ui/aura/window.cc
+++ b/ui/aura/window.cc
@@ -29,8 +29,8 @@
 #include "ui/aura/window_tree_host.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/layer.h"
+#include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/events/event_target_iterator.h"
-#include "ui/gfx/animation/multi_animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/path.h"
 #include "ui/gfx/scoped_canvas.h"
@@ -500,6 +500,15 @@ void Window::StackChildBelow(Window* child, Window* target) {
 }
 
 void Window::AddChild(Window* child) {
+  scoped_ptr<ui::ScopedLayerAnimationSettings> animation_settings;
+  if (child->layer() &&
+      child->layer()->GetAnimator() &&
+      child->layer()->GetAnimator()->is_animating()) {
+    animation_settings.reset(
+        new ui::ScopedLayerAnimationSettings(child->layer()->GetAnimator()));
+    animation_settings->SetPreemptionStrategy(
+        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);

[oshima, 2014/04/23 16:34:44]

I feels like animation should be canceled upon parent change, and the client
code should take care of applying animation correctly (after reparent in this
case)?

Is there any reason why drag code can't be fixed instead?

[varkha, 2014/04/23 16:44:05]

I was thinking that this would be an option. It is just that the paths that may
cause animation to be active during reparenting are many and not easily
controlled or visible in code inspection so fixing it here made sense. Stopping
animation also has some issues - I have seen the window bounds getting out of
sync with layers bounds because we revert window bounds_ in ReparentLayers and
UnparentLayers which only makes sense if the animation is allowed to continue.
If you look at the crashing bug
https://code.google.com/p/chromium/issues/detail?id=364498 - it has a few call
stacks mentioned that are all slightly different but all similar in that they
all involve animation during reparenting. It feels that we would have to hunt
each of those separately with the same recipe.

+  }
   WindowObserver::HierarchyChangeParams params;
   params.target = child;
   params.new_parent = this;
@@ -516,13 +525,15 @@ void Window::AddChild(Window* child) {
 
   gfx::Vector2d offset;
   aura::Window* ancestor_with_layer = GetAncestorWithLayer(&offset);
+  gfx::Rect target_bounds = child->GetTargetBounds();
+
+  child->parent_ = this;
+
   if (ancestor_with_layer) {
-    offset += child->bounds().OffsetFromOrigin();
+    offset += target_bounds.OffsetFromOrigin();
     child->ReparentLayers(ancestor_with_layer->layer(), offset);
   }
 
-  child->parent_ = this;
-
   children_.push_back(child);
   if (layout_manager_)
     layout_manager_->OnWindowAddedToLayout(child);
@@ -664,7 +675,7 @@ bool Window::ContainsPointInRoot(const gfx::Point& point_in_root) const {
 }
 
 bool Window::ContainsPoint(const gfx::Point& local_point) const {
-  return gfx::Rect(bounds().size()).Contains(local_point);
+  return gfx::Rect(GetTargetBounds().size()).Contains(local_point);
 }
 
 Window* Window::GetEventHandlerForPoint(const gfx::Point& local_point) {
@@ -1049,8 +1060,8 @@ void Window::UnparentLayers(bool has_layerless_ancestor,
       if (layer()->parent())
         layer()->parent()->Remove(layer());
       if (has_layerless_ancestor) {
-        const gfx::Rect real_bounds(bounds_);
-        gfx::Rect layer_bounds(layer()->bounds());
+        const gfx::Rect real_bounds(bounds());
+        gfx::Rect layer_bounds(layer()->GetTargetBounds());
         layer_bounds.Offset(-offset);
         layer()->SetBounds(layer_bounds);
         bounds_ = real_bounds;
@@ -1070,7 +1081,7 @@ void Window::ReparentLayers(ui::Layer* parent_layer,
   } else {
     const gfx::Rect real_bounds(bounds());
     parent_layer->Add(layer());
-    gfx::Rect layer_bounds(layer()->bounds().size());
+    gfx::Rect layer_bounds(layer()->GetTargetBounds().size());
     layer_bounds += offset;
     layer()->SetBounds(layer_bounds);
     bounds_ = real_bounds;
@@ -1082,7 +1093,7 @@ void Window::OffsetLayerBounds(const gfx::Vector2d& offset) {
     for (size_t i = 0; i < children_.size(); ++i)
       children_[i]->OffsetLayerBounds(offset);
   } else {
-    gfx::Rect layer_bounds(layer()->bounds());
+    gfx::Rect layer_bounds(layer()->GetTargetBounds());
     layer_bounds += offset;
     layer()->SetBounds(layer_bounds);
   }
@@ -1434,7 +1445,7 @@ const Window* Window::GetAncestorWithLayer(gfx::Vector2d* offset) const {
     if (window->layer())
       return window;
     if (offset)
-      *offset += window->bounds().OffsetFromOrigin();
+      *offset += window->GetTargetBounds().OffsetFromOrigin();
   }
   if (offset)
     *offset = gfx::Vector2d();