Rotate display keyboard shortcut behavior in tablet physical state

When the device is physically used as a tablet (e.g. detached or
flipped), the keyboard shortcut to rotate the display should
behave as a request to lock the user orientation to the next
rotation of the internal display.

App windows are allowed to lock the rotation to a particular
orientation in tablet UI mode on the internal display only.
In this case, the shortcut should switch between the allowed
rotations if any, or do nothing.

For example:
- If a window locks the rotation to kPortrait, the shortcut should
  switch the user rotation lock between kPortriatPrimary and
  kPortraitSecondary.
- If a window locks the rotation to kPortraitSecondary, then the
  shortcut should do nothing.

BUG=925087, 1026398
TEST=Manually, Added a new test.

Change-Id: I34bf76a336ef81de4f1f5269843a3dd8c5efcd4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1892054
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721293}

ash/accelerators/accelerator_controller_impl.cc

@@ -519,7 +519,8 @@ void HandleSwitchToLastUsedIme(const ui::Accelerator& accelerator) {
   // Else: consume the Ctrl+Space ET_KEY_RELEASED event but do not do anything.
 }
 
-display::Display::Rotation GetNextRotation(display::Display::Rotation current) {
+display::Display::Rotation GetNextRotationInClamshell(
+    display::Display::Rotation current) {
   switch (current) {
     case display::Display::ROTATE_0:
       return display::Display::ROTATE_90;
@@ -534,15 +535,95 @@ display::Display::Rotation GetNextRotation(display::Display::Rotation current) {
   return display::Display::ROTATE_0;
 }
 
+display::Display::Rotation GetNextRotationInTabletMode(
+    int64_t display_id,
+    display::Display::Rotation current) {
+  Shell* shell = Shell::Get();
+  DCHECK(shell->tablet_mode_controller()->InTabletMode());
+
+  if (!display::Display::HasInternalDisplay() ||
+      display_id != display::Display::InternalDisplayId()) {
+    return GetNextRotationInClamshell(current);
+  }
+
+  const OrientationLockType app_requested_lock =
+      shell->screen_orientation_controller()
+          ->GetCurrentAppRequestedOrientationLock();
+
+  bool add_180_degrees = false;
+  switch (app_requested_lock) {
+    case OrientationLockType::kCurrent:
+    case OrientationLockType::kLandscapePrimary:
+    case OrientationLockType::kLandscapeSecondary:
+    case OrientationLockType::kPortraitPrimary:
+    case OrientationLockType::kPortraitSecondary:
+    case OrientationLockType::kNatural:
+      // Do not change the current orientation.
+      return current;
+
+    case OrientationLockType::kLandscape:
+    case OrientationLockType::kPortrait:
+      // App allows both primary and secondary orientations in either landscape
+      // or portrait, therefore switch to the next one by adding 180 degrees.
+      add_180_degrees = true;
+      break;
+
+    default:
+      break;
+  }
+
+  switch (current) {
+    case display::Display::ROTATE_0:
+      return add_180_degrees ? display::Display::ROTATE_180
+                             : display::Display::ROTATE_90;
+    case display::Display::ROTATE_90:
+      return add_180_degrees ? display::Display::ROTATE_270
+                             : display::Display::ROTATE_180;
+    case display::Display::ROTATE_180:
+      return add_180_degrees ? display::Display::ROTATE_0
+                             : display::Display::ROTATE_270;
+    case display::Display::ROTATE_270:
+      return add_180_degrees ? display::Display::ROTATE_90
+                             : display::Display::ROTATE_0;
+  }
+  NOTREACHED() << "Unknown rotation:" << current;
+  return display::Display::ROTATE_0;
+}
+
+bool ShouldLockRotation(int64_t display_id) {
+  return display::Display::HasInternalDisplay() &&
+         display_id == display::Display::InternalDisplayId() &&
+         Shell::Get()->tablet_mode_controller()->is_in_tablet_physical_state();
+}
+
+int64_t GetDisplayIdForRotation() {
+  const gfx::Point point = display::Screen::GetScreen()->GetCursorScreenPoint();
+  return display::Screen::GetScreen()->GetDisplayNearestPoint(point).id();
+}
+
 void RotateScreen() {
-  gfx::Point point = display::Screen::GetScreen()->GetCursorScreenPoint();
-  display::Display display =
-      display::Screen::GetScreen()->GetDisplayNearestPoint(point);
+  auto* shell = Shell::Get();
+  const bool in_tablet_mode =
+      Shell::Get()->tablet_mode_controller()->InTabletMode();
+  const int64_t display_id = GetDisplayIdForRotation();
   const display::ManagedDisplayInfo& display_info =
-      Shell::Get()->display_manager()->GetDisplayInfo(display.id());
-  Shell::Get()->display_configuration_controller()->SetDisplayRotation(
-      display.id(), GetNextRotation(display_info.GetActiveRotation()),
-      display::Display::RotationSource::USER);
+      shell->display_manager()->GetDisplayInfo(display_id);
+  const auto active_rotation = display_info.GetActiveRotation();
+  const auto next_rotation =
+      in_tablet_mode ? GetNextRotationInTabletMode(display_id, active_rotation)
+                     : GetNextRotationInClamshell(active_rotation);
+  if (active_rotation == next_rotation)
+    return;
+
+  // When the device is in a physical tablet state, display rotation requests of
+  // the internal display are treated as requests to lock the user rotation.
+  if (ShouldLockRotation(display_id)) {
+    shell->screen_orientation_controller()->SetLockToRotation(next_rotation);
+    return;
+  }
+
+  shell->display_configuration_controller()->SetDisplayRotation(
+      display_id, next_rotation, display::Display::RotationSource::USER);
 }
 
 void OnRotationDialogAccepted() {

ash/accelerators/accelerator_controller_unittest.cc

@@ -38,6 +38,7 @@
 #include "ash/test_screenshot_delegate.h"
 #include "ash/wm/lock_state_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
+#include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
 #include "ash/wm/test_session_state_animator.h"
 #include "ash/wm/window_positioning_utils.h"
 #include "ash/wm/window_state.h"
@@ -235,6 +236,18 @@ class AcceleratorControllerTest : public AshTestBase {
     test_api_->GetConfirmationDialog()->CancelDialog();
   }
 
+  void TriggerRotateScreenShortcut() {
+    ui::test::EventGenerator* generator = GetEventGenerator();
+    generator->PressKey(ui::VKEY_BROWSER_REFRESH,
+                        ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN);
+    generator->ReleaseKey(ui::VKEY_BROWSER_REFRESH,
+                          ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN);
+    if (IsConfirmationDialogOpen()) {
+      AcceptConfirmationDialog();
+      base::RunLoop().RunUntilIdle();
+    }
+  }
+
   void RemoveAllNotifications() const {
     message_center()->RemoveAllNotifications(
         false /* by_user */, message_center::MessageCenter::RemoveType::ALL);
@@ -594,6 +607,126 @@ TEST_F(AcceleratorControllerTest, RotateScreen) {
   EXPECT_NE(initial_rotation, rotation_after_accept);
 }
 
+// Tests that using the keyboard shortcut to rotate the display while the device
+// is in physical tablet state behaves like a request to lock the user
+// orientation to the next rotation of the internal display, and disables auto-
+// rotation.
+TEST_F(AcceleratorControllerTest, RotateScreenInPhysicalTabletState) {
+  display::test::DisplayManagerTestApi(display_manager())
+      .SetFirstDisplayAsInternalDisplay();
+  ash::ShellTestApi().SetTabletModeEnabledForTest(true);
+  auto* tablet_mode_controller = Shell::Get()->tablet_mode_controller();
+  auto* screen_orientation_controller =
+      Shell::Get()->screen_orientation_controller();
+  EXPECT_TRUE(tablet_mode_controller->is_in_tablet_physical_state());
+  EXPECT_FALSE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_FALSE(screen_orientation_controller->rotation_locked());
+  EXPECT_EQ(OrientationLockType::kLandscapePrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  TriggerRotateScreenShortcut();
+
+  EXPECT_TRUE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_EQ(OrientationLockType::kPortraitSecondary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  // When the device is no longer used as a tablet, the original rotation will
+  // be restored.
+  ash::ShellTestApi().SetTabletModeEnabledForTest(false);
+  EXPECT_FALSE(tablet_mode_controller->is_in_tablet_physical_state());
+  EXPECT_EQ(OrientationLockType::kLandscapePrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+  // User rotation lock remains in place to be restored again when the device
+  // goes to physical tablet state again.
+  EXPECT_TRUE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_FALSE(screen_orientation_controller->rotation_locked());
+}
+
+// Tests the behavior of the shortcut when the active window requests to lock
+// the rotation to a particular orientation.
+TEST_F(AcceleratorControllerTest, RotateScreenWithWindowLockingOrientation) {
+  display::test::DisplayManagerTestApi(display_manager())
+      .SetFirstDisplayAsInternalDisplay();
+  ash::ShellTestApi().SetTabletModeEnabledForTest(true);
+  auto* tablet_mode_controller = Shell::Get()->tablet_mode_controller();
+  auto* screen_orientation_controller =
+      Shell::Get()->screen_orientation_controller();
+  EXPECT_TRUE(tablet_mode_controller->is_in_tablet_physical_state());
+  EXPECT_FALSE(screen_orientation_controller->user_rotation_locked());
+  auto win0 = CreateAppWindow(gfx::Rect{100, 300});
+  auto win1 = CreateAppWindow(gfx::Rect{200, 200});
+  screen_orientation_controller->LockOrientationForWindow(
+      win0.get(), OrientationLockType::kPortraitPrimary);
+  screen_orientation_controller->LockOrientationForWindow(
+      win1.get(), OrientationLockType::kLandscape);
+
+  // `win0` requests to lock the orientation to only portrait-primary. The
+  // shortcut therefore won't be able to change the current rotation at all.
+  wm::ActivateWindow(win0.get());
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_FALSE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_EQ(OrientationLockType::kPortraitPrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  TriggerRotateScreenShortcut();
+  // Nothing happens; user rotation is still not locked, but the rotation is
+  // app-locked.
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_FALSE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_EQ(OrientationLockType::kPortraitPrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  // Activate `win1` which allows any landscape orientations (either primary or
+  // secondary). The shortcut will switch between the two allowed orientations
+  // only.
+  wm::ActivateWindow(win1.get());
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_FALSE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_EQ(OrientationLockType::kLandscapePrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  TriggerRotateScreenShortcut();
+  // User rotation will now be locked.
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_TRUE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_EQ(OrientationLockType::kLandscapeSecondary,
+            screen_orientation_controller->GetCurrentOrientation());
+  TriggerRotateScreenShortcut();
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_TRUE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_EQ(OrientationLockType::kLandscapePrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  // Hook a mouse device, exiting tablet mode to clamshell mode (but remaining
+  // in a tablet physical state). Expect that the shortcut changes the user
+  // rotation lock in all directions regardless of which window is active, even
+  // those that requested window rotation locks.
+  TabletModeControllerTestApi().AttachExternalMouse();
+  EXPECT_TRUE(tablet_mode_controller->is_in_tablet_physical_state());
+  EXPECT_FALSE(tablet_mode_controller->InTabletMode());
+
+  wm::ActivateWindow(win0.get());
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_TRUE(screen_orientation_controller->user_rotation_locked());
+  EXPECT_EQ(OrientationLockType::kLandscapePrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+  TriggerRotateScreenShortcut();
+  EXPECT_EQ(OrientationLockType::kPortraitSecondary,
+            screen_orientation_controller->GetCurrentOrientation());
+  TriggerRotateScreenShortcut();
+  EXPECT_EQ(OrientationLockType::kLandscapeSecondary,
+            screen_orientation_controller->GetCurrentOrientation());
+
+  wm::ActivateWindow(win1.get());
+  TriggerRotateScreenShortcut();
+  EXPECT_EQ(OrientationLockType::kPortraitPrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+  TriggerRotateScreenShortcut();
+  EXPECT_EQ(OrientationLockType::kLandscapePrimary,
+            screen_orientation_controller->GetCurrentOrientation());
+}
+
 TEST_F(AcceleratorControllerTest, AutoRepeat) {
   ui::Accelerator accelerator_a(ui::VKEY_A, ui::EF_CONTROL_DOWN);
   ui::TestAcceleratorTarget target_a;

ash/display/screen_orientation_controller.cc

@@ -296,6 +296,12 @@ bool ScreenOrientationController::IsUserLockedOrientationPortrait() {
   }
 }
 
+OrientationLockType
+ScreenOrientationController::GetCurrentAppRequestedOrientationLock() const {
+  return current_app_requested_orientation_lock_.value_or(
+      OrientationLockType::kAny);
+}
+
 void ScreenOrientationController::ToggleUserRotationLock() {
   if (!display::Display::HasInternalDisplay())
     return;
@@ -623,6 +629,22 @@ void ScreenOrientationController::LoadDisplayRotationProperties() {
 }
 
 void ScreenOrientationController::ApplyLockForActiveWindow() {
+  current_app_requested_orientation_lock_ = base::nullopt;
+  if (!display::Display::HasInternalDisplay())
+    return;
+
+  aura::Window* const internal_display_root =
+      Shell::GetRootWindowForDisplayId(display::Display::InternalDisplayId());
+  if (!internal_display_root) {
+    // We might have an internal display, but no root window for it, such as in
+    // the case of Unified Display. Also, some tests may not set an internal
+    // display.
+    // Since rotation lock is applied only on internal displays (see
+    // ScreenOrientationController::SetDisplayRotation()), there's no need to
+    // continue.
+    return;
+  }
+
   bool in_tablet_mode = Shell::Get()->tablet_mode_controller()->InTabletMode();
   if (!in_tablet_mode) {
     if (IsAutoRotationAllowed()) {
@@ -635,7 +657,7 @@ void ScreenOrientationController::ApplyLockForActiveWindow() {
     return;
   }
 
-  if (SplitViewController::Get(Shell::GetPrimaryRootWindow())
+  if (SplitViewController::Get(internal_display_root)
           ->InTabletSplitViewMode()) {
     // While split view is enabled, ignore rotation lock set by windows.
     LockRotationToOrientation(user_locked_orientation_);
@@ -647,6 +669,13 @@ void ScreenOrientationController::ApplyLockForActiveWindow() {
           kActiveDesk));
 
   for (auto* window : mru_windows) {
+    if (window->GetRootWindow() != internal_display_root) {
+      // TODO(afakhry): Window may move to an external display (e.g. via
+      // shortcut) and remain active. In this case, we need to undo any
+      // orientation lock it applied on the internal display.
+      continue;
+    }
+
     if (!window->TargetVisibility())
       continue;
 
@@ -679,6 +708,8 @@ bool ScreenOrientationController::ApplyLockForWindowIfPossible(
           lock_info.orientation_lock = orientation_lock;
         }
       }
+      current_app_requested_orientation_lock_ =
+          base::make_optional<OrientationLockType>(lock_info.orientation_lock);
       return true;
     }
   }

ash/display/screen_orientation_controller.h

@@ -17,6 +17,7 @@
 #include "ash/wm/splitview/split_view_observer.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "base/optional.h"
 #include "ui/aura/window_observer.h"
 #include "ui/display/display.h"
 #include "ui/wm/public/activation_change_observer.h"
@@ -107,6 +108,12 @@ class ASH_EXPORT ScreenOrientationController
   // orientation.
   bool IsUserLockedOrientationPortrait();
 
+  // Returns the OrientationLockType that is applied on based on whether a
+  // rotation lock was requested for an app window, and whether the current
+  // system state allows it to lock the rotation (e.g. being in tablet mode, on
+  // the internal display, and splitview is inactive).
+  OrientationLockType GetCurrentAppRequestedOrientationLock() const;
+
   bool ignore_display_configuration_updates() const {
     return ignore_display_configuration_updates_;
   }
@@ -248,6 +255,10 @@ class ASH_EXPORT ScreenOrientationController
   // The orientation of the device locked by the user.
   OrientationLockType user_locked_orientation_ = OrientationLockType::kAny;
 
+  // The currently applied orientation lock that was requested by an app if any.
+  base::Optional<OrientationLockType> current_app_requested_orientation_lock_ =
+      base::nullopt;
+
   // The current rotation set by ScreenOrientationController for the internal
   // display.
   display::Display::Rotation current_rotation_;

ash/display/screen_orientation_controller_unittest.cc

@@ -24,6 +24,7 @@
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
 #include "ash/wm/window_state.h"
+#include "ash/wm/window_util.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/numerics/math_constants.h"
@@ -37,6 +38,7 @@
 #include "ui/display/manager/managed_display_info.h"
 #include "ui/display/test/display_manager_test_api.h"
 #include "ui/message_center/message_center.h"
+#include "ui/wm/core/window_util.h"
 #include "ui/wm/public/activation_client.h"
 
 namespace ash {
@@ -850,4 +852,84 @@ TEST_F(ScreenOrientationControllerTest,
   EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
 }
 
+TEST_F(ScreenOrientationControllerTest, GetCurrentAppRequestedOrientationLock) {
+  UpdateDisplay("0+0-400x300,+400+0-500x400");
+  auto win0 = CreateAppWindow(gfx::Rect{100, 200});
+  auto win1 = CreateAppWindow(gfx::Rect{460, 10, 100, 200});
+  auto roots = Shell::GetAllRootWindows();
+  ASSERT_EQ(2u, roots.size());
+  EXPECT_EQ(win0->GetRootWindow(), roots[0]);
+  EXPECT_EQ(win1->GetRootWindow(), roots[1]);
+  EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
+  EXPECT_EQ(OrientationLockType::kAny, UserLockedOrientation());
+
+  auto* screen_orientation_controller =
+      Shell::Get()->screen_orientation_controller();
+  screen_orientation_controller->LockOrientationForWindow(
+      win0.get(), OrientationLockType::kPortraitPrimary);
+  screen_orientation_controller->LockOrientationForWindow(
+      win1.get(), OrientationLockType::kLandscape);
+  EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
+  EXPECT_EQ(OrientationLockType::kAny, UserLockedOrientation());
+
+  EXPECT_EQ(
+      OrientationLockType::kAny,
+      screen_orientation_controller->GetCurrentAppRequestedOrientationLock());
+
+  // Enter tablet mode and expect nothing will change until we activate win0.
+  TabletModeControllerTestApi().DetachAllMice();
+  EnableTabletMode(true);
+  // Run a loop for mirror mode to kick in which is triggered asynchronously.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(display_manager()->IsInSoftwareMirrorMode());
+  EXPECT_EQ(OrientationLockType::kLandscape,
+            screen_orientation_controller->natural_orientation());
+  EXPECT_EQ(win1.get(), window_util::GetActiveWindow());
+  EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
+  EXPECT_EQ(OrientationLockType::kAny, UserLockedOrientation());
+
+  wm::ActivateWindow(win0.get());
+  EXPECT_EQ(
+      OrientationLockType::kPortraitPrimary,
+      screen_orientation_controller->GetCurrentAppRequestedOrientationLock());
+  EXPECT_EQ(win0.get(), window_util::GetActiveWindow());
+  EXPECT_EQ(display::Display::ROTATE_270, GetCurrentInternalDisplayRotation());
+  EXPECT_EQ(OrientationLockType::kAny, UserLockedOrientation());
+
+  display_manager()->SetMirrorMode(display::MirrorMode::kOff, base::nullopt);
+  base::RunLoop().RunUntilIdle();
+
+  roots = Shell::GetAllRootWindows();
+  ASSERT_EQ(2u, roots.size());
+  EXPECT_EQ(win0->GetRootWindow(), roots[0]);
+  EXPECT_EQ(win1->GetRootWindow(), roots[1]);
+
+  // `win1` belongs to the external display, so it is not allowed to lock the
+  // rotation.
+  EXPECT_EQ(win0.get(), window_util::GetActiveWindow());
+  EXPECT_EQ(
+      OrientationLockType::kPortraitPrimary,
+      screen_orientation_controller->GetCurrentAppRequestedOrientationLock());
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_EQ(display::Display::ROTATE_270, GetCurrentInternalDisplayRotation());
+  EXPECT_EQ(OrientationLockType::kAny, UserLockedOrientation());
+
+  // Even if you activate `win1`, internal display is not affected and remain
+  // locked to the rotation requested by `win0`.
+  wm::ActivateWindow(win1.get());
+  EXPECT_EQ(
+      OrientationLockType::kPortraitPrimary,
+      screen_orientation_controller->GetCurrentAppRequestedOrientationLock());
+  EXPECT_TRUE(screen_orientation_controller->rotation_locked());
+  EXPECT_EQ(display::Display::ROTATE_270, GetCurrentInternalDisplayRotation());
+  EXPECT_EQ(OrientationLockType::kAny, UserLockedOrientation());
+
+  // Once `win0` is snapped in splitview, it can no longer lock the rotation.
+  SplitViewController::Get(win0->GetRootWindow())
+      ->SnapWindow(win0.get(), SplitViewController::RIGHT);
+  EXPECT_EQ(
+      OrientationLockType::kAny,
+      screen_orientation_controller->GetCurrentAppRequestedOrientationLock());
+}
+
 }  // namespace ash