mash: Exit the root process if the window manager service crashes

mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
TBR=tsepez@chromium.org

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

[James Cook, 2017-01-31 23:07:30]

Description was changed from

==========
WIP: Quit mash_session and chrome --mash service runner when ash crashes

WIP

BUG=
==========

to

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
==========

[James Cook, 2017-01-31 23:08:30]

Description was changed from

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
==========

to

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
==========

[James Cook, 2017-01-31 23:11:33]

Patchset #3 (id:40001) has been deleted

[James Cook, 2017-01-31 23:11:41]

Patchset #2 (id:20001) has been deleted

[James Cook, 2017-01-31 23:13:56]

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

[James Cook, 2017-01-31 23:14:16]

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

[James Cook, 2017-01-31 23:14:16]

rockot, PTAL

https://codereview.chromium.org/2646033002/diff/80001/services/service_manage...
File
services/service_manager/background/tests/background_service_manager_unittest.cc
(right):

https://codereview.chromium.org/2646033002/diff/80001/services/service_manage...
services/service_manager/background/tests/background_service_manager_unittest.cc:84:
BackgroundServiceManager background_service_manager(nullptr, nullptr);
I looked at de-duplicating this setup with the test above, but it seemed a
little harder to read and I don't feel too bad about replicating test code.
Lemme know if this is too much copy/pasta for you.

[commit-bot: I haz the power, 2017-01-31 23:14:34]

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-31 23:26:05]

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

[commit-bot: I haz the power, 2017-01-31 23:26:06]

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

[Ken Rockot(use gerrit already), 2017-01-31 23:33:28]

lgtm

https://codereview.chromium.org/2646033002/diff/80001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2646033002/diff/80001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:206:
base::Bind(&MashRunner::OnInstanceQuitInMain, base::Unretained(this)));
Instead of making the RunLoop and return values instance variables, I would just
make them locals and pass their addresses to OnInstanceQuitInMain. We know it's
safe because that function will never be called after the RunLoop quits, and the
RunLoop is run only within this function.

https://codereview.chromium.org/2646033002/diff/80001/services/service_manage...
File
services/service_manager/background/tests/background_service_manager_unittest.cc
(right):

https://codereview.chromium.org/2646033002/diff/80001/services/service_manage...
services/service_manager/background/tests/background_service_manager_unittest.cc:84:
BackgroundServiceManager background_service_manager(nullptr, nullptr);
On 2017/01/31 at 23:14:16, James Cook wrote:
> I looked at de-duplicating this setup with the test above, but it seemed a
little harder to read and I don't feel too bad about replicating test code.
Lemme know if this is too much copy/pasta for you.

Seems fine to me. I prefer copypasta over arbitrary abstraction, and this
doesn't feel large enough to justify the effort of creating something reusable.

[Ken Rockot(use gerrit already), 2017-01-31 23:34:51]

https://codereview.chromium.org/2646033002/diff/80001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2646033002/diff/80001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:206:
base::Bind(&MashRunner::OnInstanceQuitInMain, base::Unretained(this)));
On 2017/01/31 at 23:33:28, Ken Rockot wrote:
> Instead of making the RunLoop and return values instance variables, I would
just make them locals and pass their addresses to OnInstanceQuitInMain. We know
it's safe because that function will never be called after the RunLoop quits,
and the RunLoop is run only within this function.

Note: I just realized this would also mean there's no need to make
OnInstanceQuitInMain a member function. It can be a free-floating function or,
if you're feeling saucy, a lambda.

[James Cook, 2017-02-01 00:40:45]

Description was changed from

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
==========

to

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
TBR=tsepez@chromium.org
==========

[James Cook, 2017-02-01 00:41:32]

TBR tsepez for adding a method to a mojom for a test service that only runs in
unit tests

https://codereview.chromium.org/2646033002/diff/80001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2646033002/diff/80001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:206:
base::Bind(&MashRunner::OnInstanceQuitInMain, base::Unretained(this)));
On 2017/01/31 23:34:51, Ken Rockot wrote:
> On 2017/01/31 at 23:33:28, Ken Rockot wrote:
> > Instead of making the RunLoop and return values instance variables, I would
> just make them locals and pass their addresses to OnInstanceQuitInMain. We
know
> it's safe because that function will never be called after the RunLoop quits,
> and the RunLoop is run only within this function.
> 
> Note: I just realized this would also mean there's no need to make
> OnInstanceQuitInMain a member function. It can be a free-floating function or,
> if you're feeling saucy, a lambda.

Done and done. I used an anonymous function because the lambda was pretty
unreadable.

[James Cook, 2017-02-01 00:42:37]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2017-02-01 00:42:37]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2646033002/#ps100001
(title: "review comments")

[commit-bot: I haz the power, 2017-02-01 00:43:28]

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-02-01 02:05:26]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485909757636380,
"parent_rev": "9075a09693ec32f975a12dd8207eab03a5cf48f9", "commit_rev":
"f98489f8975f814711ffecfb180ee46cbbd17519"}

[commit-bot: I haz the power, 2017-02-01 02:07:05]

Description was changed from

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
TBR=tsepez@chromium.org
==========

to

==========
mash: Exit the root process if the window manager service crashes

Both the browser and ash cache information about each other's state. For now
we've decided not to try to make ash restartable. Instead we're going to
bring everything down when ash crashes. The underlying Chrome OS session
manager will then restart chrome --mash to bring everything back up.

* Set a quit callback on all services and terminate the root process if the
ash service disconnects.
* Plumb the quit callback through the BackgroundServiceManager.
* Don't use mash_session from mash_runner. It's simpler to start the desired
services (window manager, quick_launch) explicitly.

TODO: Shut down root process when mus ui service crashes.
TODO: The window server hits a DCHECK when the root process shuts down.

BUG=678683
TEST=added to service_manager_unittests
TBR=tsepez@chromium.org

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

[commit-bot: I haz the power, 2017-02-01 02:07:06]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/f98489f8975f814711ffecfb180e...