Add second copy request after screen rotation to flatten the layers in animation.

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>
-
 #include "ash/ash_switches.h"
 #include "ash/display/window_tree_host_manager.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/rotator/screen_rotation_animation.h"
 #include "ash/rotator/screen_rotation_animator_observer.h"
 #include "ash/shell.h"
+#include "ash/utility/transformer_util.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "ui/aura/window.h"
+#include "ui/compositor/callback_layer_animation_observer.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animation_element.h"
-#include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/layer_animation_sequence.h"
 #include "ui/compositor/layer_animator.h"
 #include "ui/compositor/layer_owner.h"
 #include "ui/compositor/layer_tree_owner.h"
 #include "ui/display/display.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/manager/managed_display_info.h"
 #include "ui/gfx/animation/tween.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
@@ skipping 33 matching lines @@
   return (initial_rotation + 3) % 4 == new_rotation
              ? kCounterClockWiseRotationFactor
              : kClockWiseRotationFactor;
 }
 
 aura::Window* GetRootWindow(int64_t display_id) {
   return Shell::Get()->window_tree_host_manager()->GetRootWindowForDisplayId(
       display_id);
 }
 
+aura::Window* GetScreenRotationContainer(aura::Window* root_window) {
+  return root_window->GetChildById(kShellWindowId_ScreenRotationContainer);
+}
+
 // 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;
 }
 
 // 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:
-  // Takes WeakPtr of ScreenRotationAnimator. |this| may outlive the |animator_|
-  // instance and the |animator_| isn't detaching itself as an observer when
-  // being destroyed. However, ideally, when |animator_| is destroying,
-  // deleting |old_layer_tree_owner_| will trigger OnLayerAnimationAborted and
-  // delete |this| before |animator_| deleted.
-  explicit LayerCleanupObserver(base::WeakPtr<ScreenRotationAnimator> animator);
-  ~LayerCleanupObserver() override;
-
-  // 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:
-  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(
-    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);
+void AddLayerAtTopOfWindowLayers(aura::Window* root_window, ui::Layer* layer) {
+  // Add the cloned/copied layer tree into the root, so it will be rendered.
+  root_window->layer()->Add(layer);
+  root_window->layer()->StackAtTop(layer);
 }
 
-void LayerCleanupObserver::OnLayerAnimationEnded(
-    ui::LayerAnimationSequence* sequence) {
-  if (animator_)
-    animator_->ProcessAnimationQueue();
-
-  delete this;
+void AddLayerBelowWindowLayer(aura::Window* root_window,
+                              ui::Layer* top_layer,
+                              ui::Layer* layer) {
+  // Add the cloned/copied layer tree into the root, so it will be rendered.
+  root_window->layer()->Add(layer);
+  root_window->layer()->StackBelow(layer, top_layer);
 }
 
-void LayerCleanupObserver::OnLayerAnimationAborted(
-    ui::LayerAnimationSequence* sequence) {
-  if (animator_)
-    animator_->ProcessAnimationQueue();
-
-  delete this;
+// The Callback will be invoked when all animation sequences have
+// finished. |observer| will be destroyed after invoking the Callback if it
+// returns true.
+bool AnimationEndedCallback(
+    base::WeakPtr<ScreenRotationAnimator> animator,
+    const ui::CallbackLayerAnimationObserver& observer) {
+  if (animator)
+    animator->ProcessAnimationQueue();
+  return true;
 }
 
-void LayerCleanupObserver::OnAttachedToSequence(
-    ui::LayerAnimationSequence* sequence) {
-  sequence_ = sequence;
+// Creates a Transform for the old layer in screen rotation animation.
+gfx::Transform CreateScreenRotationOldLayerTransformForDisplay(
+    display::Display::Rotation old_rotation,
+    display::Display::Rotation new_rotation,
+    const display::Display& display) {
+  gfx::Transform inverse;
+  CHECK(CreateRotationTransform(old_rotation, new_rotation, display)
+            .GetInverse(&inverse));
+  return inverse;
 }
 
-void LayerCleanupObserver::OnDetachedFromSequence(
-    ui::LayerAnimationSequence* sequence) {
-  DCHECK_EQ(sequence, sequence_);
-  sequence_ = nullptr;
+// The |request_id| changed since last copy request, which means a
+// new rotation stated, we need to ignore this copy result.
+bool IgnoreCopyResult(int64_t request_id, int64_t current_request_id) {
+  DCHECK(request_id <= current_request_id);
+  return request_id < current_request_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.
+bool AbortRotation(int64_t display_id, cc::CopyOutputResult* result) {
+  return !IsDisplayIdValid(display_id) || result->IsEmpty();
+}
+
+// Creates a black mask layer and returns the |layer_owner|.
+std::unique_ptr<ui::LayerOwner> CreateBlackMaskLayerOwner(
+    const gfx::Rect& rect) {
+  std::unique_ptr<ui::Layer> black_mask_layer =
+      base::MakeUnique<ui::Layer>(ui::LAYER_SOLID_COLOR);
+  black_mask_layer->SetBounds(rect);
+  black_mask_layer->SetColor(SK_ColorBLACK);
+  std::unique_ptr<ui::LayerOwner> black_mask_layer_owner =
+      base::MakeUnique<ui::LayerOwner>();
+  black_mask_layer_owner->SetLayer(std::move(black_mask_layer));
+  return black_mask_layer_owner;
 }
 
 class ScreenRotationAnimationMetricsReporter
     : public ui::AnimationMetricsReporter {
  public:
   ScreenRotationAnimationMetricsReporter() {}
   ~ScreenRotationAnimationMetricsReporter() override {}
 
   void Report(int value) override {
     UMA_HISTOGRAM_PERCENTAGE("Ash.Rotation.AnimationSmoothness", value);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimationMetricsReporter);
 };
 
 }  // namespace
 
 ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
     : display_id_(display_id),
       screen_rotation_state_(IDLE),
       rotation_request_id_(0),
       metrics_reporter_(
           base::MakeUnique<ScreenRotationAnimationMetricsReporter>()),
       disable_animation_timers_for_test_(false),
+      has_switch_ash_enable_smooth_screen_rotation_(
+          base::CommandLine::ForCurrentProcess()->HasSwitch(
+              switches::kAshEnableSmoothScreenRotation)),
+      root_window_(GetRootWindow(display_id_)),
+      screen_rotation_container_layer_(
+          GetScreenRotationContainer(root_window_)->layer()),
       weak_factory_(this) {}
 
 ScreenRotationAnimator::~ScreenRotationAnimator() {
-  // To prevent a call to |LayerCleanupObserver::OnLayerAnimationAborted()| from
-  // calling a method on the |animator_|.
+  // To prevent a call to |AnimationEndedCallback()| from calling a method on
+  // the |animator_|.
   weak_factory_.InvalidateWeakPtrs();
 
   // Explicitly reset the |old_layer_tree_owner_| and |metrics_reporter_| in
   // order to make sure |metrics_reporter_| outlives the attached animation
   // sequence.
   old_layer_tree_owner_.reset();
   metrics_reporter_.reset();
 }
 
 void ScreenRotationAnimator::StartRotationAnimation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kAshEnableSmoothScreenRotation)) {
-    RequestCopyRootLayerAndAnimateRotation(std::move(rotation_request));
+  if (has_switch_ash_enable_smooth_screen_rotation_) {
+    std::unique_ptr<cc::CopyOutputRequest> copy_output_request =
+        cc::CopyOutputRequest::CreateRequest(
+            CreateAfterCopyCallbackBeforeRotation(std::move(rotation_request)));
+    RequestCopyScreenRotationContainerLayer(std::move(copy_output_request));
+    screen_rotation_state_ = COPY_REQUESTED;
   } else {
-    CreateOldLayerTree();
-    AnimateRotation(std::move(rotation_request));
+    StartSlowAnimation(std::move(rotation_request));
   }
 }
 
-void ScreenRotationAnimator::RequestCopyRootLayerAndAnimateRotation(
+void ScreenRotationAnimator::StartSlowAnimation(
     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));
+  CreateOldLayerTree();
+  SetRotation(rotation_request->old_rotation, rotation_request->new_rotation,
+              rotation_request->source);
+  AnimateRotation(std::move(rotation_request));
+}
 
-  screen_rotation_state_ = COPY_REQUESTED;
+void ScreenRotationAnimator::SetRotation(
+    display::Display::Rotation old_rotation,
+    display::Display::Rotation new_rotation,
+    display::Display::RotationSource source) {
+  Shell::Get()->display_manager()->SetDisplayRotation(display_id_, new_rotation,
+                                                      source);
+  const display::Display display =
+      Shell::Get()->display_manager()->GetDisplayForId(display_id_);
+  old_layer_tree_owner_->root()->SetTransform(
+      CreateScreenRotationOldLayerTransformForDisplay(old_rotation,
+                                                      new_rotation, display));
+}
+
+void ScreenRotationAnimator::RequestCopyScreenRotationContainerLayer(
+    std::unique_ptr<cc::CopyOutputRequest> copy_output_request) {
+  copy_output_request->set_area(
+      gfx::Rect(screen_rotation_container_layer_->size()));
+  screen_rotation_container_layer_->RequestCopyOfOutput(
+      std::move(copy_output_request));
 }
 
 ScreenRotationAnimator::CopyCallback
-ScreenRotationAnimator::CreateAfterCopyCallback(
+ScreenRotationAnimator::CreateAfterCopyCallbackBeforeRotation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
-  return base::Bind(&ScreenRotationAnimator::OnRootLayerCopiedBeforeRotation,
+  return base::Bind(&ScreenRotationAnimator::
+                        OnScreenRotationContainerLayerCopiedBeforeRotation,
                     weak_factory_.GetWeakPtr(),
                     base::Passed(&rotation_request));
 }
 
-void ScreenRotationAnimator::OnRootLayerCopiedBeforeRotation(
+ScreenRotationAnimator::CopyCallback
+ScreenRotationAnimator::CreateAfterCopyCallbackAfterRotation(
+    std::unique_ptr<ScreenRotationRequest> rotation_request) {
+  return base::Bind(&ScreenRotationAnimator::
+                        OnScreenRotationContainerLayerCopiedAfterRotation,
+                    weak_factory_.GetWeakPtr(),
+                    base::Passed(&rotation_request));
+}
+
+void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedBeforeRotation(
     std::unique_ptr<ScreenRotationRequest> rotation_request,
     std::unique_ptr<cc::CopyOutputResult> result) {
-  DCHECK(rotation_request->id <= rotation_request_id_);
-  // The |rotation_request_id_| changed since last copy request, which means a
-  // new rotation stated, we need to ignore this copy result.
-  if (rotation_request->id < rotation_request_id_)
+  if (IgnoreCopyResult(rotation_request->id, rotation_request_id_))
     return;
-
-  // 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()) {
+  if (AbortRotation(display_id_, result.get())) {
     ProcessAnimationQueue();
     return;
   }
 
-  CopyOldLayerTree(std::move(result));
+  old_layer_tree_owner_ = CopyLayerTree(std::move(result));
+  AddLayerAtTopOfWindowLayers(root_window_, old_layer_tree_owner_->root());
+  SetRotation(rotation_request->old_rotation, rotation_request->new_rotation,
+              rotation_request->source);
+  std::unique_ptr<cc::CopyOutputRequest> copy_output_request =
+      cc::CopyOutputRequest::CreateRequest(
+          CreateAfterCopyCallbackAfterRotation(std::move(rotation_request)));
+  RequestCopyScreenRotationContainerLayer(std::move(copy_output_request));
+}
+
+void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedAfterRotation(
+    std::unique_ptr<ScreenRotationRequest> rotation_request,
+    std::unique_ptr<cc::CopyOutputResult> result) {
+  if (IgnoreCopyResult(rotation_request->id, rotation_request_id_))
+    return;
+  if (AbortRotation(display_id_, result.get())) {
+    ProcessAnimationQueue();
+    return;
+  }
+
+  new_layer_tree_owner_ = CopyLayerTree(std::move(result));
+  AddLayerBelowWindowLayer(root_window_, old_layer_tree_owner_->root(),
+                           new_layer_tree_owner_->root());
   AnimateRotation(std::move(rotation_request));
 }
 
 void ScreenRotationAnimator::CreateOldLayerTree() {

[oshima, 2017/04/18 18:32:25]

can you rename this to CreateOldLayerTreeForSlowAnimation() ?

[wutao, 2017/04/18 19:56:58]

Done.

-  old_layer_tree_owner_ = ::wm::RecreateLayers(GetRootWindow(display_id_));
+  old_layer_tree_owner_ = ::wm::RecreateLayers(root_window_);
+  // |screen_rotation_container_layer_| needs update after |RecreateLayers()|.
+  screen_rotation_container_layer_ =
+      GetScreenRotationContainer(root_window_)->layer();
+  AddLayerAtTopOfWindowLayers(root_window_, old_layer_tree_owner_->root());
 }
 
-void ScreenRotationAnimator::CopyOldLayerTree(
+std::unique_ptr<ui::LayerTreeOwner> ScreenRotationAnimator::CopyLayerTree(
     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());
+  gfx::Rect rect(screen_rotation_container_layer_->size());

[oshima, 2017/04/18 18:32:25]

nit: const 

[wutao, 2017/04/18 19:56:58]

Done.

   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));
+  return 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();
-
-  const int rotation_factor = GetRotationFactor(
-      GetCurrentScreenRotation(display_id_), rotation_request->new_rotation);
-
+  const int rotation_factor = GetRotationFactor(rotation_request->old_rotation,
+                                                rotation_request->new_rotation);
   const int old_layer_initial_rotation_degrees = GetInitialDegrees(
-      GetCurrentScreenRotation(display_id_), rotation_request->new_rotation);
-
+      rotation_request->old_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;
-
-  Shell::Get()->display_manager()->SetDisplayRotation(
-      display_id_, rotation_request->new_rotation, rotation_request->source);
-
-  const gfx::Rect rotated_screen_bounds = root_window->GetTargetBounds();
+  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);
 
-  ui::Layer* screen_rotation_container_layer =
-      root_window->GetChildById(kShellWindowId_ScreenRotationContainer)
-          ->layer();
-  std::unique_ptr<ScreenRotationAnimation> current_layer_screen_rotation =
+  ui::Layer* new_root_layer;
+  if (new_layer_tree_owner_ && has_switch_ash_enable_smooth_screen_rotation_) {
+    new_root_layer = new_layer_tree_owner_->root();
+    // Add a black mask layer on top of |screen_rotation_container_layer_|.
+    black_mask_layer_owner_ = CreateBlackMaskLayerOwner(
+        gfx::Rect(screen_rotation_container_layer_->size()));
+    AddLayerBelowWindowLayer(root_window_, new_root_layer,
+                             black_mask_layer_owner_->layer());
+  } else {
+    new_root_layer = screen_rotation_container_layer_;
+  }
+
+  std::unique_ptr<ScreenRotationAnimation> new_layer_screen_rotation =
       base::MakeUnique<ScreenRotationAnimation>(
-          screen_rotation_container_layer, kRotationDegrees * rotation_factor,
-          0 /* end_degrees */, screen_rotation_container_layer->opacity(),
-          screen_rotation_container_layer->opacity() /* target_opacity */,
-          pivot, duration, tween_type);
+          new_root_layer, kRotationDegrees * rotation_factor,
+          0 /* end_degrees */, new_root_layer->opacity(),
+          new_root_layer->opacity() /* target_opacity */, pivot, duration,
+          tween_type);
 
-  ui::LayerAnimator* current_layer_animator =
-      screen_rotation_container_layer->GetAnimator();
-  current_layer_animator->set_preemption_strategy(
+  ui::LayerAnimator* new_layer_animator = new_root_layer->GetAnimator();
+  new_layer_animator->set_preemption_strategy(
       ui::LayerAnimator::REPLACE_QUEUED_ANIMATIONS);
-  std::unique_ptr<ui::LayerAnimationSequence> current_layer_animation_sequence =
+  std::unique_ptr<ui::LayerAnimationSequence> new_layer_animation_sequence =
       base::MakeUnique<ui::LayerAnimationSequence>(
-          std::move(current_layer_screen_rotation));
-  current_layer_animator->StartAnimation(
-      current_layer_animation_sequence.release());
+          std::move(new_layer_screen_rotation));
 
+  ui::Layer* old_root_layer = old_layer_tree_owner_->root();
+  const gfx::Rect original_screen_bounds = old_root_layer->GetTargetBounds();
   // 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);
   old_root_layer->SetTransform(translate_transform);
 
   std::unique_ptr<ScreenRotationAnimation> old_layer_screen_rotation =
       base::MakeUnique<ScreenRotationAnimation>(
           old_root_layer, old_layer_initial_rotation_degrees * rotation_factor,
           (old_layer_initial_rotation_degrees - kRotationDegrees) *
               rotation_factor,
           old_root_layer->opacity(), 0.0f /* target_opacity */, pivot, duration,
           tween_type);
 
   ui::LayerAnimator* old_layer_animator = old_root_layer->GetAnimator();
   old_layer_animator->set_preemption_strategy(
       ui::LayerAnimator::REPLACE_QUEUED_ANIMATIONS);
   std::unique_ptr<ui::LayerAnimationSequence> old_layer_animation_sequence =
       base::MakeUnique<ui::LayerAnimationSequence>(
           std::move(old_layer_screen_rotation));
-  // Add an observer so that the cloned layers can be cleaned up with the
-  // animation completes/aborts.
-  old_layer_animation_sequence->AddObserver(
-      old_layer_cleanup_observer.release());
+
   // In unit test, we can use ash::test::ScreenRotationAnimatorTestApi to
   // control the animation.
-  if (disable_animation_timers_for_test_)
+  if (disable_animation_timers_for_test_) {
+    if (new_layer_tree_owner_)
+      new_layer_animator->set_disable_timer_for_test(true);
     old_layer_animator->set_disable_timer_for_test(true);
+  }
   old_layer_animation_sequence->SetAnimationMetricsReporter(
       metrics_reporter_.get());
+
+  // Add an observer so that the cloned/copied layers can be cleaned up with the
+  // animation completes/aborts.
+  ui::CallbackLayerAnimationObserver* observer =
+      new ui::CallbackLayerAnimationObserver(
+          base::Bind(&AnimationEndedCallback, weak_factory_.GetWeakPtr()));
+  if (new_layer_tree_owner_)
+    new_layer_animator->AddObserver(observer);
+  new_layer_animator->StartAnimation(new_layer_animation_sequence.release());
+  old_layer_animator->AddObserver(observer);
   old_layer_animator->StartAnimation(old_layer_animation_sequence.release());
+  observer->SetActive();
 }
 
 void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
                                     display::Display::RotationSource source) {
-  if (GetCurrentScreenRotation(display_id_) == new_rotation)
+  const display::Display::Rotation current_rotation =
+      GetCurrentScreenRotation(display_id_);
+  rotation_request_id_++;
+
+  // 1. If |current_rotation| equals to |new_rotation|:
+  //   1.1 When |screen_rotation_state_| is IDEL: we already at the target
+  //     rotation, we should stop and call |ProcessAnimationQueue()| to nofity
+  //     |screen_rotation_animator_observers_|.
+  //   1.2 When |screen_rotation_state_| is ROTATING: we already called
+  //     |DisplayManager::SetDisplayRotation()| for previous rotation request
+  //     and at the target rotation we should stop and call
+  //     |ProcessAnimationQueue()| to nofity
+  //     |screen_rotation_animator_observers_|.
+  //   1.3 When |screen_rotation_state_| is COPY_REQUESTED:
+  //     1.3.1 Before the first copy callback, we have not called
+  //       |DisplayManager::SetDisplayRotation()|, but we increased the
+  //       |rotation_request_id_| so that the first copy callback will be
+  //       skipped. We should stop and call |ProcessAnimationQueue()| to nofity
+  //       |screen_rotation_animator_observers_|.
+  //     1.3.2 Before the second copy callback, we have called
+  //       |DisplayManager::SetDisplayRotation()|, we also increased the
+  //       |rotation_request_id_| so that the second copy callback will exit.
+  //       We should stop and call |ProcessAnimationQueue()| to nofity
+  //       |screen_rotation_animator_observers_|.
+  // 2. If |current_rotation| dose not equal to |new_rotation|:
+  //   2.1 When |screen_rotation_state_| is IDEL: we should call
+  //     |StartRotationAnimation()|.
+  //   2.2 When |screen_rotation_state_| is ROTATING: we should set
+  //     |last_pending_request_| and |StopAnimating()| to process the
+  //     |last_pending_request_| in the |AnimationEndedCallback()|.
+  //   2.3 When |screen_rotation_state_| is COPY_REQUESTED:
+  //     2.3.1 Before the first copy callback, we have not called
+  //       |DisplayManager::SetDisplayRotation()|, but increased the
+  //       |rotation_request_id_| so that the first copy callback will be
+  //       skipped. We should call |StartRotationAnimation()|.
+  //     2.3.2 Before the second copy callback, we have called
+  //       |DisplayManager::SetDisplayRotation()| and increased the
+  //       |rotation_request_id_| so that the second copy callback will be
+  //       skipped. We should call |StartRotationAnimation()|.

[oshima, 2017/04/18 18:32:25]

You don't have to explain everything that the code does.

Just explain how request id works to skip slate requests.

[wutao, 2017/04/18 19:56:58]

Done.

+
+  // The actions for 1.1, 1.2, 1.3.1, and 1.3.2 are the same.
+  if (current_rotation == new_rotation) {
+    // We need to call |ProcessAnimationQueue()| to notify
+    // |screen_rotation_animator_observers_|.
+    ProcessAnimationQueue();
     return;
+  }
 
-  rotation_request_id_++;
   std::unique_ptr<ScreenRotationRequest> rotation_request =
-      base::MakeUnique<ScreenRotationRequest>(rotation_request_id_,
-                                              new_rotation, source);
-
+      base::MakeUnique<ScreenRotationRequest>(
+          rotation_request_id_, current_rotation, new_rotation, source);
+  // The actions for 2.1, 2.3.1, and 2.3.2 are the same.
   switch (screen_rotation_state_) {
     case IDLE:
     case COPY_REQUESTED:
       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()|.
+      // |AnimationEndedCallback()| should be called after |StopAnimating()|.
       StopAnimating();
       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() {
   screen_rotation_state_ = IDLE;
+  if (IsDisplayIdValid(display_id_) && black_mask_layer_owner_)
+    root_window_->layer()->Remove(black_mask_layer_owner_->layer());
   old_layer_tree_owner_.reset();
+  new_layer_tree_owner_.reset();
+  black_mask_layer_owner_.reset();
   if (last_pending_request_ && IsDisplayIdValid(display_id_)) {
     display::Display::Rotation new_rotation =
         last_pending_request_->new_rotation;
     display::Display::RotationSource source = last_pending_request_->source;
     last_pending_request_.reset();

[oshima, 2017/04/18 18:32:25]

Looking more closely, looks like this increases the id then recreate the
request. Can you refactor so that it uses the same rotation request?

[wutao, 2017/04/18 19:56:58]

Thanks to catch the double increasing request id. Since in
ProcessAnimationQueue(), the screen_rotation_state_ is set to IDLE, when we call
Rotate() at this time, it will call StartRotationAnimation() if the target
rotation is different. 

I will call StartRotationAnimation() in ProcessAnimationQueue() directly after
checking the target rotation.

     Rotate(new_rotation, source);
     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() {
-  GetRootWindow(display_id_)
-      ->GetChildById(kShellWindowId_ScreenRotationContainer)
-      ->layer()
-      ->GetAnimator()
-      ->StopAnimating();
+  // |old_layer_tree_owner_| new_layer_tree_owner_| could be nullptr if another
+  // the rotation request comes before the copy request finished.
   if (old_layer_tree_owner_)
     old_layer_tree_owner_->root()->GetAnimator()->StopAnimating();
+  if (new_layer_tree_owner_)
+    new_layer_tree_owner_->root()->GetAnimator()->StopAnimating();
+  if (IsDisplayIdValid(display_id_) && black_mask_layer_owner_)
+    root_window_->layer()->Remove(black_mask_layer_owner_->layer());
 }
 
 }  // namespace ash