cc: Move ownership of surfaces to SurfaceManager

cc/surfaces/surface_manager.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/surfaces/surface_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 27 matching lines @@
       new DirectSurfaceReferenceFactory(weak_factory_.GetWeakPtr());
 }
 
 SurfaceManager::~SurfaceManager() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Remove all temporary references on shutdown.
   temporary_references_.clear();
   temporary_reference_ranges_.clear();
 
-  GarbageCollectSurfaces();
-
-  for (SurfaceDestroyList::iterator it = surfaces_to_destroy_.begin();
-       it != surfaces_to_destroy_.end();
-       ++it) {
-    UnregisterSurface((*it)->surface_id());
-  }
   surfaces_to_destroy_.clear();
+  surface_map_.clear();
 }
 
 #if DCHECK_IS_ON()
 std::string SurfaceManager::SurfaceReferencesToString() {
   std::stringstream str;
   SurfaceReferencesToStringImpl(root_surface_id_, "", &str);
   // Temporary references will have an asterisk in front of them.
   for (auto& map_entry : temporary_references_)
     SurfaceReferencesToStringImpl(map_entry.first, "* ", &str);
 
   return str.str();
 }
 #endif
 
 void SurfaceManager::SetDependencyTracker(
     SurfaceDependencyTracker* dependency_tracker) {
   dependency_tracker_ = dependency_tracker;
 }
 
 void SurfaceManager::RequestSurfaceResolution(Surface* pending_surface) {
   if (dependency_tracker_)
     dependency_tracker_->RequestSurfaceResolution(pending_surface);
 }
 
-std::unique_ptr<Surface> SurfaceManager::CreateSurface(
+base::WeakPtr<Surface> SurfaceManager::CreateSurface(
     base::WeakPtr<CompositorFrameSinkSupport> compositor_frame_sink_support,
     const SurfaceInfo& surface_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(surface_info.is_valid());
   DCHECK(compositor_frame_sink_support);
   DCHECK_EQ(surface_info.id().frame_sink_id(),
             compositor_frame_sink_support->frame_sink_id());
 
   // If no surface with this SurfaceId exists, simply create the surface and
   // return.
-  auto surface_iter = surface_map_.find(surface_info.id());
-  if (surface_iter == surface_map_.end()) {
-    auto surface =
+  auto it = surface_map_.find(surface_info.id());
+  if (it == surface_map_.end()) {
+    surface_map_[surface_info.id()] =
         base::MakeUnique<Surface>(surface_info, compositor_frame_sink_support);
-    surface_map_[surface->surface_id()] = surface.get();
-    return surface;
+    return surface_map_[surface_info.id()]->AsWeakPtr();
   }
 
-  // If a surface with this SurfaceId exists and it's not marked as destroyed,
-  // we should not receive a request to create a new surface with the same
-  // SurfaceId.
-  DCHECK(surface_iter->second->destroyed());
-
-  // If a surface with this SurfaceId exists and it's marked as destroyed,
-  // it means it's in the garbage collector's queue. We simply take it out of
-  // the queue and reuse it.
-  auto it =
-      std::find_if(surfaces_to_destroy_.begin(), surfaces_to_destroy_.end(),
-                   [&surface_info](const std::unique_ptr<Surface>& surface) {
-                     return surface->surface_id() == surface_info.id();
-                   });
-  DCHECK(it != surfaces_to_destroy_.end());
-  std::unique_ptr<Surface> surface = std::move(*it);
-  surfaces_to_destroy_.erase(it);
+  // If a surface with this SurfaceId exists, it must be marked as destroyed.
+  // Otherwise, we wouldn't receive a request to reuse the same SurfaceId.
+  // Remove the surface out of the garbage collector's queue and reuse it.
+  Surface* surface = it->second.get();
+  DCHECK(surface->destroyed());

[Fady Samuel, 2017/06/16 18:56:38]

Can we get rid of the destroy bit?

[Saman Sami, 2017/06/19 22:16:46]

Done.

+  surfaces_to_destroy_.erase(surface);
   surface->set_destroyed(false);
   DCHECK_EQ(compositor_frame_sink_support.get(),
             surface->compositor_frame_sink_support().get());
-  return surface;
+  return surface->AsWeakPtr();
 }
 
-void SurfaceManager::DestroySurface(std::unique_ptr<Surface> surface) {
+void SurfaceManager::DestroySurface(Surface* surface) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!surface->destroyed());
   surface->set_destroyed(true);
   for (auto& observer : observer_list_)
     observer.OnSurfaceDestroyed(surface->surface_id());
-  surfaces_to_destroy_.push_back(std::move(surface));
+  surfaces_to_destroy_.insert(surface);
   GarbageCollectSurfaces();
 }
 
 void SurfaceManager::RequireSequence(const SurfaceId& surface_id,
                                      const SurfaceSequence& sequence) {
   auto* surface = GetSurfaceForId(surface_id);
   if (!surface) {
     DLOG(ERROR) << "Attempting to require callback on nonexistent surface";
     return;
   }
@@ skipping 88 matching lines @@
 }
 
 void SurfaceManager::GarbageCollectSurfaces() {
   if (surfaces_to_destroy_.empty())
     return;
 
   SurfaceIdSet reachable_surfaces = using_surface_references()
                                         ? GetLiveSurfacesForReferences()
                                         : GetLiveSurfacesForSequences();
 
-  std::vector<std::unique_ptr<Surface>> surfaces_to_delete;
+  std::vector<SurfaceId> surfaces_to_delete;
 
   // Delete all destroyed and unreachable surfaces.
   for (auto iter = surfaces_to_destroy_.begin();
        iter != surfaces_to_destroy_.end();) {
     SurfaceId surface_id = (*iter)->surface_id();
     if (reachable_surfaces.count(surface_id) == 0) {
-      UnregisterSurface(surface_id);
-      surfaces_to_delete.push_back(std::move(*iter));
+      surfaces_to_delete.push_back(surface_id);
       iter = surfaces_to_destroy_.erase(iter);
     } else {
       ++iter;
     }
   }
 
   // ~Surface() draw callback could modify |surfaces_to_destroy_|.
-  surfaces_to_delete.clear();
+  for (const SurfaceId& surface_id : surfaces_to_delete)
+    DestroySurfaceInternal(surface_id);
 }
 
 SurfaceManager::SurfaceIdSet SurfaceManager::GetLiveSurfacesForReferences() {
   DCHECK(using_surface_references());
 
   SurfaceIdSet reachable_surfaces;
 
   // Walk down from the root and mark each SurfaceId we encounter as reachable.
   std::queue<SurfaceId> surface_queue;
   surface_queue.push(root_surface_id_);
@@ skipping 23 matching lines @@
   // Simple mark and sweep GC.
   // TODO(jbauman): Reduce the amount of work when nothing needs to be
   // destroyed.
   std::vector<SurfaceId> live_surfaces;
   std::unordered_set<SurfaceId, SurfaceIdHash> live_surfaces_set;
 
   // GC roots are surfaces that have not been destroyed, or have not had all
   // their destruction dependencies satisfied.
   for (auto& map_entry : surface_map_) {
     const SurfaceId& surface_id = map_entry.first;
-    Surface* surface = map_entry.second;
+    Surface* surface = map_entry.second.get();
     surface->SatisfyDestructionDependencies(&satisfied_sequences_,
                                   framesink_manager_.GetValidFrameSinkIds());
 
     if (!surface->destroyed() || surface->GetDestructionDependencyCount() > 0) {
       live_surfaces_set.insert(surface_id);
       live_surfaces.push_back(surface_id);
     }
   }
 
   // Mark all surfaces reachable from live surfaces by adding them to
   // live_surfaces and live_surfaces_set.
   for (size_t i = 0; i < live_surfaces.size(); i++) {
-    Surface* surf = surface_map_[live_surfaces[i]];
+    Surface* surf = surface_map_[live_surfaces[i]].get();
     DCHECK(surf);
 
     const auto& children = GetSurfacesReferencedByParent(surf->surface_id());
     for (const SurfaceId& id : children) {
       if (live_surfaces_set.count(id))
         continue;
 
       Surface* surf2 = GetSurfaceForId(id);
       if (surf2) {
         live_surfaces.push_back(id);
@@ skipping 136 matching lines @@
     const FrameSinkId& child_frame_sink_id) {
   framesink_manager_.UnregisterFrameSinkHierarchy(parent_frame_sink_id,
                                                 child_frame_sink_id);
 }
 
 Surface* SurfaceManager::GetSurfaceForId(const SurfaceId& surface_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   SurfaceMap::iterator it = surface_map_.find(surface_id);
   if (it == surface_map_.end())
     return nullptr;
-  return it->second;
+  return it->second.get();
 }
 
 bool SurfaceManager::SurfaceModified(const SurfaceId& surface_id,
                                      const BeginFrameAck& ack) {
   CHECK(thread_checker_.CalledOnValidThread());
   bool changed = false;
   for (auto& observer : observer_list_)
     changed |= observer.OnSurfaceDamaged(surface_id, ack);
   return changed;
 }
@@ skipping 37 matching lines @@
     dependency_tracker_->OnSurfaceDiscarded(surface);
 }
 
 void SurfaceManager::SurfaceDamageExpected(const SurfaceId& surface_id,
                                            const BeginFrameArgs& args) {
   DCHECK(thread_checker_.CalledOnValidThread());
   for (auto& observer : observer_list_)
     observer.OnSurfaceDamageExpected(surface_id, args);
 }
 
-void SurfaceManager::UnregisterSurface(const SurfaceId& surface_id) {
+void SurfaceManager::DestroySurfaceInternal(const SurfaceId& surface_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   SurfaceMap::iterator it = surface_map_.find(surface_id);
   DCHECK(it != surface_map_.end());
   surface_map_.erase(it);
   RemoveAllSurfaceReferences(surface_id);
 }
 
 #if DCHECK_IS_ON()
 void SurfaceManager::SurfaceReferencesToStringImpl(const SurfaceId& surface_id,
                                                    std::string indent,
@@ skipping 25 matching lines @@
   *str << "\n";
 
   // If the current surface has references to children, sort children and print
   // references for each child.
   for (const SurfaceId& child_id : GetSurfacesReferencedByParent(surface_id))
     SurfaceReferencesToStringImpl(child_id, indent + "  ", str);
 }
 #endif  // DCHECK_IS_ON()
 
 }  // namespace cc