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

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 <queue>
@@ skipping 70 matching lines @@
 void SurfaceManager::SetDependencyTracker(
     std::unique_ptr<SurfaceDependencyTracker> dependency_tracker) {
   dependency_tracker_ = std::move(dependency_tracker);
 }
 
 void SurfaceManager::RequestSurfaceResolution(Surface* pending_surface) {
   if (dependency_tracker_)
     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);
   DCHECK(it != surface_map_.end());
   surface_map_.erase(it);
   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 16:30:08]

const SurfaceId&

[Saman Sami, 2017/03/09 20:45:50]

Done.

+    base::WeakPtr<SurfaceFactory> surface_factory) {

[Fady Samuel, 2017/03/09 16:30:08]

Does thsi need to be a weakptr?

[Saman Sami, 2017/03/09 20:45:50]

Surface keeps a weak ptr to its factory.

+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (!surface_id.is_valid() || !surface_factory)

[Fady Samuel, 2017/03/09 16:30:08]

DCHECK instead? It seems like a bug otherwise.

[Saman Sami, 2017/03/09 20:45:50]

Done.

+    return nullptr;
+
+  auto surface_iter = surface_map_.find(surface_id);
+
+  // If the SurfaceId is not registered, simply create the surface and return.
+  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 the SurfaceId is registered but the registered surface is not
+  // destroyed, we can't create a new surface with the same SurfaceId.
+  if (!surface_iter->second->destroyed())

[Fady Samuel, 2017/03/09 16:30:08]

This is a bug right? DCHECK instead?

[Saman Sami, 2017/03/09 20:45:50]

Done.

+    return nullptr;
+
+  // If the SurfaceId is registered and the registered surface is destroyed,
+  // we can remove it from the garbage collector's queue and reuse it.
+  std::unique_ptr<Surface> surface;
+  for (auto it = surfaces_to_destroy_.begin(); it != surfaces_to_destroy_.end();

[Fady Samuel, 2017/03/09 16:30:08]

Use std::find? http://en.cppreference.com/w/cpp/algorithm/find

[Saman Sami, 2017/03/09 20:45:50]

Done.

+       it++) {
+    if ((*it)->surface_id() == surface_id) {
+      surface = std::move(*it);
+      surfaces_to_destroy_.erase(it);
+      break;
+    }
+  }
+  DCHECK(surface);
+  DCHECK_EQ(surface_factory.get(), surface->factory().get());
+  surface->set_destroyed(false);
+  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));
   GarbageCollectSurfaces();
 }
 
 void SurfaceManager::RequireSequence(const SurfaceId& surface_id,
                                      const SurfaceSequence& sequence) {
   auto* surface = GetSurfaceForId(surface_id);
   if (!surface) {
@@ skipping 540 matching lines @@
     std::vector<SurfaceId> children(iter->second.begin(), iter->second.end());
     std::sort(children.begin(), children.end());
 
     for (const SurfaceId& child_id : children)
       SurfaceReferencesToStringImpl(child_id, indent + "  ", str);
   }
 }
 #endif  // DCHECK_IS_ON()
 
 }  // namespace cc