Compact layout mananger animates windows rather than the default container layer.

With this change, the windows are animated rather than the default container layer. This allows the default container to remain the same size.

BUG=113729
TEST=run with --aura-window-mode=compact and open many windows. Screen should not shrink.

[sky, 2012-02-13 22:21:13]

https://chromiumcodereview.appspot.com/9388018/diff/1/ash/wm/compact_layout_m...
File ash/wm/compact_layout_manager.cc (right):

https://chromiumcodereview.appspot.com/9388018/diff/1/ash/wm/compact_layout_m...
ash/wm/compact_layout_manager.cc:153:
ScopedVector<ui::ScopedLayerAnimationSettings> settings;
Is there a reason you're creating this outside the for loop? Shouldn't it be
simpler to declare on the stack around line 163ish?

[alicet1, 2012-02-13 22:32:46]

Talked to ian offline, I'll also mention it here. it's hard for me to imagine
what happened at the following cases with this change, can you check they do
work? (better if on device):
- alt-tab through a set of windows, should see the wraparound animation of all
the windows
- child/transient windows ("Save As...", "Print", and "Task Manager") should 
center ok on each window after a bunch of alt-tabbing.
- holding down alt-tab 
- some sequence of open and delete windows

I'll see if I can remember more what was easy to break with this code.

thanx,
alice

http://codereview.chromium.org/9388018/diff/1/ash/wm/compact_layout_manager.cc
File ash/wm/compact_layout_manager.cc (right):

http://codereview.chromium.org/9388018/diff/1/ash/wm/compact_layout_manager.c...
ash/wm/compact_layout_manager.cc:166:
(*const_it)->layer()->SetTransform(transform);
I'm not sure this will work properly -- this will set all windows in the cycle
list to slide to -offset_x. if you have 3 windows, how does alt-tabbing through
them shows the fast sliding from window 3 -> window 1?

[Ian Vollick, 2012-02-14 01:10:49]

On 2012/02/13 22:21:13, sky wrote:
>
https://chromiumcodereview.appspot.com/9388018/diff/1/ash/wm/compact_layout_m...
> File ash/wm/compact_layout_manager.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9388018/diff/1/ash/wm/compact_layout_m...
> ash/wm/compact_layout_manager.cc:153:
> ScopedVector<ui::ScopedLayerAnimationSettings> settings;
> Is there a reason you're creating this outside the for loop? Shouldn't it be
> simpler to declare on the stack around line 163ish?
It is a bit unfortunate that the code looks like this. I did it this way so that
none of the ScopedLayerAnimationSettings objects would be destructed until all
the implicit animations had been started. Otherwise, there's a chance that
OnImplicitAnimationsCompleted might be called when window 1's animations
complete and before I've had a chance to start window 2's animations.

[Ian Vollick, 2012-02-14 02:32:15]

On 2012/02/13 22:32:46, alicet1 wrote:
> Talked to ian offline, I'll also mention it here. it's hard for me to imagine
> what happened at the following cases with this change, can you check they do
> work? (better if on device):
I hit some snags testing on device, but I will continue trying tomorrow.
> - alt-tab through a set of windows, should see the wraparound animation of all
> the windows
Fixed. The compact layout manager's function for laying out windows assumed that
window 1's bounds would have its origin at 0 which is no longer the case.
> - child/transient windows ("Save As...", "Print", and "Task Manager") should 
> center ok on each window after a bunch of alt-tabbing.
Fixed. Some code assumed that windows would have the identity transform which is
also no longer the case. Now that the animations operate in terms of bounds,
this code works again.
> - holding down alt-tab 
Fixed. Also updated the preemption strategy so that the animator handles getting
spammed with animation requests more sanely.
> - some sequence of open and delete windows
Seems to work.
> 
> I'll see if I can remember more what was easy to break with this code.
Please let me know if you notice any more fishiness. I will continue poking on
device tomorrow.
> 
> thanx,
> alice
> 
> http://codereview.chromium.org/9388018/diff/1/ash/wm/compact_layout_manager.cc
> File ash/wm/compact_layout_manager.cc (right):
> 
>
http://codereview.chromium.org/9388018/diff/1/ash/wm/compact_layout_manager.c...
> ash/wm/compact_layout_manager.cc:166:
> (*const_it)->layer()->SetTransform(transform);
> I'm not sure this will work properly -- this will set all windows in the cycle
> list to slide to -offset_x. if you have 3 windows, how does alt-tabbing
through
> them shows the fast sliding from window 3 -> window 1?
Fixed. More details above.

[sky, 2012-02-14 04:41:42]

Ick. Please add a comment for this.

  -Scott

On Mon, Feb 13, 2012 at 5:10 PM,  <vollick@chromium.org> wrote:
> On 2012/02/13 22:21:13, sky wrote:
>
>
https://chromiumcodereview.appspot.com/9388018/diff/1/ash/wm/compact_layout_m...
>>
>> File ash/wm/compact_layout_manager.cc (right):
>
>
>
>
https://chromiumcodereview.appspot.com/9388018/diff/1/ash/wm/compact_layout_m...
>>
>> ash/wm/compact_layout_manager.cc:153:
>> ScopedVector<ui::ScopedLayerAnimationSettings> settings;
>> Is there a reason you're creating this outside the for loop? Shouldn't it
>> be
>> simpler to declare on the stack around line 163ish?
>
> It is a bit unfortunate that the code looks like this. I did it this way so
> that
> none of the ScopedLayerAnimationSettings objects would be destructed until
> all
> the implicit animations had been started. Otherwise, there's a chance that
> OnImplicitAnimationsCompleted might be called when window 1's animations
> complete and before I've had a chance to start window 2's animations.
>
> https://chromiumcodereview.appspot.com/9388018/

[alicet1, 2012-02-14 16:04:39]

lgtm

thanks for checking. also please check that screen lock locks and unlocks ok on
non-first windows, the hiding/visiblity of windows is a little tricky at times.

thanx,
alice

http://codereview.chromium.org/9388018/diff/1002/ash/wm/compact_layout_manage...
File ash/wm/compact_layout_manager.cc (right):

http://codereview.chromium.org/9388018/diff/1002/ash/wm/compact_layout_manage...
ash/wm/compact_layout_manager.cc:175: settings[settings.size() -
1]->AddObserver(this);
settings.back()?

http://codereview.chromium.org/9388018/diff/1002/ash/wm/compact_layout_manage...
ash/wm/compact_layout_manager.cc:178: gfx::Rect bounds =
(*const_it)->layer()->bounds();
should this be GetTargetBounds() ?