Prevent entering maximize mode when the lid is closed or has recently opened.

BUG=387879
TEST=MaximizeModeController.CloseLidWhileInMaximizeMode
TEST=MaximizeModeController.WasLidOpenedRecently
TEST=MaximizeModeController.HingeAnglesWithLidClosed
TEST=MaximizeModeController.StableHingeAnglesWithLidOpened
TEST=MaximizeModeController.UnstableHingeAnglesWhenLidRecentlyOpened
TEST=MaximizeModeController.UnstableHingAnglesWithLidOpened

Review URL: https://codereview.chromium.org/412013002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287625 0039d316-1c4b-4281-b951-d872f2087c98

ash/wm/maximize_mode/maximize_mode_controller.cc

@@ -15,6 +15,8 @@
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
+#include "base/time/default_tick_clock.h"
+#include "base/time/tick_clock.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/events/event.h"
 #include "ui/events/event_handler.h"
@@ -25,6 +27,10 @@
 #include "ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/dbus_thread_manager.h"
+#endif  // OS_CHROMEOS
+
 namespace ash {
 
 namespace {
@@ -36,11 +42,19 @@ const float kEnterMaximizeModeAngle = 200.0f;
 // angle to enter maximize mode to prevent rapid toggling when near the angle.
 const float kExitMaximizeModeAngle = 160.0f;
 
-// When the lid is fully open 360 degrees, the accelerometer readings can
-// occasionally appear as though the lid is almost closed. If the lid appears
-// near closed but the device is on we assume it is an erroneous reading from
-// it being open 360 degrees.
-const float kFullyOpenAngleErrorTolerance = 20.0f;
+// Defines a range for which accelerometer readings are considered accurate.
+// When the lid is near open (or near closed) the accelerometer readings may be
+// inaccurate and a lid that is fully open may appear to be near closed (and
+// vice versa).
+const float kMinStableAngle = 20.0f;
+const float kMaxStableAngle = 340.0f;
+
+// The time duration to consider the lid to be recently opened.
+// This is used to prevent entering maximize mode if an erroneous accelerometer
+// reading makes the lid appear to be fully open when the user is opening the
+// lid from a closed position.
+const base::TimeDelta kLidRecentlyOpenedDuration =
+    base::TimeDelta::FromSeconds(2);
 
 // When the device approaches vertical orientation (i.e. portrait orientation)
 // the accelerometers for the base and lid approach the same values (i.e.
@@ -144,14 +158,24 @@ MaximizeModeController::MaximizeModeController()
       have_seen_accelerometer_data_(false),
       in_set_screen_rotation_(false),
       user_rotation_(gfx::Display::ROTATE_0),
-      last_touchview_transition_time_(base::Time::Now()) {
+      last_touchview_transition_time_(base::Time::Now()),
+      tick_clock_(new base::DefaultTickClock()),
+      lid_is_closed_(false) {
   Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
   Shell::GetInstance()->AddShellObserver(this);
+#if defined(OS_CHROMEOS)
+  chromeos::DBusThreadManager::Get()->
+      GetPowerManagerClient()->AddObserver(this);
+#endif  // OS_CHROMEOS
 }
 
 MaximizeModeController::~MaximizeModeController() {
   Shell::GetInstance()->RemoveShellObserver(this);
   Shell::GetInstance()->accelerometer_controller()->RemoveObserver(this);
+#if defined(OS_CHROMEOS)
+  chromeos::DBusThreadManager::Get()->
+      GetPowerManagerClient()->RemoveObserver(this);
+#endif  // OS_CHROMEOS
 }
 
 void MaximizeModeController::SetRotationLocked(bool rotation_locked) {
@@ -239,6 +263,25 @@ void MaximizeModeController::OnDisplayConfigurationChanged() {
   }
 }
 
+#if defined(OS_CHROMEOS)
+void MaximizeModeController::LidEventReceived(bool open,
+                                              const base::TimeTicks& time) {
+  if (open)
+    last_lid_open_time_ = time;
+  lid_is_closed_ = !open;
+  LeaveMaximizeMode();
+}
+
+void MaximizeModeController::SuspendImminent() {
+  RecordTouchViewStateTransition();
+}
+
+void MaximizeModeController::SuspendDone(
+    const base::TimeDelta& sleep_duration) {
+  last_touchview_transition_time_ = base::Time::Now();
+}
+#endif  // OS_CHROMEOS
+
 void MaximizeModeController::HandleHingeRotation(const gfx::Vector3dF& base,
                                                  const gfx::Vector3dF& lid) {
   static const gfx::Vector3dF hinge_vector(0.0f, 1.0f, 0.0f);
@@ -262,16 +305,24 @@ void MaximizeModeController::HandleHingeRotation(const gfx::Vector3dF& base,
   float angle = ClockwiseAngleBetweenVectorsInDegrees(base_flattened,
       lid_flattened, hinge_vector);
 
+  bool is_angle_stable = angle > kMinStableAngle && angle < kMaxStableAngle;
+
+  // Clear the last_lid_open_time_ for a stable reading so that there is less
+  // chance of a delay if the lid is moved from the close state to the fully
+  // open state very quickly.
+  if (is_angle_stable)
+    last_lid_open_time_ = base::TimeTicks();
+
   // Toggle maximize mode on or off when corresponding thresholds are passed.
   // TODO(flackr): Make MaximizeModeController own the MaximizeModeWindowManager
   // such that observations of state changes occur after the change and shell
   // has fewer states to track.
-  if (maximize_mode_engaged &&
-      angle > kFullyOpenAngleErrorTolerance &&
+  if (maximize_mode_engaged && is_angle_stable &&
       angle < kExitMaximizeModeAngle) {
     LeaveMaximizeMode();
-  } else if (!maximize_mode_engaged &&
-      angle > kEnterMaximizeModeAngle) {
+  } else if (!lid_is_closed_ && !maximize_mode_engaged &&
+             angle > kEnterMaximizeModeAngle &&
+             (is_angle_stable || !WasLidOpenedRecently())) {
     EnterMaximizeMode();
   }
 }
@@ -350,6 +401,8 @@ void MaximizeModeController::SetDisplayRotation(
 }
 
 void MaximizeModeController::EnterMaximizeMode() {
+  if (IsMaximizeModeWindowManagerEnabled())
+    return;
   DisplayManager* display_manager = Shell::GetInstance()->display_manager();
   current_rotation_ = user_rotation_ = display_manager->
       GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation();
@@ -364,6 +417,8 @@ void MaximizeModeController::EnterMaximizeMode() {
 }
 
 void MaximizeModeController::LeaveMaximizeMode() {
+  if (!IsMaximizeModeWindowManagerEnabled())
+    return;
   DisplayManager* display_manager = Shell::GetInstance()->display_manager();
   gfx::Display::Rotation current_rotation = display_manager->
       GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation();
@@ -373,14 +428,7 @@ void MaximizeModeController::LeaveMaximizeMode() {
   EnableMaximizeModeWindowManager(false);
   event_blocker_.reset();
   event_handler_.reset();
-}
-
-void MaximizeModeController::OnSuspend() {
-  RecordTouchViewStateTransition();
-}
-
-void MaximizeModeController::OnResume() {
-  last_touchview_transition_time_ = base::Time::Now();
+  Shell::GetInstance()->display_controller()->RemoveObserver(this);
 }
 
 // Called after maximize mode has started, windows might still animate though.
@@ -428,4 +476,20 @@ void MaximizeModeController::OnAppTerminating() {
   Shell::GetInstance()->display_controller()->RemoveObserver(this);
 }
 
+bool MaximizeModeController::WasLidOpenedRecently() const {
+  if (last_lid_open_time_.is_null())
+    return false;
+
+  base::TimeTicks now = tick_clock_->NowTicks();
+  DCHECK(now >= last_lid_open_time_);
+  base::TimeDelta elapsed_time = now - last_lid_open_time_;
+  return elapsed_time <= kLidRecentlyOpenedDuration;
+}
+
+void MaximizeModeController::SetTickClockForTest(
+    scoped_ptr<base::TickClock> tick_clock) {
+  DCHECK(tick_clock_);
+  tick_clock_ = tick_clock.Pass();
+}
+
 }  // namespace ash

ash/wm/maximize_mode/maximize_mode_controller.h

@@ -13,9 +13,16 @@
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
-#include "base/power_monitor/power_observer.h"
 #include "ui/gfx/display.h"
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/power_manager_client.h"
+#endif  // OS_CHROMEOS
+
+namespace base {
+class TickClock;
+}
+
 namespace ui {
 class EventHandler;
 }
@@ -30,10 +37,13 @@ class MaximizeModeWindowManagerTest;
 // MaximizeModeController listens to accelerometer events and automatically
 // enters and exits maximize mode when the lid is opened beyond the triggering
 // angle and rotates the display to match the device when in maximize mode.
-class ASH_EXPORT MaximizeModeController : public AccelerometerObserver,
-    public base::PowerObserver,
-    public ShellObserver,
-    public DisplayController::Observer {
+class ASH_EXPORT MaximizeModeController
+    : public AccelerometerObserver,
+#if defined(OS_CHROMEOS)
+      public chromeos::PowerManagerClient::Observer,
+#endif  // OS_CHROMEOS
+      public ShellObserver,
+      public DisplayController::Observer {
  public:
   // Observer that reports changes to the state of MaximizeModeController's
   // rotation lock.
@@ -98,17 +108,25 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver,
   virtual void OnMaximizeModeStarted() OVERRIDE;
   virtual void OnMaximizeModeEnded() OVERRIDE;
 
-  // base::PowerObserver:
-  virtual void OnSuspend() OVERRIDE;
-  virtual void OnResume() OVERRIDE;
-
   // DisplayController::Observer:
   virtual void OnDisplayConfigurationChanged() OVERRIDE;
 
+#if defined(OS_CHROMEOS)
+  // PowerManagerClient::Observer:
+  virtual void LidEventReceived(bool open,
+                                const base::TimeTicks& time) OVERRIDE;
+  virtual void SuspendImminent() OVERRIDE;
+  virtual void SuspendDone(const base::TimeDelta& sleep_duration) OVERRIDE;
+#endif  // OS_CHROMEOS
+
  private:
   friend class MaximizeModeControllerTest;
   friend class MaximizeModeWindowManagerTest;
 
+  // Set the TickClock. This is only to be used by tests that need to
+  // artificially and deterministically control the current time.
+  void SetTickClockForTest(scoped_ptr<base::TickClock> tick_clock);
+
   // Detect hinge rotation from |base| and |lid| accelerometers and
   // automatically start / stop maximize mode.
   void HandleHingeRotation(const gfx::Vector3dF& base,
@@ -122,6 +140,9 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver,
   void SetDisplayRotation(DisplayManager* display_manager,
                           gfx::Display::Rotation rotation);
 
+  // Returns true if the lid was recently opened.
+  bool WasLidOpenedRecently() const;
+
   // Enables MaximizeModeWindowManager, and determines the current state of
   // rotation lock.
   void EnterMaximizeMode();
@@ -169,6 +190,17 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver,
   base::TimeDelta total_touchview_time_;
   base::TimeDelta total_non_touchview_time_;
 
+  // Tracks the last time we received a lid open event. This is used to suppress
+  // erroneous accelerometer readings as the lid is opened but the accelerometer
+  // reports readings that make the lid to appear near fully open.
+  base::TimeTicks last_lid_open_time_;
+
+  // Source for the current time in base::TimeTicks.
+  scoped_ptr<base::TickClock> tick_clock_;
+
+  // Tracks when the lid is closed. Used to prevent entering maximize mode.
+  bool lid_is_closed_;
+
   DISALLOW_COPY_AND_ASSIGN(MaximizeModeController);
 };
 

ash/wm/maximize_mode/maximize_mode_controller_unittest.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <math.h>
+
 #include "ash/wm/maximize_mode/maximize_mode_controller.h"
 
 #include "ash/accelerometer/accelerometer_controller.h"
@@ -14,6 +16,7 @@
 #include "ash/test/test_screenshot_delegate.h"
 #include "ash/test/test_system_tray_delegate.h"
 #include "ash/test/test_volume_control_delegate.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "ui/events/event_handler.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/vector3d_f.h"
@@ -90,43 +93,178 @@ class MaximizeModeControllerTest : public test::AshTestBase {
         SetDisplayRotation(gfx::Display::InternalDisplayId(), rotation);
   }
 
+  // Attaches a SimpleTestTickClock to the MaximizeModeController with a non
+  // null value initial value.
+  void AttachTickClockForTest() {
+    scoped_ptr<base::TickClock> tick_clock(
+        test_tick_clock_ = new base::SimpleTestTickClock());
+    test_tick_clock_->Advance(base::TimeDelta::FromSeconds(1));
+    maximize_mode_controller()->SetTickClockForTest(tick_clock.Pass());
+  }
+
+  void AdvanceTickClock(const base::TimeDelta& delta) {
+    DCHECK(test_tick_clock_);
+    test_tick_clock_->Advance(delta);
+  }
+
+  void OpenLidToAngle(float degrees) {
+    DCHECK(degrees >= 0.0f);
+    DCHECK(degrees <= 360.0f);
+
+    float radians = degrees * kDegreesToRadians;
+    gfx::Vector3dF base_vector(1.0f, 0.0f, 0.0f);
+    gfx::Vector3dF lid_vector(cos(radians), 0.0f, sin(radians));
+    TriggerAccelerometerUpdate(base_vector, lid_vector);
+  }
+
+#if defined(OS_CHROMEOS)
+  void OpenLid() {
+    maximize_mode_controller()->LidEventReceived(true /* open */,
+        maximize_mode_controller()->tick_clock_->NowTicks());
+  }
+
+  void CloseLid() {
+    maximize_mode_controller()->LidEventReceived(false /* open */,
+        maximize_mode_controller()->tick_clock_->NowTicks());
+  }
+#endif  // OS_CHROMEOS
+
+  bool WasLidOpenedRecently() {
+    return maximize_mode_controller()->WasLidOpenedRecently();
+  }
+
  private:
+  base::SimpleTestTickClock* test_tick_clock_;
+
   DISALLOW_COPY_AND_ASSIGN(MaximizeModeControllerTest);
 };
 
-// Tests that opening the lid beyond 180 will enter touchview, and that it will
-// exit when the lid comes back from 180. Also tests the thresholds, i.e. it
-// will stick to the current mode.
-TEST_F(MaximizeModeControllerTest, EnterExitThresholds) {
-  // For the simple test the base remains steady.
-  gfx::Vector3dF base(0.0f, 0.0f, 1.0f);
+#if defined(OS_CHROMEOS)
+
+// Verify that closing the lid will exit maximize mode.
+TEST_F(MaximizeModeControllerTest, CloseLidWhileInMaximizeMode) {
+  OpenLidToAngle(315.0f);
+  ASSERT_TRUE(IsMaximizeModeStarted());
 
-  // Lid open 90 degrees.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  CloseLid();
   EXPECT_FALSE(IsMaximizeModeStarted());
+}
+
+// Verify that maximize mode will not be entered when the lid is closed.
+TEST_F(MaximizeModeControllerTest,
+    HingeAnglesWithLidClosed) {
+  AttachTickClockForTest();
 
-  // Open just past 180.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(0.05f, 0.0f, -1.0f));
+  CloseLid();
+
+  OpenLidToAngle(270.0f);
   EXPECT_FALSE(IsMaximizeModeStarted());
 
-  // Open up 270 degrees.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(1.0f, 0.0f, 0.0f));
+  OpenLidToAngle(315.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(355.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+}
+
+// Verify the maximize mode state for unstable hinge angles when the lid was
+// recently open.
+TEST_F(MaximizeModeControllerTest,
+    UnstableHingeAnglesWhenLidRecentlyOpened) {
+  AttachTickClockForTest();
+
+  OpenLid();
+  ASSERT_TRUE(WasLidOpenedRecently());
+
+  OpenLidToAngle(5.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(355.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+
+  // This is a stable reading and should clear the last lid opened time.
+  OpenLidToAngle(45.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+  EXPECT_FALSE(WasLidOpenedRecently());
+
+  OpenLidToAngle(355.0f);
+  EXPECT_TRUE(IsMaximizeModeStarted());
+}
+
+#endif  // OS_CHROMEOS
+
+// Verify the WasLidOpenedRecently signal with respect to time.
+TEST_F(MaximizeModeControllerTest, WasLidOpenedRecentlyOverTime) {
+#if defined(OS_CHROMEOS)
+
+  AttachTickClockForTest();
+
+  // No lid open time initially.
+  ASSERT_FALSE(WasLidOpenedRecently());
+
+  CloseLid();
+  EXPECT_FALSE(WasLidOpenedRecently());
+
+  OpenLid();
+  EXPECT_TRUE(WasLidOpenedRecently());
+
+  // 1 second after lid open.
+  AdvanceTickClock(base::TimeDelta::FromSeconds(1));
+  EXPECT_TRUE(WasLidOpenedRecently());
+
+  // 3 seconds after lid open.
+  AdvanceTickClock(base::TimeDelta::FromSeconds(2));
+  EXPECT_FALSE(WasLidOpenedRecently());
+
+#else
+
+  EXPECT_FALSE(WasLidOpenedRecently());
+
+#endif  // OS_CHROMEOS
+}
+
+// Verify the maximize mode enter/exit thresholds for stable angles.
+TEST_F(MaximizeModeControllerTest, StableHingeAnglesWithLidOpened) {
+  ASSERT_FALSE(IsMaximizeModeStarted());
+  ASSERT_FALSE(WasLidOpenedRecently());
+
+  OpenLidToAngle(180.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(315.0f);
   EXPECT_TRUE(IsMaximizeModeStarted());
 
-  // Open up 360 degrees and appearing to be slightly past it (i.e. as if almost
-  // closed).
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(-0.05f, 0.0f, 1.0f));
+  OpenLidToAngle(180.0f);
   EXPECT_TRUE(IsMaximizeModeStarted());
 
-  // Open just before 180.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(-0.05f, 0.0f, -1.0f));
+  OpenLidToAngle(45.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(270.0f);
   EXPECT_TRUE(IsMaximizeModeStarted());
 
-  // Open 90 degrees.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  OpenLidToAngle(90.0f);
   EXPECT_FALSE(IsMaximizeModeStarted());
 }
 
+// Verify the maximize mode state for unstable hinge angles when the lid is open
+// but not recently.
+TEST_F(MaximizeModeControllerTest, UnstableHingeAnglesWithLidOpened) {
+  AttachTickClockForTest();
+
+  ASSERT_FALSE(WasLidOpenedRecently());
+  ASSERT_FALSE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(5.0f);
+  EXPECT_FALSE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(355.0f);
+  EXPECT_TRUE(IsMaximizeModeStarted());
+
+  OpenLidToAngle(5.0f);
+  EXPECT_TRUE(IsMaximizeModeStarted());
+}
+
 // Tests that when the hinge is nearly vertically aligned, the current state
 // persists as the computed angle is highly inaccurate in this orientation.
 TEST_F(MaximizeModeControllerTest, HingeAligned) {
@@ -157,8 +295,8 @@ TEST_F(MaximizeModeControllerTest, HingeAligned) {
   EXPECT_TRUE(IsMaximizeModeStarted());
 }
 
-// Tests that accelerometer readings in each of the screen angles will trigger
-// a rotation of the internal display.
+// Tests that accelerometer readings in each of the screen angles will trigger a
+// rotation of the internal display.
 TEST_F(MaximizeModeControllerTest, DisplayRotation) {
   // Trigger maximize mode by opening to 270.
   TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, -1.0f),
@@ -275,8 +413,7 @@ TEST_F(MaximizeModeControllerTest, Screenshot) {
   delegate->set_can_take_screenshot(true);
 
   // Open up 270 degrees.
-  TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, 1.0f),
-                             gfx::Vector3dF(1.0f, 0.0f, 0.0f));
+  OpenLidToAngle(270.0f);
   ASSERT_TRUE(IsMaximizeModeStarted());
 
   // Pressing power alone does not take a screenshot.
@@ -363,22 +500,17 @@ TEST_F(MaximizeModeControllerTest, RotationLockPreventsRotation) {
 // Tests that when MaximizeModeController turns off MaximizeMode that on the
 // next accelerometer update the rotation lock is cleared.
 TEST_F(MaximizeModeControllerTest, ExitingMaximizeModeClearRotationLock) {
-  // The base remains steady.
-  gfx::Vector3dF base(0.0f, 0.0f, 1.0f);
-
   // Trigger maximize mode by opening to 270.
-  TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, -1.0f),
-  gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  OpenLidToAngle(270.0f);
   ASSERT_TRUE(IsMaximizeModeStarted());
 
   maximize_mode_controller()->SetRotationLocked(true);
 
-  // Open 90 degrees.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  OpenLidToAngle(90.0f);
   EXPECT_FALSE(IsMaximizeModeStarted());
 
-  // Send an update that would not relaunch MaximizeMode. 90 degrees.
-  TriggerAccelerometerUpdate(base, gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  // Send an update that would not relaunch MaximizeMode.
+  OpenLidToAngle(90.0f);
   EXPECT_FALSE(maximize_mode_controller()->rotation_locked());
 }