Removes WmRootWindowController subclasses

Removes WmRootWindowController subclasses

And makes mash use RootWindowController directly. My next step is to
fold WmRootWindowController back into RootWindowController.

BUG=671246
TEST=none
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2620913003
Cr-Commit-Position: refs/heads/master@{#442817}
Committed: https://chromium.googlesource.com/chromium/src/+/00f98a37895bba109714215198be8e99b3826cdc

[sky, 2017-01-10 18:43:02]

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

[commit-bot: I haz the power, 2017-01-10 18:43:44]

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, 2017-01-10 19:45:11]

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

[commit-bot: I haz the power, 2017-01-10 19:45:12]

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

[sky, 2017-01-10 20:19:16]

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

[commit-bot: I haz the power, 2017-01-10 20:19:45]

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, 2017-01-10 22:00:10]

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

[commit-bot: I haz the power, 2017-01-10 22:00:13]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[James Cook, 2017-01-10 22:41:14]

Nice to see things getting simplified / cleaned up.

https://codereview.chromium.org/2620913003/diff/40001/ash/common/wm_root_wind...
File ash/common/wm_root_window_controller.cc (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/common/wm_root_wind...
ash/common/wm_root_window_controller.cc:162: :
root_window_controller_(root_window_controller), root_(root) {}
nit: either DCHECK root_window_controller and root here, or document in header
that they cannot be null

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/bridge/wm_windo...
File ash/mus/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/bridge/wm_windo...
ash/mus/bridge/wm_window_mus.cc:64: WmRootWindowController*
WmWindowMus::GetRootWindowController() {
Could this override be eliminated in favor of WmWindowAura's version? If not,
should this be using aura_window()->GetRootWindow()?

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/root_window_con...
File ash/mus/root_window_controller.h (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/root_window_con...
ash/mus/root_window_controller.h:38: class RootWindowController {
Aside: It's super confusing we have classes named ash::RootWindowController and
also ash::mus::RootWindowController. Do you have plans to eliminate or rename
the latter?

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.cc:405: DCHECK((ash_host && !window_tree_host) ||
(!ash_host && window_tree_host));
Hooray for DCHECK!

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.cc:507: if (!WmShell::Get()->IsRunningInMash() &&
maybe this should go at the top of the function? or the caller should skip
InitTouchHuds?  I think we have a separate app for touch hud on mus. I'm not
sure if TouchHudDebug should be expected to work.

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
File ash/root_window_controller.h (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:74: enum class RootWindowType { PRIMARY, SECONDARY
};
nit: not your fault, but please document. Does SECONDARY mean for a secondary
display?

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:223: std::unique_ptr<aura::WindowTreeHost>
owned_window_tree_host_;
should this be called aura_window_tree_host_ or mus_window_tree_host_? "owned"
is a little weird, because |ash_host_| is also owned.

https://codereview.chromium.org/2620913003/diff/40001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/shell.h#newcode188
ash/shell.h:188: // TODO(sky): better isolate this.
Hrm. Yeah, this is kind of ugly. Could it live somewhere else? Maybe in
WmRootWindowController or RootWindowController? At least those are a bit less
visible than Shell.

https://codereview.chromium.org/2620913003/diff/40001/ash/shell.h#newcode427
ash/shell.h:427: static aura::WindowTreeClient* window_tree_client_;
Is this only set when running with mus? If so, please document that.

[sky, 2017-01-10 23:06:17]

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

[commit-bot: I haz the power, 2017-01-10 23:06:53]

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

[sky, 2017-01-10 23:06:57]

https://codereview.chromium.org/2620913003/diff/40001/ash/common/wm_root_wind...
File ash/common/wm_root_window_controller.cc (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/common/wm_root_wind...
ash/common/wm_root_window_controller.cc:162: :
root_window_controller_(root_window_controller), root_(root) {}
On 2017/01/10 22:41:13, James Cook wrote:
> nit: either DCHECK root_window_controller and root here, or document in header
> that they cannot be null

Done.

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/bridge/wm_windo...
File ash/mus/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/bridge/wm_windo...
ash/mus/bridge/wm_window_mus.cc:64: WmRootWindowController*
WmWindowMus::GetRootWindowController() {
On 2017/01/10 22:41:13, James Cook wrote:
> Could this override be eliminated in favor of WmWindowAura's version? If not,
> should this be using aura_window()->GetRootWindow()?

Good call. WmWindowAura's implementation works as long as change assertions in
RootWindowController::ForWindow.

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/root_window_con...
File ash/mus/root_window_controller.h (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/mus/root_window_con...
ash/mus/root_window_controller.h:38: class RootWindowController {
On 2017/01/10 22:41:13, James Cook wrote:
> Aside: It's super confusing we have classes named ash::RootWindowController
and
> also ash::mus::RootWindowController. Do you have plans to eliminate or rename
> the latter?

Agreed. I added TODO.

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.cc:507: if (!WmShell::Get()->IsRunningInMash() &&
On 2017/01/10 22:41:13, James Cook wrote:
> maybe this should go at the top of the function? or the caller should skip
> InitTouchHuds?  I think we have a separate app for touch hud on mus. I'm not
> sure if TouchHudDebug should be expected to work.

Done.

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
File ash/root_window_controller.h (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:74: enum class RootWindowType { PRIMARY, SECONDARY
};
On 2017/01/10 22:41:14, James Cook wrote:
> nit: not your fault, but please document. Does SECONDARY mean for a secondary
> display?

Yes. Added comment.

https://codereview.chromium.org/2620913003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:223: std::unique_ptr<aura::WindowTreeHost>
owned_window_tree_host_;
On 2017/01/10 22:41:14, James Cook wrote:
> should this be called aura_window_tree_host_ or mus_window_tree_host_? "owned"
> is a little weird, because |ash_host_| is also owned.

I went with mus_window_tree_host_.

https://codereview.chromium.org/2620913003/diff/40001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2620913003/diff/40001/ash/shell.h#newcode188
ash/shell.h:188: // TODO(sky): better isolate this.
On 2017/01/10 22:41:14, James Cook wrote:
> Hrm. Yeah, this is kind of ugly. Could it live somewhere else? Maybe in
> WmRootWindowController or RootWindowController? At least those are a bit less
> visible than Shell.

I put it in Shell as it isn't related to a single root (RootWindowController is
for a single root) and I suspect other places are going to need it.

https://codereview.chromium.org/2620913003/diff/40001/ash/shell.h#newcode427
ash/shell.h:427: static aura::WindowTreeClient* window_tree_client_;
On 2017/01/10 22:41:14, James Cook wrote:
> Is this only set when running with mus? If so, please document that.

Done.

[James Cook, 2017-01-10 23:23:19]

LGTM with nit

https://codereview.chromium.org/2620913003/diff/60001/ash/root_window_control...
File ash/root_window_controller.h (right):

https://codereview.chromium.org/2620913003/diff/60001/ash/root_window_control...
ash/root_window_controller.h:74: // Enumerates the type of display. If there is
only a single display them
nit: them -> then

[sky, 2017-01-10 23:35:04]

The CQ bit was checked by sky@chromium.org

[sky, 2017-01-10 23:35:05]

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/2620913003/#ps80001
(title: "spelling")

[sky, 2017-01-10 23:35:22]

https://codereview.chromium.org/2620913003/diff/60001/ash/root_window_control...
File ash/root_window_controller.h (right):

https://codereview.chromium.org/2620913003/diff/60001/ash/root_window_control...
ash/root_window_controller.h:74: // Enumerates the type of display. If there is
only a single display them
On 2017/01/10 23:23:19, James Cook wrote:
> nit: them -> then

Done.

[commit-bot: I haz the power, 2017-01-10 23:36:03]

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, 2017-01-11 01:09:09]

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

[commit-bot: I haz the power, 2017-01-11 01:09:10]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[sky, 2017-01-11 03:45:57]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-01-11 03:46:33]

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, 2017-01-11 05:17:02]

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

[commit-bot: I haz the power, 2017-01-11 05:17:03]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[sky, 2017-01-11 05:21:56]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-01-11 05:22: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, 2017-01-11 06:03:54]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484112116669520,
"parent_rev": "46a10c19dab82172fb82578b062b8929600cf376", "commit_rev":
"00f98a37895bba109714215198be8e99b3826cdc"}

[commit-bot: I haz the power, 2017-01-11 06:04:50]

Description was changed from

==========
Removes WmRootWindowController subclasses

And makes mash use RootWindowController directly. My next step is to
fold WmRootWindowController back into RootWindowController.

BUG=671246
TEST=none
R=jamescook@chromium.org
==========

to

==========
Removes WmRootWindowController subclasses

And makes mash use RootWindowController directly. My next step is to
fold WmRootWindowController back into RootWindowController.

BUG=671246
TEST=none
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2620913003
Cr-Commit-Position: refs/heads/master@{#442817}
Committed:
https://chromium.googlesource.com/chromium/src/+/00f98a37895bba109714215198be...
==========

[commit-bot: I haz the power, 2017-01-11 06:04:52]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/00f98a37895bba109714215198be...