chromeos: Move device shutdown handling out of chrome into ash

chromeos: Move device shutdown handling out of chrome into ash

Historically device shutdown was handled in chrome because it needed to check
a device "reboot on shutdown" policy during the shutdown process. Under mustash
it is possible to quit the chrome browser and leave ash running, but the old
code required launching chrome in order to shut down.

Moving the code into ash also reduces ash/chrome dependencies and simplifies
the SystemTrayDelegate code I'm trying to refactor.

* The system tray shutdown button computes its "shutdown" vs "reboot" each time
the menu is opened. It no longer changes its tooltip if the policy changed while
the menu was open. The old behavior seemed unnecessary, and doing it this way
avoids a set of policy observers in ash.
* Chrome observes the shutdown policy via ShutdownPolicyHandler. It sends the
initial value to ash via the ash::mojom::ShutdownController mojo interface.
Subsequent policy updates are sent over the same interface.

This partially reverts the ShutdownClient implementation introduced in
https://codereview.chromium.org/2471643002

BUG=647412, 628792
TEST=ash_unittests, chrome unit_tests and browser_tests (ShutdownPolicy*) and
manual testing on device

Committed: https://crrev.com/d9f152f1d2e35a35a726a3891c00a764de144b39
Cr-Commit-Position: refs/heads/master@{#432035}

[James Cook, 2016-11-12 01:27:23]

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

[commit-bot: I haz the power, 2016-11-12 01:27:52]

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

[James Cook, 2016-11-12 01:32:18]

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

[James Cook, 2016-11-12 01:32:18]

Dan, please take a look. Apologies for the size, but at least it's net negative!

I looked at landing my other patch first, but it doesn't really help -- most of
the lines I changed there end up getting deleted here.

https://codereview.chromium.org/2497123002/diff/1/chrome/browser/chromeos/chr...
File chrome/browser/chromeos/chrome_interface_factory.cc (right):

https://codereview.chromium.org/2497123002/diff/1/chrome/browser/chromeos/chr...
chrome/browser/chromeos/chrome_interface_factory.cc:182: // but a connection
from service:content_gpu does not.
I talked to rockot@ -- this doesn't hurt anything (the registry is
per-connection and the GPU connection just won't use the interfaces) but it
might be nice to clean up later.

[commit-bot: I haz the power, 2016-11-12 02:02:30]

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

[commit-bot: I haz the power, 2016-11-12 02:02:31]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Daniel Erat, 2016-11-14 15:42:41]

lgtm!

https://codereview.chromium.org/2497123002/diff/1/ash/common/shutdown_control...
File ash/common/shutdown_controller.h (right):

https://codereview.chromium.org/2497123002/diff/1/ash/common/shutdown_control...
ash/common/shutdown_controller.h:27: virtual void ShutdownOrReboot();
nit: ShutDownOrReboot ("shut down" is verb, "shutdown" is noun)

https://codereview.chromium.org/2497123002/diff/1/ash/wm/lock_state_controller.h
File ash/wm/lock_state_controller.h (right):

https://codereview.chromium.org/2497123002/diff/1/ash/wm/lock_state_controlle...
ash/wm/lock_state_controller.h:244: // Controller used to trigger the actual
shut down.
nit: s/shut down/shutdown/

[James Cook, 2016-11-14 17:21:39]

Thanks for the quick review.

https://codereview.chromium.org/2497123002/diff/1/ash/common/shutdown_control...
File ash/common/shutdown_controller.h (right):

https://codereview.chromium.org/2497123002/diff/1/ash/common/shutdown_control...
ash/common/shutdown_controller.h:27: virtual void ShutdownOrReboot();
On 2016/11/14 15:42:41, Daniel Erat wrote:
> nit: ShutDownOrReboot ("shut down" is verb, "shutdown" is noun)

Done.

https://codereview.chromium.org/2497123002/diff/1/ash/wm/lock_state_controller.h
File ash/wm/lock_state_controller.h (right):

https://codereview.chromium.org/2497123002/diff/1/ash/wm/lock_state_controlle...
ash/wm/lock_state_controller.h:244: // Controller used to trigger the actual
shut down.
On 2016/11/14 15:42:41, Daniel Erat wrote:
> nit: s/shut down/shutdown/

Done.

[James Cook, 2016-11-14 17:24:37]

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

[commit-bot: I haz the power, 2016-11-14 17:25:14]

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

[James Cook, 2016-11-14 17:28:00]

jamescook@chromium.org changed reviewers:
+ sky@chromium.org, tsepez@chromium.org

[James Cook, 2016-11-14 17:28:01]

tsepez and sky, can I get OWNERS for these files?

tsepez:
    ash/public/interfaces/shutdown.mojom

sky:
    chrome/app/mash/chrome_mash_content_browser_manifest_overlay.json
    chrome/browser/chrome_content_browser_manifest_overlay.json
    chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
    chrome/browser/ui/webui/chromeos/login/oobe_ui.h

[Tom Sepez, 2016-11-14 18:02:15]

OWNERS LGTM

[commit-bot: I haz the power, 2016-11-14 18:40:54]

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

[commit-bot: I haz the power, 2016-11-14 18:40:55]

Dry run: This issue passed the CQ dry run.

[sky, 2016-11-14 21:41:37]

LGTM

[James Cook, 2016-11-14 22:00:02]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-11-14 22:00:02]

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

[commit-bot: I haz the power, 2016-11-14 22:03:55]

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, 2016-11-15 00:53:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-15 01:04:43]

Description was changed from

==========
chromeos: Move device shutdown handling out of chrome into ash

Historically device shutdown was handled in chrome because it needed to check
a device "reboot on shutdown" policy during the shutdown process. Under mustash
it is possible to quit the chrome browser and leave ash running, but the old
code required launching chrome in order to shut down.

Moving the code into ash also reduces ash/chrome dependencies and simplifies
the SystemTrayDelegate code I'm trying to refactor.

* The system tray shutdown button computes its "shutdown" vs "reboot" each time
the menu is opened. It no longer changes its tooltip if the policy changed while
the menu was open. The old behavior seemed unnecessary, and doing it this way
avoids a set of policy observers in ash.
* Chrome observes the shutdown policy via ShutdownPolicyHandler. It sends the
initial value to ash via the ash::mojom::ShutdownController mojo interface.
Subsequent policy updates are sent over the same interface.

This partially reverts the ShutdownClient implementation introduced in
https://codereview.chromium.org/2471643002

BUG=647412,628792
TEST=ash_unittests, chrome unit_tests and browser_tests (ShutdownPolicy*) and
manual testing on device
==========

to

==========
chromeos: Move device shutdown handling out of chrome into ash

Historically device shutdown was handled in chrome because it needed to check
a device "reboot on shutdown" policy during the shutdown process. Under mustash
it is possible to quit the chrome browser and leave ash running, but the old
code required launching chrome in order to shut down.

Moving the code into ash also reduces ash/chrome dependencies and simplifies
the SystemTrayDelegate code I'm trying to refactor.

* The system tray shutdown button computes its "shutdown" vs "reboot" each time
the menu is opened. It no longer changes its tooltip if the policy changed while
the menu was open. The old behavior seemed unnecessary, and doing it this way
avoids a set of policy observers in ash.
* Chrome observes the shutdown policy via ShutdownPolicyHandler. It sends the
initial value to ash via the ash::mojom::ShutdownController mojo interface.
Subsequent policy updates are sent over the same interface.

This partially reverts the ShutdownClient implementation introduced in
https://codereview.chromium.org/2471643002

BUG=647412,628792
TEST=ash_unittests, chrome unit_tests and browser_tests (ShutdownPolicy*) and
manual testing on device

Committed: https://crrev.com/d9f152f1d2e35a35a726a3891c00a764de144b39
Cr-Commit-Position: refs/heads/master@{#432035}
==========

[commit-bot: I haz the power, 2016-11-15 01:04:44]

Patchset 4 (id:??) landed as
https://crrev.com/d9f152f1d2e35a35a726a3891c00a764de144b39
Cr-Commit-Position: refs/heads/master@{#432035}