Make WmShellMus use classic types for mus

Make WmShellMus use classic types for mus

mushrome mode should mostly use classic code. This makes WmShellMus
use classic code where appropriate.

BUG=704985
TEST=none
R=msw@chromium.org

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

[sky, 2017-03-28 04:34:53]

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

[commit-bot: I haz the power, 2017-03-28 04:35:05]

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-03-28 04:55:35]

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

[commit-bot: I haz the power, 2017-03-28 04:55:36]

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_...)

[sky, 2017-03-28 16:42:53]

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

[commit-bot: I haz the power, 2017-03-28 16:43:37]

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-03-28 17:50:43]

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

[commit-bot: I haz the power, 2017-03-28 17:50:43]

Dry run: This issue passed the CQ dry run.

[msw, 2017-03-28 20:26:32]

Would it make sense to use WmShellAura in MUS config?

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.cc (right):

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:268:
Shell::GetInstance()->metrics()->RecordUserMetricsAction(action);
Can MASH use this too, since it now instantiates and uses Shell?
(I know we'll need to use the metrics service eventually)
Similar question for other impls that just forward to Shell:
 RecordTaskSwitchMetric
 ToggleIgnoreExternalKeyboard
 SetLaserPointerEnabled
 SetPartialMagnifierEnabled

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:325: // actually used.
optional nit: cite a bug or actually create a
ScopedDisableInternalMouseAndKeyboardX11 like WmShellAura, so both can be fixed
simultaneously? (or maybe x11 is valid for non-mus chrome os?)

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:445: // caled (it is order dependent), that is
not the case with Config::MASH.
nit: called

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.h (right):

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.h:124: // Used in Config::MASH.
optional nit: make MUS/MASH comment styling match between this and
accelerator_controller_delegate_* below?

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.h:125: views::PointerWatcherEventRouter*
pointer_watcher_event_router_;
optional nit: maybe add more meaningful comments about the meaning and usage of
these pointer watcher instances? (ok if not, other members don't)

[sky, 2017-03-28 21:18:23]

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

[commit-bot: I haz the power, 2017-03-28 21:18:35]

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-03-28 21:18:48]

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

[commit-bot: I haz the power, 2017-03-28 21:18:49]

Dry run: CQ has no permission to schedule in bucket
master.tryserver.chromium.linux

[sky, 2017-03-28 21:21:01]

The mushrome build is somewhere between WmShellAura and WmShellMus, so we could
likely make either one work for mushrome. I went with WmShellMus as it is more
closely aligned with the types mushrome uses (mostly the WindowManager). I
suspect using WmShellAura would be more awkward from the mash point of view.

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.cc (right):

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:268:
Shell::GetInstance()->metrics()->RecordUserMetricsAction(action);
On 2017/03/28 20:26:32, msw wrote:
> Can MASH use this too, since it now instantiates and uses Shell?
> (I know we'll need to use the metrics service eventually)
> Similar question for other impls that just forward to Shell:
>  RecordTaskSwitchMetric
>  ToggleIgnoreExternalKeyboard
>  SetLaserPointerEnabled
>  SetPartialMagnifierEnabled

Shell only creates UserMetricsRecorder in classic and mus modes. The keyboard is
specific to classic/mus, laser pointer may just work and I think magnification
is classic/mus specific.

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:325: // actually used.
On 2017/03/28 20:26:32, msw wrote:
> optional nit: cite a bug or actually create a
> ScopedDisableInternalMouseAndKeyboardX11 like WmShellAura, so both can be
fixed
> simultaneously? (or maybe x11 is valid for non-mus chrome os?)

x11 for chromeos isn't actually used anymore outside of bots. I added reference
to bug for removing the config.

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:445: // caled (it is order dependent), that is
not the case with Config::MASH.
On 2017/03/28 20:26:32, msw wrote:
> nit: called

Done.

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.h (right):

https://codereview.chromium.org/2783563002/diff/40001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.h:124: // Used in Config::MASH.
On 2017/03/28 20:26:32, msw wrote:
> optional nit: make MUS/MASH comment styling match between this and
> accelerator_controller_delegate_* below?

I restructured this code to make it more obvious what is used where.

[sky, 2017-03-28 21:23:41]

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

[commit-bot: I haz the power, 2017-03-28 21:23: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-03-28 21:23:57]

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

[commit-bot: I haz the power, 2017-03-28 21:23:58]

Dry run: CQ has no permission to schedule in bucket
master.tryserver.chromium.linux

[msw, 2017-03-28 21:27:00]

lgtm

https://codereview.chromium.org/2783563002/diff/60001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.cc (right):

https://codereview.chromium.org/2783563002/diff/60001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:152: mus_state_->pointer_watcher_adapter.reset();
optional nit: nest in |if (GetConfig() == Config::MUS)|

[sky, 2017-03-28 21:56:08]

https://codereview.chromium.org/2783563002/diff/60001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.cc (right):

https://codereview.chromium.org/2783563002/diff/60001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:152: mus_state_->pointer_watcher_adapter.reset();
On 2017/03/28 21:27:00, msw wrote:
> optional nit: nest in |if (GetConfig() == Config::MUS)|

Done.

[sky, 2017-03-28 21:56:24]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-28 21:59:25]

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-03-28 22:14:22]

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

[commit-bot: I haz the power, 2017-03-28 22:14:23]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[sky, 2017-03-28 22:36:59]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-28 22:37:36]

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-03-29 00:01:18]

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

[commit-bot: I haz the power, 2017-03-29 00:01:19]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[sky, 2017-03-29 15:39:52]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-29 15:40:43]

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-03-29 15:47:49]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490801992475120,
"parent_rev": "1385715e4bbb9aa74264578e5dba49890c49c12d", "commit_rev":
"d8462c3cca625a374bd9ffabb5b23178a53b70e2"}

[commit-bot: I haz the power, 2017-03-29 15:48:43]

Description was changed from

==========
Make WmShellMus use classic types for mus

mushrome mode should mostly use classic code. This makes WmShellMus
use classic code where appropriate.

BUG=704985
TEST=none
R=msw@chromium.org
==========

to

==========
Make WmShellMus use classic types for mus

mushrome mode should mostly use classic code. This makes WmShellMus
use classic code where appropriate.

BUG=704985
TEST=none
R=msw@chromium.org

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

[commit-bot: I haz the power, 2017-03-29 15:48:44]

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