Keep dedicated layers for hiding animation

ui/views/corewm/window_animations.h

 // 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.
 
 #ifndef UI_VIEWS_COREWM_WINDOW_ANIMATIONS_H_
 #define UI_VIEWS_COREWM_WINDOW_ANIMATIONS_H_
 
 #include <vector>
 
+#include "base/memory/scoped_ptr.h"
 #include "ui/views/views_export.h"
 
 namespace aura {
 class Window;
 }
 namespace base {
 class TimeDelta;
 }
 namespace gfx {
 class Rect;
 }
 namespace ui {
 class ImplicitAnimationObserver;
 class Layer;
 class LayerAnimationSequence;
+class ScopedLayerAnimationSettings;
 }
 
 namespace views {
 namespace corewm {
 
 // A variety of canned animations for window transitions.
 enum WindowVisibilityAnimationType {
   WINDOW_VISIBILITY_ANIMATION_TYPE_DEFAULT = 0,     // Default. Lets the system
                                                     // decide based on window
                                                     // type.
@@ skipping 39 matching lines @@
     aura::Window* window,
     const base::TimeDelta& duration);
 
 VIEWS_EXPORT base::TimeDelta GetWindowVisibilityAnimationDuration(
     const aura::Window& window);
 
 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,
                                                         bool visible);
 VIEWS_EXPORT bool AnimateWindow(aura::Window* window, WindowAnimationType type);
 
 // Returns true if window animations are disabled for |window|. Window
 // animations are enabled by default. If |window| is NULL, this just checks
 // if the global flag disabling window animations is present.
 VIEWS_EXPORT bool WindowAnimationsDisabled(aura::Window* window);
 
 }  // namespace corewm
 }  // namespace views
 
 #endif  // UI_VIEWS_COREWM_WINDOW_ANIMATIONS_H_