Keep dedicated layers for hiding animation

ui/views/corewm/window_animations.cc

Index: ui/views/corewm/window_animations.cc
diff --git a/ui/views/corewm/window_animations.cc b/ui/views/corewm/window_animations.cc
index 42b26413129abdd166b339cd15de902c2393946e..7dfe3e8805886147f4fe95c953c19c45a9f83e46 100644
--- a/ui/views/corewm/window_animations.cc
+++ b/ui/views/corewm/window_animations.cc
@@ -107,31 +107,22 @@ int GetWindowVisibilityAnimationType(aura::Window* window) {
   return type;
 }
 
-// Observes a hide animation.
-// A window can be hidden for a variety of reasons. Sometimes, Hide() will be
-// called and life is simple. Sometimes, the window is actually bound to a
-// views::Widget and that Widget is closed, and life is a little more
-// complicated. When a Widget is closed the aura::Window* is actually not
-// destroyed immediately - it is actually just immediately hidden and then
-// destroyed when the stack unwinds. To handle this case, we start the hide
-// 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.
+// A base class for hiding animation observer. This calls
+// AnimationHost::OnWindowHidingAnimationCompleted upon completion.
 class HidingWindowAnimationObserver : public ui::ImplicitAnimationObserver,
                                       public aura::WindowObserver {
  public:
-  explicit HidingWindowAnimationObserver(aura::Window* window)
+  HidingWindowAnimationObserver(aura::Window* window,
+                                ui::ScopedLayerAnimationSettings* settings)
       : window_(window) {
-    window_->AddObserver(this);
+    window->AddObserver(this);
+    if (settings)
+      settings->AddObserver(this);
   }
+
   virtual ~HidingWindowAnimationObserver() {
-    STLDeleteElements(&layers_);
   }
 
- private:
-  // Overridden from ui::ImplicitAnimationObserver:
   virtual void OnImplicitAnimationsCompleted() OVERRIDE {
     // Window may have been destroyed by this point.
     if (window_) {
@@ -144,30 +135,121 @@ class HidingWindowAnimationObserver : public ui::ImplicitAnimationObserver,
     delete this;
   }
 
+  virtual void OnWindowDestroying(aura::Window* window) OVERRIDE {
+    DCHECK_EQ(window, window_);
+    window_->RemoveObserver(this);
+    window_ = NULL;
+  }
+
+ protected:
+  aura::Window* window_;
+
+  DISALLOW_COPY_AND_ASSIGN(HidingWindowAnimationObserver);
+};
+
+// A window can be hidden for a variety of reasons. Sometimes, Hide() will be
+// called and life is simple. Sometimes, the window is actually bound to a
+// views::Widget and that Widget is closed, and life is a little more
+// complicated. When a Widget is closed the aura::Window* is actually not
+// destroyed immediately - it is actually just immediately hidden and then
+// destroyed when the stack unwinds. To handle this case, we start the hide
+// 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 DetachLayersWhenDestroyedAnimationObserver
+    : public HidingWindowAnimationObserver {
+ public:
+  explicit DetachLayersWhenDestroyedAnimationObserver(
+      aura::Window* window,
+      ui::ScopedLayerAnimationSettings* settings)
+      : HidingWindowAnimationObserver(window, settings) {
+  }
+
+  virtual ~DetachLayersWhenDestroyedAnimationObserver() {
+    STLDeleteElements(&layers_);
+  }
+
+ private:
   // 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;
+    HidingWindowAnimationObserver::OnWindowDestroying(window);
   }
 
   void AcquireAllLayers(ui::Layer* layer) {
     if (layer->owner()) {
       ui::Layer* released = layer->owner()->AcquireLayer();
-      DCHECK_EQ(layer, released);
+      CHECK_EQ(layer, released);
       layers_.push_back(released);
     }
-    std::vector<Layer*>::const_iterator it = layer->children().begin();
+    std::vector<ui::Layer*>::const_iterator it = layer->children().begin();
     for (; it != layer->children().end(); ++it)
       AcquireAllLayers(*it);
   }
 
-  aura::Window* window_;
   std::vector<ui::Layer*> layers_;
 
-  DISALLOW_COPY_AND_ASSIGN(HidingWindowAnimationObserver);
+  DISALLOW_COPY_AND_ASSIGN(DetachLayersWhenDestroyedAnimationObserver);
+};
+
+// When hiding an active window, the animating layer can be hidden by
+// new active window's layer. This class detaches the layers for
+// hiding animation, creates new layers for the window and stack them

[sky, 2014/03/10 13:30:40]

stacks

[oshima, 2014/03/10 17:48:24]

Done.

+// above the window and its transient children so that the layer for
+// new active widnow will no hide the hiding animation.

[sky, 2014/03/10 13:30:40]

widnow->window
no->not

[oshima, 2014/03/10 17:48:24]

Done.

+class DetachAndRecreateLayersAnimationObserver
+    : HidingWindowAnimationObserver {
+ public:
+  DetachAndRecreateLayersAnimationObserver(
+      aura::Window* window,
+      ui::ScopedLayerAnimationSettings* settings)
+    : HidingWindowAnimationObserver(window, settings) {

[sky, 2014/03/10 13:30:40]

nit: spacing if off here.

[oshima, 2014/03/10 17:48:24]

Done.

+  }
+
+  virtual ~DetachAndRecreateLayersAnimationObserver() {
+    DeepDeleteLayers(layer_);
+  }
+
+  void DetachAndRecreateLayers() {
+    layer_ = RecreateWindowLayers(window_, true);
+    // If the window has transient children, move above the top most
+    // transient child so that activation change does not put the
+    // window above the animating layer.
+    if(window_->parent()) {
+      const aura::Window::Windows& transient_children =
+          GetTransientChildren(window_);
+      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());

[sky, 2014/03/10 13:30:40]

nit: move DCHECK up one line (since 229 doesn't effect it and and best to keep
DCHECK as close as possible to what you're asserting).

[oshima, 2014/03/10 17:48:24]

Done.

+      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_, topmost_transient_child->layer());
+      }
+    }
+    // Make the new layer invisible immediately.
+    window_->layer()->SetVisible(false);
+    window_->layer()->SetOpacity(0);
+  }
+
+ private:
+  ui::Layer* layer_;
+
+  DISALLOW_COPY_AND_ASSIGN(DetachAndRecreateLayersAnimationObserver);
 };
 
 void GetTransformRelativeToRoot(ui::Layer* layer, gfx::Transform* transform) {
@@ -255,7 +337,8 @@ void AnimateHideWindowCommon(aura::Window* window,
 
   // Property sets within this scope will be implicitly animated.
   ui::ScopedLayerAnimationSettings settings(window->layer()->GetAnimator());
-  settings.AddObserver(new HidingWindowAnimationObserver(window));
+  scoped_ptr<LayerDetacherForHidingAnimation> detacher =
+      DetachAndRecreateLayersForHidingAnimation(window, &settings);
 
   base::TimeDelta duration = GetWindowVisibilityAnimationDuration(*window);
   if (duration.ToInternalValue() > 0)
@@ -358,8 +441,11 @@ void AddLayerAnimationsForRotate(aura::Window* window, bool show) {
   base::TimeDelta duration = base::TimeDelta::FromMilliseconds(
       kWindowAnimation_Rotate_DurationMS);
 
+  scoped_ptr<LayerDetacherForHidingAnimation> layer_detacher;
   if (!show) {
-    new HidingWindowAnimationObserver(window);

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

This leaks the observer because it's not observing the animation.
I'll address this in separate CL.

+    // TODO(oshima): Fix observer leak.
+    layer_detacher =
+        DetachAndRecreateLayersForHidingAnimation(window, NULL).Pass();
     window->layer()->GetAnimator()->SchedulePauseForProperties(
         duration * (100 - kWindowAnimation_Rotate_OpacityDurationPercent) / 100,
         ui::LayerAnimationElement::OPACITY);
@@ -477,6 +563,26 @@ bool AnimateHideWindow(aura::Window* window) {
   }
 }
 
+class LayerDetacherForHidingAnimationImpl

[sky, 2014/03/10 13:30:40]

Add description. In fact all these classes here are rather confusing and it
isn't clear why one variant is needed over the other. Please add better
descriptions of what they are all for.

[oshima, 2014/03/10 17:48:24]

The description is in header file. I added more description there.

I added this to safely detach and recreate layers when applying animation in
less error prone way.
I wish I could simplify a bit more, and I'm happy to change if there is a better
way.

Alternatively, if we detach the layers when the focus changes (like before), the
code would be simpler because we don't have to worry about the order of the
applying animation and the adding observer.

+    : public LayerDetacherForHidingAnimation {
+ public:
+  LayerDetacherForHidingAnimationImpl(
+      aura::Window* window,
+      ui::ScopedLayerAnimationSettings* settings)
+    : observer_(
+          new DetachAndRecreateLayersAnimationObserver(window, settings)) {
+  }
+
+  virtual ~LayerDetacherForHidingAnimationImpl() {
+    observer_->DetachAndRecreateLayers();
+  }
+
+ private:
+  DetachAndRecreateLayersAnimationObserver* observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(LayerDetacherForHidingAnimationImpl);
+};
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -521,9 +627,18 @@ void SetWindowVisibilityAnimationVerticalPosition(aura::Window* window,
   window->SetProperty(kWindowVisibilityAnimationVerticalPositionKey, position);
 }
 
-ui::ImplicitAnimationObserver* CreateHidingWindowAnimationObserver(
-    aura::Window* window) {
-  return new HidingWindowAnimationObserver(window);
+scoped_ptr<LayerDetacherForHidingAnimation>
+DetachAndRecreateLayersForHidingAnimation(
+    aura::Window* window,
+    ui::ScopedLayerAnimationSettings* settings) {
+  return scoped_ptr<LayerDetacherForHidingAnimation>(
+      new LayerDetacherForHidingAnimationImpl(window, settings)).Pass();
+}
+
+void DetachLayersForHidingAnimationWhenDestroyed(
+    aura::Window* window,
+    ui::ScopedLayerAnimationSettings* settings) {
+  new DetachLayersWhenDestroyedAnimationObserver(window, settings);
 }
 
 bool AnimateOnChildWindowVisibilityChanged(aura::Window* window, bool visible) {