Commit b74f9beb authored by oshima's avatar oshima Committed by Commit Bot

Don't lock and save the orientation change made not through ScreenOrientationController

This can happen when rotation animator sets the orientation after
animation is completed. We shouldn't save this anyway, as we changed
the semantics as follows:
a) A user/API driven rotation change in tablet mode should be treated
  as user locked rotation.
b) A user will not be able to change the orientation in tablet mode
  from settings.

BUG=734107
TEST=Manual + unit test

Review-Url: https://codereview.chromium.org/2951773002
Cr-Commit-Position: refs/heads/master@{#481027}
parent 53fe8fd8
...@@ -510,16 +510,6 @@ TEST_F(ScreenOrientationControllerTest, ResetUserRotationUponExit) { ...@@ -510,16 +510,6 @@ TEST_F(ScreenOrientationControllerTest, ResetUserRotationUponExit) {
EXPECT_EQ(display::Display::ROTATE_90, GetCurrentInternalDisplayRotation()); EXPECT_EQ(display::Display::ROTATE_90, GetCurrentInternalDisplayRotation());
} }
// Tests that if a user sets a display rotation that accelerometer rotation
// becomes locked.
TEST_F(ScreenOrientationControllerTest,
NonAccelerometerRotationChangesLockRotation) {
EnableMaximizeMode(true);
ASSERT_FALSE(RotationLocked());
SetInternalDisplayRotation(display::Display::ROTATE_270);
EXPECT_TRUE(RotationLocked());
}
// Tests that if a user changes the display rotation, while rotation is locked, // Tests that if a user changes the display rotation, while rotation is locked,
// that the updates are recorded. Upon exiting maximize mode the latest user // that the updates are recorded. Upon exiting maximize mode the latest user
// rotation should be applied. // rotation should be applied.
...@@ -621,33 +611,6 @@ TEST_F(ScreenOrientationControllerTest, UserRotationLockDisallowsRotation) { ...@@ -621,33 +611,6 @@ TEST_F(ScreenOrientationControllerTest, UserRotationLockDisallowsRotation) {
EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation()); EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
} }
// Tests that when MaximizeMode is triggered before the internal display is
// ready, that ScreenOrientationController still begins listening to events,
// which require an internal display to be acted upon.
TEST_F(ScreenOrientationControllerTest, InternalDisplayNotAvailableAtStartup) {
display::test::DisplayManagerTestApi(display_manager())
.SetFirstDisplayAsInternalDisplay();
int64_t internal_display_id = display::Display::InternalDisplayId();
display::Display::SetInternalDisplayId(display::kInvalidDisplayId);
EnableMaximizeMode(true);
// Should not crash, even though there is no internal display.
SetDisplayRotationById(internal_display_id, display::Display::ROTATE_180);
EXPECT_FALSE(RotationLocked());
// Should not crash, even though the invalid display id is requested.
SetDisplayRotationById(display::kInvalidDisplayId,
display::Display::ROTATE_180);
EXPECT_FALSE(RotationLocked());
// With an internal display now available, functionality should resume.
display::Display::SetInternalDisplayId(internal_display_id);
SetInternalDisplayRotation(display::Display::ROTATE_90);
EXPECT_TRUE(RotationLocked());
}
// Verifies rotating an inactive Display is successful. // Verifies rotating an inactive Display is successful.
TEST_F(ScreenOrientationControllerTest, RotateInactiveDisplay) { TEST_F(ScreenOrientationControllerTest, RotateInactiveDisplay) {
const int64_t kInternalDisplayId = 9; const int64_t kInternalDisplayId = 9;
......
...@@ -3213,4 +3213,26 @@ TEST_F(DisplayManagerOrientationTest, LockToSpecificOrientation) { ...@@ -3213,4 +3213,26 @@ TEST_F(DisplayManagerOrientationTest, LockToSpecificOrientation) {
test_api.GetCurrentOrientation()); test_api.GetCurrentOrientation());
} }
// crbug.com/734107
TEST_F(DisplayManagerOrientationTest, DisplayChangeShouldNotSaveUserRotation) {
Shell* shell = Shell::Get();
display::DisplayManager* display_manager = shell->display_manager();
display::test::DisplayManagerTestApi test_api(display_manager);
test_api.SetFirstDisplayAsInternalDisplay();
display::Screen* screen = display::Screen::GetScreen();
Shell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
true);
// Emulate that Animator is calling this async when animation is completed.
display_manager->SetDisplayRotation(
screen->GetPrimaryDisplay().id(), display::Display::ROTATE_90,
display::Display::ROTATION_SOURCE_ACCELEROMETER);
EXPECT_EQ(display::Display::ROTATE_90,
screen->GetPrimaryDisplay().rotation());
Shell::Get()->maximize_mode_controller()->EnableMaximizeModeWindowManager(
false);
EXPECT_EQ(display::Display::ROTATE_0, screen->GetPrimaryDisplay().rotation());
}
} // namespace ash } // namespace ash
...@@ -288,20 +288,17 @@ void ScreenOrientationController::OnDisplayConfigurationChanged() { ...@@ -288,20 +288,17 @@ void ScreenOrientationController::OnDisplayConfigurationChanged() {
return; return;
if (!display::Display::HasInternalDisplay()) if (!display::Display::HasInternalDisplay())
return; return;
display::Display::Rotation user_rotation = if (!Shell::Get()->display_manager()->IsActiveDisplayId(
display::Display::InternalDisplayId())) {
return;
}
// TODO(oshima): We should disable the orientation change in settings
// because application may not work correctly.
current_rotation_ =
ShellPort::Get() ShellPort::Get()
->GetDisplayInfo(display::Display::InternalDisplayId()) ->GetDisplayInfo(display::Display::InternalDisplayId())
.GetActiveRotation(); .GetActiveRotation();
if (user_rotation != current_rotation_) {
// TODO(oshima): We should disable the orientation change in settings
// because application may not work correctly.
// A user may change other display configuration settings. When the user
// does change the rotation setting, then lock rotation to prevent the
// accelerometer from erasing their change.
SetRotationLockedInternal(true);
user_rotation_ = current_rotation_ = user_rotation;
}
} }
void ScreenOrientationController::OnMaximizeModeStarted() { void ScreenOrientationController::OnMaximizeModeStarted() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment