Keep dedicated layers for hiding animation

ui/views/corewm/window_animations.h

Index: ui/views/corewm/window_animations.h
diff --git a/ui/views/corewm/window_animations.h b/ui/views/corewm/window_animations.h
index f5728e6add64fc4ad60b8244540c8c6c28efec62..525bbf3bc2c720c68963999e531ba9c8965d40ac 100644
--- a/ui/views/corewm/window_animations.h
+++ b/ui/views/corewm/window_animations.h
@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "base/memory/scoped_ptr.h"
 #include "ui/views/views_export.h"
 
 namespace aura {
@@ -22,6 +23,7 @@ namespace ui {
 class ImplicitAnimationObserver;
 class Layer;
 class LayerAnimationSequence;
+class ScopedLayerAnimationSettings;
 }
 
 namespace views {
@@ -81,12 +83,39 @@ VIEWS_EXPORT void SetWindowVisibilityAnimationVerticalPosition(
     aura::Window* window,
     float position);
 
-// Creates an ImplicitAnimationObserver that takes ownership of the layers
-// associated with a Window so that the animation can continue after the Window
-// has been destroyed.
-// The returned object deletes itself when the animations are done.
-VIEWS_EXPORT ui::ImplicitAnimationObserver* CreateHidingWindowAnimationObserver(
-    aura::Window* window);
+// A scoped object which detaches the current layers which will animate, and
+// recreates new layers for the window upon deletion of this object.
+// Layers are owned by the animation observer that is added to the
+// deteached layer's animator, and will be deleted when the animation

[sky, 2014/03/11 16:10:53]

deteached->detached

[oshima, 2014/03/11 17:45:20]

Done.

+// is completed.
+class LayerDetacherForHidingAnimation {

[sky, 2014/03/11 16:10:53]

Why bother with this effectively pure virtual class? Why not move
LayerDetacherForHidingAnimationImpl here? If you say to avoid forward declaring
DetachAndRecreateLayersAnimationObserver that seems like a weak argument.

[oshima, 2014/03/11 17:45:20]

It also requires the constructor which a user don't care, and I thought it's
better to hide it rather than expose it. I don't feel strongly about it, so
changed it.

+ public:
+  LayerDetacherForHidingAnimation() {}
+  virtual ~LayerDetacherForHidingAnimation() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LayerDetacherForHidingAnimation);
+};
+
+// This is used to make sure the hiding animation is visible even if

[sky, 2014/03/11 16:10:53]

These two functions are nearly the same and it isn't quite clear (from the
descriptions) as to when you would use one or the other. I think you want:

The following two functions are used to animate a window hiding (this includes
minimize and close). DetachAndRecreateLayersForHidingAnimation() is intended for
use when the window is going to persist (hide or minimize) and
DetachLayersForHidingAnimationWhenDestroyed() when the window is being
destroyed. Both ensure the layers for the window are visible even if the window
is destroyed.

[oshima, 2014/03/11 17:45:20]

That's not correct. The former is to make sure the animation is visible after
activation change, regardless of whether the window will be destroyed. The
latter is the case where we simply want to make sure the animation will continue
after deletion, and can become behind the active window.

We discussed this over the chat. If you think we should use the former for the
latter and will not cause a regression, I will do so. Please let me know.

[sky, 2014/03/11 19:16:23]

My main concern with what you have here (and in the latest patch) is that it is
not clear which one I want to use in a particular situation. That needs to be
made clearer.

+// the activation changes.
+// This should be called before applying animatinos to |settings|

[sky, 2014/03/11 16:10:53]

animatinos->animations

[oshima, 2014/03/11 17:45:20]

Done.

+// as this installs an animation observer and the returned object should
+// have the same scope as |settings|.
+VIEWS_EXPORT scoped_ptr<LayerDetacherForHidingAnimation>
+    DetachAndRecreateLayersForHidingAnimation(
+        aura::Window* window,
+        ui::ScopedLayerAnimationSettings* settings) WARN_UNUSED_RESULT;
+
+// This installs a window observer that takes ownership of the layers
+// associated with a winddow. This is used to make sure that the

[sky, 2014/03/11 16:10:53]

winddow->window

[oshima, 2014/03/11 17:45:20]

Done.

+// hiding animation can continue after the window has been destroyed.
+// this will install an animation observer.
+// This should be called before applying animatinos to |settings| as
+// this installs an animation observer.
+VIEWS_EXPORT void DetachLayersForHidingAnimationWhenDestroyed(
+    aura::Window* window,
+    ui::ScopedLayerAnimationSettings* settings);
 
 // Returns false if the |window| didn't animate.
 VIEWS_EXPORT bool AnimateOnChildWindowVisibilityChanged(aura::Window* window,