diff options
author | bruthig <bruthig@chromium.org> | 2015-04-24 14:19:31 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-24 21:19:39 +0000 |
commit | f57a7bbada10a659c251400a0d400dc4efe79f31 (patch) | |
tree | 693f3279a91ecca3ae86fac828f8701a9b31e6ba | |
parent | 0a3351c2a7c81284f82e6531380a21d079f55056 (diff) | |
download | chromium_src-f57a7bbada10a659c251400a0d400dc4efe79f31.zip chromium_src-f57a7bbada10a659c251400a0d400dc4efe79f31.tar.gz chromium_src-f57a7bbada10a659c251400a0d400dc4efe79f31.tar.bz2 |
Fixed the ScreenOrientationController so that it doesn't crash by animating
screen rotations on inactive displays.
TEST=ScreenOrientationControllerTest, RotateInactiveDisplay
BUG=479503
Review URL: https://codereview.chromium.org/1108473002
Cr-Commit-Position: refs/heads/master@{#326877}
4 files changed, 81 insertions, 4 deletions
diff --git a/ash/content/display/screen_orientation_controller_chromeos.cc b/ash/content/display/screen_orientation_controller_chromeos.cc index bb4938c..eee0ae6 100644 --- a/ash/content/display/screen_orientation_controller_chromeos.cc +++ b/ash/content/display/screen_orientation_controller_chromeos.cc @@ -117,8 +117,17 @@ void ScreenOrientationController::SetDisplayRotation( current_rotation_ = rotation; base::AutoReset<bool> auto_ignore_display_configuration_updates( &ignore_display_configuration_updates_, true); - ash::ScreenRotationAnimator(gfx::Display::InternalDisplayId()) - .Rotate(rotation, source); + + ash::ScreenRotationAnimator screen_rotation_animator( + gfx::Display::InternalDisplayId()); + if (screen_rotation_animator.CanAnimate()) { + screen_rotation_animator.Rotate(rotation, source); + } else { + // TODO(bruthig): Fix the DisplayManager so that display rotations set on + // inactive displays are persisted. See www.crbug.com/480703. + Shell::GetInstance()->display_manager()->SetDisplayRotation( + gfx::Display::InternalDisplayId(), rotation, source); + } } void ScreenOrientationController::OnWindowActivated(aura::Window* gained_active, diff --git a/ash/content/display/screen_orientation_controller_chromeos_unittest.cc b/ash/content/display/screen_orientation_controller_chromeos_unittest.cc index 27ec3d6..487d44c 100644 --- a/ash/content/display/screen_orientation_controller_chromeos_unittest.cc +++ b/ash/content/display/screen_orientation_controller_chromeos_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 <vector> + #include "ash/ash_switches.h" #include "ash/content/display/screen_orientation_controller_chromeos.h" #include "ash/display/display_info.h" @@ -9,6 +11,7 @@ #include "ash/shell.h" #include "ash/test/ash_test_base.h" #include "ash/test/ash_test_helper.h" +#include "ash/test/display_manager_test_api.h" #include "ash/test/test_shell_delegate.h" #include "ash/test/test_system_tray_delegate.h" #include "ash/wm/maximize_mode/maximize_mode_controller.h" @@ -35,6 +38,12 @@ namespace { const float kDegreesToRadians = 3.1415926f / 180.0f; const float kMeanGravity = 9.8066f; +DisplayInfo CreateDisplayInfo(int64 id, const gfx::Rect& bounds) { + DisplayInfo info(id, "dummy", false); + info.SetBounds(bounds); + return info; +} + void EnableMaximizeMode(bool enable) { Shell::GetInstance() ->maximize_mode_controller() @@ -595,4 +604,48 @@ TEST_F(ScreenOrientationControllerTest, InternalDisplayNotAvailableAtStartup) { EXPECT_TRUE(RotationLocked()); } +// Verifies rotating an inactive Display is sucessful. +TEST_F(ScreenOrientationControllerTest, RotateInactiveDisplay) { + const int64 kInternalDisplayId = 9; + const int64 kExternalDisplayId = 10; + const gfx::Display::Rotation kNewRotation = gfx::Display::ROTATE_180; + + DisplayManager* display_manager = Shell::GetInstance()->display_manager(); + + const DisplayInfo internal_display_info = + CreateDisplayInfo(kInternalDisplayId, gfx::Rect(0, 0, 500, 500)); + const DisplayInfo external_display_info = + CreateDisplayInfo(kExternalDisplayId, gfx::Rect(1, 1, 500, 500)); + + std::vector<DisplayInfo> display_info_list_two_active; + display_info_list_two_active.push_back(internal_display_info); + display_info_list_two_active.push_back(external_display_info); + + std::vector<DisplayInfo> display_info_list_one_active; + display_info_list_one_active.push_back(external_display_info); + + // The DisplayInfo list with two active displays needs to be added first so + // that the DisplayManager can track the |internal_display_info| as inactive + // instead of non-existent. + ash::Shell::GetInstance()->display_manager()->UpdateDisplays( + display_info_list_two_active); + ash::Shell::GetInstance()->display_manager()->UpdateDisplays( + display_info_list_one_active); + + test::DisplayManagerTestApi(display_manager) + .SetInternalDisplayId(kInternalDisplayId); + + ASSERT_NE(kNewRotation, display_manager->GetDisplayInfo(kInternalDisplayId) + .GetActiveRotation()); + + delegate()->SetDisplayRotation(kNewRotation, + gfx::Display::ROTATION_SOURCE_ACTIVE); + + // TODO(bruthig): Uncomment when www.crbug.com/480703 is fixed. This test + // still adds value by ensuring a crash does not occur. See + // www.crbug.com/479503. + // ASSERT_EQ(kNewRotation, display_manager->GetDisplayInfo(kInternalDisplayId) + // .GetActiveRotation()); +} + } // namespace ash diff --git a/ash/rotator/screen_rotation_animator.cc b/ash/rotator/screen_rotation_animator.cc index 8517759..9b746b6 100644 --- a/ash/rotator/screen_rotation_animator.cc +++ b/ash/rotator/screen_rotation_animator.cc @@ -279,6 +279,13 @@ ScreenRotationAnimator::ScreenRotationAnimator(int64 display_id) ScreenRotationAnimator::~ScreenRotationAnimator() { } +bool ScreenRotationAnimator::CanAnimate() const { + return Shell::GetInstance() + ->display_manager() + ->GetDisplayForId(display_id_) + .is_valid(); +} + void ScreenRotationAnimator::Rotate(gfx::Display::Rotation new_rotation, gfx::Display::RotationSource source) { const gfx::Display::Rotation current_rotation = diff --git a/ash/rotator/screen_rotation_animator.h b/ash/rotator/screen_rotation_animator.h index 89c1784..6aff432 100644 --- a/ash/rotator/screen_rotation_animator.h +++ b/ash/rotator/screen_rotation_animator.h @@ -17,8 +17,16 @@ class ASH_EXPORT ScreenRotationAnimator { explicit ScreenRotationAnimator(int64 display_id); ~ScreenRotationAnimator(); - // Rotates |display_| to the |new_rotation| orientation, for the given - // |source|. The rotation will also become active. + // Returns true if the screen rotation animation can be completed + // successfully. For example an animation is not possible if |display_id_| + // specifies a gfx::Display that is not currently active. See + // www.crbug.com/479503. + bool CanAnimate() const; + + // Rotates the gfx::Display specified by |display_id_| to the |new_rotation| + // orientation, for the given |source|. The rotation will also become active. + // Clients should only call |Rotate(gfx::Display::Rotation)| if |CanAnimate()| + // returns true. void Rotate(gfx::Display::Rotation new_rotation, gfx::Display::RotationSource source); |