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

ash/wm/power_button_controller.h

Index: ash/wm/power_button_controller.h
diff --git a/ash/wm/power_button_controller.h b/ash/wm/power_button_controller.h
index 7043966f92a9d439ef664804e47343fab0d6804c..0cdc516d0e35b85b81ddcf226faed9bda99afa2f 100644
--- a/ash/wm/power_button_controller.h
+++ b/ash/wm/power_button_controller.h
@@ -8,6 +8,7 @@
 #include "ash/ash_export.h"
 #include "base/macros.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "ui/events/event_handler.h"
 
 #if defined(OS_CHROMEOS)
@@ -26,10 +27,6 @@ class Layer;
 
 namespace ash {
 
-namespace test {
-class PowerButtonControllerTest;
-}
-
 class LockStateController;
 
 // Handles power & lock button events which may result in the locking or
@@ -46,6 +43,28 @@ class ASH_EXPORT PowerButtonController
 #endif
 {
  public:
+  // Helper class used by tablet power button tests to access internal state.
+  class ASH_EXPORT TestApi {
+   public:
+    explicit TestApi(PowerButtonController* controller);
+    virtual ~TestApi();
+
+    bool tablet_pre_start_shutdown_animation_timer_is_running() const {

[Daniel Erat, 2016/11/04 16:12:52]

neither of these are trivial getters or setters, so they should use
TheRegularMethodNamingStyle and be defined in the .cc file instead of inlined
here

[Qiang(Joe) Xu, 2016/11/04 19:29:51]

Done.

+      return controller_->tablet_pre_start_shutdown_animation_timer_
+          .IsRunning();
+    }
+
+    void trigger_tablet_pre_start_shutdown_animation_timeout() {
+      controller_->OnTabletPreStartShutdownAnimationTimeout();
+      controller_->tablet_pre_start_shutdown_animation_timer_.Stop();
+    }
+
+   private:
+    PowerButtonController* controller_;  // Not owned.
+
+    DISALLOW_COPY_AND_ASSIGN(TestApi);
+  };
+
   explicit PowerButtonController(LockStateController* controller);
   ~PowerButtonController() override;
 
@@ -66,6 +85,7 @@ class ASH_EXPORT PowerButtonController
 
   // ui::EventHandler:
   void OnKeyEvent(ui::KeyEvent* event) override;
+  void OnMouseEvent(ui::MouseEvent* event) override;
 
 #if defined(OS_CHROMEOS)
   // Overriden from ui::DisplayConfigurator::Observer:
@@ -78,7 +98,41 @@ class ASH_EXPORT PowerButtonController
 #endif
 
  private:
-  friend class test::PowerButtonControllerTest;
+  // Called by OnPowerButtonEvent on convertible/tablet devices.
+  void OnTabletPowerButtonEvent(bool down, const base::TimeTicks& timestamp);
+
+  // Set backlights to |forced_off| if needed.
+  void SetBacklightsForcedOff(bool forced_off);
+
+  // Sends a request to get the backlights forced off state so that

[Daniel Erat, 2016/11/04 16:12:52]

nit: "... a request to powerd to get ..."

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+  // |backlights_forced_off_| can be initialized.
+  void GetInitialBacklightsForcedOff();
+
+  // Initializes |backlights_forced_off_|.
+  void HandleInitialBacklightsForcedOff(bool is_forced_off);

[Daniel Erat, 2016/11/04 16:12:52]

nit: OnGotInitialBacklightsForcedOff for consistency with other methods

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+
+  // When power button is pressed, spin a 1s timer before starting shutdown
+  // white animation managed by |controller_|.

[Daniel Erat, 2016/11/04 16:12:52]

// Starts |tablet_power_button_timer_| when the power button
// is pressed while in tablet mode.

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+  void StartTabletPreStartShutdownAnimationTimer();
+
+  // When this timer times out, starting shutdown white animation managed
+  // by |controller_|.

[Daniel Erat, 2016/11/04 16:12:52]

// Called by |tablet_power_button_timer_| to start the
// pre-shutdown animation.

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+  void OnTabletPreStartShutdownAnimationTimeout();
+
+  // If user has "require password to wake from sleep" pref set in profile,
+  // setting backlights forced off should be accompanied with locking screen.

[Daniel Erat, 2016/11/04 16:12:52]

// Locks the screen if the "require password to wake from sleep"
// pref is set and locking is possible.

[Qiang(Joe) Xu, 2016/11/04 19:29:51]

Done.

+  void LockScreenIfRequired();
+
+  // Returns true if device is a convertible/tablet device or has
+  // kAshEnableTouchViewTesting in test, otherwise false.
+  bool IsTabletModeSupported() const;
+
+  // Returns true if device works in tablet/maximize mode, otherwise false.

[Daniel Erat, 2016/11/04 16:12:52]

s/if device works in/if device is currently in/

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+  bool IsTabletModeActive() const;
+
+  // The one sec timer to wait for tablet power button released. If it is

[Daniel Erat, 2016/11/04 16:12:52]

nit: mind moving this down to the bottom instead of putting it above the
more-important-seeming power_button_down_ and lock_button_down_ members? i also
wouldn't mention the delay in the comment, since it's likely to become stale if
we change this. maybe something like this instead:

  // Started when the tablet power button is pressed and stopped
  // when it's released. Runs OnTabletPowerButtonTimeout() to start
  // shutdown.

[Qiang(Joe) Xu, 2016/11/04 19:29:51]

Done.

+  // timeout, |controller_| will start shutdown white animation.
+  base::OneShotTimer tablet_pre_start_shutdown_animation_timer_;

[Daniel Erat, 2016/11/04 16:12:52]

maybe just call this tablet_power_button_timer_? i know that's a bit less
descriptive but pre_start_shutdown_animation is pretty confusing. :-) if we need
additional timers later, it's easy to give this a longer name then.

if so, i'd also rename the methods to StartTabletPowerButtonTimer and
OnTabletPowerButtonTimeout.

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

OK. I agree with this change.

 
   // Are the power or lock buttons currently held?
   bool power_button_down_;
@@ -110,6 +164,14 @@ class ASH_EXPORT PowerButtonController
 
   LockStateController* controller_;  // Not owned.
 
+  // To differentiate a power button pressed is taking on while screen is off
+  // or not.

[Daniel Erat, 2016/11/04 16:12:52]

// True if the screen was off when the power button was pressed.

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+  bool power_button_down_while_screen_off_;
+
+  // Saving backlights forced off state, so that it doesn't need to bother
+  // calling GetBacklightsForcedOff method.

[Daniel Erat, 2016/11/04 16:12:52]

// Current forced-off state of backlights.

[Qiang(Joe) Xu, 2016/11/04 19:29:52]

Done.

+  bool backlights_forced_off_;
+
   DISALLOW_COPY_AND_ASSIGN(PowerButtonController);
 };