Moving temporary reference logic to SurfaceManager

cc/surfaces/surface_manager.cc

Index: cc/surfaces/surface_manager.cc
diff --git a/cc/surfaces/surface_manager.cc b/cc/surfaces/surface_manager.cc
index c4dcc5b46cf5eb58a3ee195ddbba829760429155..d6e43b97a26e49709457e78bc169ce53a97b0429 100644
--- a/cc/surfaces/surface_manager.cc
+++ b/cc/surfaces/surface_manager.cc
@@ -15,6 +15,7 @@
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_factory_client.h"
 #include "cc/surfaces/surface_id_allocator.h"
+#include "cc/surfaces/surface_info.h"
 
 namespace cc {
 
@@ -41,6 +42,19 @@ SurfaceManager::SurfaceManager(LifetimeType lifetime_type)
 
 SurfaceManager::~SurfaceManager() {
   DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (lifetime_type_ == LifetimeType::REFERENCES) {
+    // Remove all temporary references on shutdown.
+    for (auto& map_entry : temp_references_) {
+      const FrameSinkId& frame_sink_id = map_entry.first;
+      for (auto& local_frame_id : map_entry.second) {
+        RemoveSurfaceReferenceImpl(GetRootSurfaceId(),
+                                   SurfaceId(frame_sink_id, local_frame_id));
+      }
+    }
+    GarbageCollectSurfaces();
+  }
+
   for (SurfaceDestroyList::iterator it = surfaces_to_destroy_.begin();
        it != surfaces_to_destroy_.end();
        ++it) {
@@ -127,6 +141,58 @@ void SurfaceManager::AddSurfaceReference(const SurfaceId& parent_id,
     return;
   }
 
+  auto vector_iter = temp_references_.find(child_id.frame_sink_id());
+
+  // If there are no temporary references for the FrameSinkId then we can just
+  // add reference and return.
+  if (vector_iter == temp_references_.end()) {
+    AddSurfaceReferenceImpl(parent_id, child_id);
+    return;
+  }
+
+  // Get the vector<LocalFrameId> for the appropriate FrameSinkId and look for
+  // |child_id.local_frame_id| in that vector. If found, there is a temporary
+  // reference to |child_id|.
+  std::vector<LocalFrameId>& refs = vector_iter->second;
+  auto temp_ref_iter =
+      std::find(refs.begin(), refs.end(), child_id.local_frame_id());
+
+  if (temp_ref_iter == refs.end()) {
+    AddSurfaceReferenceImpl(parent_id, child_id);
+    return;
+  }
+
+  // All surfaces get a temporary reference to the top level root. If the parent
+  // wants to add a reference to the top level root then we do nothing.
+  // Otherwise remove the temporary reference and add the reference.
+  if (parent_id != GetRootSurfaceId()) {
+    AddSurfaceReferenceImpl(parent_id, child_id);
+    RemoveSurfaceReference(GetRootSurfaceId(), child_id);
+  }
+
+  // Remove temporary references for surfaces with the same FrameSinkId that
+  // were created before |child_id|. The earlier surfaces were never embedded in
+  // the parent and the parent is embedding a later surface, so we know the
+  // parent doesn't need them anymore.
+  for (auto iter = refs.begin(); iter != temp_ref_iter; ++iter) {
+    SurfaceId id = SurfaceId(child_id.frame_sink_id(), *iter);
+    RemoveSurfaceReference(GetRootSurfaceId(), id);
+  }
+
+  // Remove markers for temporary references up to |child_id|, as the temporary
+  // references they correspond to were removed above. If |temp_ref_iter| points
+  // at the last element in |refs| then we are removing all temporary references
+  // for the FrameSinkId and can remove the map entry entirely.
+  if (++temp_ref_iter == refs.end())
+    temp_references_.erase(child_id.frame_sink_id());
+  else
+    refs.erase(refs.begin(), temp_ref_iter);
+}
+
+void SurfaceManager::AddSurfaceReferenceImpl(const SurfaceId& parent_id,
+                                             const SurfaceId& child_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());

[kylechar, 2017/01/17 16:15:35]

The private method doesn't also need a 
DCHECK(thread_checker_.CalledOnValidThread()) since the public method has one
too.

[Saman Sami, 2017/01/17 18:29:05]

Done.

+
   parent_to_child_refs_[parent_id].insert(child_id);
   child_to_parent_refs_[child_id].insert(parent_id);
 }
@@ -504,6 +570,13 @@ bool SurfaceManager::SurfaceModified(const SurfaceId& surface_id) {
 
 void SurfaceManager::SurfaceCreated(const SurfaceInfo& surface_info) {
   CHECK(thread_checker_.CalledOnValidThread());
+
+  if (lifetime_type_ == LifetimeType::REFERENCES) {

[kylechar, 2017/01/17 16:15:35]

There was a comment that went with this code in DisplayCompositor that should be
moved/modified for here. Otherwise it's a bit of a mystery what is happening.

[Saman Sami, 2017/01/17 18:29:05]

Done.

+    AddSurfaceReferenceImpl(GetRootSurfaceId(), surface_info.id());
+    temp_references_[surface_info.id().frame_sink_id()].push_back(
+        surface_info.id().local_frame_id());
+  }
+
   for (auto& observer : observer_list_)
     observer.OnSurfaceCreated(surface_info);
 }