cc: Move ownership of surfaces to SurfaceManager

cc/surfaces/surface_manager.h

Index: cc/surfaces/surface_manager.h
diff --git a/cc/surfaces/surface_manager.h b/cc/surfaces/surface_manager.h
index 01d18b2dc1986ca3c0f22aad6105610916b313a5..4b4e158841955d65794f03ea543164efd7026e66 100644
--- a/cc/surfaces/surface_manager.h
+++ b/cc/surfaces/surface_manager.h
@@ -7,8 +7,8 @@
 
 #include <stdint.h>
 
-#include <list>
 #include <memory>
+#include <set>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
@@ -67,12 +67,12 @@ class CC_SURFACES_EXPORT SurfaceManager {
 
   void RequestSurfaceResolution(Surface* pending_surface);
 
-  std::unique_ptr<Surface> CreateSurface(
+  base::WeakPtr<Surface> CreateSurface(
       base::WeakPtr<CompositorFrameSinkSupport> compositor_frame_sink_support,
       const SurfaceInfo& surface_info);
 
   // Destroy the Surface once a set of sequence numbers has been satisfied.
-  void DestroySurface(std::unique_ptr<Surface> surface);
+  void DestroySurface(Surface* surface);

[kylechar, 2017/06/16 19:13:30]

Can this just take a SurfaceId instead of a Surface*?

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

Done.

 
   Surface* GetSurfaceForId(const SurfaceId& surface_id);
 
@@ -257,9 +257,8 @@ class CC_SURFACES_EXPORT SurfaceManager {
   // |surface_id| that were added before |surface_id| will also be removed.
   void RemoveTemporaryReference(const SurfaceId& surface_id, bool remove_range);
 
-  // Called when a surface is destroyed and it needs to be removed from the
-  // surface map.
-  void UnregisterSurface(const SurfaceId& surface_id);
+  // Called by the garbage collector when a surafce needs to be destroyed.
+  void DestroySurfaceInternal(const SurfaceId& surface_id);
 
 #if DCHECK_IS_ON()
   // Recursively prints surface references starting at |surface_id| to |str|.
@@ -273,15 +272,13 @@ class CC_SURFACES_EXPORT SurfaceManager {
 
   FrameSinkManager framesink_manager_;
 
-  using SurfaceMap = std::unordered_map<SurfaceId, Surface*, SurfaceIdHash>;
+  using SurfaceMap =
+      std::unordered_map<SurfaceId, std::unique_ptr<Surface>, SurfaceIdHash>;
   SurfaceMap surface_map_;
   base::ObserverList<SurfaceObserver> observer_list_;
   base::ThreadChecker thread_checker_;
 
-  // List of surfaces to be destroyed, along with what sequences they're still
-  // waiting on.
-  using SurfaceDestroyList = std::list<std::unique_ptr<Surface>>;
-  SurfaceDestroyList surfaces_to_destroy_;
+  std::set<Surface*> surfaces_to_destroy_;

[kylechar, 2017/06/16 19:13:30]

Can you use base::flat_set here? This should almost always be very small. Also
you only use the pointer to get a SurfaceId from, you could just have a set of
SurfaceIds?

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

Done.

 
   // Set of SurfaceSequences that have been satisfied by a frame but not yet
   // waited on.