Tablet-like power button behavior on Convertible/Tablet ChromeOS devices

ash/wm/tablet_power_button_controller.cc

Index: ash/wm/tablet_power_button_controller.cc
diff --git a/ash/wm/tablet_power_button_controller.cc b/ash/wm/tablet_power_button_controller.cc
new file mode 100644
index 0000000000000000000000000000000000000000..40a821bd3da0c7ba44468435f9e71ddc294bb2f7
--- /dev/null
+++ b/ash/wm/tablet_power_button_controller.cc
@@ -0,0 +1,149 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/wm/tablet_power_button_controller.h"
+
+#include "ash/common/session/session_state_delegate.h"
+#include "ash/common/wm/maximize_mode/maximize_mode_controller.h"
+#include "ash/common/wm_shell.h"
+#include "ash/shell.h"
+#include "ash/wm/lock_state_controller.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/power_manager_client.h"
+
+namespace ash {
+
+namespace {
+
+// Amount of time the power button must be held to start the pre-shutdown
+// animation when in tablet mode.
+constexpr int kTabletShutdownTimeoutMs = 1000;

[Daniel Erat, 2016/11/07 20:54:41]

nit: remove "Tablet" from name

[Qiang(Joe) Xu, 2016/11/10 22:18:44]

Done.

+
+}  // namespace
+
+TabletPowerButtonController::TestApi::TestApi(
+    TabletPowerButtonController* controller)
+    : controller_(controller) {}
+
+TabletPowerButtonController::TestApi::~TestApi() {}
+
+bool TabletPowerButtonController::TestApi::TabletPowerButtonTimerIsRunning()
+    const {
+  return controller_->tablet_power_button_timer_.IsRunning();
+}
+
+void TabletPowerButtonController::TestApi::TriggerTabletPowerButtonTimeout() {
+  controller_->OnTabletPowerButtonTimeout();

[Daniel Erat, 2016/11/07 20:54:41]

i'd make sure that the timer is actually running here first

[Qiang(Joe) Xu, 2016/11/10 22:18:45]

Done.

+  controller_->tablet_power_button_timer_.Stop();
+}
+
+TabletPowerButtonController::TabletPowerButtonController(
+    LockStateController* controller) {
+  controller_ = controller;

[Daniel Erat, 2016/11/07 20:54:41]

nit: use an initialization list for controller_

[Qiang(Joe) Xu, 2016/11/10 22:18:44]

Done.

+  Shell::GetInstance()->PrependPreTargetHandler(this);
+  GetInitialBacklightsForcedOff();

[Daniel Erat, 2016/11/07 20:54:41]

i think you also need to handle the case where powerd restarts. you could do
this by using PowerManagerClient::Observer::PowerManagerRestarted to either a)
query powerd's new state, or b) set powerd's state to backlights_forced_off_. b)
seems like it might be slightly better...

[Qiang(Joe) Xu, 2016/11/10 22:18:44]

Done.

+}
+
+TabletPowerButtonController::~TabletPowerButtonController() {
+  Shell::GetInstance()->RemovePreTargetHandler(this);
+}
+
+bool TabletPowerButtonController::ShouldHandlePowerButtonEvents() const {
+  return IsTabletModeSupported();

[Daniel Erat, 2016/11/07 20:54:41]

would it be cleaner to make IsTabletModeSupported public? maybe it could even be
static, and then you could avoid instantiating TabletPowerButtonController if
it's unneeded (assuming that the "supported" state can't change at runtime)

[Qiang(Joe) Xu, 2016/11/10 22:18:44]

Avoid instantiating makes sense to me. However, I find CanEnterMaximizeMode is
not updated in PBC's ctor. That means OnAccelerometerUpdated is being notified
at a later time. I don't know whether we have a way to process this..

I am doing the change by moving IsTabletModeSupported and IsTabletModeActive to
anonymous space.

+}
+
+void TabletPowerButtonController::OnPowerButtonEvent(
+    bool down,
+    const base::TimeTicks& timestamp,
+    bool brightness_is_zero) {
+  if (down) {
+    power_button_down_while_screen_off_ = brightness_is_zero;
+    SetBacklightsForcedOff(false);
+    StartTabletPowerButtonTimer();

[Daniel Erat, 2016/11/07 20:54:41]

should you only start this if we aren't already shutting down? otherwise, it
looks like OnTabletPowerButtonTimeout could call StartShutdownAnimation a second
time, which seems like it could be problematic.

[Qiang(Joe) Xu, 2016/11/10 22:18:44]

This case will be handled by PBC::OnPowerButtonEvent.

+  } else {
+    if (tablet_power_button_timer_.IsRunning()) {
+      tablet_power_button_timer_.Stop();
+      if (!power_button_down_while_screen_off_) {
+        SetBacklightsForcedOff(true);
+        LockScreenIfRequired();
+      }
+    }
+    power_button_down_while_screen_off_ = false;
+
+    // When power button is released, Cancel Shutdown animation whenever it is

[Daniel Erat, 2016/11/07 20:54:41]

nit: s/Shutdown/shutdown/

[Qiang(Joe) Xu, 2016/11/10 22:18:45]

Done.

+    // still cancellable.
+    if (controller_->CanCancelShutdownAnimation())
+      controller_->CancelShutdownAnimation();
+  }
+}
+
+void TabletPowerButtonController::OnKeyEvent(ui::KeyEvent* event) {
+  if (!IsTabletModeActive() && backlights_forced_off_)
+    SetBacklightsForcedOff(false);
+}
+
+void TabletPowerButtonController::OnMouseEvent(ui::MouseEvent* event) {

[Daniel Erat, 2016/11/07 20:54:41]

i'm assuming that we intentionally don't want to stop forcing the backlights off
for touch events, but what about stylus events (assuming that we can distinguish
those)?

[Qiang(Joe) Xu, 2016/11/10 22:18:44]

I learned from Jacob that stylus event is a mouse event. I updated the code.

I discussed with Tom. Stylus touchscreen events should keep forcing the
backlights off. However, taking stylus from device should stop forcing the
backlights off.

I add changes to the new patch. However, Stylus touchscreen events should keep
forcing the backlights off doesn't work on kevin due to this: crbug.com/664002.

+  if (!IsTabletModeActive() && backlights_forced_off_)
+    SetBacklightsForcedOff(false);
+}
+
+void TabletPowerButtonController::SetBacklightsForcedOff(bool forced_off) {
+  if (backlights_forced_off_ == forced_off)
+    return;
+
+  chromeos::DBusThreadManager::Get()
+      ->GetPowerManagerClient()
+      ->SetBacklightsForcedOff(forced_off);
+  backlights_forced_off_ = forced_off;
+}
+
+void TabletPowerButtonController::GetInitialBacklightsForcedOff() {
+  chromeos::DBusThreadManager::Get()
+      ->GetPowerManagerClient()
+      ->GetBacklightsForcedOff(base::Bind(
+          &TabletPowerButtonController::OnGotInitialBacklightsForcedOff,
+          base::Unretained(this)));
+}
+
+void TabletPowerButtonController::OnGotInitialBacklightsForcedOff(
+    bool is_forced_off) {
+  backlights_forced_off_ = is_forced_off;
+}
+
+void TabletPowerButtonController::StartTabletPowerButtonTimer() {
+  tablet_power_button_timer_.Start(
+      FROM_HERE, base::TimeDelta::FromMilliseconds(kTabletShutdownTimeoutMs),
+      this, &TabletPowerButtonController::OnTabletPowerButtonTimeout);
+}
+
+void TabletPowerButtonController::OnTabletPowerButtonTimeout() {
+  controller_->StartShutdownAnimation();
+}
+
+void TabletPowerButtonController::LockScreenIfRequired() {
+  SessionStateDelegate* session_state_delegate =
+      WmShell::Get()->GetSessionStateDelegate();
+  if (session_state_delegate->ShouldLockScreenAutomatically() &&
+      session_state_delegate->CanLockScreen() &&
+      !session_state_delegate->IsUserSessionBlocked() &&
+      !controller_->LockRequested()) {
+    session_state_delegate->LockScreen();
+  }
+}
+
+bool TabletPowerButtonController::IsTabletModeSupported() const {
+  MaximizeModeController* maximize_mode_controller =
+      WmShell::Get()->maximize_mode_controller();
+  return maximize_mode_controller &&
+         maximize_mode_controller->CanEnterMaximizeMode();
+}
+
+bool TabletPowerButtonController::IsTabletModeActive() const {
+  MaximizeModeController* maximize_mode_controller =
+      WmShell::Get()->maximize_mode_controller();
+  return maximize_mode_controller &&
+         maximize_mode_controller->IsMaximizeModeWindowManagerEnabled();
+}
+
+}  // namespace ash