Handles users rotating screen too early

ash/rotator/screen_rotation_animator.cc

 // Copyright 2015 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 <string>
 #include <utility>
 #include <vector>
 
@@ skipping 16 matching lines @@
 #include "ui/gfx/animation/tween.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
 #include "ui/gfx/transform.h"
 #include "ui/gfx/transform_util.h"
 #include "ui/wm/core/window_util.h"
 
 namespace ash {
 
+ScreenRotationAnimator::ScreenRotationAnimatorObserver::
+    ScreenRotationAnimatorObserver() {}
+
+ScreenRotationAnimator::ScreenRotationAnimatorObserver::
+    ~ScreenRotationAnimatorObserver() {}
+
+void ScreenRotationAnimator::ScreenRotationAnimatorObserver::
+    OnEndedOrAbortedScreenRotationAnimation(ScreenRotationAnimator* animator) {}
+
 namespace {
 
 // The number of degrees that the rotation animations animate through.
 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) {
-  return Shell::GetInstance()
-      ->display_manager()
-      ->GetDisplayInfo(display_id)
-      .GetActiveRotation();
+// The rotation directions
+const int kCounterClockWiseRotation = 1;
+const int kClockWiseRotation = -1;
+
+int GetRotationFactor(display::Display::Rotation initial_rotation,
+                      display::Display::Rotation new_rotation) {
+  return (initial_rotation + 3) % 4 == new_rotation ? kCounterClockWiseRotation
+                                                    : kClockWiseRotation;
 }
 
 // Returns true if the rotation between |initial_rotation| and |new_rotation| is
 // 180 degrees.
 bool Is180DegreeFlip(display::Display::Rotation initial_rotation,
                      display::Display::Rotation new_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);
+}
+
+aura::Window* GetRootWindow(int64_t display_id) {
+  return Shell::GetInstance()
+      ->window_tree_host_manager()
+      ->GetRootWindowForDisplayId(display_id);
+}
+
+// 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);
   ~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;
   void OnLayerAnimationScheduled(
       ui::LayerAnimationSequence* sequence) override {}
 
  protected:
   // ui::LayerAnimationObserver:
   bool RequiresNotificationWhenAnimatorDestroyed() const override {
     return true;
   }
   void OnAttachedToSequence(ui::LayerAnimationSequence* sequence) override;
   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.
   ui::LayerAnimationSequence* sequence_;
 
   DISALLOW_COPY_AND_ASSIGN(LayerCleanupObserver);
 };
 
 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_) {

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

nit: No need for the '{}' if the 'if' and 'body' only span 2 lines.  Applies
here and below.

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

Done.

+    animator_->OnLayerAnimationEnded();
+  }
+
   delete this;
 }
 
 void LayerCleanupObserver::OnLayerAnimationAborted(
     ui::LayerAnimationSequence* sequence) {
+  if (animator_) {
+    animator_->OnLayerAnimationAborted();
+  }
+
   delete this;
 }
 
 void LayerCleanupObserver::OnAttachedToSequence(
     ui::LayerAnimationSequence* sequence) {
   sequence_ = sequence;
 }
 
 void LayerCleanupObserver::OnDetachedFromSequence(
     ui::LayerAnimationSequence* sequence) {
   DCHECK_EQ(sequence, sequence_);
   sequence_ = nullptr;
 }
 
-void LayerCleanupObserver::AbortAnimations(ui::Layer* layer) {
-  for (ui::Layer* child_layer : layer->children())
-    AbortAnimations(child_layer);
-  layer->GetAnimator()->AbortAllAnimations();
-}
-
 // 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);
+void RotateScreen(
+    const ScreenRotationAnimator::ScreenRotationRequest& rotation_request) {
+  aura::Window* root_window = GetRootWindow(rotation_request.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(
+      rotation_request.initial_rotation, 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(
+      rotation_request.initial_rotation, rotation_request.new_rotation);
 
   const base::TimeDelta duration =
       base::TimeDelta::FromMilliseconds(kRotationDurationInMs);
 
   const gfx::Tween::Type tween_type = gfx::Tween::FAST_OUT_LINEAR_IN;
 
   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)));
+  ScreenRotationAnimator* screen_rotation_animator = rotation_request.animator;
+  screen_rotation_animator->set_old_layer_tree_owner(old_layer_tree.release());
+  std::unique_ptr<LayerCleanupObserver> old_layer_cleanup_observer(
+      new LayerCleanupObserver(screen_rotation_animator->WeakPtr()));
 
   Shell::GetInstance()->display_manager()->SetDisplayRotation(
-      display_id, new_rotation, source);
+      rotation_request.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,
                                       rotated_screen_bounds.height() / 2);
 
   // We must animate each non-cloned child layer individually because the cloned
   // layer was added as a child to |root_window|'s layer so that it will be
   // rendered.
   // TODO(bruthig): Add a NOT_DRAWN layer in between the root_window's layer and
   // its current children so that we only need to initiate two
   // 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 == screen_rotation_animator->GetOldLayerTreeRootLayer())
       continue;
 
     std::unique_ptr<ScreenRotationAnimation> screen_rotation =
         base::MakeUnique<ScreenRotationAnimation>(
             child_layer, kRotationDegrees * rotation_factor,
             0 /* end_degrees */, child_layer->opacity(),
             1.0f /* target_opacity */, pivot, duration, tween_type);
 
     ui::LayerAnimator* animator = child_layer->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));
     animator->StartAnimation(animation_sequence.release());
   }
 
   // The old layer will also be transformed into the new orientation. We will
   // translate it so that the old layer's center point aligns with the new
   // orientation's center point and use that center point as the pivot for the
   // rotation animation.
   gfx::Transform translate_transform;
   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);
+  screen_rotation_animator->GetOldLayerTreeRootLayer()->SetTransform(
+      translate_transform);
 
   std::unique_ptr<ScreenRotationAnimation> screen_rotation =
       base::MakeUnique<ScreenRotationAnimation>(
-          layer_cleanup_observer->GetRootLayer(),
+          screen_rotation_animator->GetOldLayerTreeRootLayer(),
           old_layer_initial_rotation_degrees * rotation_factor,
           (old_layer_initial_rotation_degrees - kRotationDegrees) *
               rotation_factor,
-          layer_cleanup_observer->GetRootLayer()->opacity(),
+          screen_rotation_animator->GetOldLayerTreeRootLayer()->opacity(),
           0.0f /* target_opacity */, pivot, duration, tween_type);
 
   ui::LayerAnimator* animator =
-      layer_cleanup_observer->GetRootLayer()->GetAnimator();
+      screen_rotation_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());
   animator->StartAnimation(animation_sequence.release());
 }
 
+display::Display::Rotation GetCurrentScreenRotation(int64_t display_id) {
+  return Shell::GetInstance()
+      ->display_manager()
+      ->GetDisplayInfo(display_id)
+      .GetActiveRotation();
+}
+
 }  // namespace
 
 ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
-    : display_id_(display_id) {}
+    : display_id_(display_id),
+      is_rotating_(false),
+      screen_rotation_animator_observer_(nullptr),
+      weak_factory_(this) {}
 
 ScreenRotationAnimator::~ScreenRotationAnimator() {}
 
-bool ScreenRotationAnimator::CanAnimate() const {
-  return Shell::GetInstance()
-      ->display_manager()
-      ->GetDisplayForId(display_id_)
-      .is_valid();
+void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
+                                    display::Display::RotationSource source) {
+  const display::Display::Rotation initial_rotation =
+      GetCurrentScreenRotation(display_id_);
+
+  if (initial_rotation == new_rotation)
+    return;
+
+  std::unique_ptr<ScreenRotationRequest> rotation_request =
+      base::MakeUnique<ScreenRotationRequest>();
+  rotation_request->animator = this;
+  rotation_request->display_id = display_id_;
+  rotation_request->initial_rotation = initial_rotation;
+  rotation_request->new_rotation = new_rotation;
+  rotation_request->source = source;
+
+  if (is_rotating_) {
+    last_pending_request_.reset(rotation_request.release());
+    StopAnimating();
+  } else {
+    last_pending_request_.reset();
+    is_rotating_ = true;
+    RotateScreen(*rotation_request);
+    rotation_request.reset();
+  }
 }
 
-void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
-                                    display::Display::RotationSource source) {
-  const display::Display::Rotation current_rotation =
-      GetCurrentRotation(display_id_);
+ui::Layer* ScreenRotationAnimator::GetOldLayerTreeRootLayer() {
+  if (old_layer_tree_owner_) {
+    return old_layer_tree_owner_->root();
+  } else {

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

nit: The 'else' is not really necessary since the 'if' block above will always
return;

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

What if old_layer_tree_owner_ is not assigned yet? Here we just want to make
sure we will not call GetOldLayerTreeRootLayer() before old_layer_tree_owner_
has value.

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

I'm not suggesting to remove the body of the 'else' block, just the else block.
i.e.

...
  if (old_layer_tree_owner_)
    return old_layer_tree_owner_->root();
  NOTREACHED();
  return nullptr;
}

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

Done.

+    NOTREACHED();
+    return nullptr;
+  }
+}
 
-  if (current_rotation == new_rotation)
-    return;
+void ScreenRotationAnimator::OnLayerAnimationEnded() {
+  ContinueOrCleanUp();
+}
 
-  RotateScreen(display_id_, new_rotation, source);
+void ScreenRotationAnimator::OnLayerAnimationAborted() {
+  AbortAnimations(old_layer_tree_owner_->root());
+  ContinueOrCleanUp();
+}
+
+void ScreenRotationAnimator::ContinueOrCleanUp() {
+  is_rotating_ = false;
+  old_layer_tree_owner_.reset();
+  if (last_pending_request_) {
+    const ScreenRotationAnimator::ScreenRotationRequest pending_request =

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

nit: Instead of creating a copy of the |last_pending_request_| I think it would
be ok if you just re-used |last_pending_reqest_| in place of  |pending_request|
below.  

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

lol. Right. Forget this when I moved the code to here from
display_configuration_controller.

With the change below, this if block is much clean now.

+        *last_pending_request_;
+    const display::Display::Rotation new_rotation =
+        pending_request.new_rotation;
+    if (GetCurrentScreenRotation(display_id_) != new_rotation) {

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

nit: I don't think duplicating this early return is adding much value.  It's
already done in Rotate() on line 277.

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

Done.

+      Rotate(new_rotation, pending_request.source);
+      return;
+    }
+  }
+
+  if (screen_rotation_animator_observer_)
+    screen_rotation_animator_observer_->OnEndedOrAbortedScreenRotationAnimation(
+        this);

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

If the body wraps onto more than one line the '{}' should be included.

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

Done.

+}
+
+void ScreenRotationAnimator::AbortAnimations(ui::Layer* layer) {

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

nit: It might make sense to make this a non-member, helper function in the
anonymous namespace.

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

Done.

+  for (ui::Layer* child_layer : layer->children())
+    AbortAnimations(child_layer);
+  layer->GetAnimator()->AbortAllAnimations();
+}
+
+void ScreenRotationAnimator::StopAnimating() {
+  aura::Window* root_window = GetRootWindow(display_id_);
+  for (ui::Layer* child_layer : root_window->layer()->children()) {
+    if (old_layer_tree_owner_ && child_layer == old_layer_tree_owner_->root())

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

Adding a quick return if (!old_layer_tree_owner_) might make this more
readable/efficient.

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

Do we assume if (!old_layer_tree_owner_), then there is no animation? Looks like
is this situation.
Then it should not enter StopAnimating() any way. As you mentioned in patch 2,
maybe this kind of early return would be masking a bug. I plan to remove all the
if (old_layer_tree_owner_) check.

+      continue;
+
+    child_layer->GetAnimator()->StopAnimating();
+  }
+
+  if (old_layer_tree_owner_) {
+    old_layer_tree_owner_->root()->GetAnimator()->StopAnimating();
+  }
 }
 
 }  // namespace ash