CC Animations: Make LayerAnimationController creation optional

ui/compositor/layer.cc

Index: ui/compositor/layer.cc
diff --git a/ui/compositor/layer.cc b/ui/compositor/layer.cc
index 2b6f9219c574b40c1bdd6d6e0742c0d47d0a1478..f0f12df9d52219e9f08cbb24990db064e6751d54 100644
--- a/ui/compositor/layer.cc
+++ b/ui/compositor/layer.cc
@@ -124,7 +124,10 @@ Layer::~Layer() {
     layer_mask_back_link_->SetMaskLayer(NULL);
   for (size_t i = 0; i < children_.size(); ++i)
     children_[i]->parent_ = NULL;
-  cc_layer_->RemoveLayerAnimationEventObserver(this);
+
+  if (cc_layer_->layer_animation_controller())
+    cc_layer_->RemoveLayerAnimationEventObserver(this);
+
   cc_layer_->RemoveFromParent();
 }
 
@@ -141,22 +144,29 @@ float Layer::opacity() const {
   return cc_layer_->opacity();
 }
 
-void Layer::SetCompositor(Compositor* compositor) {
-  // This function must only be called to set the compositor on the root layer,
-  // or to reset it.
-  DCHECK(!compositor || !compositor_);
-  DCHECK(!compositor || compositor->root_layer() == this);
+void Layer::SetCompositor(Compositor* compositor,
+                          scoped_refptr<cc::Layer> root_layer) {
+  // This function must only be called to set the compositor on the root ui
+  // layer.
+  DCHECK(compositor);
+  DCHECK(!compositor_);
+  DCHECK(compositor->root_layer() == this);
   DCHECK(!parent_);
-  if (compositor_) {
+
+  compositor_ = compositor;
+  OnDeviceScaleFactorChanged(compositor->device_scale_factor());
+  AddAnimatorsInTreeToCollection(compositor_->layer_animator_collection());
+
+  root_layer->AddChild(cc_layer_);
+  SendPendingThreadedAnimations();
+}
+
+void Layer::ResetCompositor() {
+  DCHECK(!parent_);
+  if (compositor_)
     RemoveAnimatorsInTreeFromCollection(
         compositor_->layer_animator_collection());
-  }
-  compositor_ = compositor;
-  if (compositor) {
-    OnDeviceScaleFactorChanged(compositor->device_scale_factor());
-    SendPendingThreadedAnimations();
-    AddAnimatorsInTreeToCollection(compositor_->layer_animator_collection());
-  }
+  compositor_ = nullptr;
 }
 
 void Layer::Add(Layer* child) {
@@ -337,8 +347,7 @@ void Layer::SetMaskLayer(Layer* layer_mask) {
   if (layer_mask_)
     layer_mask_->layer_mask_back_link_ = NULL;
   layer_mask_ = layer_mask;
-  cc_layer_->SetMaskLayer(
-      layer_mask ? layer_mask->cc_layer() : NULL);
+  cc_layer_->SetMaskLayer(layer_mask ? layer_mask->cc_layer_ : NULL);
   // We need to reference the linked object so that it can properly break the
   // link to us when it gets deleted.
   if (layer_mask) {
@@ -475,6 +484,9 @@ void Layer::SetFillsBoundsCompletely(bool fills_bounds_completely) {
 }
 
 void Layer::SwitchToLayer(scoped_refptr<cc::Layer> new_layer) {
+  DCHECK(!new_layer->parent());
+  DCHECK(!new_layer->layer_animation_controller());
+
   // Finish animations being handled by cc_layer_.
   if (animator_.get()) {
     animator_->StopAnimatingProperty(LayerAnimationElement::TRANSFORM);
@@ -490,7 +502,8 @@ void Layer::SwitchToLayer(scoped_refptr<cc::Layer> new_layer) {
     cc_layer_->parent()->ReplaceChild(cc_layer_, new_layer);
   }
   cc_layer_->SetLayerClient(NULL);
-  cc_layer_->RemoveLayerAnimationEventObserver(this);
+  if (cc_layer_->layer_animation_controller())
+    cc_layer_->RemoveLayerAnimationEventObserver(this);
   new_layer->SetOpacity(cc_layer_->opacity());
   new_layer->SetTransform(cc_layer_->transform());
   new_layer->SetPosition(cc_layer_->position());
@@ -503,7 +516,9 @@ void Layer::SwitchToLayer(scoped_refptr<cc::Layer> new_layer) {
   delegated_renderer_layer_ = NULL;
   surface_layer_ = NULL;
 
-  cc_layer_->AddLayerAnimationEventObserver(this);
+  if (cc_layer_->layer_animation_controller())
+    cc_layer_->AddLayerAnimationEventObserver(this);
+
   for (size_t i = 0; i < children_.size(); ++i) {
     DCHECK(children_[i]->cc_layer_);
     cc_layer_->AddChild(children_[i]->cc_layer_);
@@ -1005,6 +1020,9 @@ LayerAnimatorCollection* Layer::GetLayerAnimatorCollection() {
 }
 
 void Layer::SendPendingThreadedAnimations() {
+  DCHECK(cc_layer_->layer_animation_controller());

[piman, 2015/04/30 00:40:38]

Is this DCHECK valid? SendPendingThreadedAnimations is called any time the layer
is added to the tree, regardless of animation controllers.

[loyso (OOO), 2015/04/30 01:03:04]

SendPendingThreadedAnimations is only called after AddChild so LayerTreeHost /
RegisterForAnimations are set up.

SendPendingThreadedAnimations requires LAC anyway. It adds animations into LAC.

In fact, we can remove it because we have the same one in
cc_layer_->AddLayerAnimationEventObserver.

[loyso (OOO), 2015/04/30 01:04:03]

* Remove - I meant duplicate DCHECK.

+  cc_layer_->AddLayerAnimationEventObserver(this);

[piman, 2015/04/30 00:40:38]

Is this idempotent? SendPendingThreadedAnimations is called when we add the
layer to the tree, but we don't call RemoveLayerAnimationEventObserver when we
remove from the tree.

What is the invariant wrt the AnimationEventObserver?

Can we add unit tests to verify that?

[loyso (OOO), 2015/04/30 01:03:04]

Yes, it's idempotent. Apart from SwithToLayer case, we detach ui::Layer from
cc::Layer in ui::Layer destructor only.

In general, the idea is simple. We defer AddLayerAnimationEventObserver from
CreateCCLayer call until SetCompositor OR Add cases. That's it.

"What is the invariant wrt the AnimationEventObserver?"
Sorry, I haven't caught the question.

[loyso (OOO), 2015/05/01 03:46:21]

We call SendPendingThreadedAnimations at two places: Layer::SetCompositor and
Layer::Add.
1) In this CL it's clearly seen that in Layer::SetCompositor we always have
LTHs/LACs for the whole subtree prepared (root_web_layer always has a host set
and we add root ui layer into root_webLayer in the line right before the
SendPendingThreadedAnimations call).
2) In Layer::Add we skip SendPendingThreadedAnimations if Layer is orphaned.
Once we add the orphaned sub-tree into a layer with Compositor+LTH+LAC set, we
call SendPendingThreadedAnimations recursively for the whole previously orphaned
subtree.

+
   for (cc::ScopedPtrVector<cc::Animation>::iterator it =
            pending_threaded_animations_.begin();
        it != pending_threaded_animations_.end();
@@ -1034,7 +1052,6 @@ void Layer::CreateCcLayer() {
   cc_layer_->SetTransformOrigin(gfx::Point3F());
   cc_layer_->SetContentsOpaque(true);
   cc_layer_->SetIsDrawable(type_ != LAYER_NOT_DRAWN);
-  cc_layer_->AddLayerAnimationEventObserver(this);
   cc_layer_->SetLayerClient(this);
   RecomputePosition();
 }