mash: Partially migrate ShelfLayoutManager to ash common types.

mash: Partially migrate ShelfLayoutManager to ash common types.

ActivationChangeObserver -> WmActivationObserver.
Always create RootWindowControllerObserverImpl.
Make RootWindowControllerObserverImpl add/remove itself.

Compare display ids instead of root windows.
Use wm::GetWindowForFullscreenMode instead of ash::RWC.
Use WmLookup instead of WmWindowAura::Get.
Use Widget::IsMouseEventsEnabled instead of CursorClient.
Fix IWYU issues; remove unused forward decls/friends.

BUG=631215
TEST=Automated tests; no behavior changes.
R=jamescook@chromium.org

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

[msw, 2016-07-27 00:00:50]

Description was changed from

==========
mash: Partially migrate ShelfLayoutManager to ash common types.

BUG=631215
==========

to

==========
mash: Partially migrate ShelfLayoutManager to ash common types.

ActivationChangeObserver -> WmActivationObserver.
Always create RootWindowControllerObserverImpl.
Make RootWindowControllerObserverImpl add/remove itself.

Compare display ids instead of root windows.
Use wm::GetWindowForFullscreenMode instead of ash::RWC.
Use WmLookup instead of WmWindowAura::Get.
Use Widget::IsMouseEventsEnabled instead of CursorClient.
Fix IWYU issues; remove unused forward decls/friends.

BUG=631215
TEST=Automated tests; no behavior changes.
R=jamescook@chromium.org
==========

[msw, 2016-07-27 00:00:50]

msw@chromium.org changed reviewers:
+ jamescook@chromium.org

[msw, 2016-07-27 00:02:34]

Hey James, please take a look; thanks!

[msw, 2016-07-27 00:02:41]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 00:03:13]

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

[msw, 2016-07-27 00:10:57]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 00:11:31]

Dry run: 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-07-27 00:43:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 00:43:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[James Cook, 2016-07-27 03:15:11]

A couple questions.

Also, the bots look unhappy. I've been bitten by
WallpaperManagerBrowserTest.LoadCustomLargeWallpaperForLargeExternalScreen
before when I tried to reorder stuff. When it failed for me it was a browser
process crash. It does something weird where it connects an external display,
which adds a new rootwindowcontroller/shelf, but doesn't necessarily simulate
login correctly. :-(

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:179: if (widget &&
WmLookup::Get()->GetWindowForWidget(widget)) {
Just curious, is |widget| null in tests, or does that happen in normal shutdown?

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:311:
WmLookup::Get()->GetWindowForWidget(shelf_widget_));
totally optional: Are you thinking that long-term |root_window_| will go away
and everything will compute it from |shelf_widget_|?  If you think we'll need it
longer-term you could cache a |wm_root_window_| in the constructor. Or introduce
a GetRootWindowForShelf() utility method.

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:1053: int64_t display_id = WmLookup::Get()
Would it work to use this for the applist check?

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.h (right):

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.h:145: // Overriden from WmActivationObserver:
nit: "Overridden"

https://codereview.chromium.org/2187513003/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_resizer_unittest.cc (left):

https://codereview.chromium.org/2187513003/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_resizer_unittest.cc:237: ShelfModel* model_;
Nice cleanup.

[msw, 2016-07-27 22:30:49]

Patchset #4 (id:60001) has been deleted

[msw, 2016-07-27 22:31:18]

Please take another look; thanks!
(hopefully I fixed all the test crashes/failures)

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:179: if (widget &&
WmLookup::Get()->GetWindowForWidget(widget)) {
On 2016/07/27 03:15:11, James Cook wrote:
> Just curious, is |widget| null in tests, or does that happen in normal
shutdown?

Neither. |window| may be null; I removed the extra widget check.

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:311:
WmLookup::Get()->GetWindowForWidget(shelf_widget_));
On 2016/07/27 03:15:11, James Cook wrote:
> totally optional: Are you thinking that long-term |root_window_| will go away
> and everything will compute it from |shelf_widget_|?  If you think we'll need
it
> longer-term you could cache a |wm_root_window_| in the constructor. Or
introduce
> a GetRootWindowForShelf() utility method.

Yeah, we can probably nuke |root_window_| soon; computing the WmWindow from
|shelf_widget_| makes sense for now, but we don't need the root WmWindow for
most stuff; caching a WmWindow* |shelf_window_| (or adding some helper) might
make sense, but I'll punt on that for now.

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:1053: int64_t display_id = WmLookup::Get()
On 2016/07/27 03:15:11, James Cook wrote:
> Would it work to use this for the applist check?

Done.

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.h (right):

https://codereview.chromium.org/2187513003/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.h:145: // Overriden from WmActivationObserver:
On 2016/07/27 03:15:11, James Cook wrote:
> nit: "Overridden"

Done.

https://codereview.chromium.org/2187513003/diff/40001/ash/wm/dock/docked_wind...
File ash/wm/dock/docked_window_resizer_unittest.cc (left):

https://codereview.chromium.org/2187513003/diff/40001/ash/wm/dock/docked_wind...
ash/wm/dock/docked_window_resizer_unittest.cc:237: ShelfModel* model_;
On 2016/07/27 03:15:11, James Cook wrote:
> Nice cleanup.

Acknowledged.

[msw, 2016-07-27 22:31:28]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 22:31:47]

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

[James Cook, 2016-07-27 22:38:22]

LGTM

[msw, 2016-07-27 22:40:39]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2016-07-27 22:41:17]

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-07-27 23:29:50]

Description was changed from

==========
mash: Partially migrate ShelfLayoutManager to ash common types.

ActivationChangeObserver -> WmActivationObserver.
Always create RootWindowControllerObserverImpl.
Make RootWindowControllerObserverImpl add/remove itself.

Compare display ids instead of root windows.
Use wm::GetWindowForFullscreenMode instead of ash::RWC.
Use WmLookup instead of WmWindowAura::Get.
Use Widget::IsMouseEventsEnabled instead of CursorClient.
Fix IWYU issues; remove unused forward decls/friends.

BUG=631215
TEST=Automated tests; no behavior changes.
R=jamescook@chromium.org
==========

to

==========
mash: Partially migrate ShelfLayoutManager to ash common types.

ActivationChangeObserver -> WmActivationObserver.
Always create RootWindowControllerObserverImpl.
Make RootWindowControllerObserverImpl add/remove itself.

Compare display ids instead of root windows.
Use wm::GetWindowForFullscreenMode instead of ash::RWC.
Use WmLookup instead of WmWindowAura::Get.
Use Widget::IsMouseEventsEnabled instead of CursorClient.
Fix IWYU issues; remove unused forward decls/friends.

BUG=631215
TEST=Automated tests; no behavior changes.
R=jamescook@chromium.org
==========

[commit-bot: I haz the power, 2016-07-27 23:29:51]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-07-27 23:32:53]

Description was changed from

==========
mash: Partially migrate ShelfLayoutManager to ash common types.

ActivationChangeObserver -> WmActivationObserver.
Always create RootWindowControllerObserverImpl.
Make RootWindowControllerObserverImpl add/remove itself.

Compare display ids instead of root windows.
Use wm::GetWindowForFullscreenMode instead of ash::RWC.
Use WmLookup instead of WmWindowAura::Get.
Use Widget::IsMouseEventsEnabled instead of CursorClient.
Fix IWYU issues; remove unused forward decls/friends.

BUG=631215
TEST=Automated tests; no behavior changes.
R=jamescook@chromium.org
==========

to

==========
mash: Partially migrate ShelfLayoutManager to ash common types.

ActivationChangeObserver -> WmActivationObserver.
Always create RootWindowControllerObserverImpl.
Make RootWindowControllerObserverImpl add/remove itself.

Compare display ids instead of root windows.
Use wm::GetWindowForFullscreenMode instead of ash::RWC.
Use WmLookup instead of WmWindowAura::Get.
Use Widget::IsMouseEventsEnabled instead of CursorClient.
Fix IWYU issues; remove unused forward decls/friends.

BUG=631215
TEST=Automated tests; no behavior changes.
R=jamescook@chromium.org

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

[commit-bot: I haz the power, 2016-07-27 23:32:54]

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