Keep dedicated layers for hiding animation

ui/views/corewm/window_animations.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/views/corewm/window_animations.h"
 
 #include <math.h>
 
 #include <algorithm>
 #include <vector>
@@ skipping 16 matching lines @@
 #include "ui/compositor/layer_animation_sequence.h"
 #include "ui/compositor/layer_animator.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/animation/animation.h"
 #include "ui/gfx/interpolated_transform.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/screen.h"
 #include "ui/gfx/vector2d.h"
 #include "ui/gfx/vector3d_f.h"
 #include "ui/views/corewm/corewm_switches.h"
+#include "ui/views/corewm/transient_window_manager.h"
 #include "ui/views/corewm/window_util.h"
 
 DECLARE_WINDOW_PROPERTY_TYPE(int)
 DECLARE_WINDOW_PROPERTY_TYPE(views::corewm::WindowVisibilityAnimationType)
 DECLARE_WINDOW_PROPERTY_TYPE(views::corewm::WindowVisibilityAnimationTransition)
 DECLARE_WINDOW_PROPERTY_TYPE(float)
 DECLARE_EXPORTED_WINDOW_PROPERTY_TYPE(VIEWS_EXPORT, bool)
 
 using aura::Window;
 using base::TimeDelta;
@@ skipping 70 matching lines @@
 // animation immediately when the window is hidden, then when the window is
 // subsequently destroyed this object acquires ownership of the window's layer,
 // so that it can continue animating it until the animation completes.
 // Regardless of whether or not the window is destroyed, this object deletes
 // itself when the animation completes.
 class HidingWindowAnimationObserver : public ui::ImplicitAnimationObserver,
                                       public aura::WindowObserver {
  public:
   explicit HidingWindowAnimationObserver(aura::Window* window)
       : window_(window) {
     window_->AddObserver(this);

[sky, 2014/03/07 15:14:54]

This code previously cared about the window so that if the window was desctroyed
it would grab the layers. Since you now grab the layers right away it seems like
it shouldn't need to be a WindowObserver nor have the window_ member. The only
thing I'm not sure about is 175-178 (new). I honestly don't understand that
code. Is it needed?

[oshima, 2014/03/08 00:14:30]

I tried that but it broke the test due to the AnimationHost not being called at
the end,
and this seems to be used by desktop aura.

(It's added here https://chromiumcodereview.appspot.com/12342028)

   }
+
   virtual ~HidingWindowAnimationObserver() {
-    STLDeleteElements(&layers_);
+  }
+
+  void DetachAndRecreateLayers() {
+    layer_.reset(views::corewm::RecreateWindowLayers(window_, true));

[sky, 2014/03/07 15:14:54]

nit: don't need views::corem here as you're in that namespace already.

[sky, 2014/03/07 15:14:54]

Layer does not own its children. So, the way you have it now you leak all the
descendants.

[Ben Goodger (Google), 2014/03/07 15:51:54]

Yeah RecreateWindowLayers seems like a weird API. How can it not leak all the
descendents (and view layers too).

Also this would reintroduce the bug that my last CL here fixed. Seems like
RecreateWindowLayers should become RecreateLayers() and do a proper deep copy of
the layer tree, returning ownership of the original tree to the caller.

[sky, 2014/03/07 16:34:57]

I'm going to change the return value of RecreateWindowLayers now as part of
another patch I'm working on.

[oshima, 2014/03/08 00:14:30]

Fixed to use DeepDeleteLayers for now. I'll update when sky's change is in.

+    // Use const window to avoid creating a TransientWindowManager
+    // if the window doesn't have one.
+    const aura::Window* const_window = window_;

[sky, 2014/03/07 15:14:54]

If you use the function in window_util it'll do the right thing and you don't
need the cast.

[oshima, 2014/03/08 00:14:30]

Ah, I didn't see that. Fixed

+    const TransientWindowManager* transient_window_manager =
+        TransientWindowManager::Get(const_window);
+    // If the window has transient children, move above the top most

[sky, 2014/03/07 15:14:54]

Document why you need to do this.

[oshima, 2014/03/08 00:14:30]

Done.

+    // transient child.
+    if (transient_window_manager) {
+      const aura::Window::Windows& transient_children =
+          transient_window_manager->transient_children();
+      aura::Window::Windows::const_iterator iter =
+          std::find(window_->parent()->children().begin(),
+                    window_->parent()->children().end(),
+                    window_);
+      aura::Window* topmost_transient_child = NULL;
+      DCHECK(iter != window_->parent()->children().end());
+      for (++iter; iter != window_->parent()->children().end(); ++iter) {
+        if (std::find(transient_children.begin(),
+                      transient_children.end(),
+                      *iter) !=
+            transient_children.end()) {
+          topmost_transient_child = *iter;
+        }
+      }
+      if (topmost_transient_child) {
+        window_->parent()->layer()->StackAbove(
+            layer_.get(), topmost_transient_child->layer());
+      }
+    }
+    // Make the new layer unvisible immediately.

[Ben Goodger (Google), 2014/03/07 15:51:54]

invisible

[oshima, 2014/03/08 00:14:30]

Done.

+    window_->layer()->SetVisible(false);
+    window_->layer()->SetOpacity(0);
   }
 
  private:
   // Overridden from ui::ImplicitAnimationObserver:
   virtual void OnImplicitAnimationsCompleted() OVERRIDE {
     // Window may have been destroyed by this point.
     if (window_) {
       aura::client::AnimationHost* animation_host =
           aura::client::GetAnimationHost(window_);
       if (animation_host)
         animation_host->OnWindowHidingAnimationCompleted();
       window_->RemoveObserver(this);
     }
     delete this;
   }
 
   // Overridden from aura::WindowObserver:
   virtual void OnWindowDestroying(aura::Window* window) OVERRIDE {
     DCHECK_EQ(window, window_);
-    DCHECK(layers_.empty());
-    AcquireAllLayers(window_->layer());
     window_->RemoveObserver(this);
     window_ = NULL;
   }
 
-  void AcquireAllLayers(ui::Layer* layer) {
-    if (layer->owner()) {
-      ui::Layer* released = layer->owner()->AcquireLayer();
-      DCHECK_EQ(layer, released);
-      layers_.push_back(released);
-    }
-    std::vector<Layer*>::const_iterator it = layer->children().begin();
-    for (; it != layer->children().end(); ++it)
-      AcquireAllLayers(*it);
+  aura::Window* window_;
+  scoped_ptr<ui::Layer> layer_;
+
+  DISALLOW_COPY_AND_ASSIGN(HidingWindowAnimationObserver);
+};
+
+class ScopedLayerDetacher {

[sky, 2014/03/07 15:14:54]

Description?

[oshima, 2014/03/08 00:14:30]

Moved to header and added comment.

+ public:
+  explicit ScopedLayerDetacher(HidingWindowAnimationObserver* observer)
+      : observer_(observer) {}
+  ~ScopedLayerDetacher() {
+    observer_->DetachAndRecreateLayers();
   }
 
-  aura::Window* window_;
-  std::vector<ui::Layer*> layers_;
+  HidingWindowAnimationObserver* observer() { return observer_; }
 
-  DISALLOW_COPY_AND_ASSIGN(HidingWindowAnimationObserver);
+ private:
+  HidingWindowAnimationObserver* observer_ ;  // not owned.
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedLayerDetacher);
 };
 
 void GetTransformRelativeToRoot(ui::Layer* layer, gfx::Transform* transform) {
   const Layer* root = layer;
   while (root->parent())
     root = root->parent();
   layer->GetTargetTransformRelativeTo(root, transform);
 }
 
 gfx::Rect GetLayerWorldBoundsAfterTransform(ui::Layer* layer,
@@ skipping 67 matching lines @@
 
 // Hides a window using an animation, animating its opacity from 1.f to 0.f,
 // its visibility to false, and its transform to |end_transform|.
 void AnimateHideWindowCommon(aura::Window* window,
                              const gfx::Transform& end_transform) {
   AugmentWindowSize(window, end_transform);
   window->layer()->set_delegate(NULL);
 
   // Property sets within this scope will be implicitly animated.
   ui::ScopedLayerAnimationSettings settings(window->layer()->GetAnimator());
-  settings.AddObserver(new HidingWindowAnimationObserver(window));
+  ScopedLayerDetacher detacher(new HidingWindowAnimationObserver(window));
+  settings.AddObserver(detacher.observer());
 
   base::TimeDelta duration = GetWindowVisibilityAnimationDuration(*window);
   if (duration.ToInternalValue() > 0)
     settings.SetTransitionDuration(duration);
 
   window->layer()->SetOpacity(kWindowAnimation_HideOpacity);
   window->layer()->SetTransform(end_transform);
   window->layer()->SetVisible(false);
 }
 
@@ skipping 81 matching lines @@
   window->layer()->GetAnimator()->StartAnimation(sequence.release());
 }
 
 void AddLayerAnimationsForRotate(aura::Window* window, bool show) {
   window->layer()->set_delegate(window);
   if (show)
     window->layer()->SetOpacity(kWindowAnimation_HideOpacity);
 
   base::TimeDelta duration = base::TimeDelta::FromMilliseconds(
       kWindowAnimation_Rotate_DurationMS);
+  scoped_ptr<ScopedLayerDetacher> layer_detacher;
 
   if (!show) {
-    new HidingWindowAnimationObserver(window);
+    layer_detacher.reset(new ScopedLayerDetacher(
+        new HidingWindowAnimationObserver(window)));
     window->layer()->GetAnimator()->SchedulePauseForProperties(
         duration * (100 - kWindowAnimation_Rotate_OpacityDurationPercent) / 100,
         ui::LayerAnimationElement::OPACITY);
   }
   scoped_ptr<ui::LayerAnimationElement> opacity(
       ui::LayerAnimationElement::CreateOpacityElement(
           show ? kWindowAnimation_ShowOpacity : kWindowAnimation_HideOpacity,
           duration * kWindowAnimation_Rotate_OpacityDurationPercent / 100));
   opacity->set_tween_type(gfx::Tween::EASE_IN_OUT);
   window->layer()->GetAnimator()->ScheduleAnimation(
@@ skipping 143 matching lines @@
       window.GetProperty(kWindowVisibilityAnimationDurationKey));
 }
 
 void SetWindowVisibilityAnimationVerticalPosition(aura::Window* window,
                                                   float position) {
   window->SetProperty(kWindowVisibilityAnimationVerticalPositionKey, position);
 }
 
 ui::ImplicitAnimationObserver* CreateHidingWindowAnimationObserver(
     aura::Window* window) {
   return new HidingWindowAnimationObserver(window);

[Ben Goodger (Google), 2014/03/07 15:51:54]

do you run into the same problem when this method is called?

[oshima, 2014/03/08 00:14:30]

Thanks. I looked at usages and looks like I should support both scenarios
(detach upon deletion, and detach & recreate when animation starts).

I replaced this with two utility functions

DetachAndRecreateLayersForHidingAnimation and
DetachLayersForHidingAnimationWhenDestroyed.

 }
 
 bool AnimateOnChildWindowVisibilityChanged(aura::Window* window, bool visible) {
   if (WindowAnimationsDisabled(window))
     return false;
   if (visible)
     return AnimateShowWindow(window);
   // Don't start hiding the window again if it's already being hidden.
   return window->layer()->GetTargetOpacity() != 0.0f &&
       AnimateHideWindow(window);
@@ skipping 12 matching lines @@
 
 bool WindowAnimationsDisabled(aura::Window* window) {
   return (!gfx::Animation::ShouldRenderRichAnimation() || (window &&
           window->GetProperty(aura::client::kAnimationsDisabledKey)) ||
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kWindowAnimationsDisabled));
 }
 
 }  // namespace corewm
 }  // namespace views