cros: Do not cache |root_window| in ScreenRotationAnimator.

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 aaadcdb64169539b07e0630bdc9d7c56cfe586e1..e692ecd28f67b5ca1681a5a3a9ace977f74edeba 100644
--- a/ash/rotator/screen_rotation_animator.cc
+++ b/ash/rotator/screen_rotation_animator.cc
@@ -134,17 +134,19 @@ bool IgnoreCopyResult(int64_t request_id, int64_t current_request_id) {
   return request_id < current_request_id;
 }
 
-// Creates a black mask layer and returns the |layer_owner|.
-std::unique_ptr<ui::LayerOwner> CreateBlackMaskLayerOwner(
+bool RootWindowChangedForDisplayId(aura::Window* root_window,
+                                   int64_t display_id) {
+  return root_window != GetRootWindow(display_id);
+}
+
+// Creates a mask layer and returns the |mask_layer_tree_owner|.
+std::unique_ptr<ui::LayerTreeOwner> CreateMaskLayerTreeOwner(
     const gfx::Rect& rect) {
-  std::unique_ptr<ui::Layer> black_mask_layer =
+  std::unique_ptr<ui::Layer> 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;
+  mask_layer->SetBounds(rect);
+  mask_layer->SetColor(SK_ColorBLACK);
+  return base::MakeUnique<ui::LayerTreeOwner>(std::move(mask_layer));
 }
 
 class ScreenRotationAnimationMetricsReporter
@@ -163,8 +165,10 @@ class ScreenRotationAnimationMetricsReporter
 
 }  // namespace
 
-ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
-    : display_id_(display_id),
+ScreenRotationAnimator::ScreenRotationAnimator(aura::Window* root_window)
+    : root_window_(root_window),
+      screen_rotation_container_layer_(
+          GetScreenRotationContainer(root_window_)->layer()),
       screen_rotation_state_(IDLE),
       rotation_request_id_(0),
       metrics_reporter_(
@@ -173,9 +177,6 @@ ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id)
       has_switch_ash_disable_smooth_screen_rotation_(
           base::CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kAshDisableSmoothScreenRotation)),
-      root_window_(GetRootWindow(display_id_)),
-      screen_rotation_container_layer_(
-          GetScreenRotationContainer(root_window_)->layer()),
       weak_factory_(this) {}
 
 ScreenRotationAnimator::~ScreenRotationAnimator() {
@@ -193,7 +194,7 @@ ScreenRotationAnimator::~ScreenRotationAnimator() {
 void ScreenRotationAnimator::StartRotationAnimation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
   const display::Display::Rotation current_rotation =
-      GetCurrentScreenRotation(display_id_);
+      GetCurrentScreenRotation(rotation_request->display_id);
   if (current_rotation == rotation_request->new_rotation) {
     // We need to call |ProcessAnimationQueue()| to prepare for next rotation
     // request.
@@ -216,12 +217,13 @@ void ScreenRotationAnimator::StartRotationAnimation(
 void ScreenRotationAnimator::StartSlowAnimation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
   CreateOldLayerTreeForSlowAnimation();
-  SetRotation(rotation_request->old_rotation, rotation_request->new_rotation,
-              rotation_request->source);
+  SetRotation(rotation_request->display_id, rotation_request->old_rotation,
+              rotation_request->new_rotation, rotation_request->source);
   AnimateRotation(std::move(rotation_request));
 }
 
 void ScreenRotationAnimator::SetRotation(
+    int64_t display_id,
     display::Display::Rotation old_rotation,
     display::Display::Rotation new_rotation,
     display::Display::RotationSource source) {
@@ -230,11 +232,11 @@ void ScreenRotationAnimator::SetRotation(
   // and ARC apps.
   ui::Compositor* compositor = root_window_->layer()->GetCompositor();
   compositor->set_allow_locks_to_extend_timeout(true);
-  Shell::Get()->display_manager()->SetDisplayRotation(display_id_, new_rotation,
+  Shell::Get()->display_manager()->SetDisplayRotation(display_id, new_rotation,
                                                       source);
   compositor->set_allow_locks_to_extend_timeout(false);
   const display::Display display =
-      Shell::Get()->display_manager()->GetDisplayForId(display_id_);
+      Shell::Get()->display_manager()->GetDisplayForId(display_id);
   old_layer_tree_owner_->root()->SetTransform(
       CreateScreenRotationOldLayerTransformForDisplay(old_rotation,
                                                       new_rotation, display));
@@ -271,8 +273,11 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedBeforeRotation(
     std::unique_ptr<cc::CopyOutputResult> result) {
   if (IgnoreCopyResult(rotation_request->id, rotation_request_id_))
     return;
-  // Abort rotation and animation if the display was removed.
-  if (!IsDisplayIdValid(display_id_)) {
+  // Abort rotation and animation if the display was removed or the
+  // |root_window| was changed for |display_id|.
+  if (!IsDisplayIdValid(rotation_request->display_id) ||

[oshima, 2017/06/01 15:54:59]

you should be able t remove this because if it's invalid, then this root window
should have new display id.

[wutao, 2017/06/01 22:48:03]

Done.

+      RootWindowChangedForDisplayId(root_window_,
+                                    rotation_request->display_id)) {
     ProcessAnimationQueue();
     return;
   }
@@ -282,15 +287,16 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedBeforeRotation(
   // request to the compositor. b) The compositor is shutdown.
   if (result->IsEmpty()) {
     Shell::Get()->display_manager()->SetDisplayRotation(
-        display_id_, rotation_request->new_rotation, rotation_request->source);
+        rotation_request->display_id, rotation_request->new_rotation,
+        rotation_request->source);
     ProcessAnimationQueue();
     return;
   }
 
   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);
+  SetRotation(rotation_request->display_id, 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)));
@@ -304,11 +310,15 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedAfterRotation(
     return;
   // In the following cases, abort animation:
   // 1) if the display was removed,
-  // 2) the copy request has been canceled or failed. It would fail if,
+  // 2) if the |root_window| was changed for |display_id|,
+  // 3) 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 (!IsDisplayIdValid(rotation_request->display_id) ||

[oshima, 2017/06/01 15:54:59]

ditto

[wutao, 2017/06/01 22:48:03]

Done.

+      RootWindowChangedForDisplayId(root_window_,
+                                    rotation_request->display_id) ||
+      result->IsEmpty()) {
     ProcessAnimationQueue();
     return;
   }
@@ -363,10 +373,10 @@ void ScreenRotationAnimator::AnimateRotation(
   } else {
     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(
+    mask_layer_tree_owner_ = CreateMaskLayerTreeOwner(
         gfx::Rect(screen_rotation_container_layer_->size()));
     AddLayerBelowWindowLayer(root_window_, new_root_layer,
-                             black_mask_layer_owner_->layer());
+                             mask_layer_tree_owner_->root());
   }
 
   std::unique_ptr<ScreenRotationAnimation> new_layer_screen_rotation =
@@ -433,7 +443,8 @@ void ScreenRotationAnimator::AnimateRotation(
   observer->SetActive();
 }
 
-void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
+void ScreenRotationAnimator::Rotate(int64_t display_id,

[oshima, 2017/06/01 15:54:59]

Use the display id of the root window instead.

[wutao, 2017/06/01 22:48:04]

Done.

+                                    display::Display::Rotation new_rotation,
                                     display::Display::RotationSource source) {
   // |rotation_request_id_| is used to skip stale requests. Before the layer
   // CopyOutputResult callback called, there could have new rotation request.
@@ -442,7 +453,7 @@ void ScreenRotationAnimator::Rotate(display::Display::Rotation new_rotation,
   // determine the stale status.
   rotation_request_id_++;
   std::unique_ptr<ScreenRotationRequest> rotation_request =
-      base::MakeUnique<ScreenRotationRequest>(rotation_request_id_,
+      base::MakeUnique<ScreenRotationRequest>(rotation_request_id_, display_id,
                                               new_rotation, source);
   switch (screen_rotation_state_) {
     case IDLE:
@@ -470,12 +481,13 @@ void ScreenRotationAnimator::RemoveScreenRotationAnimatorObserver(
 
 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_)) {
+  mask_layer_tree_owner_.reset();
+  if (last_pending_request_ &&
+      IsDisplayIdValid(last_pending_request_->display_id) &&

[oshima, 2017/06/01 15:54:59]

ditto

[wutao, 2017/06/01 22:48:03]

Done.

+      !RootWindowChangedForDisplayId(root_window_,
+                                     last_pending_request_->display_id)) {
     StartRotationAnimation(std::move(last_pending_request_));
     return;
   }
@@ -492,8 +504,7 @@ void ScreenRotationAnimator::StopAnimating() {
     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());
+  mask_layer_tree_owner_.reset();
 }
 
 }  // namespace ash