CC Animations: Make LayerAnimationController creation optional

cc/layers/layer.cc

 // Copyright 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/layers/layer.h"
 
 #include <algorithm>
 
 #include "base/atomic_sequence_num.h"
 #include "base/location.h"
@@ skipping 65 matching lines @@
       scroll_blocks_on_(SCROLL_BLOCKS_ON_NONE),
       background_color_(0),
       opacity_(1.f),
       blend_mode_(SkXfermode::kSrcOver_Mode),
       scroll_parent_(nullptr),
       clip_parent_(nullptr),
       replica_layer_(nullptr),
       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() {
   // Our parent should be holding a reference to us so there should be no
   // way for us to be destroyed while we still have a parent.
   DCHECK(!parent());
   // Similarly we shouldn't have a layer tree host since it also keeps a
   // 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();
 
   // Remove the parent reference from all children and dependents.
   RemoveAllChildren();
   if (mask_layer_.get()) {
     DCHECK_EQ(this, mask_layer_->parent());
     mask_layer_->RemoveFromParent();
   }
@@ skipping 15 matching lines @@
 
   for (size_t i = 0; i < children_.size(); ++i)
     children_[i]->SetLayerTreeHost(host);
 
   if (mask_layer_.get())
     mask_layer_->SetLayerTreeHost(host);
   if (replica_layer_.get())
     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();
   }
 
   if (host && layer_animation_controller_->has_any_animation())
     host->SetNeedsCommit();
 }
 
 void Layer::SetNeedsUpdate() {
   if (layer_tree_host_ && !ignore_set_needs_commit_)
@@ skipping 1026 matching lines @@
 void Layer::OnAnimationWaitingForDeletion() {
   // Animations are only deleted during PushProperties.
   SetNeedsPushProperties();
 }
 
 bool Layer::IsActive() const {
   return true;
 }
 
 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 &&
       !layer_animation_controller_->animation_registrar()
            ->supports_scroll_animations())
     return false;
 
   UMA_HISTOGRAM_BOOLEAN("Renderer.AnimationAddedToOrphanLayer",
                         !layer_tree_host_);
   layer_animation_controller_->AddAnimation(animation.Pass());
@@ skipping 23 matching lines @@
   layer_animation_controller_->RemoveValueObserver(this);
   layer_animation_controller_ = controller;
   layer_animation_controller_->AddValueObserver(this);
   SetNeedsCommit();
 }
 
 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);
 }
 
 SimpleEnclosedRegion Layer::VisibleContentOpaqueRegion() const {
   if (contents_opaque())
     return SimpleEnclosedRegion(visible_content_rect());
   return SimpleEnclosedRegion();
 }
 
 ScrollbarLayerInterface* Layer::ToScrollbarLayer() {
@@ skipping 129 matching lines @@
 
 void Layer::DidBeginTracing() {
   // We'll be dumping layer trees as part of trace, so make sure
   // PushPropertiesTo() propagates layer debug info to the impl
   // side -- otherwise this won't happen for the the layers that
   // remain unchanged since tracing started.
   SetNeedsPushProperties();
 }
 
 }  // namespace cc