Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Merge Prevent DisplayPreferences from saving incorrect rotations.
Browse files Browse the repository at this point in the history
Refactor DisplayInfo::rotation_ to track different sources of rotations

Currently DisplayInfo only tracks the active rotation for the given display.
DisplayPreferences however saves a user rotation, as well as an accelerometer
rotation. DisplayController::Observer::OnDisplayConfigurationChanged triggers
the saving of display preferences.

This has been leading to active accelerometer rotations being saved as user
preferences, and being re-applied upon reboot.

This change refactors DisplayInfo to track one rotation per source of rotation
changes. DisplayPreferences has been updated to save based on these states.

TEST=DisplayPreferencesTest.DontSaveMaximizeModeControllerRotations, also ran
ash_unittests, and unit_tests
BUG=chrome-os-partner:37555, 469752, 466861
[email protected], [email protected], [email protected]
NOTRY=true
NOPRESUBMIT=true

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

Cr-Commit-Position: refs/heads/master@{#326614}
(cherry picked from commit d01de7f)

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

Cr-Commit-Position: refs/branch-heads/2357@{#248}
Cr-Branched-From: 59d4494-refs/heads/master@{#323860}
  • Loading branch information
jonross authored and Commit bot committed Apr 28, 2015
1 parent 58e5cab commit bb34612
Show file tree
Hide file tree
Showing 28 changed files with 284 additions and 213 deletions.
3 changes: 2 additions & 1 deletion ash/accelerators/accelerator_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,8 @@ void HandleRotateScreen() {
const DisplayInfo& display_info =
Shell::GetInstance()->display_manager()->GetDisplayInfo(display.id());
ash::ScreenRotationAnimator(display.id())
.Rotate(GetNextRotation(display_info.rotation()));
.Rotate(GetNextRotation(display_info.GetActiveRotation()),
gfx::Display::ROTATION_SOURCE_USER);
}

// Rotate the active window.
Expand Down
41 changes: 25 additions & 16 deletions ash/content/display/screen_orientation_controller_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ blink::WebScreenOrientationLockType GetDisplayNaturalOrientation() {
ash::DisplayInfo info =
display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId());
gfx::Size size = info.size_in_pixel();
switch (info.rotation()) {
switch (info.GetActiveRotation()) {
case gfx::Display::ROTATE_0:
case gfx::Display::ROTATE_180:
return size.height() >= size.width()
Expand Down Expand Up @@ -112,15 +112,16 @@ void ScreenOrientationController::SetRotationLocked(bool rotation_locked) {
}

void ScreenOrientationController::SetDisplayRotation(
gfx::Display::Rotation rotation) {
gfx::Display::Rotation rotation,
gfx::Display::RotationSource source) {
DisplayManager* display_manager = Shell::GetInstance()->display_manager();
if (!display_manager->HasInternalDisplay())
return;
current_rotation_ = rotation;
base::AutoReset<bool> auto_ignore_display_configuration_updates(
&ignore_display_configuration_updates_, true);
ash::ScreenRotationAnimator(gfx::Display::InternalDisplayId())
.Rotate(rotation);
.Rotate(rotation, source);
}

void ScreenOrientationController::OnWindowActivated(aura::Window* gained_active,
Expand Down Expand Up @@ -201,7 +202,7 @@ void ScreenOrientationController::OnDisplayConfigurationChanged() {
return;
gfx::Display::Rotation user_rotation =
display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId())
.rotation();
.GetActiveRotation();
if (user_rotation != current_rotation_) {
// A user may change other display configuration settings. When the user
// does change the rotation setting, then lock rotation to prevent the
Expand All @@ -219,7 +220,7 @@ void ScreenOrientationController::OnMaximizeModeStarted() {
if (display_manager->HasInternalDisplay()) {
current_rotation_ = user_rotation_ =
display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId())
.rotation();
.GetActiveRotation();
}
if (!rotation_locked_)
LoadDisplayRotationProperties();
Expand All @@ -231,13 +232,14 @@ void ScreenOrientationController::OnMaximizeModeEnded() {
chromeos::AccelerometerReader::GetInstance()->RemoveObserver(this);
Shell::GetInstance()->display_controller()->RemoveObserver(this);
if (current_rotation_ != user_rotation_)
SetDisplayRotation(user_rotation_);
SetDisplayRotation(user_rotation_, gfx::Display::ROTATION_SOURCE_USER);
}

void ScreenOrientationController::LockRotation(
gfx::Display::Rotation rotation) {
gfx::Display::Rotation rotation,
gfx::Display::RotationSource source) {
SetRotationLocked(true);
SetDisplayRotation(rotation);
SetDisplayRotation(rotation, source);
}

void ScreenOrientationController::LockRotationToOrientation(
Expand Down Expand Up @@ -270,7 +272,8 @@ void ScreenOrientationController::LockRotationToOrientation(
blink::WebScreenOrientationLockLandscape);
break;
case blink::WebScreenOrientationLockNatural:
LockRotation(gfx::Display::ROTATE_0);
LockRotation(gfx::Display::ROTATE_0,
gfx::Display::ROTATION_SOURCE_ACTIVE);
break;
default:
NOTREACHED();
Expand All @@ -282,14 +285,16 @@ void ScreenOrientationController::LockRotationToPrimaryOrientation(
blink::WebScreenOrientationLockType lock_orientation) {
LockRotation(natural_orientation_ == lock_orientation
? gfx::Display::ROTATE_0
: gfx::Display::ROTATE_90);
: gfx::Display::ROTATE_90,
gfx::Display::ROTATION_SOURCE_ACTIVE);
}

void ScreenOrientationController::LockRotationToSecondaryOrientation(
blink::WebScreenOrientationLockType lock_orientation) {
LockRotation(natural_orientation_ == lock_orientation
? gfx::Display::ROTATE_180
: gfx::Display::ROTATE_270);
: gfx::Display::ROTATE_270,
gfx::Display::ROTATION_SOURCE_ACTIVE);
}

void ScreenOrientationController::LockToRotationMatchingOrientation(
Expand All @@ -300,20 +305,22 @@ void ScreenOrientationController::LockToRotationMatchingOrientation(

gfx::Display::Rotation rotation =
display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId())
.rotation();
.GetActiveRotation();
if (natural_orientation_ == lock_orientation) {
if (rotation == gfx::Display::ROTATE_0 ||
rotation == gfx::Display::ROTATE_180) {
SetRotationLocked(true);
} else {
LockRotation(gfx::Display::ROTATE_0);
LockRotation(gfx::Display::ROTATE_0,
gfx::Display::ROTATION_SOURCE_ACTIVE);
}
} else {
if (rotation == gfx::Display::ROTATE_90 ||
rotation == gfx::Display::ROTATE_270) {
SetRotationLocked(true);
} else {
LockRotation(gfx::Display::ROTATE_90);
LockRotation(gfx::Display::ROTATE_90,
gfx::Display::ROTATION_SOURCE_ACTIVE);
}
}
}
Expand Down Expand Up @@ -365,14 +372,16 @@ void ScreenOrientationController::HandleScreenRotation(

if (new_rotation != current_rotation_ &&
IsRotationAllowedInLockedState(new_rotation))
SetDisplayRotation(new_rotation);
SetDisplayRotation(new_rotation,
gfx::Display::ROTATION_SOURCE_ACCELEROMETER);
}

void ScreenOrientationController::LoadDisplayRotationProperties() {
DisplayManager* display_manager = Shell::GetInstance()->display_manager();
if (!display_manager->registered_internal_display_rotation_lock())
return;
SetDisplayRotation(display_manager->registered_internal_display_rotation());
SetDisplayRotation(display_manager->registered_internal_display_rotation(),
gfx::Display::ROTATION_SOURCE_ACCELEROMETER);
SetRotationLocked(true);
}

Expand Down
10 changes: 7 additions & 3 deletions ash/content/display/screen_orientation_controller_chromeos.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,11 @@ class ASH_EXPORT ScreenOrientationController
// display rotation.
void SetRotationLocked(bool rotation_locked);

// Sets the display rotation and suppresses display notifications.
void SetDisplayRotation(gfx::Display::Rotation rotation);
// Sets the display rotation for the given |source|. The new |rotation| will
// also become active. Display changed notifications are surpressed for this
// change.
void SetDisplayRotation(gfx::Display::Rotation rotation,
gfx::Display::RotationSource source);

// aura::client::ActivationChangeObserver:
void OnWindowActivated(aura::Window* gained_active,
Expand Down Expand Up @@ -102,7 +105,8 @@ class ASH_EXPORT ScreenOrientationController
// Sets the display rotation to |rotation|. Future accelerometer updates
// should not be used to change the rotation. SetRotationLocked(false) removes
// the rotation lock.
void LockRotation(gfx::Display::Rotation rotation);
void LockRotation(gfx::Display::Rotation rotation,
gfx::Display::RotationSource source);

// Sets the display rotation based on |lock_orientation|. Future accelerometer
// updates should not be used to change the rotation. SetRotationLocked(false)
Expand Down
Loading

0 comments on commit bb34612

Please sign in to comment.