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..8acde24394bce22c572a4907279f2709df58e636
--- /dev/null
+++ b/ash/rotator/screen_rotation_animator_unittest.cc
@@ -0,0 +1,140 @@
+// 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/shell.h"
+#include "ash/test/ash_test_base.h"
+#include "base/memory/ptr_util.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
+#include "ui/display/display.h"
+#include "ui/display/manager/display_manager.h"
+#include "ui/display/screen.h"
+
+namespace ash {
+
+namespace {
+
+display::Display::Rotation GetDisplayRotation(int64_t display_id) {
+  return Shell::GetInstance()
+      ->display_manager()
+      ->GetDisplayInfo(display_id)
+      .GetActiveRotation();
+}
+
+void SetDisplayRotation(int64_t display_id,
+                        display::Display::Rotation rotation) {
+  Shell::GetInstance()->display_manager()->SetDisplayRotation(
+      display_id, rotation,
+      display::Display::RotationSource::ROTATION_SOURCE_USER);
+}
+
+}  // namespace
+
+namespace test {
+
+class AnimationObserver
+    : public ScreenRotationAnimator::ScreenRotationAnimatorObserver {
+ public:
+  std::unique_ptr<ScreenRotationAnimator> animator_;
+
+  void OnEndedOrAbortedScreenRotationAnimation(
+      ScreenRotationAnimator* animator) override;
+};
+
+void AnimationObserver::OnEndedOrAbortedScreenRotationAnimation(
+    ScreenRotationAnimator* animator) {
+  animator_.reset();
+}
+
+class ScreenRotationAnimatorTest : public AshTestBase {

[bruthig, 2017/03/10 22:34:08]

Thanks again for adding this missing test fixture!!

[wutao, 2017/03/14 16:46:38]

Acknowledged.

+ public:
+  ScreenRotationAnimatorTest() {}
+  ~ScreenRotationAnimatorTest() override {}
+
+  // AshTestBase:
+  void SetUp() override;
+
+  int64_t display_id();
+
+ protected:
+  std::unique_ptr<ui::ScopedAnimationDurationScaleMode> non_zero_duration_mode_;
+
+ private:
+  display::Display display_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimatorTest);
+};
+
+void ScreenRotationAnimatorTest::SetUp() {
+  AshTestBase::SetUp();
+  non_zero_duration_mode_.reset(new ui::ScopedAnimationDurationScaleMode(
+      ui::ScopedAnimationDurationScaleMode::SLOW_DURATION));
+
+  display_ = display::Screen::GetScreen()->GetPrimaryDisplay();
+  SetDisplayRotation(display_.id(), display::Display::ROTATE_0);
+}
+
+int64_t ScreenRotationAnimatorTest::display_id() {
+  return display_.id();
+}
+
+TEST_F(ScreenRotationAnimatorTest, DeletesAnimator) {

[bruthig, 2017/03/10 22:34:08]

Isn't this more verifying that the observer is called.  Verifying that the
animator_ is actually deleted is mixing in the implementation details of the
DisplayConfigurationController?

[wutao, 2017/03/14 16:46:38]

You are right. Deleting animator is tested in the
DisplayConfigurationCOntroller. I will modify the test.

+  // TODO(wutao): needs displayManager::GetDisplayInfo http://crbug.com/622480.
+  if (WmShell::Get()->IsRunningInMash())
+    return;
+
+  non_zero_duration_mode_.reset(new ui::ScopedAnimationDurationScaleMode(
+      ui::ScopedAnimationDurationScaleMode::ZERO_DURATION));
+
+  AnimationObserver observer;
+  observer.animator_.reset(new ScreenRotationAnimator(display_id()));
+  observer.animator_->SetScreenRotationAnimatorObserver(&observer);
+  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;
+
+  ScreenRotationAnimator animator(display_id());
+  animator.Rotate(display::Display::ROTATE_90,
+                  display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_TRUE(animator.is_rotating_for_test());
+}
+
+TEST_F(ScreenRotationAnimatorTest, ShouldNotRotateTheSameRotation) {
+  // TODO(wutao): needs displayManager::GetDisplayInfo http://crbug.com/622480.
+  if (WmShell::Get()->IsRunningInMash())
+    return;
+
+  ScreenRotationAnimator animator(display_id());
+  animator.Rotate(display::Display::ROTATE_0,
+                  display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_FALSE(animator.is_rotating_for_test());
+}
+
+// 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;
+
+  ScreenRotationAnimator animator(display_id());
+  animator.Rotate(display::Display::ROTATE_90,
+                  display::Display::RotationSource::ROTATION_SOURCE_USER);
+  animator.Rotate(display::Display::ROTATE_180,
+                  display::Display::RotationSource::ROTATION_SOURCE_USER);
+  EXPECT_EQ(display::Display::ROTATE_180, GetDisplayRotation(display_id()));
+}
+

[bruthig, 2017/03/10 22:34:08]

I think it would be VERY valuable to have a test that queues up a second
animation while one is in progress and ensures the animations don't continue
forever.  The processing of the |ScreenRotationAnimator::last_pending_request_|
is not easy to follow.

Check out the MultiLayerAnimatorTestController and
MultiLayerAnimatorTestControllerDelegate classes on how you can control the
rotation animations from tests and not have to use insufficient
DurationScaleModes.

[wutao, 2017/03/14 16:46:38]

I will add a new test with a ScreenRotationAnimatorTestApi and
MultiLayerAnimatorTestController etc.

+}  // namespace test
+}  // namespace ash