Refactor MutatorHostClient from LayerTreeHost to LayerTree.

cc/trees/layer_tree_host.cc

Index: cc/trees/layer_tree_host.cc
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc
index 8e0493a8c1af61f5f87ab840f4ae5be1a468df18..6d3edc4e1831a48ab4e01815cb2cfe5a80f59002 100644
--- a/cc/trees/layer_tree_host.cc
+++ b/cc/trees/layer_tree_host.cc
@@ -213,6 +213,8 @@ std::unique_ptr<LayerTreeHost> LayerTreeHost::CreateRemoteClient(
 
 LayerTreeHost::LayerTreeHost(InitParams* params, CompositorMode mode)
     : micro_benchmark_controller_(this),
+      layer_tree_(base::WrapUnique(
+          new LayerTree(std::move(params->animation_host), this))),
       next_ui_resource_id_(1),
       compositor_mode_(mode),
       needs_full_tree_sync_(true),
@@ -247,12 +249,9 @@ LayerTreeHost::LayerTreeHost(InitParams* params, CompositorMode mode)
       task_graph_runner_(params->task_graph_runner),
       image_serialization_processor_(params->image_serialization_processor),
       surface_client_id_(0u),
-      next_surface_sequence_(1u),
-      layer_tree_(std::move(params->animation_host)) {
+      next_surface_sequence_(1u) {
   DCHECK(task_graph_runner_);
 
-  layer_tree_.animation_host()->SetMutatorHostClient(this);
-
   rendering_stats_instrumentation_->set_record_rendering_stats(
       debug_state_.RecordRenderingStats());
 }
@@ -362,15 +361,13 @@ void LayerTreeHost::InitializeProxy(
   proxy_ = std::move(proxy);
   proxy_->Start(std::move(external_begin_frame_source));
 
-  layer_tree_.animation_host()->SetSupportsScrollAnimations(
+  layer_tree_->animation_host()->SetSupportsScrollAnimations(

[loyso (OOO), 2016/08/16 00:21:17]

Notice that LayerTreeHost still deals with layer_tree_->animation_host() calls
so the logic is scattered. But I can't suggest anything (apart from creating
wrapper functions in LayerTree to hide animation_host() OR passing bool to
LayerTree ctor for this case.

[xingliu, 2016/08/16 02:25:25]

I agree in general this breaks the symmetric, but since we don't want to copy
code between different implementations of LTH (base class are also not preferred
according to some previous discussion), it makes some sense to put animation
host inside LayerTree.

Also the impl side doesn't need to abstract out interface to control commit
behavior, so the general structure on the big pictures of two sides has
diverged.

@Khushal, please feel free to provide thoughts on this.

       proxy_->SupportsImplScrolling());
 }
 
 LayerTreeHost::~LayerTreeHost() {
   TRACE_EVENT0("cc", "LayerTreeHost::~LayerTreeHost");
 
-  layer_tree_.animation_host()->SetMutatorHostClient(nullptr);
-
   if (root_layer_.get())
     root_layer_->SetLayerTreeHost(NULL);
 
@@ -438,7 +435,7 @@ void LayerTreeHost::FinishCommitOnImplThread(LayerTreeHostImpl* host_impl) {
       frame_viewer_instrumentation::IsTracingLayerTreeSnapshots() &&
       root_layer()) {
     LayerTreeHostCommon::CallFunctionForEveryLayer(
-        this, [](Layer* layer) { layer->DidBeginTracing(); });
+        layer_tree_.get(), [](Layer* layer) { layer->DidBeginTracing(); });
   }
 
   LayerTreeImpl* sync_tree = host_impl->sync_tree();
@@ -547,7 +544,7 @@ void LayerTreeHost::FinishCommitOnImplThread(LayerTreeHostImpl* host_impl) {
   {
     TRACE_EVENT0("cc", "LayerTreeHost::PushProperties");
 
-    TreeSynchronizer::PushLayerProperties(&layer_tree_, sync_tree);
+    TreeSynchronizer::PushLayerProperties(layer_tree_.get(), sync_tree);
 
     // This must happen after synchronizing property trees and after push
     // properties, which updates property tree indices, but before animation
@@ -558,7 +555,8 @@ void LayerTreeHost::FinishCommitOnImplThread(LayerTreeHostImpl* host_impl) {
 
     TRACE_EVENT0("cc", "LayerTreeHost::AnimationHost::PushProperties");
     DCHECK(host_impl->animation_host());
-    layer_tree_.animation_host()->PushPropertiesTo(host_impl->animation_host());
+    layer_tree_->animation_host()->PushPropertiesTo(

[loyso (OOO), 2016/08/16 00:21:17]

and here.

[loyso (OOO), 2016/08/16 00:28:05]

And if you add LayerTree::PushProperties (as in other entities) then there is q
question, what would the argument would be. Right now it must be LTHI (which
holds AnimationHost impl instance).
That's what I meant by symmetry.

[xingliu, 2016/08/16 02:25:25]

For PushPropertiesTo, there is another refactoring CL which will create wrapper
in LayerTree.(please refer to https://codereview.chromium.org/2183403002/ ).

+        host_impl->animation_host());
   }
 
   // This must happen after synchronizing property trees and after pushing
@@ -645,7 +643,8 @@ std::unique_ptr<LayerTreeHostImpl> LayerTreeHost::CreateLayerTreeHostImpl(
 
   const bool supports_impl_scrolling = task_runner_provider_->HasImplThread();
   std::unique_ptr<AnimationHost> animation_host_impl =
-      layer_tree_.animation_host()->CreateImplInstance(supports_impl_scrolling);
+      layer_tree_->animation_host()->CreateImplInstance(

[loyso (OOO), 2016/08/16 00:21:17]

Same here. This is the logic how to pass AnimationHost impl to LTHI.

+          supports_impl_scrolling);
 
   std::unique_ptr<LayerTreeHostImpl> host_impl = LayerTreeHostImpl::Create(
       settings_, client, task_runner_provider_.get(),
@@ -677,11 +676,6 @@ void LayerTreeHost::SetDeferCommits(bool defer_commits) {
   proxy_->SetDeferCommits(defer_commits);
 }
 
-void LayerTreeHost::SetNeedsDisplayOnAllLayers() {
-  for (auto* layer : *this)
-    layer->SetNeedsDisplay();
-}
-
 const RendererCapabilities& LayerTreeHost::GetRendererCapabilities() const {
   return proxy_->GetRendererCapabilities();
 }
@@ -746,7 +740,7 @@ void LayerTreeHost::SetNextCommitForcesRedraw() {
 void LayerTreeHost::SetAnimationEvents(
     std::unique_ptr<AnimationEvents> events) {
   DCHECK(task_runner_provider_->IsMainThread());
-  layer_tree_.animation_host()->SetAnimationEvents(std::move(events));
+  layer_tree_->animation_host()->SetAnimationEvents(std::move(events));

[loyso (OOO), 2016/08/16 00:21:17]

.. and here.

 }
 
 void LayerTreeHost::SetRootLayer(scoped_refptr<Layer> root_layer) {
@@ -916,22 +910,6 @@ bool LayerTreeHost::UpdateLayers() {
   return result || next_commit_forces_redraw_;
 }
 
-LayerListIterator<Layer> LayerTreeHost::begin() const {
-  return LayerListIterator<Layer>(root_layer_.get());
-}
-
-LayerListIterator<Layer> LayerTreeHost::end() const {
-  return LayerListIterator<Layer>(nullptr);
-}
-
-LayerListReverseIterator<Layer> LayerTreeHost::rbegin() {
-  return LayerListReverseIterator<Layer>(root_layer_.get());
-}
-
-LayerListReverseIterator<Layer> LayerTreeHost::rend() {
-  return LayerListReverseIterator<Layer>(nullptr);
-}
-
 void LayerTreeHost::DidCompletePageScaleAnimation() {
   did_complete_scale_animation_ = true;
 }
@@ -973,14 +951,6 @@ void LayerTreeHost::BuildPropertyTreesForTesting() {
       gfx::Rect(device_viewport_size_), identity_transform, &property_trees_);
 }
 
-static void SetElementIdForTesting(Layer* layer) {
-  layer->SetElementId(LayerIdToElementIdForTesting(layer->id()));
-}
-
-void LayerTreeHost::SetElementIdsForTesting() {
-  LayerTreeHostCommon::CallFunctionForEveryLayer(this, SetElementIdForTesting);
-}
-
 bool LayerTreeHost::UsingSharedMemoryResources() {
   return GetRendererCapabilities().using_shared_memory_resources;
 }
@@ -1034,16 +1004,16 @@ bool LayerTreeHost::DoUpdateLayers(Layer* root_layer) {
     draw_property_utils::UpdatePropertyTrees(&property_trees_,
                                              can_render_to_separate_surface);
     draw_property_utils::FindLayersThatNeedUpdates(
-        this, property_trees_.transform_tree, property_trees_.effect_tree,
-        &update_layer_list);
+        layer_tree_.get(), property_trees_.transform_tree,
+        property_trees_.effect_tree, &update_layer_list);
   }
 
   for (const auto& layer : update_layer_list)
     layer->SavePaintProperties();
 
   bool content_is_suitable_for_gpu = true;
-  bool did_paint_content =
-      layer_tree_.UpdateLayers(update_layer_list, &content_is_suitable_for_gpu);
+  bool did_paint_content = layer_tree_->UpdateLayers(
+      update_layer_list, &content_is_suitable_for_gpu);
 
   if (content_is_suitable_for_gpu) {
     ++num_consecutive_frames_suitable_for_gpu_;
@@ -1100,7 +1070,7 @@ void LayerTreeHost::ApplyScrollAndScale(ScrollAndScaleSet* info) {
 
   if (root_layer_.get()) {
     for (size_t i = 0; i < info->scrolls.size(); ++i) {
-      Layer* layer = LayerById(info->scrolls[i].layer_id);
+      Layer* layer = layer_tree_->LayerById(info->scrolls[i].layer_id);
       if (!layer)
         continue;
       layer->SetScrollOffsetFromImplSide(gfx::ScrollOffsetWithDelta(
@@ -1142,7 +1112,7 @@ void LayerTreeHost::UpdateTopControlsState(TopControlsState constraints,
 }
 
 void LayerTreeHost::AnimateLayers(base::TimeTicks monotonic_time) {
-  AnimationHost* animation_host = layer_tree_.animation_host();
+  AnimationHost* animation_host = layer_tree_->animation_host();

[loyso (OOO), 2016/08/16 00:21:17]

Same here. Move AnimateLayers method to LayerTree?

   std::unique_ptr<AnimationEvents> events = animation_host->CreateEvents();
 
   if (animation_host->AnimateLayers(monotonic_time))
@@ -1310,149 +1280,6 @@ void LayerTreeHost::SetLayerTreeMutator(
   proxy_->SetMutator(std::move(mutator));
 }
 
-Layer* LayerTreeHost::LayerById(int id) const {
-  return layer_tree_.LayerById(id);
-}
-
-Layer* LayerTreeHost::LayerByElementId(ElementId element_id) const {
-  ElementLayersMap::const_iterator iter = element_layers_map_.find(element_id);
-  return iter != element_layers_map_.end() ? iter->second : nullptr;
-}
-
-void LayerTreeHost::AddToElementMap(Layer* layer) {
-  if (!layer->element_id())
-    return;
-
-  element_layers_map_[layer->element_id()] = layer;
-}
-
-void LayerTreeHost::RemoveFromElementMap(Layer* layer) {
-  if (!layer->element_id())
-    return;
-
-  element_layers_map_.erase(layer->element_id());
-}
-
-bool LayerTreeHost::IsElementInList(ElementId element_id,
-                                    ElementListType list_type) const {
-  return list_type == ElementListType::ACTIVE && LayerByElementId(element_id);
-}
-
-void LayerTreeHost::SetMutatorsNeedCommit() {
-  SetNeedsCommit();
-}
-
-void LayerTreeHost::SetMutatorsNeedRebuildPropertyTrees() {
-  property_trees_.needs_rebuild = true;
-}
-
-void LayerTreeHost::SetElementFilterMutated(ElementId element_id,
-                                            ElementListType list_type,
-                                            const FilterOperations& filters) {
-  Layer* layer = LayerByElementId(element_id);
-  DCHECK(layer);
-  layer->OnFilterAnimated(filters);
-}
-
-void LayerTreeHost::SetElementOpacityMutated(ElementId element_id,
-                                             ElementListType list_type,
-                                             float opacity) {
-  Layer* layer = LayerByElementId(element_id);
-  DCHECK(layer);
-  layer->OnOpacityAnimated(opacity);
-}
-
-void LayerTreeHost::SetElementTransformMutated(
-    ElementId element_id,
-    ElementListType list_type,
-    const gfx::Transform& transform) {
-  Layer* layer = LayerByElementId(element_id);
-  DCHECK(layer);
-  layer->OnTransformAnimated(transform);
-}
-
-void LayerTreeHost::SetElementScrollOffsetMutated(
-    ElementId element_id,
-    ElementListType list_type,
-    const gfx::ScrollOffset& scroll_offset) {
-  Layer* layer = LayerByElementId(element_id);
-  DCHECK(layer);
-  layer->OnScrollOffsetAnimated(scroll_offset);
-}
-
-void LayerTreeHost::ElementTransformIsAnimatingChanged(
-    ElementId element_id,
-    ElementListType list_type,
-    AnimationChangeType change_type,
-    bool is_animating) {
-  Layer* layer = LayerByElementId(element_id);
-  if (layer) {
-    switch (change_type) {
-      case AnimationChangeType::POTENTIAL:
-        layer->OnTransformIsPotentiallyAnimatingChanged(is_animating);
-        break;
-      case AnimationChangeType::RUNNING:
-        layer->OnTransformIsCurrentlyAnimatingChanged(is_animating);
-        break;
-      case AnimationChangeType::BOTH:
-        layer->OnTransformIsPotentiallyAnimatingChanged(is_animating);
-        layer->OnTransformIsCurrentlyAnimatingChanged(is_animating);
-        break;
-    }
-  }
-}
-
-void LayerTreeHost::ElementOpacityIsAnimatingChanged(
-    ElementId element_id,
-    ElementListType list_type,
-    AnimationChangeType change_type,
-    bool is_animating) {
-  Layer* layer = LayerByElementId(element_id);
-  if (layer) {
-    switch (change_type) {
-      case AnimationChangeType::POTENTIAL:
-        layer->OnOpacityIsPotentiallyAnimatingChanged(is_animating);
-        break;
-      case AnimationChangeType::RUNNING:
-        layer->OnOpacityIsCurrentlyAnimatingChanged(is_animating);
-        break;
-      case AnimationChangeType::BOTH:
-        layer->OnOpacityIsPotentiallyAnimatingChanged(is_animating);
-        layer->OnOpacityIsCurrentlyAnimatingChanged(is_animating);
-        break;
-    }
-  }
-}
-
-void LayerTreeHost::ElementFilterIsAnimatingChanged(
-    ElementId element_id,
-    ElementListType list_type,
-    AnimationChangeType change_type,
-    bool is_animating) {
-  Layer* layer = LayerByElementId(element_id);
-  if (layer) {
-    switch (change_type) {
-      case AnimationChangeType::POTENTIAL:
-        layer->OnFilterIsPotentiallyAnimatingChanged(is_animating);
-        break;
-      case AnimationChangeType::RUNNING:
-        layer->OnFilterIsCurrentlyAnimatingChanged(is_animating);
-        break;
-      case AnimationChangeType::BOTH:
-        layer->OnFilterIsPotentiallyAnimatingChanged(is_animating);
-        layer->OnFilterIsCurrentlyAnimatingChanged(is_animating);
-        break;
-    }
-  }
-}
-
-gfx::ScrollOffset LayerTreeHost::GetScrollOffsetForAnimation(
-    ElementId element_id) const {
-  Layer* layer = LayerByElementId(element_id);
-  DCHECK(layer);
-  return layer->ScrollOffsetForAnimation();
-}
-
 bool LayerTreeHost::IsSingleThreaded() const {
   DCHECK(compositor_mode_ != CompositorMode::SINGLE_THREADED ||
          !task_runner_provider_->HasImplThread());
@@ -1514,7 +1341,7 @@ void LayerTreeHost::ToProtobufForCommit(
   // Serialize the LayerTree before serializing the properties. During layer
   // property serialization, we clear the list |layer_that_should_properties_|
   // from the LayerTree.
-  layer_tree_.ToProtobuf(proto->mutable_layer_tree());
+  layer_tree_->ToProtobuf(proto->mutable_layer_tree());
 
   LayerProtoConverter::SerializeLayerProperties(this,
                                                 proto->mutable_layer_updates());
@@ -1592,7 +1419,7 @@ void LayerTreeHost::FromProtobufForCommit(const proto::LayerTreeHost& proto) {
     root_layer_ = new_root_layer;
   }
 
-  layer_tree_.FromProtobuf(proto.layer_tree());
+  layer_tree_->FromProtobuf(proto.layer_tree());
 
   // Ensure ClientPictureCache contains all the necessary SkPictures before
   // deserializing the properties.
@@ -1639,19 +1466,19 @@ void LayerTreeHost::FromProtobufForCommit(const proto::LayerTreeHost& proto) {
   id_ = proto.id();
   next_commit_forces_redraw_ = proto.next_commit_forces_redraw();
 
-  hud_layer_ = static_cast<HeadsUpDisplayLayer*>(
-      UpdateAndGetLayer(hud_layer_.get(), proto.hud_layer_id(), &layer_tree_));
-  overscroll_elasticity_layer_ =
-      UpdateAndGetLayer(overscroll_elasticity_layer_.get(),
-                        proto.overscroll_elasticity_layer_id(), &layer_tree_);
+  hud_layer_ = static_cast<HeadsUpDisplayLayer*>(UpdateAndGetLayer(
+      hud_layer_.get(), proto.hud_layer_id(), layer_tree_.get()));
+  overscroll_elasticity_layer_ = UpdateAndGetLayer(
+      overscroll_elasticity_layer_.get(),
+      proto.overscroll_elasticity_layer_id(), layer_tree_.get());
   page_scale_layer_ = UpdateAndGetLayer(
-      page_scale_layer_.get(), proto.page_scale_layer_id(), &layer_tree_);
-  inner_viewport_scroll_layer_ =
-      UpdateAndGetLayer(inner_viewport_scroll_layer_.get(),
-                        proto.inner_viewport_scroll_layer_id(), &layer_tree_);
-  outer_viewport_scroll_layer_ =
-      UpdateAndGetLayer(outer_viewport_scroll_layer_.get(),
-                        proto.outer_viewport_scroll_layer_id(), &layer_tree_);
+      page_scale_layer_.get(), proto.page_scale_layer_id(), layer_tree_.get());
+  inner_viewport_scroll_layer_ = UpdateAndGetLayer(
+      inner_viewport_scroll_layer_.get(),
+      proto.inner_viewport_scroll_layer_id(), layer_tree_.get());
+  outer_viewport_scroll_layer_ = UpdateAndGetLayer(
+      outer_viewport_scroll_layer_.get(),
+      proto.outer_viewport_scroll_layer_id(), layer_tree_.get());
 
   LayerSelectionFromProtobuf(&selection_, proto.selection());
 
@@ -1665,16 +1492,12 @@ void LayerTreeHost::FromProtobufForCommit(const proto::LayerTreeHost& proto) {
   // updated for other reasons. All layers that at this point are part of the
   // layer tree are valid, so it is OK that they have a valid sequence number.
   int seq_num = property_trees_.sequence_number;
-  LayerTreeHostCommon::CallFunctionForEveryLayer(this, [seq_num](Layer* layer) {
-    layer->set_property_tree_sequence_number(seq_num);
-  });
+  LayerTreeHostCommon::CallFunctionForEveryLayer(
+      layer_tree_.get(), [seq_num](Layer* layer) {
+        layer->set_property_tree_sequence_number(seq_num);
+      });
 
   surface_client_id_ = proto.surface_client_id();
   next_surface_sequence_ = proto.next_surface_sequence();
 }
-
-AnimationHost* LayerTreeHost::animation_host() const {
-  return layer_tree_.animation_host();
-}
-
 }  // namespace cc