Handles users rotating screen too early

ash/rotator/screen_rotation_animator.cc

Index: ash/rotator/screen_rotation_animator.cc
diff --git a/ash/rotator/screen_rotation_animator.cc b/ash/rotator/screen_rotation_animator.cc
index 5bf704e705dc7712b0bc07b3b1c096b2465d94c3..22ca1b2eba78716e4e8e2334eec5330281989d90 100644
--- a/ash/rotator/screen_rotation_animator.cc
+++ b/ash/rotator/screen_rotation_animator.cc
@@ -10,6 +10,7 @@
 
 #include "ash/display/window_tree_host_manager.h"
 #include "ash/rotator/screen_rotation_animation.h"
+#include "ash/rotator/screen_rotation_animator_observer.h"
 #include "ash/shell.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
@@ -42,15 +43,37 @@ const int kRotationDegrees = 20;
 // The time it takes for the rotation animations to run.
 const int kRotationDurationInMs = 250;
 
-// Gets the current display rotation for the display with the specified
-// |display_id|.
-display::Display::Rotation GetCurrentRotation(int64_t display_id) {
+// The rotation directions
+const int kCounterClockWiseRotation = 1;
+const int kClockWiseRotation = -1;
+
+// Aborts the active animations of the layer, and recurses upon its child
+// layers.
+void AbortAnimations(ui::Layer* layer) {
+  for (ui::Layer* child_layer : layer->children())
+    AbortAnimations(child_layer);
+  layer->GetAnimator()->AbortAllAnimations();
+}
+
+display::Display::Rotation GetCurrentScreenRotation(int64_t display_id) {
   return Shell::GetInstance()
       ->display_manager()
       ->GetDisplayInfo(display_id)
       .GetActiveRotation();
 }
 
+int GetRotationFactor(display::Display::Rotation initial_rotation,
+                      display::Display::Rotation new_rotation) {
+  return (initial_rotation + 3) % 4 == new_rotation ? kCounterClockWiseRotation
+                                                    : kClockWiseRotation;
+}
+
+aura::Window* GetRootWindow(int64_t display_id) {
+  return Shell::GetInstance()
+      ->window_tree_host_manager()
+      ->GetRootWindowForDisplayId(display_id);
+}
+
 // Returns true if the rotation between |initial_rotation| and |new_rotation| is
 // 180 degrees.
 bool Is180DegreeFlip(display::Display::Rotation initial_rotation,
@@ -58,17 +81,19 @@ bool Is180DegreeFlip(display::Display::Rotation initial_rotation,
   return (initial_rotation + 2) % 4 == new_rotation;
 }
 
-// A LayerAnimationObserver that will destroy the contained LayerTreeOwner when
-// notified that a layer animation has ended or was aborted.
+// Returns the initial degrees the old layer animation to begin with.
+int GetInitialDegrees(display::Display::Rotation initial_rotation,
+                      display::Display::Rotation new_rotation) {
+  return (Is180DegreeFlip(initial_rotation, new_rotation) ? 180 : 90);
+}
+
+// A LayerAnimationObserver that will destroy the contained LayerTreeOwner
+// when notified that a layer animation has ended or was aborted.
 class LayerCleanupObserver : public ui::LayerAnimationObserver {
  public:
-  explicit LayerCleanupObserver(
-      std::unique_ptr<ui::LayerTreeOwner> layer_tree_owner);
+  explicit LayerCleanupObserver(base::WeakPtr<ScreenRotationAnimator> animator);

[bruthig, 2017/03/15 23:12:37]

Docs for accepts an |animator| and why it is a WeakPtr.

[wutao, 2017/03/16 07:37:59]

Done.

   ~LayerCleanupObserver() override;
 
-  // Get the root layer of the owned layer tree.
-  ui::Layer* GetRootLayer();
-
   // ui::LayerAnimationObserver:
   void OnLayerAnimationEnded(ui::LayerAnimationSequence* sequence) override;
   void OnLayerAnimationAborted(ui::LayerAnimationSequence* sequence) override;
@@ -84,12 +109,7 @@ class LayerCleanupObserver : public ui::LayerAnimationObserver {
   void OnDetachedFromSequence(ui::LayerAnimationSequence* sequence) override;
 
  private:
-  // Aborts the active animations of the layer, and recurses upon its child
-  // layers.
-  void AbortAnimations(ui::Layer* layer);
-
-  // The owned layer tree.
-  std::unique_ptr<ui::LayerTreeOwner> layer_tree_owner_;
+  base::WeakPtr<ScreenRotationAnimator> animator_;
 
   // The LayerAnimationSequence that |this| has been attached to. Defaults to
   // nullptr.
@@ -99,28 +119,29 @@ class LayerCleanupObserver : public ui::LayerAnimationObserver {
 };
 
 LayerCleanupObserver::LayerCleanupObserver(
-    std::unique_ptr<ui::LayerTreeOwner> layer_tree_owner)
-    : layer_tree_owner_(std::move(layer_tree_owner)), sequence_(nullptr) {}
+    base::WeakPtr<ScreenRotationAnimator> animator)
+    : animator_(animator), sequence_(nullptr) {}
 
 LayerCleanupObserver::~LayerCleanupObserver() {
   // We must eplicitly detach from |sequence_| because we return true from
   // RequiresNotificationWhenAnimatorDestroyed.
   if (sequence_)
     sequence_->RemoveObserver(this);
-  AbortAnimations(layer_tree_owner_->root());
-}
-
-ui::Layer* LayerCleanupObserver::GetRootLayer() {
-  return layer_tree_owner_->root();
 }
 
 void LayerCleanupObserver::OnLayerAnimationEnded(
     ui::LayerAnimationSequence* sequence) {
+  if (animator_)
+    animator_->OnLayerAnimationEnded();
+
   delete this;
 }
 
 void LayerCleanupObserver::OnLayerAnimationAborted(
     ui::LayerAnimationSequence* sequence) {
+  if (animator_)
+    animator_->OnLayerAnimationAborted();
+
   delete this;
 }
 
@@ -135,34 +156,38 @@ void LayerCleanupObserver::OnDetachedFromSequence(
   sequence_ = nullptr;
 }
 
-void LayerCleanupObserver::AbortAnimations(ui::Layer* layer) {
-  for (ui::Layer* child_layer : layer->children())
-    AbortAnimations(child_layer);
-  layer->GetAnimator()->AbortAllAnimations();
-}
+}  // namespace
+
+struct ScreenRotationAnimator::ScreenRotationRequest {
+  display::Display::Rotation new_rotation;
+  display::Display::RotationSource source;
+};
+
+ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
+    : display_id_(display_id),
+      is_rotating_(false),
+      disable_animation_timers_for_test_(false),
+      screen_rotation_animator_observer_(nullptr),
+      weak_factory_(this) {}
+
+ScreenRotationAnimator::~ScreenRotationAnimator() {}
 
-// Set the screen orientation for the given |display_id| to |new_rotation| and
-// animate the change. The animation will rotate the initial orientation's
-// layer towards the new orientation through |rotation_degrees| while fading
-// out, and the new orientation's layer will be rotated in to the
-// |new_orientation| through |rotation_degrees| arc.
-void RotateScreen(int64_t display_id,
-                  display::Display::Rotation new_rotation,
-                  display::Display::RotationSource source) {
-  aura::Window* root_window = Shell::GetInstance()
-                                  ->window_tree_host_manager()
-                                  ->GetRootWindowForDisplayId(display_id);
-
-  const display::Display::Rotation initial_orientation =
-      GetCurrentRotation(display_id);
+// Set the screen orientation to |new_rotation| and animate the change. The

[bruthig, 2017/03/15 23:12:37]

Should these docs be moved to the .h file?

[wutao, 2017/03/16 07:37:59]

Done.

+// animation will rotate the initial orientation's layer towards the new
+// orientation through |rotation_degrees| while fading out, and the new
+// orientation's layer will be rotated in to the |new_orientation| through
+// |rotation_degrees| arc.
+void ScreenRotationAnimator::AnimateRotation(
+    const ScreenRotationRequest& rotation_request) {
+  aura::Window* root_window = GetRootWindow(display_id_);
 
   const gfx::Rect original_screen_bounds = root_window->GetTargetBounds();
   // 180 degree rotations should animate clock-wise.
-  const int rotation_factor =
-      (initial_orientation + 3) % 4 == new_rotation ? 1 : -1;
+  const int rotation_factor = GetRotationFactor(
+      GetCurrentScreenRotation(display_id_), rotation_request.new_rotation);
 
-  const int old_layer_initial_rotation_degrees =
-      (Is180DegreeFlip(initial_orientation, new_rotation) ? 180 : 90);
+  const int old_layer_initial_rotation_degrees = GetInitialDegrees(
+      GetCurrentScreenRotation(display_id_), rotation_request.new_rotation);
 
   const base::TimeDelta duration =
       base::TimeDelta::FromMilliseconds(kRotationDurationInMs);
@@ -171,16 +196,18 @@ void RotateScreen(int64_t display_id,
 
   std::unique_ptr<ui::LayerTreeOwner> old_layer_tree =
       ::wm::RecreateLayers(root_window);
+  old_layer_tree->root()->set_name("ScreenRotationAnimator:old_layer_tree");
 
   // Add the cloned layer tree in to the root, so it will be rendered.
   root_window->layer()->Add(old_layer_tree->root());
   root_window->layer()->StackAtTop(old_layer_tree->root());
 
-  std::unique_ptr<LayerCleanupObserver> layer_cleanup_observer(
-      new LayerCleanupObserver(std::move(old_layer_tree)));
+  old_layer_tree_owner_.reset(old_layer_tree.release());
+  std::unique_ptr<LayerCleanupObserver> old_layer_cleanup_observer(
+      new LayerCleanupObserver(WeakPtr()));
 
   Shell::GetInstance()->display_manager()->SetDisplayRotation(
-      display_id, new_rotation, source);
+      display_id_, rotation_request.new_rotation, rotation_request.source);
 
   const gfx::Rect rotated_screen_bounds = root_window->GetTargetBounds();
   const gfx::Point pivot = gfx::Point(rotated_screen_bounds.width() / 2,
@@ -194,7 +221,7 @@ void RotateScreen(int64_t display_id,
   // LayerAnimationSequences. One for the new layers and one for the old layer.
   for (ui::Layer* child_layer : root_window->layer()->children()) {
     // Skip the cloned layer because it has a different animation.
-    if (child_layer == layer_cleanup_observer->GetRootLayer())
+    if (child_layer == GetOldLayerTreeRootLayer())
       continue;
 
     std::unique_ptr<ScreenRotationAnimation> screen_rotation =
@@ -220,52 +247,102 @@ void RotateScreen(int64_t display_id,
   translate_transform.Translate(
       (rotated_screen_bounds.width() - original_screen_bounds.width()) / 2,
       (rotated_screen_bounds.height() - original_screen_bounds.height()) / 2);
-  layer_cleanup_observer->GetRootLayer()->SetTransform(translate_transform);
+  GetOldLayerTreeRootLayer()->SetTransform(translate_transform);
 
   std::unique_ptr<ScreenRotationAnimation> screen_rotation =
       base::MakeUnique<ScreenRotationAnimation>(
-          layer_cleanup_observer->GetRootLayer(),
+          GetOldLayerTreeRootLayer(),
           old_layer_initial_rotation_degrees * rotation_factor,
           (old_layer_initial_rotation_degrees - kRotationDegrees) *
               rotation_factor,
-          layer_cleanup_observer->GetRootLayer()->opacity(),
-          0.0f /* target_opacity */, pivot, duration, tween_type);
+          GetOldLayerTreeRootLayer()->opacity(), 0.0f /* target_opacity */,
+          pivot, duration, tween_type);
 
-  ui::LayerAnimator* animator =
-      layer_cleanup_observer->GetRootLayer()->GetAnimator();
+  ui::LayerAnimator* animator = GetOldLayerTreeRootLayer()->GetAnimator();
   animator->set_preemption_strategy(
       ui::LayerAnimator::REPLACE_QUEUED_ANIMATIONS);
   std::unique_ptr<ui::LayerAnimationSequence> animation_sequence =
       base::MakeUnique<ui::LayerAnimationSequence>(std::move(screen_rotation));
   // Add an observer so that the cloned layers can be cleaned up with the
   // animation completes/aborts.
-  animation_sequence->AddObserver(layer_cleanup_observer.release());
+  animation_sequence->AddObserver(old_layer_cleanup_observer.release());
+  // In unit test, we can use ui::test::MultiLayerAnimatorTestController to
+  // controll the animation.

[bruthig, 2017/03/15 23:12:37]

nit: 'controll' -> 'control'

[wutao, 2017/03/16 07:37:59]

Done.

+  if (disable_animation_timers_for_test_)
+    animator->set_disable_timer_for_test(true);
   animator->StartAnimation(animation_sequence.release());
 }
 
-}  // namespace
+void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
+                                    display::Display::RotationSource source) {
+  if (GetCurrentScreenRotation(display_id_) == new_rotation)
+    return;
 
-ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
-    : display_id_(display_id) {}
+  std::unique_ptr<ScreenRotationRequest> rotation_request =
+      base::MakeUnique<ScreenRotationRequest>();
+  rotation_request->new_rotation = new_rotation;
+  rotation_request->source = source;
+
+  if (is_rotating_) {
+    last_pending_request_.reset(rotation_request.release());
+    StopAnimating();

[bruthig, 2017/03/15 23:12:37]

nit: A comment here explaining that the queue will be processed because the
OnLayerAnimation(Ended|Aborted) methods should be called would be helpful.

[wutao, 2017/03/16 07:37:59]

Done.

+  } else {
+    last_pending_request_.reset();
+    is_rotating_ = true;
+    AnimateRotation(*rotation_request);
+    rotation_request.reset();
+  }
+}
 
-ScreenRotationAnimator::~ScreenRotationAnimator() {}
+void ScreenRotationAnimator::OnLayerAnimationEnded() {
+  ProcessAnimationQueue();
+}
 
-bool ScreenRotationAnimator::CanAnimate() const {
-  return Shell::GetInstance()
-      ->display_manager()
-      ->GetDisplayForId(display_id_)
-      .is_valid();
+void ScreenRotationAnimator::OnLayerAnimationAborted() {
+  AbortAnimations(old_layer_tree_owner_->root());
+  ProcessAnimationQueue();
 }
 
-void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
-                                    display::Display::RotationSource source) {
-  const display::Display::Rotation current_rotation =
-      GetCurrentRotation(display_id_);
+base::WeakPtr<ScreenRotationAnimator> ScreenRotationAnimator::WeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
 
-  if (current_rotation == new_rotation)
-    return;
+ui::Layer* ScreenRotationAnimator::GetOldLayerTreeRootLayer() {
+  if (old_layer_tree_owner_) {
+    return old_layer_tree_owner_->root();
+  } else {
+    NOTREACHED();
+    return nullptr;
+  }
+}
+
+void ScreenRotationAnimator::ProcessAnimationQueue() {
+  is_rotating_ = false;
+  old_layer_tree_owner_.reset();
+  if (last_pending_request_)
+    Rotate(last_pending_request_->new_rotation, last_pending_request_->source);

[bruthig, 2017/03/15 23:12:37]

Can we pop the request off the queue here or does it have to be done in
::Rotate() ?

[wutao, 2017/03/16 07:37:59]

We might.
We can make a copy and reset the last_pending_request_ before Rotate().
We also can pop after Rotate(). But the concern is that, when animation
aborted/ended, if the last_pending_request_ is not reset, then Rotate() will be
called again.

[bruthig, 2017/03/16 20:41:57]

I think it would be easier to follow if you pop it off the queue here, making a
copy, prior to calling Rotate().

+
+  if (screen_rotation_animator_observer_) {
+    screen_rotation_animator_observer_->OnEndedOrAbortedScreenRotationAnimation(
+        this);
+  }
+}
+
+void ScreenRotationAnimator::set_disable_animation_timers_for_test(
+    bool disable_timers) {
+  disable_animation_timers_for_test_ = disable_timers;
+}
+
+void ScreenRotationAnimator::StopAnimating() {
+  aura::Window* root_window = GetRootWindow(display_id_);
+  for (ui::Layer* child_layer : root_window->layer()->children()) {
+    if (child_layer == old_layer_tree_owner_->root())
+      continue;
+
+    child_layer->GetAnimator()->StopAnimating();
+  }
 
-  RotateScreen(display_id_, new_rotation, source);
+  old_layer_tree_owner_->root()->GetAnimator()->StopAnimating();
 }
 
 }  // namespace ash