Handles users rotating screen too early

ash/rotator/screen_rotation_animator_unittest.cc

Index: ash/rotator/screen_rotation_animator_unittest.cc
diff --git a/ash/rotator/screen_rotation_animator_unittest.cc b/ash/rotator/screen_rotation_animator_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..22ba0826dcd278009e388f8b16b9f1709ea83e20
--- /dev/null
+++ b/ash/rotator/screen_rotation_animator_unittest.cc
@@ -0,0 +1,112 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/rotator/screen_rotation_animator.h"
+
+#include "ash/common/wm_shell.h"
+#include "ash/test/ash_test_base.h"
+#include "base/memory/ptr_util.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
+
+namespace ash {
+namespace test {
+
+class ScreenRotationAnimatorTest : public AshTestBase {
+ public:
+  ScreenRotationAnimatorTest() {}
+  ~ScreenRotationAnimatorTest() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimatorTest);
+};
+
+class AnimationObserver
+    : public ScreenRotationAnimator::ScreenRotationAnimatorObserver {
+ public:
+  std::unique_ptr<ScreenRotationAnimator> animator_;
+
+  void OnEndedOrAbortedAnimation(
+      base::WeakPtr<ScreenRotationAnimator> animator) override;
+};
+
+void AnimationObserver::OnEndedOrAbortedAnimation(
+    base::WeakPtr<ScreenRotationAnimator> animator) {
+  animator_.reset();
+}
+
+TEST_F(ScreenRotationAnimatorTest, DeletesAnimator) {
+  // TODO(wutao): needs displayManager::GetDisplayInfo http://crbug.com/622480.
+  if (WmShell::Get()->IsRunningInMash())
+    return;
+
+  display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  std::unique_ptr<AnimationObserver> observer =
+      base::MakeUnique<AnimationObserver>();
+  observer->animator_.reset(new ScreenRotationAnimator(display.id()));
+  observer->animator_->SetObserver(observer.get());
+
+  display.set_rotation(display::Display::ROTATE_0);
+  observer->animator_->Rotate(
+      display::Display::ROTATE_90,
+      display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_FALSE(observer->animator_);
+}
+
+TEST_F(ScreenRotationAnimatorTest, RotatesToDifferentRotation) {
+  // TODO(wutao): needs displayManager::GetDisplayInfo http://crbug.com/622480.
+  if (WmShell::Get()->IsRunningInMash())
+    return;
+
+  display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  std::unique_ptr<ScreenRotationAnimator> animator =
+      base::MakeUnique<ScreenRotationAnimator>(display.id());
+
+  display.set_rotation(display::Display::ROTATE_0);
+  animator->Rotate(display::Display::ROTATE_90,
+                   display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_TRUE(animator->is_animating());
+}
+
+TEST_F(ScreenRotationAnimatorTest, ShouldNotRotateTheSameRotation) {
+  // TODO(wutao): needs displayManager::GetDisplayInfo http://crbug.com/622480.
+  if (WmShell::Get()->IsRunningInMash())
+    return;
+
+  display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  std::unique_ptr<ScreenRotationAnimator> animator =
+      base::MakeUnique<ScreenRotationAnimator>(display.id());
+
+  display.set_rotation(display::Display::ROTATE_0);
+  animator->Rotate(display::Display::ROTATE_0,
+                   display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_FALSE(animator->is_animating());
+}
+
+// Simulates the situation that if there is a new rotation request during
+// animation, it should stop the animation immediately and add the new rotation
+// request to the |last_pending_request_|.
+TEST_F(ScreenRotationAnimatorTest, AddsToPendingRequest) {
+  // TODO(wutao): needs displayManager::GetDisplayInfo http://crbug.com/622480.
+  if (WmShell::Get()->IsRunningInMash())
+    return;
+
+  display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  std::unique_ptr<ScreenRotationAnimator> animator =
+      base::MakeUnique<ScreenRotationAnimator>(display.id());
+
+  EXPECT_FALSE(animator->last_pending_request());
+  display.set_rotation(display::Display::ROTATE_0);
+  animator->Rotate(display::Display::ROTATE_90,

[bruthig, 2017/03/02 18:25:45]

FYI AshTestBase via AshTestHelper::SetUp() uses the ZERO_DURATION
ui::ScopedAnimationDurationScaleMode to control animations.  I'm pretty sure it
is in effect here which means that the animation triggered on line 101 will
execute immediately and the second animation might not actually be getting
queued up like you expect.

I think you should be able to create a local ScopedAnimationDurationScaleMode
instance with a SLOW_DURATION scale mode to have a higher chance of the second
animation actually being queued up.

[wutao, 2017/03/03 02:45:52]

Done.
You are right. The trick here is that in the layer clean up observer, I do not
reset the is_animating state so that the test will pass. I will modify the test
to reset the state on-animation-ended or aborted and add the SLOW_DURATION.

+                   display::Display::RotationSource::ROTATION_SOURCE_USER);
+  animator->Rotate(display::Display::ROTATE_180,
+                   display::Display::RotationSource::ROTATION_SOURCE_USER);
+
+  EXPECT_TRUE(animator->last_pending_request());
+  EXPECT_EQ(display::Display::ROTATE_180,
+            animator->last_pending_request()->new_rotation);
+}
+
+}  // namespace test
+}  // namespace ash