cc : Store surface layer ids on LayerTreeHost and push them at commit

cc/layers/surface_layer.cc

Index: cc/layers/surface_layer.cc
diff --git a/cc/layers/surface_layer.cc b/cc/layers/surface_layer.cc
index 573f97b9b13280a3c9f5806052f0c7d81e52bd74..5909c47db205b71056433f1854c44b85b35b26e6 100644
--- a/cc/layers/surface_layer.cc
+++ b/cc/layers/surface_layer.cc
@@ -64,22 +64,38 @@ SurfaceLayer::~SurfaceLayer() {
 }
 
 void SurfaceLayer::SetPrimarySurfaceInfo(const SurfaceInfo& surface_info) {
+  const SurfaceId* old_surface_id = GetReferencedSurfaceId();
   RemoveReference(std::move(primary_reference_returner_));
   primary_surface_info_ = surface_info;
   if (layer_tree_host()) {
     primary_reference_returner_ = ref_factory_->CreateReference(
         layer_tree_host(), primary_surface_info_.id());
+    const SurfaceId* new_surface_id = GetReferencedSurfaceId();
+    if (old_surface_id != new_surface_id) {
+      if (old_surface_id)
+        layer_tree_host()->RemoveSurfaceLayerId(*old_surface_id);
+      if (new_surface_id)
+        layer_tree_host()->AddSurfaceLayerId(*new_surface_id);
+    }
   }
   UpdateDrawsContent(HasDrawableContent());
   SetNeedsCommit();
 }
 
 void SurfaceLayer::SetFallbackSurfaceInfo(const SurfaceInfo& surface_info) {
+  const SurfaceId* old_surface_id = GetReferencedSurfaceId();
   RemoveReference(std::move(fallback_reference_returner_));
   fallback_surface_info_ = surface_info;
   if (layer_tree_host()) {
     fallback_reference_returner_ = ref_factory_->CreateReference(
         layer_tree_host(), fallback_surface_info_.id());
+    const SurfaceId* new_surface_id = GetReferencedSurfaceId();
+    if (old_surface_id != new_surface_id) {
+      if (old_surface_id)
+        layer_tree_host()->RemoveSurfaceLayerId(*old_surface_id);
+      if (new_surface_id)
+        layer_tree_host()->AddSurfaceLayerId(*new_surface_id);
+    }
   }
   SetNeedsCommit();
 }
@@ -99,11 +115,41 @@ bool SurfaceLayer::HasDrawableContent() const {
   return primary_surface_info_.is_valid() && Layer::HasDrawableContent();
 }
 
+const SurfaceId* SurfaceLayer::GetReferencedSurfaceId(LayerTreeHost* new_host) {

[enne (OOO), 2017/05/24 18:44:54]

This is a lot of logic in SurfaceLayer to try to avoid sending too many
referenced surfaces.  Is there anything wrong with just sending all primary and
fallback surface ids regardless of surface synchronization settings?

Alternatively, could all of this move to the compositor thread? In other words,
SurfaceLayerImpl::SetPrimary/FallbackSurfaceInfo could call some new
LayerTreeImpl::Add/RemoveSurfaceLayerId instead of having SurfaceLayer do it,
and have to handle changing LayerTreeHosts as well?

[jaydasika, 2017/05/24 21:39:54]

Done (sending all primary and fallback surface ids).
If we are not going to push hidden layers, we won't have SurfaceLayerImpl and so
we can't move this to compositor thread. 

[Fady Samuel, 2017/05/24 22:46:24]

I meant to reply here:

[jaydasika, 2017/05/25 18:00:28]

Ok. I have changed the CL again to record only one of the surface ids, primary
or fallback based on surface synchronization settings.

+  LayerTreeHost* old_host = layer_tree_host();
+  if (!old_host && !new_host)
+    return nullptr;
+  DCHECK(!old_host || !new_host ||
+         old_host->GetSettings().enable_surface_synchronization ==
+             old_host->GetSettings().enable_surface_synchronization);
+
+  bool enable_surface_synchronization;
+  if (old_host) {
+    enable_surface_synchronization =
+        old_host->GetSettings().enable_surface_synchronization;
+  } else {
+    enable_surface_synchronization =
+        new_host->GetSettings().enable_surface_synchronization;
+  }
+
+  if (enable_surface_synchronization) {
+    if (fallback_surface_info_.is_valid())
+      return &fallback_surface_info_.id();
+  } else if (primary_surface_info_.is_valid()) {
+    return &primary_surface_info_.id();
+  }
+  return nullptr;
+}
+
 void SurfaceLayer::SetLayerTreeHost(LayerTreeHost* host) {
   if (layer_tree_host() == host) {
     Layer::SetLayerTreeHost(host);
     return;
   }
+  const SurfaceId* surface_id = GetReferencedSurfaceId(host);
+
+  if (layer_tree_host() && surface_id)
+    layer_tree_host()->RemoveSurfaceLayerId(*surface_id);
   RemoveReference(std::move(primary_reference_returner_));
   RemoveReference(std::move(fallback_reference_returner_));
   Layer::SetLayerTreeHost(host);
@@ -116,6 +162,8 @@ void SurfaceLayer::SetLayerTreeHost(LayerTreeHost* host) {
       fallback_reference_returner_ = ref_factory_->CreateReference(
           layer_tree_host(), fallback_surface_info_.id());
     }
+    if (surface_id)
+      layer_tree_host()->AddSurfaceLayerId(*surface_id);
   }
 }