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

cc/layers/surface_layer.cc

 // Copyright 2014 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/surface_layer.h"
 
 #include <stdint.h>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ skipping 45 matching lines @@
   return make_scoped_refptr(new SurfaceLayer(std::move(ref_factory)));
 }
 
 SurfaceLayer::SurfaceLayer(scoped_refptr<SurfaceReferenceFactory> ref_factory)
     : ref_factory_(std::move(ref_factory)) {}
 
 SurfaceLayer::~SurfaceLayer() {
   DCHECK(!layer_tree_host());
 }
 
+static inline const SurfaceId* GetReferencedSurfaceId(
+    const SurfaceInfo& surface_info) {
+  return surface_info.is_valid() ? &surface_info.id() : nullptr;
+}
+
 void SurfaceLayer::SetPrimarySurfaceInfo(const SurfaceInfo& surface_info) {
+  const SurfaceId* old_surface_id =
+      GetReferencedSurfaceId(primary_surface_info_);
   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(primary_surface_info_);
+    if (old_surface_id != new_surface_id) {

[enne (OOO), 2017/05/24 21:57:42]

Hmm.  This check is comparing pointers? Can you have the same surface id by
value but different pointers? That said, it looks like callers of this function
already don't call it if the surface id hasn't changed, so I'm not sure it's
worth having the conditional.

I feel like all this code could be simplified too if you had a
AddSurfaceId(SurfaceInfo&) and RemoveSurfaceId(SurfaceInfo&) that had the
conditional inside of it, and then you don't need the const SurfaceId*
everywhere.  You'd just need RemoveSurfaceId(primary_surface_info_) and then
AddSurfaceId(primary_surface_info) here and elsewhere.

Does that sound reasonable?

[jaydasika, 2017/05/24 22:12:56]

Done.

+      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(fallback_surface_info_);
   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(fallback_surface_info_);
+    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();
 }
 
 void SurfaceLayer::SetStretchContentToFillBounds(
     bool stretch_content_to_fill_bounds) {
   stretch_content_to_fill_bounds_ = stretch_content_to_fill_bounds;
   SetNeedsPushProperties();
 }
 
 std::unique_ptr<LayerImpl> SurfaceLayer::CreateLayerImpl(
     LayerTreeImpl* tree_impl) {
   return SurfaceLayerImpl::Create(tree_impl, id());
 }
 
 bool SurfaceLayer::HasDrawableContent() const {
   return primary_surface_info_.is_valid() && Layer::HasDrawableContent();
 }
 
 void SurfaceLayer::SetLayerTreeHost(LayerTreeHost* host) {
   if (layer_tree_host() == host) {
     Layer::SetLayerTreeHost(host);
     return;
   }
+  const SurfaceId* primary_surface_id =
+      GetReferencedSurfaceId(primary_surface_info_);
+  const SurfaceId* fallback_surface_id =
+      GetReferencedSurfaceId(fallback_surface_info_);
+
+  if (layer_tree_host()) {
+    if (primary_surface_id)
+      layer_tree_host()->RemoveSurfaceLayerId(*primary_surface_id);
+    if (fallback_surface_id)
+      layer_tree_host()->RemoveSurfaceLayerId(*fallback_surface_id);
+  }
   RemoveReference(std::move(primary_reference_returner_));
   RemoveReference(std::move(fallback_reference_returner_));
   Layer::SetLayerTreeHost(host);
   if (layer_tree_host()) {
     if (primary_surface_info_.is_valid()) {
       primary_reference_returner_ = ref_factory_->CreateReference(
           layer_tree_host(), primary_surface_info_.id());
     }
     if (fallback_surface_info_.is_valid()) {
       fallback_reference_returner_ = ref_factory_->CreateReference(
           layer_tree_host(), fallback_surface_info_.id());
     }
+    if (primary_surface_id)
+      layer_tree_host()->AddSurfaceLayerId(*primary_surface_id);
+    if (fallback_surface_id)
+      layer_tree_host()->RemoveSurfaceLayerId(*fallback_surface_id);

[enne (OOO), 2017/05/24 21:57:42]

add?

[jaydasika, 2017/05/24 22:12:56]

Done.

   }
 }
 
 void SurfaceLayer::PushPropertiesTo(LayerImpl* layer) {
   Layer::PushPropertiesTo(layer);
   TRACE_EVENT0("cc", "SurfaceLayer::PushPropertiesTo");
   SurfaceLayerImpl* layer_impl = static_cast<SurfaceLayerImpl*>(layer);
   layer_impl->SetPrimarySurfaceInfo(primary_surface_info_);
   layer_impl->SetFallbackSurfaceInfo(fallback_surface_info_);
   layer_impl->SetStretchContentToFillBounds(stretch_content_to_fill_bounds_);
 }
 
 void SurfaceLayer::RemoveReference(base::Closure reference_returner) {
   if (!reference_returner)
     return;
   auto swap_promise = base::MakeUnique<SatisfySwapPromise>(
       std::move(reference_returner),
       layer_tree_host()->GetTaskRunnerProvider()->MainThreadTaskRunner());
   layer_tree_host()->GetSwapPromiseManager()->QueueSwapPromise(
       std::move(swap_promise));
 }
 
 }  // namespace cc