Folds WmRootWindowController into RootWindowController

Folds WmRootWindowController into RootWindowController

This is (mostly) the bare minimum to merge the two. Future patches
will cleanup WmWindow usage.

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

Review-Url: https://codereview.chromium.org/2620153003
Cr-Commit-Position: refs/heads/master@{#443003}
Committed: https://chromium.googlesource.com/chromium/src/+/b664383006661b34cbe214ace8d488e684cb6670

[sky, 2017-01-11 05:55:36]

This is the same as https://codereview.chromium.org/2625843002/ , but I kept the
definition in wm_root_window_controller.cc to make it easier to review. After
this I'll lands I'll TBR and patch that moves the definitions into
root_window_controller.cc.

[sky, 2017-01-11 15:27:41]

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

[commit-bot: I haz the power, 2017-01-11 15:28:08]

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-11 16:38:21]

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

[commit-bot: I haz the power, 2017-01-11 16:38:22]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-01-11 17:00:54]

LGTM with nits. Thanks for moving the code back to the other .cc file -- it was
much easier to review.

https://codereview.chromium.org/2620153003/diff/20001/ash/common/accelerators...
File ash/common/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/2620153003/diff/20001/ash/common/accelerators...
ash/common/accelerators/accelerator_controller.cc:55: #include
"ui/keyboard/keyboard_controller.h"
Thanks for cleaning up the includes.

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

https://codereview.chromium.org/2620153003/diff/40001/ash/common/wm_root_wind...
ash/common/wm_root_window_controller.cc:268: void
RootWindowController::ConfigureWidgetInitParamsForContainer(
Not for this CL: It would be nice to get rid of this function at some point. I
remember doing a lot of plumbing to call this function -- I think code used to
just get the root window for a window and grab the container as a child.

https://codereview.chromium.org/2620153003/diff/40001/ash/common/wm_root_wind...
ash/common/wm_root_window_controller.cc:278:
::wm::ConvertPointFromScreen(GetRootWindow(), &location_in_root);
nit: maybe cache root_window instead of calling GetRootWindow() three times?

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

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:106: // TODO(sky): remove this,
http://crbug.com/671246.
heh, you have a bug for it already. hooray!

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:129: WorkspaceController* workspace_controller() {
optional nit: I'm not sure it's part of the style guide, but how about putting
all the inline getters together near the top? Or maybe just move the HasShelf /
GetShelf stuff down near GetShelfLayoutManager?

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:377: TouchHudDebug* touch_hud_debug_;
nit: initialize to null in header, similar to the other members you moved over.
Or init the ones you moved in the .cc file.

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

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:404:
controller->GetSystemModalLayoutManager(NULL));
nit: nullptr if you're touching the line

[sky, 2017-01-11 18:57:42]

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

https://codereview.chromium.org/2620153003/diff/40001/ash/common/wm_root_wind...
ash/common/wm_root_window_controller.cc:268: void
RootWindowController::ConfigureWidgetInitParamsForContainer(
On 2017/01/11 17:00:53, James Cook wrote:
> Not for this CL: It would be nice to get rid of this function at some point. I
> remember doing a lot of plumbing to call this function -- I think code used to
> just get the root window for a window and grab the container as a child.

Agreed. I was tempted to do the change here, but it turns out there is ~35 calls
to said function. Yowza!

https://codereview.chromium.org/2620153003/diff/40001/ash/common/wm_root_wind...
ash/common/wm_root_window_controller.cc:278:
::wm::ConvertPointFromScreen(GetRootWindow(), &location_in_root);
On 2017/01/11 17:00:53, James Cook wrote:
> nit: maybe cache root_window instead of calling GetRootWindow() three times?

Done.

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

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:106: // TODO(sky): remove this,
http://crbug.com/671246.
On 2017/01/11 17:00:54, James Cook wrote:
> heh, you have a bug for it already. hooray!

Well, this is actually the meta bug for unraveling the porting layer. I'm adding
it to a lot of places and have to make sure I address them all.

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:129: WorkspaceController* workspace_controller() {
On 2017/01/11 17:00:54, James Cook wrote:
> optional nit: I'm not sure it's part of the style guide, but how about putting
> all the inline getters together near the top? Or maybe just move the HasShelf
/
> GetShelf stuff down near GetShelfLayoutManager?

I tried to keep related things grouped together. So workspace_controller() is
workspace related and right next to GetWorkspaceWindowState(). Similarly below
all the wallpaper related functions are grouped together.

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller.h:377: TouchHudDebug* touch_hud_debug_;
On 2017/01/11 17:00:53, James Cook wrote:
> nit: initialize to null in header, similar to the other members you moved
over.
> Or init the ones you moved in the .cc file.

Done.

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

https://codereview.chromium.org/2620153003/diff/40001/ash/root_window_control...
ash/root_window_controller_unittest.cc:404:
controller->GetSystemModalLayoutManager(NULL));
On 2017/01/11 17:00:54, James Cook wrote:
> nit: nullptr if you're touching the line

Done.

[sky, 2017-01-11 18:58:18]

The CQ bit was checked by sky@chromium.org

[sky, 2017-01-11 18:58:19]

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/2620153003/#ps60001
(title: "feedback")

[commit-bot: I haz the power, 2017-01-11 18:59:06]

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 19:04:48]

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

[commit-bot: I haz the power, 2017-01-11 19:04:49]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sky, 2017-01-11 19:12:34]

The CQ bit was checked by sky@chromium.org

[sky, 2017-01-11 19:12:34]

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

[commit-bot: I haz the power, 2017-01-11 19:13:14]

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 22:08:49]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484161954190250,
"parent_rev": "5221fd1d6caa0d84ad5a0f8abb4bd745f0170b94", "commit_rev":
"b664383006661b34cbe214ace8d488e684cb6670"}

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

Description was changed from

==========
Folds WmRootWindowController into RootWindowController

This is (mostly) the bare minimum to merge the two. Future patches
will cleanup WmWindow usage.

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

to

==========
Folds WmRootWindowController into RootWindowController

This is (mostly) the bare minimum to merge the two. Future patches
will cleanup WmWindow usage.

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

Review-Url: https://codereview.chromium.org/2620153003
Cr-Commit-Position: refs/heads/master@{#443003}
Committed:
https://chromium.googlesource.com/chromium/src/+/b664383006661b34cbe214ace8d4...
==========

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

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