Fixed the ScreenOrientationController so that it doesn't crash by ani…

…mating

screen rotations on inactive displays.

TEST=ScreenOrientationControllerTest, RotateInactiveDisplay

BUG=479503

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

Cr-Commit-Position: refs/heads/master@{#326877}

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,

ash/content/display/screen_orientation_controller_chromeos_unittest.cc

@@ -2,13 +2,16 @@
 // 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"
 #include "ash/display/display_manager.h"
 #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

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 =

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);