Update setting of Compositor on new root Layer

When calling LayerOwner::RecreateLayer() on the root Layer, the Compositor gets
moved to the new layer after all children are added.

If there are ongoing animations while this occurs they are cancelled. If any
animation observer attempts to access the compositor in response it will crash
Chrome as the compositor is null.

Update LayerOwner::RecreateLayer() to set the Compositor on the new root layer
bfore adding children. This also brings it to the same timing as when the
compositor would be set while performing RecreateLayer on non-root layers.

TEST=LayerOwnerTestWithCompositor.RecreateRootLayerDuringAnimation,
compositor_unittests, ash_unittests, views_unittests
BUG=chrome-os-partner:40032

Review URL: https://codereview.chromium.org/1128103003

Cr-Commit-Position: refs/heads/master@{#329446}

ui/compositor/layer_owner.cc

@@ -52,10 +52,14 @@ scoped_ptr<Layer> LayerOwner::RecreateLayer() {
   if (alpha_shape)
     new_layer->SetAlphaShape(make_scoped_ptr(new SkRegion(*alpha_shape)));
 
-  // Install new layer as a sibling of the old layer, stacked below it.
   if (old_layer->parent()) {
+    // Install new layer as a sibling of the old layer, stacked below it.
     old_layer->parent()->Add(new_layer);
     old_layer->parent()->StackBelow(new_layer, old_layer.get());
+  } else if (old_layer->GetCompositor()) {
+    // If old_layer was the layer tree root then we need to move the Compositor
+    // over to the new root.
+    old_layer->GetCompositor()->SetRootLayer(new_layer);
   }
 
   // Migrate all the child layers over to the new layer. Copy the list because
@@ -68,12 +72,6 @@ scoped_ptr<Layer> LayerOwner::RecreateLayer() {
     new_layer->Add(child);
   }
 
-  // If old_layer was the layer tree root then we need to move the Compositor
-  // over to the new root.
-  const bool is_root = !old_layer->parent();
-  if (is_root && old_layer->GetCompositor())
-    old_layer->GetCompositor()->SetRootLayer(new_layer);
-
   // Install the delegate last so that the delegate isn't notified as we copy
   // state to the new layer.
   new_layer->set_delegate(old_delegate);

ui/compositor/layer_owner_unittest.cc

@@ -8,7 +8,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/layer.h"
+#include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/layer_animator.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/compositor/test/context_factories_for_test.h"
 #include "ui/gfx/native_widget_types.h"
@@ -16,6 +18,24 @@
 namespace ui {
 namespace {
 
+// An animation observer that confirms upon animation completion, that the
+// compositor is not null.
+class TestLayerAnimationObserver : public ImplicitAnimationObserver {
+ public:
+  TestLayerAnimationObserver(Layer* layer) : layer_(layer) {}
+  ~TestLayerAnimationObserver() override {}
+
+  // ImplicitAnimationObserver:
+  void OnImplicitAnimationsCompleted() override {
+    EXPECT_NE(nullptr, layer_->GetCompositor());
+  }
+
+ private:
+  Layer* layer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestLayerAnimationObserver);
+};
+
 // Test fixture for LayerOwner tests that require a ui::Compositor.
 class LayerOwnerTestWithCompositor : public testing::Test {
  public:
@@ -106,4 +126,71 @@ TEST_F(LayerOwnerTestWithCompositor, RecreateRootLayerWithCompositor) {
   EXPECT_EQ(nullptr, layer_copy->GetCompositor());
 }
 
+// Tests that recreating the root layer, while one of its children is animating,
+// properly updates the compositor. So that compositor is not null for observers
+// of animations being cancelled.
+TEST_F(LayerOwnerTestWithCompositor, RecreateRootLayerDuringAnimation) {
+  LayerOwner owner;
+  Layer* layer = new Layer;
+  owner.SetLayer(layer);
+  compositor()->SetRootLayer(layer);
+
+  scoped_ptr<Layer> child(new Layer);
+  child->SetBounds(gfx::Rect(0, 0, 100, 100));
+  layer->Add(child.get());
+
+  // This observer checks that the compositor of |child| is not null upon
+  // animation completion.
+  scoped_ptr<TestLayerAnimationObserver> observer(
+      new TestLayerAnimationObserver(child.get()));
+  scoped_ptr<ui::ScopedAnimationDurationScaleMode> long_duration_animation(
+      new ui::ScopedAnimationDurationScaleMode(
+          ui::ScopedAnimationDurationScaleMode::SLOW_DURATION));
+  {
+    ui::ScopedLayerAnimationSettings animation(child->GetAnimator());
+    animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(1000));
+    animation.AddObserver(observer.get());
+    gfx::Transform transform;
+    transform.Scale(0.5f, 0.5f);
+    child->SetTransform(transform);
+  }
+
+  scoped_ptr<Layer> layer_copy = owner.RecreateLayer();
+}
+
+// Tests that recreating a non-root layer, while one of its children is
+// animating, properly updates the compositor. So that compositor is not null
+// for observers of animations being cancelled.
+TEST_F(LayerOwnerTestWithCompositor, RecreateNonRootLayerDuringAnimation) {
+  scoped_ptr<Layer> root_layer(new Layer);
+  compositor()->SetRootLayer(root_layer.get());
+
+  LayerOwner owner;
+  Layer* layer = new Layer;
+  owner.SetLayer(layer);
+  root_layer->Add(layer);
+
+  scoped_ptr<Layer> child(new Layer);
+  child->SetBounds(gfx::Rect(0, 0, 100, 100));
+  layer->Add(child.get());
+
+  // This observer checks that the compositor of |child| is not null upon
+  // animation completion.
+  scoped_ptr<TestLayerAnimationObserver> observer(
+      new TestLayerAnimationObserver(child.get()));
+  scoped_ptr<ui::ScopedAnimationDurationScaleMode> long_duration_animation(
+      new ui::ScopedAnimationDurationScaleMode(
+          ui::ScopedAnimationDurationScaleMode::SLOW_DURATION));
+  {
+    ui::ScopedLayerAnimationSettings animation(child->GetAnimator());
+    animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(1000));
+    animation.AddObserver(observer.get());
+    gfx::Transform transform;
+    transform.Scale(0.5f, 0.5f);
+    child->SetTransform(transform);
+  }
+
+  scoped_ptr<Layer> layer_copy = owner.RecreateLayer();
+}
+
 }  // namespace ui