Stops animations when removing a window from its parent

ui/aura/window.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/aura/window.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 497 matching lines @@
   params.phase = WindowObserver::HierarchyChangeParams::HIERARCHY_CHANGING;
   NotifyWindowHierarchyChange(params);
 
   Window* old_root = child->GetRootWindow();
 
   DCHECK(std::find(children_.begin(), children_.end(), child) ==
       children_.end());
   if (child->parent())
     child->parent()->RemoveChildImpl(child, this);
 
+  child->parent_ = this;
+
   gfx::Vector2d offset;
   aura::Window* ancestor_with_layer = GetAncestorWithLayer(&offset);
   if (ancestor_with_layer) {
-    offset += child->bounds().OffsetFromOrigin();
+    offset += child->GetTargetBounds().OffsetFromOrigin();

[oshima, 2014/04/22 20:47:11]

Sorry if I'm misunderstanding something, but I wonder if this correct for all
cases. GetTargetBounds seem to depend on the ancestor layer, while the next line
can change the relative location of the layer to ancestor layer.

maybe what you want to do here is something like this?

layer() ? layer()->GetTargetBounds() : bounds()

[varkha, 2014/04/22 20:59:05]

Isn't that how Window::GetTargetBounds is implemented for !layer() case?

[oshima, 2014/04/22 22:57:11]

It looks to me that this is wrong if a) the child has a layer and b) the parent
(this) does not have a layer and c) the parent has ancestor, because
Window::GetTargetBounds subtracts the offset of the ancestor from the
layer()->GetTargetBounds(), even though the layer hasn't adjusted to the
ancestor's bounds yet.

I may be missing something though. I think either sky@ or ben@ should review
this.

     child->ReparentLayers(ancestor_with_layer->layer(), offset);
   }
 
-  child->parent_ = this;
-
   children_.push_back(child);
   if (layout_manager_)
     layout_manager_->OnWindowAddedToLayout(child);
   FOR_EACH_OBSERVER(WindowObserver, observers_, OnWindowAdded(child));
   child->OnParentChanged();
 
   Window* root_window = GetRootWindow();
   if (root_window && old_root != root_window) {
     root_window->GetHost()->dispatcher()->OnWindowAddedToRootWindow(child);
     child->NotifyAddedToRootWindow();
@@ skipping 121 matching lines @@
 bool Window::ContainsPointInRoot(const gfx::Point& point_in_root) const {
   const Window* root_window = GetRootWindow();
   if (!root_window)
     return false;
   gfx::Point local_point(point_in_root);
   ConvertPointToTarget(root_window, this, &local_point);
   return gfx::Rect(GetTargetBounds().size()).Contains(local_point);
 }
 
 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) {
   return GetWindowForPoint(local_point, true, true);
 }
 
 Window* Window::GetTopWindowContainingPoint(const gfx::Point& local_point) {
   return GetWindowForPoint(local_point, false, false);
 }
 
@@ skipping 364 matching lines @@
       children_[i]->UnparentLayers(true, new_offset);
     }
   } else {
     // Only remove the layer if we still own it.  Someone else may have acquired
     // ownership of it via AcquireLayer() and may expect the hierarchy to go
     // unchanged as the Window is destroyed.
     if (OwnsLayer()) {
       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(GetTargetBounds());
+        gfx::Rect layer_bounds(layer()->GetTargetBounds());
         layer_bounds.Offset(-offset);
         layer()->SetBounds(layer_bounds);
         bounds_ = real_bounds;
       }
     }
   }
 }
 
 void Window::ReparentLayers(ui::Layer* parent_layer,
                             const gfx::Vector2d& offset) {
   if (!layer()) {
     for (size_t i = 0; i < children_.size(); ++i) {
       children_[i]->ReparentLayers(
           parent_layer,
-          offset + children_[i]->bounds().OffsetFromOrigin());
+          offset + children_[i]->GetTargetBounds().OffsetFromOrigin());
     }
   } else {
-    const gfx::Rect real_bounds(bounds());
+    const gfx::Rect real_bounds(GetTargetBounds());
     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;
   }
 }
 
 void Window::OffsetLayerBounds(const gfx::Vector2d& offset) {
   if (!layer()) {
     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);
   }
 }
 
 void Window::OnParentChanged() {
   FOR_EACH_OBSERVER(
       WindowObserver, observers_, OnWindowParentChanged(this, parent_));
 }
 
@@ skipping 331 matching lines @@
         ContainsPointInRoot(host->dispatcher()->GetLastMouseLocationInRoot());
   }
   return contains_mouse;
 }
 
 const Window* Window::GetAncestorWithLayer(gfx::Vector2d* offset) const {
   for (const aura::Window* window = this; window; window = window->parent()) {
     if (window->layer())
       return window;
     if (offset)
-      *offset += window->bounds().OffsetFromOrigin();
+      *offset += window->GetTargetBounds().OffsetFromOrigin();
   }
   if (offset)
     *offset = gfx::Vector2d();
   return NULL;
 }
 
 }  // namespace aura