Converts MaximizeModeWindowManager to use ash/common types

Converts MaximizeModeWindowManager to use ash/common types

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

Committed: https://crrev.com/e09bfe93b19d0823d49eea79881c0563e6239cd1
Cr-Commit-Position: refs/heads/master@{#402846}

[James Cook, 2016-06-29 14:11:40]

Looks fine but one question.

https://codereview.chromium.org/2103913003/diff/20001/ash/wm/maximize_mode/ma...
File ash/wm/maximize_mode/maximize_mode_window_manager.cc (right):

https://codereview.chromium.org/2103913003/diff/20001/ash/wm/maximize_mode/ma...
ash/wm/maximize_mode/maximize_mode_window_manager.cc:113:
IsContainerWindow(window) &&
just to double-check: Is it necessary to check that |window| (which has the
observer) is also a container? What if |window| is an ordinary window with an
observer added by MaximizeAndTrackWindow()?

[sky, 2016-06-29 15:41:11]

https://codereview.chromium.org/2103913003/diff/20001/ash/wm/maximize_mode/ma...
File ash/wm/maximize_mode/maximize_mode_window_manager.cc (right):

https://codereview.chromium.org/2103913003/diff/20001/ash/wm/maximize_mode/ma...
ash/wm/maximize_mode/maximize_mode_window_manager.cc:113:
IsContainerWindow(window) &&
On 2016/06/29 14:11:40, James Cook wrote:
> just to double-check: Is it necessary to check that |window| (which has the
> observer) is also a container? What if |window| is an ordinary window with an
> observer added by MaximizeAndTrackWindow()?

Good eye. I was being extra cautious here as this class observers both the
default container, and some of its children. OnWindowTreeChanged has different
semantics, in that it's sent for all observed windows, which means this is
getting called more than it was previously. But the !ContainsKey() check on 114
should deal with that, so I removed it. Take another look?

[James Cook, 2016-06-29 16:07:40]

LGTM

https://codereview.chromium.org/2103913003/diff/20001/ash/wm/maximize_mode/ma...
File ash/wm/maximize_mode/maximize_mode_window_manager.cc (right):

https://codereview.chromium.org/2103913003/diff/20001/ash/wm/maximize_mode/ma...
ash/wm/maximize_mode/maximize_mode_window_manager.cc:113:
IsContainerWindow(window) &&
On 2016/06/29 15:41:11, sky wrote:
> On 2016/06/29 14:11:40, James Cook wrote:
> > just to double-check: Is it necessary to check that |window| (which has the
> > observer) is also a container? What if |window| is an ordinary window with
an
> > observer added by MaximizeAndTrackWindow()?
> 
> Good eye. I was being extra cautious here as this class observers both the
> default container, and some of its children. OnWindowTreeChanged has different
> semantics, in that it's sent for all observed windows, which means this is
> getting called more than it was previously. But the !ContainsKey() check on
114
> should deal with that, so I removed it. Take another look?

It sounds like it will work correctly either way, so whichever way you prefer is
fine. I just wanted to double-check.

[sky, 2016-06-29 16:20:46]

The CQ bit was checked by sky@chromium.org

[sky, 2016-06-29 16:20:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2103913003/#ps80001
(title: "fix chrome")

[commit-bot: I haz the power, 2016-06-29 16:21:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 17:18:02]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-29 17:19:21]

Description was changed from

==========
Converts MaximizeModeWindowManager to use ash/common types

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

to

==========
Converts MaximizeModeWindowManager to use ash/common types

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

Committed: https://crrev.com/e09bfe93b19d0823d49eea79881c0563e6239cd1
Cr-Commit-Position: refs/heads/master@{#402846}
==========

[commit-bot: I haz the power, 2016-06-29 17:19:22]

Patchset 5 (id:??) landed as
https://crrev.com/e09bfe93b19d0823d49eea79881c0563e6239cd1
Cr-Commit-Position: refs/heads/master@{#402846}