SurfaceIds must be reusable as soon as their surfaces are marked destroyed

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 bf72fce1f3a30bc5497190135fcabe9dcb252791..3a8e6cefb52449986b4d12a1bc15f9dd313503c4 100644
--- a/cc/surfaces/surface_manager.cc
+++ b/cc/surfaces/surface_manager.cc
@@ -88,13 +88,6 @@ void SurfaceManager::RequestSurfaceResolution(Surface* pending_surface) {
     dependency_tracker_->RequestSurfaceResolution(pending_surface);
 }
 
-void SurfaceManager::RegisterSurface(Surface* surface) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(surface);
-  DCHECK(!surface_map_.count(surface->surface_id()));
-  surface_map_[surface->surface_id()] = surface;
-}
-
 void SurfaceManager::DeregisterSurface(const SurfaceId& surface_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   SurfaceMap::iterator it = surface_map_.find(surface_id);
@@ -103,7 +96,43 @@ void SurfaceManager::DeregisterSurface(const SurfaceId& surface_id) {
   RemoveAllSurfaceReferences(surface_id);
 }
 
-void SurfaceManager::Destroy(std::unique_ptr<Surface> surface) {
+std::unique_ptr<Surface> SurfaceManager::CreateSurface(
+    SurfaceId surface_id,

[Fady Samuel, 2017/03/09 21:32:16]

looks like you missed making this const SurfaceId& 

[Saman Sami, 2017/03/09 21:45:17]

Sorry one of my commits got deleted and I had to redo all the fixes. Forgot
about this one.

+    base::WeakPtr<SurfaceFactory> surface_factory) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(surface_id.is_valid() && surface_factory);
+
+  // If no surface with this SurfaceId exists, simply create the surface and
+  // return.
+  auto surface_iter = surface_map_.find(surface_id);
+  if (surface_iter == surface_map_.end()) {
+    auto surface = base::MakeUnique<Surface>(surface_id, surface_factory);
+    surface_map_[surface->surface_id()] = surface.get();
+    return surface;
+  }
+
+  // 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_id](const std::unique_ptr<Surface>& surface) {
+                     return surface->surface_id() == surface_id;
+                   });
+  DCHECK(it != surfaces_to_destroy_.end());
+  std::unique_ptr<Surface> surface = std::move(*it);
+  surfaces_to_destroy_.erase(it);
+  surface->set_destroyed(false);
+  DCHECK_EQ(surface_factory.get(), surface->factory().get());
+  return surface;
+}
+
+void SurfaceManager::DestroySurface(std::unique_ptr<Surface> surface) {
   DCHECK(thread_checker_.CalledOnValidThread());
   surface->set_destroyed(true);
   surfaces_to_destroy_.push_back(std::move(surface));