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..90a83c2271763d9e3bb8aff8b190434f1034810c 100644
--- a/ui/compositor/layer.cc
+++ b/ui/compositor/layer.cc
@@ -26,6 +26,7 @@
 #include "cc/output/filter_operation.h"
 #include "cc/output/filter_operations.h"
 #include "cc/resources/transferable_resource.h"
+#include "cc/trees/layer_tree_host.h"
 #include "ui/compositor/compositor_switches.h"
 #include "ui/compositor/dip_util.h"
 #include "ui/compositor/layer_animator.h"
@@ -124,7 +125,9 @@ 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);
+
+  DetachAnimationObservers();
+
   cc_layer_->RemoveFromParent();
 }
 
@@ -142,21 +145,33 @@ float Layer::opacity() const {
 }
 
 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);
+  // 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_);
+
+  compositor_ = compositor;
+  OnDeviceScaleFactorChanged(compositor->device_scale_factor());
+  AddAnimatorsInTreeToCollection(compositor_->layer_animator_collection());
+}
+
+void Layer::SetCompositorRootCCLayer(scoped_refptr<cc::Layer> root_layer) {
+  // This function must only be called on the root ui layer.
+  root_layer->AddChild(cc_layer_);
+  AttachAnimationObservers();
+  SendPendingThreadedAnimations();
+}
+
+void Layer::ResetCompositor() {
   DCHECK(!parent_);
   if (compositor_) {
+    DetachAnimationObservers();
     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) {
@@ -167,8 +182,11 @@ void Layer::Add(Layer* child) {
   children_.push_back(child);
   cc_layer_->AddChild(child->cc_layer_);
   child->OnDeviceScaleFactorChanged(device_scale_factor_);
-  if (GetCompositor())
+  Compositor* compositor = GetCompositor();
+  if (compositor) {
+    child->AttachAnimationObservers();
     child->SendPendingThreadedAnimations();
+  }
   LayerAnimatorCollection* collection = GetLayerAnimatorCollection();
   if (collection)
     child->AddAnimatorsInTreeToCollection(collection);
@@ -184,6 +202,8 @@ void Layer::Remove(Layer* child) {
   if (collection)
     child->RemoveAnimatorsInTreeFromCollection(collection);
 
+  child->DetachAnimationObservers();
+
   std::vector<Layer*>::iterator i =
       std::find(children_.begin(), children_.end(), child);
   DCHECK(i != children_.end());
@@ -337,8 +357,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) {
@@ -490,7 +509,9 @@ 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);
+
+  DetachAnimationObservers();
+
   new_layer->SetOpacity(cc_layer_->opacity());
   new_layer->SetTransform(cc_layer_->transform());
   new_layer->SetPosition(cc_layer_->position());
@@ -503,7 +524,8 @@ void Layer::SwitchToLayer(scoped_refptr<cc::Layer> new_layer) {
   delegated_renderer_layer_ = NULL;
   surface_layer_ = NULL;
 
-  cc_layer_->AddLayerAnimationEventObserver(this);
+  AttachAnimationObservers();
+
   for (size_t i = 0; i < children_.size(); ++i) {
     DCHECK(children_[i]->cc_layer_);
     cc_layer_->AddChild(children_[i]->cc_layer_);
@@ -1034,11 +1056,26 @@ 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();
 }
 
+void Layer::AttachAnimationObservers() {
+  if (cc_layer_->layer_animation_controller())

[ajuma, 2015/04/27 14:51:56]

It's probably better to move this null check to
cc::Layer::AddLayerAnimationEventObserver.

[loyso (OOO), 2015/04/28 05:10:36]

This is the only place in the project where
cc::Layer::AddLayerAnimationEventObserver is called.
And you mentioned that it would be fragile.
In that case a subscriber can subscribe and don't get any notification. That
would be a bug which is hard to track.

[loyso (OOO), 2015/04/28 05:23:13]

May we put a DCHECK here?

[ajuma, 2015/04/28 14:36:04]

If a DCHECK works, that'd be great. If not, perhaps add a TODO to fix this when
we switch over to the new system (e.g. so that either the call to
AddLayerAnimationEventObserver returns false when it fails, or it  always
succeeds).

+    cc_layer_->AddLayerAnimationEventObserver(this);
+
+  for (size_t i = 0; i < children_.size(); ++i)
+    children_[i]->AttachAnimationObservers();
+}
+
+void Layer::DetachAnimationObservers() {
+  if (cc_layer_->layer_animation_controller())
+    cc_layer_->RemoveLayerAnimationEventObserver(this);
+
+  for (size_t i = 0; i < children_.size(); ++i)
+    children_[i]->DetachAnimationObservers();
+}
+
 gfx::Transform Layer::transform() const {
   return cc_layer_->transform();
 }