CC Animations: Make LayerAnimationController creation optional

cc/layers/layer.cc

Index: cc/layers/layer.cc
diff --git a/cc/layers/layer.cc b/cc/layers/layer.cc
index 938449cb968e2e57ac3e792c1c4d9d6ddd1c548b..70e4de328f20439968410f972d645ca07cfe8abb 100644
--- a/cc/layers/layer.cc
+++ b/cc/layers/layer.cc
@@ -83,9 +83,6 @@ Layer::Layer()
       raster_scale_(0.f),
       client_(nullptr),
       frame_timing_requests_dirty_(false) {
-  layer_animation_controller_ = LayerAnimationController::Create(layer_id_);
-  layer_animation_controller_->AddValueObserver(this);
-  layer_animation_controller_->set_value_provider(this);
 }
 
 Layer::~Layer() {
@@ -96,8 +93,10 @@ Layer::~Layer() {
   // reference to us.
   DCHECK(!layer_tree_host());
 
-  layer_animation_controller_->RemoveValueObserver(this);
-  layer_animation_controller_->remove_value_provider(this);
+  if (layer_animation_controller_) {
+    layer_animation_controller_->RemoveValueObserver(this);
+    layer_animation_controller_->remove_value_provider(this);
+  }
 
   RemoveFromScrollTree();
   RemoveFromClipTree();
@@ -133,9 +132,7 @@ void Layer::SetLayerTreeHost(LayerTreeHost* host) {
     replica_layer_->SetLayerTreeHost(host);
 
   if (host) {
-    layer_animation_controller_->SetAnimationRegistrar(
-        host->animation_registrar());
-
+    RegisterForAnimations(host->animation_registrar(), host->settings());
     if (host->settings().layer_transforms_should_scale_layer_contents)
       reset_raster_scale_to_unknown();
   }
@@ -1182,7 +1179,8 @@ bool Layer::IsActive() const {
 }
 
 bool Layer::AddAnimation(scoped_ptr <Animation> animation) {
-  if (!layer_animation_controller_->animation_registrar())
+  if (!layer_animation_controller_ ||
+      !layer_animation_controller_->animation_registrar())
     return false;
 
   if (animation->target_property() == Animation::SCROLL_OFFSET &&
@@ -1226,13 +1224,26 @@ bool Layer::HasActiveAnimation() const {
   return layer_animation_controller_->HasActiveAnimation();
 }
 
+void Layer::RegisterForAnimations(AnimationRegistrar* registrar,
+                                  const LayerTreeSettings& settings) {
+  if (!layer_animation_controller_) {
+    layer_animation_controller_ = LayerAnimationController::Create(layer_id_);
+    layer_animation_controller_->AddValueObserver(this);
+    layer_animation_controller_->set_value_provider(this);
+  }
+
+  layer_animation_controller_->SetAnimationRegistrar(registrar);
+}
+
 void Layer::AddLayerAnimationEventObserver(
     LayerAnimationEventObserver* animation_observer) {
+  DCHECK(layer_animation_controller_);

[loyso (OOO), 2015/04/23 08:26:33]

webkit_unit_tests are failing. I'm considering to accumulate all
observers/delegates and pass them to LAC object on it's creation in
RegisterForAnimations.

[ajuma, 2015/04/23 15:09:15]

Hmm, these failures suggest that WebViewImpl::registerForAnimation is getting
called while m_layerTreeView is null (so that registerForAnimation is earlying
out). If this is correct, and I'm reading WebViewImpl correctly, this can only
happen after WebViewImpl::willCloseLayerTreeView has been called, and there's no
way to later have a non-null m_layerTreeView. So another option might be to
change this DCHECK (and the one in RemoveLayerAnimationEventObserver below) to
an 'if', but this seems fragile. Your suggestion sounds like a better approach.

[loyso (OOO), 2015/04/24 03:44:52]

Well, not really. We had a bunch of problems here:
1) LinkHighlight creates it's own content layer w/o registering it for
animations. See my fix and further explanations here:
https://codereview.chromium.org/1089043003/

2) We use WebLayerTreeViewImplForTesting for m_layerTreeView.
registerForAnimations was not plumbed to LayerTreeHost in that implementations.
See my fix in the upcoming PatchSet.

3) Some times we destroy layers without calling registerForAnimations or
setAnimationDelegate at all. Also see my fix in the upcoming PatchSet.

Very interesting :) It's cool that we identified this sub-task as a separate CL
and we isolate risks.

   layer_animation_controller_->AddEventObserver(animation_observer);
 }
 
 void Layer::RemoveLayerAnimationEventObserver(
     LayerAnimationEventObserver* animation_observer) {
+  DCHECK(layer_animation_controller_);
   layer_animation_controller_->RemoveEventObserver(animation_observer);
 }