[Athena] Make a quick power button tap suspend the device

[Athena] Make a quick power button tap suspend the device

BUG=404661, 179713
TEST=Manual, see bug
R=flackr, oshima, derat
TBR=stevenjb

Committed: https://crrev.com/4bcbeb30b6a26829dfb762539b8621b67e232843
Cr-Commit-Position: refs/heads/master@{#296515}

[pkotwicz, 2014-09-18 22:30:17]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2014-09-18 23:25:56]

pkotwicz@chromium.org changed reviewers:
+ flackr@chromium.org

[pkotwicz, 2014-09-18 23:25:56]

Rob, can you please take a look?

[Daniel Erat, 2014-09-18 23:30:05]

mind also adding http://crbug.com/179713 to the BUG line?

[flackr, 2014-09-19 14:45:30]

Looks good, just a few comments.

https://codereview.chromium.org/585623003/diff/40001/athena/system/power_butt...
File athena/system/power_button_controller.cc (right):

https://codereview.chromium.org/585623003/diff/40001/athena/system/power_butt...
athena/system/power_button_controller.cc:43: void
PowerButtonController::ShowShutdownWarningDialog() {
I think when this shows we should no longer suspend, since when I see this
dialog my impression is that if I stop holding it will cancel the shutdown.

https://codereview.chromium.org/585623003/diff/40001/athena/system/power_butt...
athena/system/power_button_controller.cc:52: base::UTF8ToUTF16("Keep pressing
power button to shutdown."));
nit s/pressing/holding

https://codereview.chromium.org/585623003/diff/40001/athena/system/system_ui_...
File athena/system/system_ui_impl.cc (right):

https://codereview.chromium.org/585623003/diff/40001/athena/system/system_ui_...
athena/system/system_ui_impl.cc:102: new
PowerButtonController(system_modal_container_));
This container doesn't seem to show above the login screen.

[pkotwicz, 2014-09-20 00:01:59]

Rob, can you please take another look?

https://codereview.chromium.org/585623003/diff/40001/athena/system/system_ui_...
File athena/system/system_ui_impl.cc (right):

https://codereview.chromium.org/585623003/diff/40001/athena/system/system_ui_...
athena/system/system_ui_impl.cc:102: new
PowerButtonController(system_modal_container_));
Thanks for catching this!

I have added a new container with priority CP_LOGIN_SCREEN_SYSTEM_MODAL
Eventually, we will want to pick the appropriate container based on whether the
user has logged in. However, I am leaving that for later

[flackr, 2014-09-22 13:49:30]

LGTM

[pkotwicz, 2014-09-22 14:43:17]

pkotwicz@chromium.org changed reviewers:
+ oshima@chromium.org, stevenjb@chromium.org

[pkotwicz, 2014-09-22 14:43:18]

oshima@ for athena/ OWNERS
stevenjb@ for chromeos/dbus/ OWNERS

[oshima, 2014-09-22 17:20:19]

lgtm with nits

https://codereview.chromium.org/585623003/diff/60001/athena/system/power_butt...
File athena/system/power_button_controller.cc (right):

https://codereview.chromium.org/585623003/diff/60001/athena/system/power_butt...
athena/system/power_button_controller.cc:54: base::UTF8ToUTF16("Keep holding
power button to shutdown."));
TODO to use IDS

https://codereview.chromium.org/585623003/diff/60001/athena/system/power_butt...
athena/system/power_button_controller.cc:104: return;
can you move this to the top of the method?

https://codereview.chromium.org/585623003/diff/60001/athena/system/system_ui_...
File athena/system/system_ui_impl.cc (right):

https://codereview.chromium.org/585623003/diff/60001/athena/system/system_ui_...
athena/system/system_ui_impl.cc:108: // user has logged in.
ideally modal dialogs should use the regular (either normal or login) container
with topmost property + event targeter.

I'll work on API. lgtm for now.

[pkotwicz, 2014-09-22 18:11:03]

Addressed Oshima's nits

[pkotwicz, 2014-09-24 03:45:57]

stevenjb@, ping?

[Daniel Erat, 2014-09-24 14:34:54]

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

[Daniel Erat, 2014-09-24 14:34:54]

lgtm for chromeos/dbus/

(i think that steven is traveling, and i'm probably the best person to review
anything power-related anyway :-P)

[stevenjb, 2014-09-24 14:57:38]

What Dan said.


On Wed, Sep 24, 2014 at 7:34 AM, <derat@chromium.org> wrote:

> lgtm for chromeos/dbus/
>
> (i think that steven is traveling, and i'm probably the best person to
> review
> anything power-related anyway :-P)
>
> https://codereview.chromium.org/585623003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[pkotwicz, 2014-09-24 19:18:43]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2014-09-24 19:18:47]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2014-09-24 19:19:04]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-09-24 19:20:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/585623003/100001

[commit-bot: I haz the power, 2014-09-24 20:02:24]

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

[commit-bot: I haz the power, 2014-09-24 20:02:25]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pkotwicz, 2014-09-24 20:16:37]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-09-24 20:18:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/585623003/120001

[commit-bot: I haz the power, 2014-09-24 21:03:22]

Committed patchset #6 (id:120001) as e27c344e4a2e6d43d55f252a014f685523829913

[commit-bot: I haz the power, 2014-09-24 21:04:05]

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