Remember the target rotation when entering the tablet mode

New animation first makes copies of screen, and chagnes
the rotation after these copies are made. If a device enters
tablet mode while coyping it may remember the wrong rotation.

BUG=742605
TEST=covered by unit tests. Also tested manually:
 1) Start portrait only ARC++ app
 2) Try to close and open the lid multiple times

Change-Id: I835b3409f34f101dac45784eda501635f7fe085e
Reviewed-on: https://chromium-review.googlesource.com/571258
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486687}

ash/display/display_configuration_controller.cc

@@ -112,10 +112,8 @@ void DisplayConfigurationController::SetDisplayRotation(
     display::Display::Rotation rotation,
     display::Display::RotationSource source) {
   if (display_manager_->IsDisplayIdValid(display_id)) {
-    if (display_manager_->GetDisplayInfo(display_id).GetActiveRotation() ==
-        rotation) {
+    if (GetTargetRotation(display_id) == rotation)
       return;
-    }
     ScreenRotationAnimator* screen_rotation_animator =
         GetScreenRotationAnimatorForDisplay(display_id);
     screen_rotation_animator->Rotate(rotation, source);
@@ -124,6 +122,19 @@ void DisplayConfigurationController::SetDisplayRotation(
   }
 }
 
+display::Display::Rotation DisplayConfigurationController::GetTargetRotation(
+    int64_t display_id) {
+  if (!display_manager_->IsDisplayIdValid(display_id))
+    return display::Display::ROTATE_0;
+
+  ScreenRotationAnimator* animator =
+      GetScreenRotationAnimatorForDisplay(display_id);
+  if (animator->IsRotating())
+    return animator->GetTargetRotation();
+
+  return display_manager_->GetDisplayInfo(display_id).GetActiveRotation();
+}
+
 void DisplayConfigurationController::SetPrimaryDisplayId(int64_t display_id) {
   if (display_manager_->GetNumDisplays() <= 1 || IsLimited())
     return;

ash/display/display_configuration_controller.h

@@ -54,6 +54,10 @@ class ASH_EXPORT DisplayConfigurationController
                           display::Display::Rotation rotation,
                           display::Display::RotationSource source);
 
+  // Returns the rotation of the display given by |display_id|. This returns
+  // the target rotation when the display is being rotated.
+  display::Display::Rotation GetTargetRotation(int64_t display_id);
+
   // Sets the primary display id.
   void SetPrimaryDisplayId(int64_t display_id);
 

ash/display/display_configuration_controller_unittest.cc

@@ -2,15 +2,45 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ash/display/display_configuration_controller.h"
+
+#include "ash/ash_switches.h"
 #include "ash/public/cpp/config.h"
 #include "ash/rotator/screen_rotation_animator.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/display_configuration_controller_test_api.h"
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "ui/display/manager/display_manager.h"
 
 namespace ash {
+namespace {
+
+display::Display::Rotation GetDisplayRotation(int64_t display_id) {
+  return Shell::Get()
+      ->display_manager()
+      ->GetDisplayInfo(display_id)
+      .GetActiveRotation();
+}
+
+class DisplayConfigurationControllerSmoothRotationTest
+    : public test::AshTestBase {
+ public:
+  DisplayConfigurationControllerSmoothRotationTest() {}
+  ~DisplayConfigurationControllerSmoothRotationTest() override {}
+
+  void SetUp() override {
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kAshDisableSmoothScreenRotation, "false");
+    test::AshTestBase::SetUp();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DisplayConfigurationControllerSmoothRotationTest);
+};
+
+}  // namespace
 
 using DisplayConfigurationControllerTest = test::AshTestBase;
 
@@ -41,4 +71,18 @@ TEST_F(DisplayConfigurationControllerTest, OnlyHasOneAnimator) {
   EXPECT_EQ(old_screen_rotation_animator, new_screen_rotation_animator);
 }
 
+TEST_F(DisplayConfigurationControllerSmoothRotationTest,
+       GetTargetRotationWithAnimation) {
+  display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  DisplayConfigurationController* controller =
+      Shell::Get()->display_configuration_controller();
+  test::DisplayConfigurationControllerTestApi testapi(controller);
+  controller->SetDisplayRotation(
+      display.id(), display::Display::ROTATE_180,
+      display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_EQ(display::Display::ROTATE_180,
+            controller->GetTargetRotation(display.id()));
+  EXPECT_EQ(display::Display::ROTATE_0, GetDisplayRotation(display.id()));
+}
+
 }  // namespace ash

ash/display/screen_orientation_controller_chromeos.cc

@@ -315,20 +315,19 @@ void ScreenOrientationController::OnDisplayConfigurationChanged() {
 }
 
 void ScreenOrientationController::OnMaximizeModeStarted() {
+  Shell* shell = Shell::Get();
   // Do not exit early, as the internal display can be determined after Maximize
   // Mode has started. (chrome-os-partner:38796)
   // Always start observing.
   if (display::Display::HasInternalDisplay()) {
     current_rotation_ = user_rotation_ =
-        Shell::Get()
-            ->display_manager()
-            ->GetDisplayInfo(display::Display::InternalDisplayId())
-            .GetActiveRotation();
+        shell->display_configuration_controller()->GetTargetRotation(
+            display::Display::InternalDisplayId());
   }
   if (!rotation_locked_)
     LoadDisplayRotationProperties();
   chromeos::AccelerometerReader::GetInstance()->AddObserver(this);
-  Shell::Get()->window_tree_host_manager()->AddObserver(this);
+  shell->window_tree_host_manager()->AddObserver(this);
 
   if (!display::Display::HasInternalDisplay())
     return;

ash/rotator/screen_rotation_animator.cc

@@ -165,7 +165,9 @@ ScreenRotationAnimator::ScreenRotationAnimator(aura::Window* root_window)
       disable_animation_timers_for_test_(false),
       has_switch_ash_disable_smooth_screen_rotation_(
           base::CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kAshDisableSmoothScreenRotation)),
+              switches::kAshDisableSmoothScreenRotation) &&
+          base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+              switches::kAshDisableSmoothScreenRotation) != "false"),
       weak_factory_(this) {}
 
 ScreenRotationAnimator::~ScreenRotationAnimator() {
@@ -444,6 +446,7 @@ void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
   std::unique_ptr<ScreenRotationRequest> rotation_request =
       base::MakeUnique<ScreenRotationRequest>(rotation_request_id_, display_id,
                                               new_rotation, source);
+  target_rotation_ = new_rotation;
   switch (screen_rotation_state_) {
     case IDLE:
     case COPY_REQUESTED:
@@ -485,6 +488,14 @@ void ScreenRotationAnimator::ProcessAnimationQueue() {
     observer.OnScreenRotationAnimationFinished(this);
 }
 
+bool ScreenRotationAnimator::IsRotating() const {
+  return screen_rotation_state_ != IDLE;
+}
+
+display::Display::Rotation ScreenRotationAnimator::GetTargetRotation() const {
+  return target_rotation_;
+}
+
 void ScreenRotationAnimator::StopAnimating() {
   // |old_layer_tree_owner_| new_layer_tree_owner_| could be nullptr if another
   // the rotation request comes before the copy request finished.

ash/rotator/screen_rotation_animator.h

@@ -61,6 +61,13 @@ class ASH_EXPORT ScreenRotationAnimator {
   // notifies |screen_rotation_animator_observer_|.
   void ProcessAnimationQueue();
 
+  // True if the screen is in rotating state (not IDLE).
+  bool IsRotating() const;
+
+  // Returns the target (new) rotation. This will return the last requested
+  // orientation if |IsRotating()| is false.
+  display::Display::Rotation GetTargetRotation() const;
+
  protected:
   using CopyCallback =
       base::Callback<void(std::unique_ptr<cc::CopyOutputResult> result)>;
@@ -177,6 +184,7 @@ class ASH_EXPORT ScreenRotationAnimator {
   std::unique_ptr<ui::LayerTreeOwner> mask_layer_tree_owner_;
   std::unique_ptr<ScreenRotationRequest> last_pending_request_;
   bool has_switch_ash_disable_smooth_screen_rotation_;
+  display::Display::Rotation target_rotation_ = display::Display::ROTATE_0;
   base::WeakPtrFactory<ScreenRotationAnimator> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimator);

ash/rotator/screen_rotation_animator_unittest.cc

@@ -319,12 +319,19 @@ TEST_F(ScreenRotationAnimatorSlowAnimationTest, RotatesDuringRotation) {
   SetDisplayRotation(display_id(), display::Display::ROTATE_0);
   animator()->Rotate(display::Display::ROTATE_90,
                      display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_TRUE(animator()->IsRotating());
+  EXPECT_EQ(display::Display::ROTATE_90, animator()->GetTargetRotation());
+
   animator()->Rotate(display::Display::ROTATE_180,
                      display::Display::RotationSource::ROTATION_SOURCE_USER);
   EXPECT_TRUE(test_api()->HasActiveAnimations());
+  EXPECT_TRUE(animator()->IsRotating());
+  EXPECT_EQ(display::Display::ROTATE_180, animator()->GetTargetRotation());
 
   test_api()->CompleteAnimations();
   EXPECT_FALSE(test_api()->HasActiveAnimations());
+  EXPECT_FALSE(animator()->IsRotating());
+
   EXPECT_EQ(display::Display::ROTATE_180, GetDisplayRotation(display_id()));
 }
 
@@ -390,8 +397,17 @@ TEST_F(ScreenRotationAnimatorSmoothAnimationTest,
   SetDisplayRotation(display_id, display::Display::ROTATE_0);
   animator()->Rotate(display::Display::ROTATE_90,
                      display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_TRUE(animator()->IsRotating());
+
+  EXPECT_EQ(display::Display::ROTATE_90, animator()->GetTargetRotation());
+  EXPECT_NE(display::Display::ROTATE_90, GetDisplayRotation(display_id));
+
   WaitForCopyCallback();
   EXPECT_TRUE(test_api()->HasActiveAnimations());
+  EXPECT_EQ(display::Display::ROTATE_90, animator()->GetTargetRotation());
+  // Once copy is made, the rotation is set to the target, with the
+  // image that was rotated to the original orientation.
+  EXPECT_EQ(display::Display::ROTATE_90, GetDisplayRotation(display_id));
 
   test_api()->CompleteAnimations();
   EXPECT_FALSE(test_api()->HasActiveAnimations());