Lock rotation when screen is manually rotated.

Lock Rotation when screen is manually rotated.
This re-lands: https://codereview.chromium.org/261163004
which was reverted by: https://codereview.chromium.org/271133002
64065053.

Additionally DisplayManager now accepts a source of display rotations. Those triggered by explicit user input (display settins, ctrl+shift+f3) will be stored as a part of display settings. Those triggered by rotation will not.

Upon exiting maximize mode the display will be returned to the last rotation explicitly set by a user.

TEST=MaximizeControllerModeTest
TEST=DisplayPreferencesTest
BUG=371426
BUG=369505

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

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

ash/shell.cc

@@ -477,6 +477,14 @@ bool Shell::IsMaximizeModeWindowManagerEnabled() {
   return maximize_mode_window_manager_.get() != NULL;
 }
 
+#if defined(OS_CHROMEOS)
+bool Shell::ShouldSaveDisplaySettings() {
+  return !((IsMaximizeModeWindowManagerEnabled() &&
+            maximize_mode_controller_->in_set_screen_rotation()) ||
+           resolution_notification_controller_->DoesNotificationTimeout());
+}
+#endif
+
 void Shell::UpdateShelfVisibility() {
   RootWindowControllerList controllers = GetAllRootWindowControllers();
   for (RootWindowControllerList::iterator iter = controllers.begin();

ash/shell.h

@@ -332,6 +332,14 @@ class ASH_EXPORT Shell : public SystemModalContainerEventFilterDelegate,
   // Test if the MaximizeModeWindowManager is enabled or not.
   bool IsMaximizeModeWindowManagerEnabled();
 
+#if defined(OS_CHROMEOS)
+  // Test if MaximizeModeWindowManager is not enabled, and if
+  // MaximizeModeController is not currently setting a display rotation. Or if
+  // the |resolution_notification_controller_| is not showing its confirmation
+  // dialog. If true then changes to display settings can be saved.
+  bool ShouldSaveDisplaySettings();
+#endif
+
   AcceleratorController* accelerator_controller() {
     return accelerator_controller_.get();
   }

ash/wm/maximize_mode/maximize_mode_controller.cc

@@ -136,7 +136,8 @@ void ScreenshotActionHandler::OnKeyEvent(ui::KeyEvent* event) {
 MaximizeModeController::MaximizeModeController()
     : rotation_locked_(false),
       have_seen_accelerometer_data_(false),
-      in_set_screen_rotation_(false) {
+      in_set_screen_rotation_(false),
+      user_rotation_(gfx::Display::ROTATE_0) {
   Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
 }
 
@@ -207,16 +208,10 @@ void MaximizeModeController::HandleHingeRotation(const gfx::Vector3dF& base,
   if (maximize_mode_engaged &&
       angle > kFullyOpenAngleErrorTolerance &&
       angle < kExitMaximizeModeAngle) {
-    Shell::GetInstance()->EnableMaximizeModeWindowManager(false);
-    event_blocker_.reset();
-    event_handler_.reset();
+    LeaveMaximizeMode();
   } else if (!maximize_mode_engaged &&
       angle > kEnterMaximizeModeAngle) {
-    Shell::GetInstance()->EnableMaximizeModeWindowManager(true);
-    event_blocker_.reset(new MaximizeModeEventBlocker);
-#if defined(OS_CHROMEOS)
-    event_handler_.reset(new ScreenshotActionHandler);
-#endif
+    EnterMaximizeMode();
   }
 }
 
@@ -224,31 +219,14 @@ void MaximizeModeController::HandleScreenRotation(const gfx::Vector3dF& lid) {
   bool maximize_mode_engaged =
       Shell::GetInstance()->IsMaximizeModeWindowManagerEnabled();
 
+  if (!maximize_mode_engaged || rotation_locked_)
+    return;
+
   DisplayManager* display_manager =
       Shell::GetInstance()->display_manager();
   gfx::Display::Rotation current_rotation = display_manager->GetDisplayInfo(
       gfx::Display::InternalDisplayId()).rotation();
 
-  // If maximize mode is not engaged, ensure the screen is not rotated and
-  // do not rotate to match the current device orientation.
-  if (!maximize_mode_engaged) {
-    if (current_rotation != gfx::Display::ROTATE_0) {
-      // TODO(flackr): Currently this will prevent setting a manual rotation on
-      // the screen of a device with an accelerometer, this should only set it
-      // back to ROTATE_0 if it was last set by the accelerometer.
-      // Also, SetDisplayRotation will save the setting to the local store,
-      // this should be stored in a way that we can distinguish what the
-      // rotation was set by.
-      SetDisplayRotation(display_manager,
-                         gfx::Display::ROTATE_0);
-    }
-    rotation_locked_ = false;
-    return;
-  }
-
-  if (rotation_locked_)
-    return;
-
   // After determining maximize mode state, determine if the screen should
   // be rotated.
   gfx::Vector3dF lid_flattened(lid.x(), lid.y(), 0.0f);
@@ -294,13 +272,8 @@ void MaximizeModeController::HandleScreenRotation(const gfx::Vector3dF& lid) {
   else if (angle < 270.0f)
     new_rotation = gfx::Display::ROTATE_180;
 
-  // When exiting maximize mode return rotation to 0. When entering, rotate to
-  // match screen orientation.
-  if (new_rotation == gfx::Display::ROTATE_0 ||
-      maximize_mode_engaged) {
-    SetDisplayRotation(display_manager,
-                       new_rotation);
-  }
+  if (new_rotation != current_rotation)
+    SetDisplayRotation(display_manager, new_rotation);
 }
 
 void MaximizeModeController::SetDisplayRotation(
@@ -312,4 +285,31 @@ void MaximizeModeController::SetDisplayRotation(
                                       rotation);
 }
 
+void MaximizeModeController::EnterMaximizeMode() {
+  // TODO(jonross): Listen for display configuration changes. If the user
+  // causes a rotation change a rotation lock should be applied.
+  // https://crbug.com/369505
+  DisplayManager* display_manager = Shell::GetInstance()->display_manager();
+  user_rotation_ = display_manager->
+      GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation();
+  Shell::GetInstance()->EnableMaximizeModeWindowManager(true);
+  event_blocker_.reset(new MaximizeModeEventBlocker);
+#if defined(OS_CHROMEOS)
+  event_handler_.reset(new ScreenshotActionHandler);
+#endif
+}
+
+void MaximizeModeController::LeaveMaximizeMode() {
+  DisplayManager* display_manager = Shell::GetInstance()->display_manager();
+  DisplayInfo info = display_manager->
+      GetDisplayInfo(gfx::Display::InternalDisplayId());
+  gfx::Display::Rotation current_rotation = info.rotation();
+  if (current_rotation != user_rotation_)
+    SetDisplayRotation(display_manager, user_rotation_);
+  rotation_locked_ = false;
+  Shell::GetInstance()->EnableMaximizeModeWindowManager(false);
+  event_blocker_.reset();
+  event_handler_.reset();
+}
+
 }  // namespace ash

ash/wm/maximize_mode/maximize_mode_controller.h

@@ -10,6 +10,7 @@
 #include "ash/display/display_manager.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "ui/gfx/display.h"
 
 namespace ui {
 class EventHandler;
@@ -69,6 +70,14 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver {
   void SetDisplayRotation(DisplayManager* display_manager,
                           gfx::Display::Rotation rotation);
 
+  // Enables MaximizeModeWindowManager, and determines the current state of
+  // rotation lock.
+  void EnterMaximizeMode();
+
+  // Removes MaximizeModeWindowManager and resets the display rotation if there
+  // is no rotation lock.
+  void LeaveMaximizeMode();
+
   // An event targeter controller which traps mouse and keyboard events while
   // maximize mode is engaged.
   scoped_ptr<MaximizeModeEventBlocker> event_blocker_;
@@ -85,6 +94,10 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver {
   // True when the screen's orientation is being changed.
   bool in_set_screen_rotation_;
 
+  // The rotation of the display set by the user. This rotation will be
+  // restored upon exiting maximize mode.
+  gfx::Display::Rotation user_rotation_;
+
   DISALLOW_COPY_AND_ASSIGN(MaximizeModeController);
 };
 

ash/wm/maximize_mode/maximize_mode_controller_unittest.cc

@@ -450,7 +450,7 @@ TEST_F(MaximizeModeControllerTest, LaptopTest) {
   // Feeds in sample accelerometer data and verifies that there are no
   // transitions into touchview / maximize mode while shaking the device around
   // with the hinge at less than 180 degrees.
-  ASSERT_TRUE(kAccelerometerLaptopModeTestDataLength % 6 == 0);
+  ASSERT_EQ(0u, kAccelerometerLaptopModeTestDataLength % 6);
   for (size_t i = 0; i < kAccelerometerLaptopModeTestDataLength / 6; ++i) {
     gfx::Vector3dF base(kAccelerometerLaptopModeTestData[i * 6],
                         kAccelerometerLaptopModeTestData[i * 6 + 1],
@@ -474,7 +474,7 @@ TEST_F(MaximizeModeControllerTest, MaximizeModeTest) {
   // Feeds in sample accelerometer data and verifies that there are no
   // transitions out of touchview / maximize mode while shaking the device
   // around.
-  ASSERT_TRUE(kAccelerometerFullyOpenTestDataLength % 6 == 0);
+  ASSERT_EQ(0u, kAccelerometerFullyOpenTestDataLength % 6);
   for (size_t i = 0; i < kAccelerometerFullyOpenTestDataLength / 6; ++i) {
     gfx::Vector3dF base(kAccelerometerFullyOpenTestData[i * 6],
                         kAccelerometerFullyOpenTestData[i * 6 + 1],
@@ -521,7 +521,7 @@ TEST_F(MaximizeModeControllerTest, ExitingMaximizeModeClearRotationLock) {
 
   // Trigger maximize mode by opening to 270.
   TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, -1.0f),
-                             gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
   ASSERT_TRUE(IsMaximizeModeStarted());
 
   maximize_mode_controller()->set_rotation_locked(true);
@@ -596,4 +596,27 @@ TEST_F(MaximizeModeControllerTest, BlockRotationNotifications) {
 }
 #endif
 
+// Tests that if a user has set a display rotation that it is restored upon
+// exiting maximize mode.
+TEST_F(MaximizeModeControllerTest, ResetUserRotationUponExit) {
+  DisplayManager* display_manager = Shell::GetInstance()->display_manager();
+  display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
+                                      gfx::Display::ROTATE_90);
+
+  // Trigger maximize mode
+  TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, -1.0f),
+                             gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  ASSERT_TRUE(IsMaximizeModeStarted());
+
+  TriggerAccelerometerUpdate(gfx::Vector3dF(1.0f, 0.0f, 0.0f),
+                             gfx::Vector3dF(1.0f, 0.0f, 0.0f));
+  EXPECT_EQ(gfx::Display::ROTATE_180, GetInternalDisplayRotation());
+
+  // Exit maximize mode
+  TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, 1.0f),
+                             gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  EXPECT_FALSE(IsMaximizeModeStarted());
+  EXPECT_EQ(gfx::Display::ROTATE_90, GetInternalDisplayRotation());
+}
+
 }  // namespace ash

chrome/browser/chromeos/display/display_preferences.cc

@@ -7,7 +7,6 @@
 #include "ash/display/display_layout_store.h"
 #include "ash/display/display_manager.h"
 #include "ash/display/display_pref_util.h"
-#include "ash/display/resolution_notification_controller.h"
 #include "ash/shell.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
@@ -305,10 +304,10 @@ void StoreDisplayPrefs() {
 
   // Do not store prefs when the confirmation dialog is shown.
   if (!UserCanSaveDisplayPreference() ||
-      ash::Shell::GetInstance()->resolution_notification_controller()->
-          DoesNotificationTimeout()) {
+      !ash::Shell::GetInstance()->ShouldSaveDisplaySettings()) {
     return;
   }
+
   StoreCurrentDisplayLayoutPrefs();
   StoreCurrentDisplayProperties();
 }

chrome/browser/chromeos/display/display_preferences_unittest.cc

@@ -12,6 +12,7 @@
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/display_manager_test_api.h"
+#include "ash/wm/maximize_mode/maximize_mode_controller.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/prefs/testing_pref_service.h"
 #include "base/strings/string_number_conversions.h"
@@ -22,6 +23,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "ui/display/chromeos/display_configurator.h"
+#include "ui/gfx/vector3d_f.h"
 #include "ui/message_center/message_center.h"
 
 using ash::ResolutionNotificationController;
@@ -620,4 +622,41 @@ TEST_F(DisplayPreferencesTest, DontSaveAndRestoreAllOff) {
             shell->display_configurator()->power_state());
 }
 
+// Tests that display configuration changes caused by MaximizeModeController
+// are not saved.
+TEST_F(DisplayPreferencesTest, DontSaveMaximizeModeControllerRotations) {
+  ash::Shell* shell = ash::Shell::GetInstance();
+  ash::MaximizeModeController* controller = shell->maximize_mode_controller();
+  gfx::Display::SetInternalDisplayId(
+      gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().id());
+  ash::DisplayManager* display_manager = shell->display_manager();
+  LoggedInAsUser();
+  // Populate the properties.
+  display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
+                                      gfx::Display::ROTATE_180);
+  // Reset property to avoid rotation lock
+  display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
+                                      gfx::Display::ROTATE_0);
+
+  // Open up 270 degrees to trigger maximize mode
+  controller->OnAccelerometerUpdated(gfx::Vector3dF(0.0f, 0.0f, -1.0f),
+                                     gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  EXPECT_TRUE(shell->IsMaximizeModeWindowManagerEnabled());
+
+  // Trigger 90 degree rotation
+  controller->OnAccelerometerUpdated(gfx::Vector3dF(0.0f, 1.0f, 0.0f),
+                                     gfx::Vector3dF(0.0f, 1.0f, 0.0f));
+  EXPECT_EQ(gfx::Display::ROTATE_90, display_manager->
+                GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation());
+
+  const base::DictionaryValue* properties =
+      local_state()->GetDictionary(prefs::kDisplayProperties);
+  const base::DictionaryValue* property = NULL;
+  EXPECT_TRUE(properties->GetDictionary(
+      base::Int64ToString(gfx::Display::InternalDisplayId()), &property));
+  int rotation = -1;
+  EXPECT_TRUE(property->GetInteger("rotation", &rotation));
+  EXPECT_EQ(gfx::Display::ROTATE_0, rotation);
+}
+
 }  // namespace chromeos