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..d718f906af1aeebe7ac9f184f07e84a9eb60e420 100644
--- a/ash/rotator/screen_rotation_animator.cc
+++ b/ash/rotator/screen_rotation_animator.cc
@@ -105,6 +105,15 @@ void AddLayerBelowWindowLayer(aura::Window* root_window,
   root_window->layer()->StackBelow(layer, top_layer);
 }
 
+void RemoveLayerFromRootWindowForDisplayId(int64_t display_id,
+                                           ui::Layer* layer) {
+  if (IsDisplayIdValid(display_id)) {
+    ui::Layer* root_layer = GetRootWindow(display_id)->layer();
+    if (root_layer->Contains(layer))
+      root_layer->Remove(layer);
+  }
+}
+
 // The Callback will be invoked when all animation sequences have
 // finished. |observer| will be destroyed after invoking the Callback if it
 // returns true.
@@ -173,9 +182,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() {
@@ -228,7 +234,8 @@ void ScreenRotationAnimator::SetRotation(
   // Allow compositor locks to extend timeout, so that screen rotation only
   // takes output copy after contents are properlly resized, such as wallpaper
   // and ARC apps.
-  ui::Compositor* compositor = root_window_->layer()->GetCompositor();
+  ui::Compositor* compositor =
+      GetRootWindow(display_id_)->layer()->GetCompositor();
   compositor->set_allow_locks_to_extend_timeout(true);
   Shell::Get()->display_manager()->SetDisplayRotation(display_id_, new_rotation,
                                                       source);
@@ -242,9 +249,11 @@ void ScreenRotationAnimator::SetRotation(
 
 void ScreenRotationAnimator::RequestCopyScreenRotationContainerLayer(
     std::unique_ptr<cc::CopyOutputRequest> copy_output_request) {
+  ui::Layer* screen_rotation_container_layer =
+      GetScreenRotationContainer(GetRootWindow(display_id_))->layer();
   copy_output_request->set_area(
-      gfx::Rect(screen_rotation_container_layer_->size()));
-  screen_rotation_container_layer_->RequestCopyOfOutput(
+      gfx::Rect(screen_rotation_container_layer->size()));
+  screen_rotation_container_layer->RequestCopyOfOutput(
       std::move(copy_output_request));
 }
 
@@ -288,7 +297,8 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedBeforeRotation(
   }
 
   old_layer_tree_owner_ = CopyLayerTree(std::move(result));
-  AddLayerAtTopOfWindowLayers(root_window_, old_layer_tree_owner_->root());
+  AddLayerAtTopOfWindowLayers(GetRootWindow(display_id_),
+                              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 =
@@ -313,18 +323,23 @@ void ScreenRotationAnimator::OnScreenRotationContainerLayerCopiedAfterRotation(
     return;
   }
 
+  aura::Window* root_window = GetRootWindow(display_id_);
+  // |root_window| changed while waiting for the copy request.
+  if (!root_window->layer()->Contains(old_layer_tree_owner_->root())) {

[oshima, 2017/05/31 22:37:57]

Hmm, I probably made a bad recommendation. I'm very sorry about that.

It's probably better to create a rotator for root window (then you don't have to
worry about these condition, or invalid display in this class).

In GetScreenRotationAnimatorForDisplay, you can find the root window, and then
create the animator for the root window.

+    ProcessAnimationQueue();
+    return;
+  }
+
   new_layer_tree_owner_ = CopyLayerTree(std::move(result));
-  AddLayerBelowWindowLayer(root_window_, old_layer_tree_owner_->root(),
+  AddLayerBelowWindowLayer(root_window, old_layer_tree_owner_->root(),
                            new_layer_tree_owner_->root());
   AnimateRotation(std::move(rotation_request));
 }
 
 void ScreenRotationAnimator::CreateOldLayerTreeForSlowAnimation() {
-  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());
+  aura::Window* root_window = GetRootWindow(display_id_);
+  old_layer_tree_owner_ = ::wm::RecreateLayers(root_window);
+  AddLayerAtTopOfWindowLayers(root_window, old_layer_tree_owner_->root());
 }
 
 std::unique_ptr<ui::LayerTreeOwner> ScreenRotationAnimator::CopyLayerTree(
@@ -333,8 +348,8 @@ std::unique_ptr<ui::LayerTreeOwner> ScreenRotationAnimator::CopyLayerTree(
   std::unique_ptr<cc::SingleReleaseCallback> release_callback;
   result->TakeTexture(&texture_mailbox, &release_callback);
   DCHECK(texture_mailbox.IsTexture());
-
-  const gfx::Rect rect(screen_rotation_container_layer_->size());
+  const gfx::Rect rect(
+      GetScreenRotationContainer(GetRootWindow(display_id_))->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),
@@ -344,6 +359,10 @@ std::unique_ptr<ui::LayerTreeOwner> ScreenRotationAnimator::CopyLayerTree(
 
 void ScreenRotationAnimator::AnimateRotation(
     std::unique_ptr<ScreenRotationRequest> rotation_request) {
+  aura::Window* root_window = GetRootWindow(display_id_);
+  ui::Layer* screen_rotation_container_layer =
+      GetScreenRotationContainer(root_window)->layer();
+
   screen_rotation_state_ = ROTATING;
   const int rotation_factor = GetRotationFactor(rotation_request->old_rotation,
                                                 rotation_request->new_rotation);
@@ -352,20 +371,20 @@ void ScreenRotationAnimator::AnimateRotation(
   const base::TimeDelta duration =
       base::TimeDelta::FromMilliseconds(kRotationDurationInMs);
   const gfx::Tween::Type tween_type = gfx::Tween::FAST_OUT_LINEAR_IN;
-  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* new_root_layer;
   if (!new_layer_tree_owner_ ||
       has_switch_ash_disable_smooth_screen_rotation_) {
-    new_root_layer = screen_rotation_container_layer_;
+    new_root_layer = screen_rotation_container_layer;
   } else {
     new_root_layer = new_layer_tree_owner_->root();
-    // Add a black mask layer on top of |screen_rotation_container_layer_|.
+    // 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,
+        gfx::Rect(screen_rotation_container_layer->size()));
+    AddLayerBelowWindowLayer(root_window, new_root_layer,
                              black_mask_layer_owner_->layer());
   }
 
@@ -470,8 +489,10 @@ 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());
+  if (black_mask_layer_owner_) {
+    RemoveLayerFromRootWindowForDisplayId(display_id_,
+                                          black_mask_layer_owner_->layer());
+  }
   old_layer_tree_owner_.reset();
   new_layer_tree_owner_.reset();
   black_mask_layer_owner_.reset();
@@ -492,8 +513,10 @@ 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());
+  if (black_mask_layer_owner_) {
+    RemoveLayerFromRootWindowForDisplayId(display_id_,
+                                          black_mask_layer_owner_->layer());
+  }
 }
 
 }  // namespace ash