ash: Animate changes in the uber tray.

ash: Animate changes in the uber tray.

Specifically, animate when a view in the tray hides/shows itself. For now, the
tray does not animate when the size of a view changes (e.g. date-view changes
size because it's 10:00).

BUG=117209
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=133526

[sadrul, 2012-04-02 20:41:12]

Hi! This is the approach I landed on. I tried doing things with
ScopedLayerAnimationSettings, and BoundsAnimator, but this seems to be the
simplest approach. (mostly because at each step of the animation, we have to
re-layout the widget).

This is also buggy (see note below). Suggestions?

http://codereview.chromium.org/9969068/diff/1/ash/system/tray/system_tray.cc
File ash/system/tray/system_tray.cc (right):

http://codereview.chromium.org/9969068/diff/1/ash/system/tray/system_tray.cc#...
ash/system/tray/system_tray.cc:126: child_->SetVisible(true);
This is a bit hacky. This can cause a bug if the icon is re-displayed before
this animation ends (e.g. quickly toggles the caps-lock). Something like
aura::Window::AcquireLayer for views::View could be useful here?

[sadrul, 2012-04-20 21:05:51]

I made some changes to the approach, and created a base-class that handles all
the items in the tray. PTAL.

A note: I considered using ScopedLayerAnimationSettings, BoundsAnimator etc, but
it would still be necessary to install an AnimationDelegate for the
implicit-animation to make sure all the contents in the tray get laid-out
correctly at each step of the animation. So I went ahead with an explicit
animation.

[Ben Goodger (Google), 2012-04-23 15:33:42]

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/system_tray_...
File ash/system/tray/system_tray_item.h (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/system_tray_...
ash/system/tray/system_tray_item.h:27: // NOTE: The returned view should almost
always be a TrayItemView.
You should document a case where it wouldn't be.

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
File ash/system/tray/tray_item_view.cc (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.cc:43: if (!GetWidget()) {
Can you implement this by overriding VisibilityChanged instead of overriding
SetVisible?

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.cc:79: gfx::Size TrayItemView::GetPreferredSize()
{
.cc function order must match .h

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
File ash/system/tray/tray_item_view.h (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.h:60: // Overridden from ui::AnimationDelegate.
Can these be private? If so, flow them after GetPreferredSize, so that
GetPreferredSize and PreferredSizeChanged can run together in the .cc file.

[sadrul, 2012-04-23 16:07:58]

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/system_tray_...
File ash/system/tray/system_tray_item.h (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/system_tray_...
ash/system/tray/system_tray_item.h:27: // NOTE: The returned view should almost
always be a TrayItemView.
On 2012/04/23 15:33:42, Ben Goodger (Google) wrote:
> You should document a case where it wouldn't be.

Done.

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
File ash/system/tray/tray_item_view.cc (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.cc:43: if (!GetWidget()) {
On 2012/04/23 15:33:42, Ben Goodger (Google) wrote:
> Can you implement this by overriding VisibilityChanged instead of overriding
> SetVisible?

This would work when showing the view, however, when hiding, we need to make
sure the view keeps painting while the view is being hidden, so we need to delay
the call to View::SetVisible because that would destroy the layer. So, we make
sure we start the animation here, and defer the View::SetVisible(false) until
the animation ends. (something like a View::AcquireLayer would probably have
been useful here to avoid delaying the call to View::SetVisible)

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.cc:79: gfx::Size TrayItemView::GetPreferredSize()
{
On 2012/04/23 15:33:42, Ben Goodger (Google) wrote:
> .cc function order must match .h

Fixed.

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
File ash/system/tray/tray_item_view.h (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.h:60: // Overridden from ui::AnimationDelegate.
On 2012/04/23 15:33:42, Ben Goodger (Google) wrote:
> Can these be private? If so, flow them after GetPreferredSize, so that
> GetPreferredSize and PreferredSizeChanged can run together in the .cc file.

Done.

[Ben Goodger (Google), 2012-04-23 17:43:38]

lgtm

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
File ash/system/tray/tray_item_view.cc (right):

http://codereview.chromium.org/9969068/diff/8002/ash/system/tray/tray_item_vi...
ash/system/tray/tray_item_view.cc:43: if (!GetWidget()) {
On 2012/04/23 16:07:58, sadrul wrote:
> On 2012/04/23 15:33:42, Ben Goodger (Google) wrote:
> > Can you implement this by overriding VisibilityChanged instead of overriding
> > SetVisible?
> 
> This would work when showing the view, however, when hiding, we need to make
> sure the view keeps painting while the view is being hidden, so we need to
delay
> the call to View::SetVisible because that would destroy the layer. So, we make
> sure we start the animation here, and defer the View::SetVisible(false) until
> the animation ends. (something like a View::AcquireLayer would probably have
> been useful here to avoid delaying the call to View::SetVisible)

Ah. I guess we have better support for this sort of thing in Aura. I guess this
can do for now. I'd like to come up with a better strategy for this in the long
run, but I'll take the AI to do that.