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..dc5504eef63a4c6953a0780f1e1f9b60a670645d 100644
--- a/ash/wm/power_button_controller.h
+++ b/ash/wm/power_button_controller.h
@@ -6,8 +6,10 @@
 #define ASH_WM_POWER_BUTTON_CONTROLLER_H_
 
 #include "ash/ash_export.h"
+#include "base/callback.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,12 +28,23 @@ class Layer;
 
 namespace ash {
 
-namespace test {
-class PowerButtonControllerTest;
-}
-
 class LockStateController;
 
+// Tablet power state delegate to handle environment specific work.
+class ASH_EXPORT TabletPowerStateDelegate {
+ public:
+  TabletPowerStateDelegate() {}
+  virtual ~TabletPowerStateDelegate() {}
+
+  virtual void SetBacklightsForcedOff(bool forced_off) = 0;
+
+  virtual void GetBacklightsForcedOff(
+      const base::Callback<void(bool)>& callback) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TabletPowerStateDelegate);
+};
+
 // Handles power & lock button events which may result in the locking or
 // shutting down of the system as well as taking screen shots while in maximize
 // mode.
@@ -46,6 +59,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 {
+      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 +101,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 +114,45 @@ class ASH_EXPORT PowerButtonController
 #endif
 
  private:
-  friend class test::PowerButtonControllerTest;
+  // A helper method to be called on convertible/tablet device.

[Daniel Erat, 2016/11/03 21:28:08]

nit: // Called by OnPowerButtonEvent on convertible/tablet devices.

[Qiang(Joe) Xu, 2016/11/04 01:14:04]

Done.

+  void OnTabletPowerButtonEvent(bool down, const base::TimeTicks& timestamp);
+
+  // |tablet_power_state_delegate_| will set backlights to |forced_off|
+  // if needed.
+  void SetBacklightsForcedOff(bool forced_off);
+
+  // Sends a request to get the backlights forced off state so that
+  // |backlights_forced_off_| can be initialized.
+  void GetInitialBacklightsForcedOff();
+
+  // Updates |backlights_forced_off_| requested by
+  // GetInitialBacklightsForcedOff, if |is_forced_off|, then set backlights
+  // forced off to false.
+  void HandleInitialBacklightsForcedOff(bool is_forced_off);
+
+  // When power button is pressed, spin a 1s timer before starting shutdown
+  // white animation managed by |controller_|.
+  void StartTabletPreStartShutdownAnimationTimer();
+
+  // When this timer times out, starting shutdown white animation managed
+  // by |controller_|.
+  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.
+  void LockScreenIfRequired();
+
+  // Allow KeyEvent/MouseEvent wake convertible device when it is working on
+  // laptop mode.
+  void WakeOnLaptopMode();
+
+  // Return true (need taking screenshot operation) when |power_button_pressed|
+  // is true, otherwise false.
+  bool MaybeTakeScreenshot(bool power_button_pressed);

[Daniel Erat, 2016/11/03 21:28:08]

"MaybeTakeScreenshot" suggests that this method might take a screenshot when
called, but it looks like it's actually just checking whether a screenshot
should be taken (without any side effects).

but i think you should just remove this and inline the code -- see the comment
in the .cc file.

[Qiang(Joe) Xu, 2016/11/04 01:14:04]

Done.

+
+  // The one sec timer to wait for tablet power button released. If it is
+  // timeout, |controller_| will start shutdown white animation.
+  base::OneShotTimer tablet_pre_start_shutdown_animation_timer_;
 
   // Are the power or lock buttons currently held?
   bool power_button_down_;
@@ -110,6 +184,17 @@ class ASH_EXPORT PowerButtonController
 
   LockStateController* controller_;  // Not owned.
 
+  // To differentiate a power button pressed is taking on screen off or not.
+  bool pressed_on_screen_off_;

[Daniel Erat, 2016/11/03 21:28:08]

this name is a bit vague. how about "power_button_down_while_screen_off_"?

[Qiang(Joe) Xu, 2016/11/04 01:14:04]

Done.

+
+  // Saving backlights forced off state, so that it doesn't need to bother
+  // calling GetBacklightsForcedOff method.
+  bool backlights_forced_off_;
+
+  // TabletPowerStateDelegate object, currently supported on ChromeOS only, for
+  // other environments, it is nullptr.
+  std::unique_ptr<TabletPowerStateDelegate> tablet_power_state_delegate_;
+
   DISALLOW_COPY_AND_ASSIGN(PowerButtonController);
 };