DirectCompositorFrameSink Uses CompositorFrameSinkSupport

cc/surfaces/direct_compositor_frame_sink.cc

Index: cc/surfaces/direct_compositor_frame_sink.cc
diff --git a/cc/surfaces/direct_compositor_frame_sink.cc b/cc/surfaces/direct_compositor_frame_sink.cc
index 01239ee9f822df7c1f589bf8691e0857cf804881..098f1ce1fed6b8186fe392281b74030304a19d95 100644
--- a/cc/surfaces/direct_compositor_frame_sink.cc
+++ b/cc/surfaces/direct_compositor_frame_sink.cc
@@ -29,14 +29,12 @@ DirectCompositorFrameSink::DirectCompositorFrameSink(
                           shared_bitmap_manager),
       frame_sink_id_(frame_sink_id),
       surface_manager_(surface_manager),
-      display_(display),
-      factory_(frame_sink_id, surface_manager, this) {
+      display_(display) {
   DCHECK(thread_checker_.CalledOnValidThread());
   capabilities_.can_force_reclaim_resources = true;
   // Display and DirectCompositorFrameSink share a GL context, so sync
   // points aren't needed when passing resources between them.
   capabilities_.delegated_sync_points_required = false;
-  factory_.set_needs_sync_points(false);
 }
 
 DirectCompositorFrameSink::DirectCompositorFrameSink(
@@ -47,8 +45,7 @@ DirectCompositorFrameSink::DirectCompositorFrameSink(
     : CompositorFrameSink(std::move(vulkan_context_provider)),
       frame_sink_id_(frame_sink_id),
       surface_manager_(surface_manager),
-      display_(display),
-      factory_(frame_sink_id_, surface_manager, this) {
+      display_(display) {
   DCHECK(thread_checker_.CalledOnValidThread());
   capabilities_.can_force_reclaim_resources = true;
 }
@@ -64,8 +61,9 @@ bool DirectCompositorFrameSink::BindToClient(
   if (!CompositorFrameSink::BindToClient(client))
     return false;
 
-  surface_manager_->RegisterSurfaceFactoryClient(frame_sink_id_, this);
-
+  support_.reset(new CompositorFrameSinkSupport(
+      this, surface_manager_, frame_sink_id_, display_, nullptr));
+  support_->SetNeedsSyncPoints(capabilities_.delegated_sync_points_required);

[Fady Samuel, 2017/01/05 00:36:38]

To be honest, I'm not entirely sure what this is used for. jbauman@ may provide
some insight. I'd prefer not exposing this API if it's not necessary.

[danakj, 2017/01/05 16:52:30]

You need sync points if the display and ui compositors are using different GL
contexts. You don't if they are using the same context. Right now they use the
same context, so the CFS reports that they are not required. In future it will
be OOP and will be required.

The capability is there for ui compositor to know if it should insert sync
points.

The surface aggregator also wants to know if it should use sync points when
returning resources back to the ui compositor, which it currently reads off the
surfacefactory.

   // We want the Display's output surface to hear about lost context, and since
   // this shares a context with it, we should not be listening for lost context
   // callbacks on the context here.
@@ -74,16 +72,15 @@ bool DirectCompositorFrameSink::BindToClient(
 
   // Avoid initializing GL context here, as this should be sharing the
   // Display's context.
-  display_->Initialize(this, surface_manager_);
+
+  begin_frame_source_ = base::MakeUnique<ExternalBeginFrameSource>(this);
+  client_->SetBeginFrameSource(begin_frame_source_.get());
   return true;
 }
 
 void DirectCompositorFrameSink::DetachFromClient() {
-  // Unregister the SurfaceFactoryClient here instead of the dtor so that only
-  // one client is alive for this namespace at any given time.
-  surface_manager_->UnregisterSurfaceFactoryClient(frame_sink_id_);
-  factory_.EvictSurface();
-
+  support_.reset();
+  begin_frame_source_.reset();
   CompositorFrameSink::DetachFromClient();
 }
 
@@ -93,30 +90,11 @@ void DirectCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) {
     delegated_local_frame_id_ = surface_id_allocator_.GenerateId();
     last_swap_frame_size_ = frame_size;
   }
-  display_->SetLocalFrameId(delegated_local_frame_id_,
-                            frame.metadata.device_scale_factor);
-
-  factory_.SubmitCompositorFrame(
-      delegated_local_frame_id_, std::move(frame),
-      base::Bind(&DirectCompositorFrameSink::DidDrawCallback,
-                 base::Unretained(this)));
+  support_->SubmitCompositorFrame(delegated_local_frame_id_, std::move(frame));
 }
 
 void DirectCompositorFrameSink::ForceReclaimResources() {
-  if (delegated_local_frame_id_.is_valid())
-    factory_.ClearSurface();
-}
-
-void DirectCompositorFrameSink::ReturnResources(
-    const ReturnedResourceArray& resources) {
-  if (client_)
-    client_->ReclaimResources(resources);
-}
-
-void DirectCompositorFrameSink::SetBeginFrameSource(
-    BeginFrameSource* begin_frame_source) {
-  DCHECK(client_);
-  client_->SetBeginFrameSource(begin_frame_source);
+  support_->ForceReclaimResources();
 }
 
 void DirectCompositorFrameSink::DisplayOutputSurfaceLost() {
@@ -136,8 +114,32 @@ void DirectCompositorFrameSink::DisplayDidDrawAndSwap() {
   // be drawn.
 }
 
+void DirectCompositorFrameSink::DidReceiveCompositorFrameAck() {
+  if (client_)
+    client_->DidReceiveCompositorFrameAck();
+}
+
+void DirectCompositorFrameSink::OnBeginFrame(const BeginFrameArgs& args) {
+  begin_frame_source_->OnBeginFrame(args);
+}
+
+void DirectCompositorFrameSink::ReclaimResources(
+    const ReturnedResourceArray& resources) {
+  if (client_)
+    client_->ReclaimResources(resources);
+}
+
+void DirectCompositorFrameSink::WillDrawSurface() {
+  // TODO(staraz): Implement this.
+}
+
 void DirectCompositorFrameSink::DidDrawCallback() {
   client_->DidReceiveCompositorFrameAck();
 }
 
+void DirectCompositorFrameSink::OnNeedsBeginFrames(bool needs_begin_frame) {
+  if (support_)
+    support_->SetNeedsBeginFrame(needs_begin_frame);
+}
+
 }  // namespace cc