cc: Move ownership of surfaces to SurfaceManager

cc/surfaces/compositor_frame_sink_support.cc

Index: cc/surfaces/compositor_frame_sink_support.cc
diff --git a/cc/surfaces/compositor_frame_sink_support.cc b/cc/surfaces/compositor_frame_sink_support.cc
index 6ac4753fe0a1d81e10882913824793c388aae7b4..e5483b248d9e549dcd476a5e95ec45a6323395ac 100644
--- a/cc/surfaces/compositor_frame_sink_support.cc
+++ b/cc/surfaces/compositor_frame_sink_support.cc
@@ -79,7 +79,8 @@ void CompositorFrameSinkSupport::SetBeginFrameSource(
 void CompositorFrameSinkSupport::EvictCurrentSurface() {
   if (!current_surface_)
     return;
-  DestroyCurrentSurface();
+  base::WeakPtr<Surface> tmp = std::move(current_surface_);
+  surface_manager_->DestroySurface(tmp.get());
 }
 
 void CompositorFrameSinkSupport::SetNeedsBeginFrame(bool needs_begin_frame) {
@@ -127,13 +128,9 @@ bool CompositorFrameSinkSupport::SubmitCompositorFrame(
     }
   }
 
-  std::unique_ptr<Surface> surface;
-  bool create_new_surface =
-      (!current_surface_ ||
-       local_surface_id != current_surface_->surface_id().local_surface_id());
-  if (!create_new_surface) {
-    surface = std::move(current_surface_);
-  } else {
+  Surface* prev_surface = current_surface_.get();

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

This is the only place you really use the Surface*. Can you hold onto a
SurfaceId and look it up here? That would eliminate all of the added WeakPtr
code.

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

Done.

+  if (!current_surface_ ||
+      local_surface_id != current_surface_->surface_id().local_surface_id()) {
     SurfaceId surface_id(frame_sink_id_, local_surface_id);
     gfx::Size frame_size = frame.render_pass_list.back()->output_rect.size();
     float device_scale_factor = frame.metadata.device_scale_factor;
@@ -143,7 +140,7 @@ bool CompositorFrameSinkSupport::SubmitCompositorFrame(
       TRACE_EVENT_INSTANT0("cc", "Invalid SurfaceInfo",
                            TRACE_EVENT_SCOPE_THREAD);
       if (current_surface_)
-        DestroyCurrentSurface();
+        EvictCurrentSurface();
       ReturnedResourceArray resources;
       TransferableResource::ReturnResources(frame.resource_list, &resources);
       ReturnResources(resources);
@@ -151,10 +148,10 @@ bool CompositorFrameSinkSupport::SubmitCompositorFrame(
       return true;
     }
 
-    surface = CreateSurface(surface_info);
+    current_surface_ = CreateSurface(surface_info);
   }
 
-  bool result = surface->QueueFrame(
+  bool result = current_surface_->QueueFrame(
       std::move(frame),
       base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
                  weak_factory_.GetWeakPtr()),
@@ -162,15 +159,14 @@ bool CompositorFrameSinkSupport::SubmitCompositorFrame(
                           weak_factory_.GetWeakPtr()));
 
   if (!result) {
-    surface_manager_->DestroySurface(std::move(surface));
+    EvictCurrentSurface();
     return false;
   }
 
-  if (current_surface_) {
-    surface->SetPreviousFrameSurface(current_surface_.get());
-    DestroyCurrentSurface();
+  if (prev_surface && prev_surface != current_surface_.get()) {
+    current_surface_->SetPreviousFrameSurface(prev_surface);
+    surface_manager_->DestroySurface(prev_surface);
   }
-  current_surface_ = std::move(surface);
 
   if (begin_frame_source_)
     begin_frame_source_->DidFinishFrame(this);
@@ -340,17 +336,13 @@ void CompositorFrameSinkSupport::UpdateNeedsBeginFramesInternal() {
     begin_frame_source_->RemoveObserver(this);
 }
 
-std::unique_ptr<Surface> CompositorFrameSinkSupport::CreateSurface(
+base::WeakPtr<Surface> CompositorFrameSinkSupport::CreateSurface(
     const SurfaceInfo& surface_info) {
   seen_first_frame_activation_ = false;
   return surface_manager_->CreateSurface(weak_factory_.GetWeakPtr(),
                                          surface_info);
 }
 
-void CompositorFrameSinkSupport::DestroyCurrentSurface() {
-  surface_manager_->DestroySurface(std::move(current_surface_));
-}
-
 void CompositorFrameSinkSupport::RequestCopyOfSurface(
     std::unique_ptr<CopyOutputRequest> copy_request) {
   if (!current_surface_)