Fix corner case if another rotation request comes before the copy request finishes.

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 171 matching lines @@
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimationMetricsReporter);
 };
 
 }  // namespace
 
 ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
     : display_id_(display_id),
-      is_rotating_(false),
+      screen_rotation_state_(IDLE),
       metrics_reporter_(
           base::MakeUnique<ScreenRotationAnimationMetricsReporter>()),
       disable_animation_timers_for_test_(false),
       weak_factory_(this) {}
 
 ScreenRotationAnimator::~ScreenRotationAnimator() {
   // To prevent a call to |LayerCleanupObserver::OnLayerAnimationAborted()| from
   // calling a method on the |animator_|.
   weak_factory_.InvalidateWeakPtrs();
 
@@ skipping 16 matching lines @@
 }
 
 void ScreenRotationAnimator::RequestCopyRootLayerAndAnimateRotation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
   std::unique_ptr<cc::CopyOutputRequest> copy_output_request =
       cc::CopyOutputRequest::CreateRequest(
           CreateAfterCopyCallback(std::move(rotation_request)));
   ui::Layer* layer = GetRootWindow(display_id_)->layer();
   copy_output_request->set_area(gfx::Rect(layer->size()));
   layer->RequestCopyOfOutput(std::move(copy_output_request));
+
+  screen_rotation_state_ = COPY_REQUESTED;
 }
 
 ScreenRotationAnimator::CopyCallback
 ScreenRotationAnimator::CreateAfterCopyCallback(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
   return base::Bind(&ScreenRotationAnimator::OnRootLayerCopiedBeforeRotation,
                     weak_factory_.GetWeakPtr(),
                     base::Passed(&rotation_request));
 }
 
 void ScreenRotationAnimator::OnRootLayerCopiedBeforeRotation(
     std::unique_ptr<ScreenRotationRequest> rotation_request,
     std::unique_ptr<cc::CopyOutputResult> result) {
-  // If display was removed, should abort rotation.
-  if (!IsDisplayIdValid(display_id_)) {
+  // In the following cases, abort rotation:
+  // 1) if the display was removed,
+  // 2) the copy request has been canceled or failed. It would fail if,
+  // for examples: a) The layer is removed from the compositor and destroye
+  // before committing the request to the compositor. b) The compositor is
+  // shutdown.
+  if (!IsDisplayIdValid(display_id_) || result->IsEmpty()) {
     ProcessAnimationQueue();
+    rotation_request.reset();
     return;
   }
 
-  // Fall back to recreate layers solution when the copy request has been
-  // canceled or failed. It would fail if, for examples: a) The layer is removed
-  // from the compositor and destroyed before committing the request to the
-  // compositor. b) The compositor is shutdown.
-  if (result->IsEmpty())
-    CreateOldLayerTree();
-  else
-    CopyOldLayerTree(std::move(result));
+  CopyOldLayerTree(std::move(result));
   AnimateRotation(std::move(rotation_request));
 }
 
 void ScreenRotationAnimator::CreateOldLayerTree() {
   old_layer_tree_owner_ = ::wm::RecreateLayers(GetRootWindow(display_id_));
 }
 
 void ScreenRotationAnimator::CopyOldLayerTree(
     std::unique_ptr<cc::CopyOutputResult> result) {
   cc::TextureMailbox texture_mailbox;
   std::unique_ptr<cc::SingleReleaseCallback> release_callback;
   result->TakeTexture(&texture_mailbox, &release_callback);
   DCHECK(texture_mailbox.IsTexture());
 
   aura::Window* root_window = GetRootWindow(display_id_);
   gfx::Rect rect(root_window->layer()->size());
   std::unique_ptr<ui::Layer> copy_layer = base::MakeUnique<ui::Layer>();
   copy_layer->SetBounds(rect);
   copy_layer->SetTextureMailbox(texture_mailbox, std::move(release_callback),
                                 rect.size());
   old_layer_tree_owner_ =
       base::MakeUnique<ui::LayerTreeOwner>(std::move(copy_layer));
 }
 
 void ScreenRotationAnimator::AnimateRotation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
+  screen_rotation_state_ = ROTATING;
+
   aura::Window* root_window = GetRootWindow(display_id_);
   std::unique_ptr<LayerCleanupObserver> old_layer_cleanup_observer(
       new LayerCleanupObserver(weak_factory_.GetWeakPtr()));
   ui::Layer* old_root_layer = old_layer_tree_owner_->root();
   old_root_layer->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_root_layer);
   root_window->layer()->StackAtTop(old_root_layer);
 
   const gfx::Rect original_screen_bounds = root_window->GetTargetBounds();
@@ skipping 79 matching lines @@
 }
 
 void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
                                     display::Display::RotationSource source) {
   if (GetCurrentScreenRotation(display_id_) == new_rotation)
     return;
 
   std::unique_ptr<ScreenRotationRequest> rotation_request =
       base::MakeUnique<ScreenRotationRequest>(new_rotation, source);
 
-  if (is_rotating_) {
-    last_pending_request_ = std::move(rotation_request);
-    // The pending request will be processed when the
-    // OnLayerAnimation(Ended|Aborted) methods should be called after
-    // StopAnimating().
-    StopAnimating();
-  } else {
-    is_rotating_ = true;
-    StartRotationAnimation(std::move(rotation_request));
+  switch (screen_rotation_state_) {
+    case IDLE:
+      StartRotationAnimation(std::move(rotation_request));
+      break;
+    case ROTATING:
+      last_pending_request_ = std::move(rotation_request);
+      // The pending request will be processed when the
+      // OnLayerAnimation(Ended|Aborted) methods should be called after
+      // |StopAnimating()|.
+      StopAnimating();
+      break;
+    case COPY_REQUESTED:
+      // TODO(wutao): We need to handle this otherwise the device will be in
+      // inconsistent state.
+      break;
   }
 }
 
 void ScreenRotationAnimator::AddScreenRotationAnimatorObserver(
     ScreenRotationAnimatorObserver* observer) {
   screen_rotation_animator_observers_.AddObserver(observer);
 }
 
 void ScreenRotationAnimator::RemoveScreenRotationAnimatorObserver(
     ScreenRotationAnimatorObserver* observer) {
   screen_rotation_animator_observers_.RemoveObserver(observer);
 }
 
 void ScreenRotationAnimator::ProcessAnimationQueue() {
-  is_rotating_ = false;
+  screen_rotation_state_ = IDLE;
   old_layer_tree_owner_.reset();
   if (last_pending_request_ && IsDisplayIdValid(display_id_)) {
     std::unique_ptr<ScreenRotationRequest> rotation_request =
         std::move(last_pending_request_);

[oshima, 2017/04/10 23:20:39]

by the way, do you need to move this to stack?

[wutao, 2017/04/10 23:56:18]

Talked offline. I will change the unique_ptr to local variable in the other cl.

We should be able reset |last_pending_request_| after calling |Rotate()|. It
should never happen when we reach this code again and |last_pending_request_|
has not been cleared. But simpler way is to just keep the rotation/source on the
stack and reset before calling it.

     Rotate(rotation_request->new_rotation, rotation_request->source);
     rotation_request.reset();
     return;
   }
 
   for (auto& observer : screen_rotation_animator_observers_)
     observer.OnScreenRotationAnimationFinished(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();
   }
 
   old_layer_tree_owner_->root()->GetAnimator()->StopAnimating();
 }
 
 }  // namespace ash