add compact layout manager window transition unittest.

add compact layout manager window transition unittest.

BUG=None
TEST=added unittest.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=120023

[alicet1, 2012-01-30 21:59:14]

hi guys,

test case for compact layout manager layer transition.

james, for the change at the test_shell_delegate.h for window cycle list -- in
linear mode we don't need reversing of the windows list, no?

sky, please take a look.

thanx,
alice

[James Cook, 2012-01-30 22:12:19]

As long as the other tests pass, I don't think it matters whether or not the
cycle order is reversed in test_shell_delegate.

http://codereview.chromium.org/9301016/diff/1/ash/test/test_shell_delegate.cc
File ash/test/test_shell_delegate.cc (right):

http://codereview.chromium.org/9301016/diff/1/ash/test/test_shell_delegate.cc...
ash/test/test_shell_delegate.cc:48: // if we are mimicing MRU.
nit: mimicing -> mimicking

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
File ash/wm/compact_layout_manager_unittest.cc (right):

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:93: static gfx::Rect kMaxBounds =
gfx::Rect(0, 0, 600, 600);
Are static class initializers allowed in test code?  I know they're not allowed
in non-test code.

Also, consider using different numbers for height and width.

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:111: layout_manager_ =  new
internal::CompactLayoutManager();
nit: one space after =

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:124: aura::Window*
CreateNormalWindow(int id,
Is there some sort of Aura test utility class or window test class that has
factory functions for windows?

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:136: aura::Window* default_container()
const {
Hooray for simple utility functions to make the test clearer!

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:159: // Create 3 windows, check that
the layer grow as each one is added
Maybe assert the initial state of the container, before any windows are added?

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:166: EXPECT_EQ(kMaxBounds.width() * 2,
default_container_layer_width());
I like that you're using constant * 2 instead of a fixed number.

[alicet1, 2012-01-30 23:52:53]

http://codereview.chromium.org/9301016/diff/1/ash/test/test_shell_delegate.cc
File ash/test/test_shell_delegate.cc (right):

http://codereview.chromium.org/9301016/diff/1/ash/test/test_shell_delegate.cc...
ash/test/test_shell_delegate.cc:48: // if we are mimicing MRU.
On 2012/01/30 22:12:20, James Cook (Chromium) wrote:
> nit: mimicing -> mimicking

Done.

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
File ash/wm/compact_layout_manager_unittest.cc (right):

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:93: static gfx::Rect kMaxBounds =
gfx::Rect(0, 0, 600, 600);
On 2012/01/30 22:12:20, James Cook (Chromium) wrote:
> Are static class initializers allowed in test code?  I know they're not
allowed
> in non-test code.
> 
> Also, consider using different numbers for height and width.

changed.

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:111: layout_manager_ =  new
internal::CompactLayoutManager();
On 2012/01/30 22:12:20, James Cook (Chromium) wrote:
> nit: one space after =
> 

Done.

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:124: aura::Window*
CreateNormalWindow(int id,
On 2012/01/30 22:12:20, James Cook (Chromium) wrote:
> Is there some sort of Aura test utility class or window test class that has
> factory functions for windows?

kinda, test_windows.cc, but here I need to create a maximized window and then
show it. so I will still end up with a convenience method here with the extra
setup.

http://codereview.chromium.org/9301016/diff/1/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:159: // Create 3 windows, check that
the layer grow as each one is added
On 2012/01/30 22:12:20, James Cook (Chromium) wrote:
> Maybe assert the initial state of the container, before any windows are added?

Done.

[James Cook, 2012-01-31 00:45:09]

LGTM assuming "const" fixed.

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
File ash/wm/compact_layout_manager_unittest.cc (right):

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:93: static gfx::Rect kMaxBounds(0, 0,
600, 600);
I meant, do you need to use plain-old-data for constants in test files like you
do for regular code:

const int kMaxWidth = 800;
const int kMaxHeight = 600;

Then construct the gfx::Rect at runtime?  You're supposed to do this in Chrome
code per http://www.chromium.org/developers/coding-style#Static_Variables but I
don't know if there's an exception for test code.

Also, needs "const" qualifier, and doesn't need "static".

[sky, 2012-01-31 01:20:14]

LGTM

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
File ash/wm/compact_layout_manager_unittest.cc (right):

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:156: };
nit: private: DISALLOW_COPY_AND_ASSIGN

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:158: 
nit: only one empty line.

[alicet1, 2012-02-01 00:32:54]

submitting, thanks!

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
File ash/wm/compact_layout_manager_unittest.cc (right):

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:93: static gfx::Rect kMaxBounds(0, 0,
600, 600);
On 2012/01/31 00:45:09, James Cook (Chromium) wrote:
> I meant, do you need to use plain-old-data for constants in test files like
you
> do for regular code:
> 
> const int kMaxWidth = 800;
> const int kMaxHeight = 600;
> 
> Then construct the gfx::Rect at runtime?  You're supposed to do this in Chrome
> code per http://www.chromium.org/developers/coding-style#Static_Variables but
I
> don't know if there's an exception for test code.
> 
> Also, needs "const" qualifier, and doesn't need "static".
good point, I really only need the sizes. changed.

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:156: };
On 2012/01/31 01:20:14, sky wrote:
> nit: private: DISALLOW_COPY_AND_ASSIGN

Done.

http://codereview.chromium.org/9301016/diff/5/ash/wm/compact_layout_manager_u...
ash/wm/compact_layout_manager_unittest.cc:158: 
On 2012/01/31 01:20:14, sky wrote:
> nit: only one empty line.

Done.

[commit-bot: I haz the power, 2012-02-01 01:30:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alicet@chromium.org/9301016/8001

[commit-bot: I haz the power, 2012-02-01 04:02:27]

Change committed as 120023