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

ash/system/chromeos/power/tablet_power_button_controller_unittest.cc

Index: ash/system/chromeos/power/tablet_power_button_controller_unittest.cc
diff --git a/ash/system/chromeos/power/tablet_power_button_controller_unittest.cc b/ash/system/chromeos/power/tablet_power_button_controller_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8879e52da5b11be5e20050e7f8b9b7fcbcda6930
--- /dev/null
+++ b/ash/system/chromeos/power/tablet_power_button_controller_unittest.cc
@@ -0,0 +1,299 @@
+// 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/system/chromeos/power/tablet_power_button_controller.h"
+
+#include <memory>
+
+#include "ash/common/ash_switches.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/test/ash_test_base.h"
+#include "ash/test/lock_state_controller_test_api.h"
+#include "ash/wm/lock_state_controller.h"
+#include "ash/wm/power_button_controller.h"
+#include "base/command_line.h"
+#include "base/compiler_specific.h"
+#include "base/run_loop.h"
+#include "base/test/simple_test_tick_clock.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_power_manager_client.h"
+#include "ui/events/test/event_generator.h"
+
+namespace ash {
+namespace test {
+
+namespace {
+
+// A non-zero brightness used for test.
+constexpr int kNonZeroBrightness = 10;
+
+void CopyResult(bool* dest, bool src) {
+  *dest = src;
+}
+
+}  // namespace
+
+class TabletPowerButtonControllerTest : public AshTestBase {
+ public:
+  TabletPowerButtonControllerTest() {}
+  ~TabletPowerButtonControllerTest() override {}
+
+  void SetUp() override {
+    // This also initializes DBusThreadManager.
+    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
+        chromeos::DBusThreadManager::GetSetterForTesting();
+    power_manager_client_ = new chromeos::FakePowerManagerClient();
+    dbus_setter->SetPowerManagerClient(base::WrapUnique(power_manager_client_));
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kAshEnableTouchViewTesting);
+    AshTestBase::SetUp();
+
+    lock_state_controller_ = Shell::GetInstance()->lock_state_controller();
+    tablet_controller_ = Shell::GetInstance()
+                             ->power_button_controller()
+                             ->tablet_power_button_controller_for_test();
+    test_api_ = base::MakeUnique<TabletPowerButtonController::TestApi>(
+        tablet_controller_);
+    generator_ = &AshTestBase::GetEventGenerator();
+    power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, false);
+    EXPECT_FALSE(GetBacklightsForcedOff());
+  }
+
+  void TearDown() override {
+    generator_ = nullptr;
+    AshTestBase::TearDown();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+
+ protected:
+  void PressPowerButton() {
+    tablet_controller_->OnPowerButtonEvent(true, base::TimeTicks::Now());
+  }
+
+  void ReleasePowerButton() {
+    tablet_controller_->OnPowerButtonEvent(false, base::TimeTicks::Now());
+  }
+
+  void SystemUnlocks() {

[Daniel Erat, 2016/11/14 16:15:31]

nit: UnlockScreen?

[Qiang(Joe) Xu, 2016/11/14 18:47:56]

Done.

+    lock_state_controller_->OnLockStateChanged(false);
+    WmShell::Get()->GetSessionStateDelegate()->UnlockScreen();
+  }
+
+  void Initialize(LoginStatus status) {
+    lock_state_controller_->OnLoginStateChanged(status);
+    SetUserLoggedIn(status != LoginStatus::NOT_LOGGED_IN);
+    lock_state_controller_->OnLockStateChanged(false);
+  }
+
+  void EnableMaximizeMode(bool enabled) {
+    WmShell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
+        enabled);
+  }
+
+  void CheckLockedState(bool expected_locked) {

[Daniel Erat, 2016/11/14 16:15:31]

how about:

  bool GetLockedState()

so that you can use EXPECT_TRUE/FALSE in tests and get correct line numbers on
failures?

[Qiang(Joe) Xu, 2016/11/14 18:47:57]

Done.

+    EXPECT_EQ(WmShell::Get()->GetSessionStateDelegate()->IsScreenLocked(),
+              expected_locked);
+  }
+
+  bool GetBacklightsForcedOff() WARN_UNUSED_RESULT {
+    bool forced_off = false;
+    power_manager_client_->GetBacklightsForcedOff(
+        base::Bind(&CopyResult, base::Unretained(&forced_off)));
+    base::RunLoop().RunUntilIdle();
+    return forced_off;
+  }
+
+  // Ownership is passed on to chromeos::DBusThreadManager.
+  chromeos::FakePowerManagerClient* power_manager_client_;
+
+  LockStateController* lock_state_controller_;      // Not owned.
+  TabletPowerButtonController* tablet_controller_;  // Not owned.
+  std::unique_ptr<TabletPowerButtonController::TestApi> test_api_;
+  ui::test::EventGenerator* generator_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(TabletPowerButtonControllerTest);
+};
+
+TEST_F(TabletPowerButtonControllerTest, LockScreenIfRequired) {
+  Initialize(LoginStatus::USER);
+  SetShouldLockScreenAutomatically(true);
+  CheckLockedState(false);
+
+  // On User logged in status, power-button-press-release should lock screen if
+  // should lock screen automatically.

[Daniel Erat, 2016/11/14 16:15:31]

nit: "... if automatic screen-locking was requested."

[Qiang(Joe) Xu, 2016/11/14 18:47:57]

Done.

+  PressPowerButton();
+  ReleasePowerButton();
+  CheckLockedState(true);
+
+  // On locked state, power-button-press-release should do nothing.
+  PressPowerButton();
+  ReleasePowerButton();
+  CheckLockedState(true);
+
+  // Unlock the sceen.
+  SystemUnlocks();
+  CheckLockedState(false);

[Daniel Erat, 2016/11/14 16:15:31]

if you change to GetLockedState as requested above, i'd use ASSERT_FALSE here

[Qiang(Joe) Xu, 2016/11/14 18:47:57]

Done.

+
+  // power-button-press-release should not lock the screen if should not lock
+  // screen automatically.

[Daniel Erat, 2016/11/14 16:15:31]

nit: "... if automatic screen-locking wasn't requested."

[Qiang(Joe) Xu, 2016/11/14 18:47:56]

Done.

+  SetShouldLockScreenAutomatically(false);
+  PressPowerButton();
+  ReleasePowerButton();
+  CheckLockedState(false);
+}
+
+// Tests that shutdown animation is not started if the power button is released
+// quickly.
+TEST_F(TabletPowerButtonControllerTest,
+       ReleasePowerButtonBeforeStartingShutdownAnimation) {
+  PressPowerButton();
+  EXPECT_TRUE(test_api_->ShutdownTimerIsRunning());
+  EXPECT_FALSE(GetBacklightsForcedOff());
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_FALSE(test_api_->ShutdownTimerIsRunning());
+  EXPECT_TRUE(GetBacklightsForcedOff());
+
+  PressPowerButton();
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, true);
+  EXPECT_TRUE(test_api_->ShutdownTimerIsRunning());
+  EXPECT_FALSE(GetBacklightsForcedOff());
+  ReleasePowerButton();
+  EXPECT_FALSE(test_api_->ShutdownTimerIsRunning());
+  EXPECT_FALSE(GetBacklightsForcedOff());
+}
+
+// Tests the power button released is done after shutdown timer is timeout,

[Daniel Erat, 2016/11/14 16:15:31]

// Tests that the shutdown animation is started when the power button
// is released after the timer fires.

[Qiang(Joe) Xu, 2016/11/14 18:47:57]

Done.

+// where it will start shutdown animation timer.
+TEST_F(TabletPowerButtonControllerTest,
+       ReleasePowerButtonDuringShutdownAnimation) {
+  // Initializes LockStateControllerTestApi to observe shutdown animation timer.
+  std::unique_ptr<LockStateControllerTestApi> lock_state_test_api =
+      base::MakeUnique<LockStateControllerTestApi>(lock_state_controller_);
+
+  PressPowerButton();
+  test_api_->TriggerShutdownTimeout();
+  EXPECT_TRUE(lock_state_test_api->shutdown_timer_is_running());
+  ReleasePowerButton();
+  EXPECT_FALSE(lock_state_test_api->shutdown_timer_is_running());
+  EXPECT_FALSE(GetBacklightsForcedOff());
+
+  // Test again when backlights is forced off.
+  PressPowerButton();
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+
+  PressPowerButton();
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, true);
+  EXPECT_FALSE(GetBacklightsForcedOff());
+  test_api_->TriggerShutdownTimeout();
+  EXPECT_TRUE(lock_state_test_api->shutdown_timer_is_running());
+  ReleasePowerButton();
+  EXPECT_FALSE(lock_state_test_api->shutdown_timer_is_running());
+  EXPECT_FALSE(GetBacklightsForcedOff());
+}
+
+// Tests tapping power button when screen is idle off.
+TEST_F(TabletPowerButtonControllerTest, TappingPowerButtonWhenScreenIsIdleOff) {
+  power_manager_client_->SendBrightnessChanged(0, false);
+  PressPowerButton();
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, true);
+  EXPECT_FALSE(GetBacklightsForcedOff());

[Daniel Erat, 2016/11/14 16:15:31]

nit: check this before sending the non-zero brightness?

[Qiang(Joe) Xu, 2016/11/14 18:47:57]

Done.

+  ReleasePowerButton();
+  EXPECT_FALSE(GetBacklightsForcedOff());
+}
+
+// Tests tapping power button when device is suspended.
+TEST_F(TabletPowerButtonControllerTest, TappingPowerButtonWhenSuspended) {
+  base::SimpleTestTickClock* tick_clock = new base::SimpleTestTickClock;
+  // |tick_clock| owned by |tablet_controller_|.
+  tablet_controller_->SetTickClockForTesting(
+      std::unique_ptr<base::TickClock>(tick_clock));
+
+  power_manager_client_->SendSuspendImminent();
+  power_manager_client_->SendBrightnessChanged(0, false);
+  // There is a power button pressed here, but PowerButtonEvent is sent later.
+  power_manager_client_->SendSuspendDone();
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, false);
+
+  // Test PowerButtonEvent is sent with a delay.

[Daniel Erat, 2016/11/14 16:15:31]

nit: // Send the power button event after a short delay and check that it is
ignored.

[Qiang(Joe) Xu, 2016/11/14 18:47:56]

Done.

+  tick_clock->Advance(base::TimeDelta::FromMilliseconds(500));
+  power_manager_client_->SendPowerButtonEvent(true, tick_clock->NowTicks());
+  power_manager_client_->SendPowerButtonEvent(false, tick_clock->NowTicks());
+  EXPECT_FALSE(GetBacklightsForcedOff());

[Daniel Erat, 2016/11/14 16:15:31]

please send another event after another 500-600 ms and check that it's honored

[Qiang(Joe) Xu, 2016/11/14 18:47:57]

Since the time duration to wait is 2s, I think the suggestion here is 1500-1600
ms?

[Daniel Erat, 2016/11/14 19:22:50]

whoops. yes, sorry -- got the delays confused

+}
+
+// For convertible device working on laptop mode, tests keyboard/mouse event
+// when screen is off.
+TEST_F(TabletPowerButtonControllerTest, ConvertibleOnLaptopMode) {
+  EnableMaximizeMode(false);
+
+  // KeyEvent should SetBacklightsForcedOff(false).
+  PressPowerButton();
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->PressKey(ui::VKEY_L, ui::EF_NONE);
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, true);
+  EXPECT_FALSE(GetBacklightsForcedOff());
+
+  // Regular mouse event should SetBacklightsForcedOff(false).
+  PressPowerButton();
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->MoveMouseBy(1, 1);
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, true);
+  EXPECT_FALSE(GetBacklightsForcedOff());
+
+  // Touchpad mouse event should SetBacklightsForcedOff(false).
+  PressPowerButton();
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->PressTouch();
+  power_manager_client_->SendBrightnessChanged(kNonZeroBrightness, true);
+  EXPECT_FALSE(GetBacklightsForcedOff());
+
+  // Stylus mouse event should not SetBacklightsForcedOff(false).
+  PressPowerButton();
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->EnterPenPointerMode();
+  generator_->MoveMouseBy(1, 1);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->ExitPenPointerMode();
+}
+
+// For convertible device working on tablet mode, keyboard/mouse event should
+// not SetBacklightsForcedOff(false) when screen is off.
+TEST_F(TabletPowerButtonControllerTest, ConvertibleOnMaximizeMode) {
+  EnableMaximizeMode(true);
+
+  PressPowerButton();
+  ReleasePowerButton();
+  power_manager_client_->SendBrightnessChanged(0, true);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->PressKey(ui::VKEY_L, ui::EF_NONE);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+
+  generator_->MoveMouseBy(1, 1);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+
+  generator_->PressTouch();
+  EXPECT_TRUE(GetBacklightsForcedOff());
+
+  generator_->EnterPenPointerMode();
+  generator_->MoveMouseBy(1, 1);
+  EXPECT_TRUE(GetBacklightsForcedOff());
+  generator_->ExitPenPointerMode();
+}
+
+}  // namespace test
+}  // namespace ash