Hook up ui::Compositor to Display's BeginFrameSource

cc/surfaces/surface_display_output_surface.cc

Index: cc/surfaces/surface_display_output_surface.cc
diff --git a/cc/surfaces/surface_display_output_surface.cc b/cc/surfaces/surface_display_output_surface.cc
index dfb491ede62bb7a5c41755c89514d399b93eb950..f616624a6dd345683d7bdf48572ae2e9f0370400 100644
--- a/cc/surfaces/surface_display_output_surface.cc
+++ b/cc/surfaces/surface_display_output_surface.cc
@@ -21,7 +21,7 @@ SurfaceDisplayOutputSurface::SurfaceDisplayOutputSurface(
     scoped_refptr<ContextProvider> worker_context_provider)
     : OutputSurface(std::move(context_provider),
                     std::move(worker_context_provider)),
-      display_client_(NULL),
+      display_client_(nullptr),
       factory_(surface_manager, this),
       allocator_(allocator) {
   factory_.set_needs_sync_points(false);
@@ -41,12 +41,6 @@ SurfaceDisplayOutputSurface::~SurfaceDisplayOutputSurface() {
   }
 }
 
-void SurfaceDisplayOutputSurface::ReceivedVSyncParameters(
-    base::TimeTicks timebase,
-    base::TimeDelta interval) {
-  CommitVSyncParameters(timebase, interval);
-}
-
 void SurfaceDisplayOutputSurface::SwapBuffers(CompositorFrame* frame) {
   gfx::Size frame_size =
       frame->delegated_frame_data->render_pass_list.back()->output_rect.size();
@@ -76,11 +70,18 @@ bool SurfaceDisplayOutputSurface::BindToClient(OutputSurfaceClient* client) {
   DCHECK(display_client_);
   factory_.manager()->RegisterSurfaceFactoryClient(allocator_->id_namespace(),
                                                    this);
-
   client_ = client;
+
   // Avoid initializing GL context here, as this should be sharing the
   // Display's context.
-  return display_client_->Initialize();
+  if (!display_client_->Initialize())
+    return false;
+
+  BeginFrameSource* source = display_client_->BeginFrameSourceForChildren();
+  DCHECK(source);
+  factory_.manager()->RegisterBeginFrameSource(source,
+                                               allocator_->id_namespace());

[sunnyps, 2016/03/22 21:52:44]

I think eventually we want the scheduler to make these associations and change
them if required. I would rather that we associate cc::OutputSurfaces with
surface id namespaces so that the scheduler can set the BFS on the output
surface directly. Or something along those lines.

[enne (OOO), 2016/03/22 22:16:21]

I'm not sure what you mean by associating cc::OutputSurfaces with surface id
namespaces.  BrowserCompositorOutputSurface doesn't involve itself with surfaces
at all, although arguably a single surface from the display is drawn to it. 
It'd be a little out of place.

The general pattern here is that SurfaceFactoryClients are consumers of
BeginFrameSources.  It also turns out that SurfaceDisplayOutputSurface is also
(awkwardly) the producer of a BeginFrameSource.  So, in this world it registers
a BFS with the SurfaceFactory which will turn around and SetBeginFrameSource on
it.  One option would be to hoist that BFS registration up to the Display or
DisplayScheduler, which would make SurfaceDisplayOutputSurface only a consumer
and then the Display(Scheduler) could change that as needed.

[sunnyps, 2016/03/24 23:14:25]

Sorry, I keep forgetting that the BFS is associated with the
SurfaceFactoryClient on the browser.
Hmmm, I think maybe GpuBrowserCompositorOutputSurface (and it's siblings like
the surfaceless one) should own a BFS - it's the place in the browser which gets
vsync parameters from the GPU process and we could make it get vsync signals on
android too. That is also the output surface cc::Display is initialized with so
it also ends up being the BFS of the root surface.

+  return true;
 }
 
 void SurfaceDisplayOutputSurface::ForceReclaimResources() {
@@ -91,6 +92,11 @@ void SurfaceDisplayOutputSurface::ForceReclaimResources() {
 
 void SurfaceDisplayOutputSurface::DetachFromClient() {
   DCHECK(HasClient());
+
+  BeginFrameSource* source = display_client_->BeginFrameSourceForChildren();
+  DCHECK(source);
+  factory_.manager()->UnregisterBeginFrameSource(source);
+
   // Unregister the SurfaceFactoryClient here instead of the dtor so that only
   // one client is alive for this namespace at any given time.
   factory_.manager()->UnregisterSurfaceFactoryClient(
@@ -109,7 +115,8 @@ void SurfaceDisplayOutputSurface::ReturnResources(
 
 void SurfaceDisplayOutputSurface::SetBeginFrameSource(
     BeginFrameSource* begin_frame_source) {
-  // TODO(tansell): Hook this up.
+  DCHECK(client_);
+  client_->SetBeginFrameSource(begin_frame_source);
 }
 
 void SurfaceDisplayOutputSurface::SwapBuffersComplete(SurfaceDrawStatus drawn) {