Adds ability for ash_unittests to run in mushrome mode

Adds ability for ash_unittests to run in mushrome mode

This makes it so that if you supply --mus to ash*unittests it'll run
in mushrome mode. A slew of tests fail with --mus, but this at least
enables mushrome mode for tests.

BUG=705715
TEST=test only changes
R=msw@chromium.org, reveman@chromium.org

Review-Url: https://codereview.chromium.org/2788463003
Cr-Commit-Position: refs/heads/master@{#461119}
Committed: https://chromium.googlesource.com/chromium/src/+/25b49b2809f59ba95b9abc95ac0a0adaf4c97722

[sky, 2017-03-30 21:37:26]

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

[sky, 2017-03-30 21:37:40]

Description was changed from

==========
Adds ability for ash_unittests to run in mushrome mode

This makes it so that if you supply --mus to ash*unittests it'll run
in mushrome mode. A slew of tests fail with --mus, but this at least
enables mushrome mode for tests.

BUG=705715
TEST=test only changes
R=msw@chromium.org
==========

to

==========
Adds ability for ash_unittests to run in mushrome mode

This makes it so that if you supply --mus to ash*unittests it'll run
in mushrome mode. A slew of tests fail with --mus, but this at least
enables mushrome mode for tests.

BUG=705715
TEST=test only changes
R=msw@chromium.org, reveman@chromium.org
==========

[sky, 2017-03-30 21:37:40]

sky@chromium.org changed reviewers:
+ reveman@chromium.org

[commit-bot: I haz the power, 2017-03-30 21:38:06]

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

[sky, 2017-03-30 21:38:18]

+reveman for exo build changes. exo uses ash_test_suite, so it needs to get
additional deps ash_test_suite now needs.

[msw, 2017-03-30 23:06:05]

lgtm with nits and qs

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
File ash/test/ash_test_helper.cc (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:137: if (config_ != Config::CLASSIC)
q: Do we actually want to create the mash window manager for mus?

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:224: // WindowManager owns the CaptureController for
mash.
optional nit: "mash and mus configs"?

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:248: void AshTestHelper::UpdateDisplayForMash(const
std::string& display_spec) {
optional nit: [add a todo to] rename this for Mus[OrMash]?

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:289: void AshTestHelper::CreateMashWindowManager() {
ditto optional nit: [add a todo to] rename this for Mus[OrMash]?

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helper.h
File ash/test/ash_test_helper.h (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.h:46: enum class Config;
optional nit: order before RootWindowController?

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.h:55: class AshTestSuite;
optional nit: technically, I don't think this fwd decl is needed

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.h:115: // The two TestSuites need to manipulate
|config_|.
nit: These? optionally expose |config_for_testing()| accessor instead?

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite.cc
File ash/test/ash_test_suite.cc (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite...
ash/test/ash_test_suite.cc:60: const bool is_classic =
optional nit: is_mus would be a one-liner.

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite.h
File ash/test/ash_test_suite.h (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite...
ash/test/ash_test_suite.h:37: // Only used when running in Config::MUS.
nit: explain what it's used for?

https://codereview.chromium.org/2788463003/diff/20001/components/exo/BUILD.gn
File components/exo/BUILD.gn (right):

https://codereview.chromium.org/2788463003/diff/20001/components/exo/BUILD.gn...
components/exo/BUILD.gn:166: "//ash/public/cpp:ash_public_cpp",
Are either of these actually needed/used by this target?

[commit-bot: I haz the power, 2017-03-30 23:06:39]

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

[commit-bot: I haz the power, 2017-03-30 23:06:40]

Dry run: This issue passed the CQ dry run.

[sky, 2017-03-30 23:35:46]

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
File ash/test/ash_test_helper.cc (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:137: if (config_ != Config::CLASSIC)
On 2017/03/30 23:06:05, msw wrote:
> q: Do we actually want to create the mash window manager for mus?

Yes. WindowManager talks to mus, which we still need to do in the mushrome
world.

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:224: // WindowManager owns the CaptureController for
mash.
On 2017/03/30 23:06:05, msw wrote:
> optional nit: "mash and mus configs"?

Done.

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.cc:248: void AshTestHelper::UpdateDisplayForMash(const
std::string& display_spec) {
On 2017/03/30 23:06:05, msw wrote:
> optional nit: [add a todo to] rename this for Mus[OrMash]?

I had proposed we use mash for mus and/or mash code in the naming thread. I'm
going to leave this and we'll see how it goes...

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helper.h
File ash/test/ash_test_helper.h (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.h:46: enum class Config;
On 2017/03/30 23:06:05, msw wrote:
> optional nit: order before RootWindowController?

I actually prefer separating them out and there doesn't appear to be a standard
here...

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_helpe...
ash/test/ash_test_helper.h:115: // The two TestSuites need to manipulate
|config_|.
On 2017/03/30 23:06:05, msw wrote:
> nit: These? optionally expose |config_for_testing()| accessor instead?

They need to set config_, which I would like to restrict to the TestSuites.

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite.cc
File ash/test/ash_test_suite.cc (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite...
ash/test/ash_test_suite.cc:60: const bool is_classic =
On 2017/03/30 23:06:05, msw wrote:
> optional nit: is_mus would be a one-liner.

Nice!

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite.h
File ash/test/ash_test_suite.h (right):

https://codereview.chromium.org/2788463003/diff/20001/ash/test/ash_test_suite...
ash/test/ash_test_suite.h:37: // Only used when running in Config::MUS.
On 2017/03/30 23:06:05, msw wrote:
> nit: explain what it's used for?

Done.

https://codereview.chromium.org/2788463003/diff/20001/components/exo/BUILD.gn
File components/exo/BUILD.gn (right):

https://codereview.chromium.org/2788463003/diff/20001/components/exo/BUILD.gn...
components/exo/BUILD.gn:166: "//ash/public/cpp:ash_public_cpp",
On 2017/03/30 23:06:05, msw wrote:
> Are either of these actually needed/used by this target?

Yes. This code uses ash_test_suite above, which I changed, hence the additional
deps.

[sky, 2017-03-30 23:35:59]

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

[commit-bot: I haz the power, 2017-03-30 23:36: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-03-31 00:49:43]

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

[commit-bot: I haz the power, 2017-03-31 00:49:43]

Dry run: This issue passed the CQ dry run.

[reveman, 2017-03-31 08:41:47]

components/exo lgtm

[sky, 2017-03-31 14:24:18]

The CQ bit was checked by sky@chromium.org

[sky, 2017-03-31 14:24:19]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2788463003/#ps60001
(title: "feedback")

[commit-bot: I haz the power, 2017-03-31 14:24:39]

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-31 14:41:20]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490970258103480,
"parent_rev": "1f7424c4927085e46ba1b0d56637e708d70e978b", "commit_rev":
"25b49b2809f59ba95b9abc95ac0a0adaf4c97722"}

[commit-bot: I haz the power, 2017-03-31 14:42:04]

Description was changed from

==========
Adds ability for ash_unittests to run in mushrome mode

This makes it so that if you supply --mus to ash*unittests it'll run
in mushrome mode. A slew of tests fail with --mus, but this at least
enables mushrome mode for tests.

BUG=705715
TEST=test only changes
R=msw@chromium.org, reveman@chromium.org
==========

to

==========
Adds ability for ash_unittests to run in mushrome mode

This makes it so that if you supply --mus to ash*unittests it'll run
in mushrome mode. A slew of tests fail with --mus, but this at least
enables mushrome mode for tests.

BUG=705715
TEST=test only changes
R=msw@chromium.org, reveman@chromium.org

Review-Url: https://codereview.chromium.org/2788463003
Cr-Commit-Position: refs/heads/master@{#461119}
Committed:
https://chromium.googlesource.com/chromium/src/+/25b49b2809f59ba95b9abc95ac0a...
==========

[commit-bot: I haz the power, 2017-03-31 14:42:05]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/25b49b2809f59ba95b9abc95ac0a...