Remove SurfaceFactory::Create and SurfaceFactory::Destroy

content/browser/frame_host/render_widget_host_view_child_frame.cc

Index: content/browser/frame_host/render_widget_host_view_child_frame.cc
diff --git a/content/browser/frame_host/render_widget_host_view_child_frame.cc b/content/browser/frame_host/render_widget_host_view_child_frame.cc
index 45141bcea5ab6a6522f0cb5d592678cd277543eb..99e3e242b3a291a691e22cfb1b453563a90c7f1e 100644
--- a/content/browser/frame_host/render_widget_host_view_child_frame.cc
+++ b/content/browser/frame_host/render_widget_host_view_child_frame.cc
@@ -69,9 +69,7 @@ RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
 }
 
 RenderWidgetHostViewChildFrame::~RenderWidgetHostViewChildFrame() {
-  if (local_frame_id_.is_valid())
-    surface_factory_->Destroy(local_frame_id_);
-
+  surface_factory_->EvictFrame();
   if (GetSurfaceManager())
     GetSurfaceManager()->InvalidateFrameSinkId(frame_sink_id_);
 }
@@ -400,10 +398,22 @@ void RenderWidgetHostViewChildFrame::OnSwapCompositorFrame(
     current_surface_scale_factor_ = scale_factor;
   }
 
+  bool allocated_new_local_frame_id = false;
   if (!local_frame_id_.is_valid()) {
     local_frame_id_ = id_allocator_->GenerateId();
-    surface_factory_->Create(local_frame_id_);
+    allocated_new_local_frame_id = true;
+  }
+
+  cc::SurfaceFactory::DrawCallback ack_callback =

[boliu, 2016/11/11 23:50:44]

any reason why this is moved to the up? I don't see any data dependency between
and "if (allocated_new_local_frame_id)" block

[Fady Samuel, 2016/11/11 23:57:33]

Surfaces are now created lazily when CompositorFrames are submitted. Thus, we
must submit a CompositorFrame before we can access it via GetSurfaceForId. It's
probably worth adding a comment here about this.

+      base::Bind(&RenderWidgetHostViewChildFrame::SurfaceDrawn, AsWeakPtr(),
+                 compositor_frame_sink_id);
+  ack_pending_count_++;
+  // If this value grows very large, something is going wrong.
+  DCHECK_LT(ack_pending_count_, 1000U);
+  surface_factory_->SubmitCompositorFrame(local_frame_id_, std::move(frame),
+                                          ack_callback);
 
+  if (allocated_new_local_frame_id) {
     cc::SurfaceSequence sequence =
         cc::SurfaceSequence(frame_sink_id_, next_surface_sequence_++);
     // The renderer process will satisfy this dependency when it creates a
@@ -415,16 +425,6 @@ void RenderWidgetHostViewChildFrame::OnSwapCompositorFrame(
         cc::SurfaceId(frame_sink_id_, local_frame_id_), frame_size,
         scale_factor, sequence);
   }
-
-  cc::SurfaceFactory::DrawCallback ack_callback =
-      base::Bind(&RenderWidgetHostViewChildFrame::SurfaceDrawn, AsWeakPtr(),
-                 compositor_frame_sink_id);
-  ack_pending_count_++;
-  // If this value grows very large, something is going wrong.
-  DCHECK_LT(ack_pending_count_, 1000U);
-  surface_factory_->SubmitCompositorFrame(local_frame_id_, std::move(frame),
-                                          ack_callback);
-
   ProcessFrameSwappedCallbacks();
 }
 
@@ -619,7 +619,7 @@ void RenderWidgetHostViewChildFrame::SubmitSurfaceCopyRequest(
   if (!src_subrect.IsEmpty())
     request->set_area(src_subrect);
 
-  surface_factory_->RequestCopyOfSurface(local_frame_id_, std::move(request));
+  surface_factory_->RequestCopyOfSurface(std::move(request));
 }
 
 void RenderWidgetHostViewChildFrame::CopyFromCompositingSurfaceToVideoFrame(
@@ -726,8 +726,7 @@ RenderWidgetHostViewChildFrame::CreateBrowserAccessibilityManager(
 }
 
 void RenderWidgetHostViewChildFrame::ClearCompositorSurfaceIfNecessary() {
-  if (local_frame_id_.is_valid())
-    surface_factory_->Destroy(local_frame_id_);
+  surface_factory_->EvictFrame();
   local_frame_id_ = cc::LocalFrameId();
 }