Moves DefaultContainer and PanelContainer to wm_shell_window_ids

Moves DefaultContainer and PanelContainer to wm_shell_window_ids

BUG=603369
TEST=covered by tests
R=jamescook@chromium.org

Committed: https://crrev.com/9f216cf9913e98a67270ac36b451a9291cdeb3fc
Cr-Commit-Position: refs/heads/master@{#389976}

[James Cook, 2016-04-26 23:36:18]

LGTM with nits

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h
File ash/shell_window_ids.h (right):

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h#newc...
ash/shell_window_ids.h:42: static_assert(kShellWindowId_VirtualKeyboardContainer
+ 1 ==
nit: These lines might be slightly clearer as:

static_assert(kShellWindowId_DefaultContainer ==
    kShellWindowId_VirtualKeyboardContainer + 1,
    ....)

This would put the "DefaultContainer" part next to the comment above.

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h#newc...
ash/shell_window_ids.h:44: "docked container must be immediately after always on
top");
nit: update error message, or just get rid of it

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h#newc...
ash/shell_window_ids.h:51: "docked container must be immediately after always on
top");
ditto

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h#newc...
ash/shell_window_ids.h:73: "docked container must be immediately after always on
top");
ditto

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h#newc...
ash/shell_window_ids.h:80: "shelf container must be immediately after docked
container");
ditto

[sky, 2016-04-26 23:58:27]

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h
File ash/shell_window_ids.h (right):

https://codereview.chromium.org/1921423002/diff/1/ash/shell_window_ids.h#newc...
ash/shell_window_ids.h:42: static_assert(kShellWindowId_VirtualKeyboardContainer
+ 1 ==
On 2016/04/26 23:36:18, James Cook wrote:
> nit: These lines might be slightly clearer as:
> 
> static_assert(kShellWindowId_DefaultContainer ==
>     kShellWindowId_VirtualKeyboardContainer + 1,
>     ....)
> 
> This would put the "DefaultContainer" part next to the comment above.

I wasn't really happy with how this file was going. I added back the constants
(commented out) as I feel that makes it more readable. And I moved the asserts
to the end of the file as putting them inline hurts readability too. I also made
an assert covering both the previous and next container. Again, I think that
improves readability. Hopefully you agree.

As to the message, apparently it's not until c++17 that it becomes optional.
Yowza!

Lastly, I'm running git cl format, so it picks how this code looks.

[James Cook, 2016-04-27 00:53:31]

Still LGTM

https://codereview.chromium.org/1921423002/diff/20001/ash/shell_window_ids.h
File ash/shell_window_ids.h (right):

https://codereview.chromium.org/1921423002/diff/20001/ash/shell_window_ids.h#...
ash/shell_window_ids.h:41: // kShellWindowId_DefaultContainer = 6;
Yeah, this is much easier for me to read.

https://codereview.chromium.org/1921423002/diff/20001/ash/shell_window_ids.h#...
ash/shell_window_ids.h:109: "default between keyboard and always-on-top");
optional: Maybe stick the container that is in the middle of the list in the
middle of the statement?

static_assert((kShellWindowId_VirtualKeyboardContainer ==
               kShellWindowId_DefaultContainer - 1) &&
                  (kShellWindowId_DefaultContainer + 1 ==
                   kShellWindowId_AlwaysOnTopContainer),
              "default between keyboard and always-on-top");

or

static_assert((kShellWindowId_VirtualKeyboardContainer + 1 ==
               kShellWindowId_DefaultContainer) &&
                  (kShellWindowId_DefaultContainer ==
                   kShellWindowId_AlwaysOnTopContainer - 1),
              "default between keyboard and always-on-top");

?

[sky, 2016-04-27 02:43:01]

I feel like the ordering is a bit subjective. So I'm going with what I
have here.

On Tue, Apr 26, 2016 at 5:53 PM,  <jamescook@chromium.org> wrote:
> Still LGTM
>
>
> https://codereview.chromium.org/1921423002/diff/20001/ash/shell_window_ids.h
> File ash/shell_window_ids.h (right):
>
>
https://codereview.chromium.org/1921423002/diff/20001/ash/shell_window_ids.h#...
> ash/shell_window_ids.h:41: // kShellWindowId_DefaultContainer = 6;
> Yeah, this is much easier for me to read.
>
>
https://codereview.chromium.org/1921423002/diff/20001/ash/shell_window_ids.h#...
> ash/shell_window_ids.h:109: "default between keyboard and
> always-on-top");
> optional: Maybe stick the container that is in the middle of the list in
> the middle of the statement?
>
> static_assert((kShellWindowId_VirtualKeyboardContainer ==
> kShellWindowId_DefaultContainer - 1) &&
> (kShellWindowId_DefaultContainer + 1 ==
> kShellWindowId_AlwaysOnTopContainer),
> "default between keyboard and always-on-top");
>
> or
>
> static_assert((kShellWindowId_VirtualKeyboardContainer + 1 ==
> kShellWindowId_DefaultContainer) &&
> (kShellWindowId_DefaultContainer ==
> kShellWindowId_AlwaysOnTopContainer - 1),
> "default between keyboard and always-on-top");
>
> ?
>
> https://codereview.chromium.org/1921423002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2016-04-27 02:43:16]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-04-27 02:43:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1921423002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1921423002/20001

[commit-bot: I haz the power, 2016-04-27 02:48:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-27 02:50:20]

Description was changed from

==========
Moves DefaultContainer and PanelContainer to wm_shell_window_ids

BUG=603369
TEST=covered by tests
R=jamescook@chromium.org
==========

to

==========
Moves DefaultContainer and PanelContainer to wm_shell_window_ids

BUG=603369
TEST=covered by tests
R=jamescook@chromium.org

Committed: https://crrev.com/9f216cf9913e98a67270ac36b451a9291cdeb3fc
Cr-Commit-Position: refs/heads/master@{#389976}
==========

[commit-bot: I haz the power, 2016-04-27 02:50:21]

Patchset 2 (id:??) landed as
https://crrev.com/9f216cf9913e98a67270ac36b451a9291cdeb3fc
Cr-Commit-Position: refs/heads/master@{#389976}