Decouple BrowserCompositorOutputSurface from BeginFrameSource.

content/browser/compositor/gpu_process_transport_factory.cc

Index: content/browser/compositor/gpu_process_transport_factory.cc
diff --git a/content/browser/compositor/gpu_process_transport_factory.cc b/content/browser/compositor/gpu_process_transport_factory.cc
index 7d1cf0c2d283a6620d1de210bc8fc8348d66ee4d..82cc8a8fcf3a7d0dc0c5c85dd9412fcfe21e4134 100644
--- a/content/browser/compositor/gpu_process_transport_factory.cc
+++ b/content/browser/compositor/gpu_process_transport_factory.cc
@@ -52,6 +52,7 @@
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/compositor_constants.h"
 #include "ui/compositor/compositor_switches.h"
+#include "ui/compositor/compositor_vsync_manager.h"
 #include "ui/compositor/layer.h"
 #include "ui/display/types/display_snapshot.h"
 #include "ui/gfx/geometry/size.h"
@@ -447,13 +448,16 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
             compositor->task_runner().get())));
   }
 
+  BrowserCompositorOutputSurface::UpdateVSyncParametersCallback vsync_callback =
+      base::Bind(&GpuProcessTransportFactory::UpdateVSyncParametersFromGpu,
+                 base::Unretained(this), compositor);
+
   std::unique_ptr<BrowserCompositorOutputSurface> display_output_surface;
 #if defined(ENABLE_VULKAN)
   std::unique_ptr<VulkanBrowserCompositorOutputSurface> vulkan_surface;
   if (vulkan_context_provider) {
     vulkan_surface.reset(new VulkanBrowserCompositorOutputSurface(
-        vulkan_context_provider, compositor->vsync_manager(),
-        begin_frame_source.get()));
+        vulkan_context_provider, vsync_callback));
     if (!vulkan_surface->Initialize(compositor.get()->widget())) {
       vulkan_surface->Destroy();
       vulkan_surface.reset();
@@ -467,8 +471,7 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
     if (!create_gpu_output_surface) {
       display_output_surface =
           base::MakeUnique<SoftwareBrowserCompositorOutputSurface>(
-              CreateSoftwareOutputDevice(compositor.get()),
-              compositor->vsync_manager(), begin_frame_source.get(),
+              CreateSoftwareOutputDevice(compositor.get()), vsync_callback,
               compositor->task_runner());
     } else {
       DCHECK(context_provider);
@@ -476,22 +479,20 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
       if (data->surface_handle == gpu::kNullSurfaceHandle) {
         display_output_surface =
             base::MakeUnique<OffscreenBrowserCompositorOutputSurface>(
-                context_provider, compositor->vsync_manager(),
-                begin_frame_source.get(),
+                context_provider, vsync_callback,
                 std::unique_ptr<
                     display_compositor::CompositorOverlayCandidateValidator>());
       } else if (capabilities.surfaceless) {
 #if defined(OS_MACOSX)
         display_output_surface = base::MakeUnique<GpuOutputSurfaceMac>(
             compositor->widget(), context_provider, data->surface_handle,
-            compositor->vsync_manager(), begin_frame_source.get(),
+            vsync_callback,
             CreateOverlayCandidateValidator(compositor->widget()),
             GetGpuMemoryBufferManager());
 #else
         display_output_surface =
             base::MakeUnique<GpuSurfacelessBrowserCompositorOutputSurface>(
-                context_provider, data->surface_handle,
-                compositor->vsync_manager(), begin_frame_source.get(),
+                context_provider, data->surface_handle, vsync_callback,
                 CreateOverlayCandidateValidator(compositor->widget()),
                 GL_TEXTURE_2D, GL_RGB, ui::DisplaySnapshot::PrimaryFormat(),
                 GetGpuMemoryBufferManager());
@@ -508,8 +509,7 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
         if (!use_mus) {
           display_output_surface =
               base::MakeUnique<GpuBrowserCompositorOutputSurface>(
-                  context_provider, compositor->vsync_manager(),
-                  begin_frame_source.get(), std::move(validator));
+                  context_provider, vsync_callback, std::move(validator));
         } else {
 #if defined(USE_AURA)
           if (compositor->window()) {
@@ -518,8 +518,8 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
             display_output_surface =
                 base::MakeUnique<MusBrowserCompositorOutputSurface>(
                     compositor->window(), context_provider,
-                    GetGpuMemoryBufferManager(), compositor->vsync_manager(),
-                    begin_frame_source.get(), std::move(validator));
+                    GetGpuMemoryBufferManager(), vsync_callback,
+                    std::move(validator));
           } else {
             aura::WindowTreeHost* host =
                 aura::WindowTreeHost::GetForAcceleratedWidget(
@@ -527,8 +527,8 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
             display_output_surface =
                 base::MakeUnique<MusBrowserCompositorOutputSurface>(
                     host->window(), context_provider,
-                    GetGpuMemoryBufferManager(), compositor->vsync_manager(),
-                    begin_frame_source.get(), std::move(validator));
+                    GetGpuMemoryBufferManager(), vsync_callback,
+                    std::move(validator));
           }
 #else
           NOTREACHED();
@@ -614,6 +614,7 @@ void GpuProcessTransportFactory::RemoveCompositor(ui::Compositor* compositor) {
   if (data->surface_handle)
     gpu::GpuSurfaceTracker::Get()->RemoveSurface(data->surface_handle);
 #endif
+
   per_compositor_data_.erase(it);
   if (per_compositor_data_.empty()) {
     // Destroying the GLHelper may cause some async actions to be cancelled,
@@ -727,6 +728,17 @@ void GpuProcessTransportFactory::SetDisplayVSyncParameters(
     data->begin_frame_source->OnUpdateVSyncParameters(timebase, interval);
 }
 
+void GpuProcessTransportFactory::UpdateVSyncParametersFromGpu(
+    base::WeakPtr<ui::Compositor> compositor,
+    base::TimeTicks timebase,
+    base::TimeDelta interval) {
+  if (!compositor)
+    return;
+
+  SetDisplayVSyncParameters(compositor.get(), timebase, interval);

[enne (OOO), 2016/11/29 21:25:03]

I think UpdateVSyncParametersFromGPU and SetDisplayVSyncParameters are too
similar here.  Also, "from GPU" isn't really true for the software case.

Could you just bind SetDisplayVSyncParameters instead of writing a new function,
and have that function update the vsync manager as well? It's probably an
oversight that it hasn't done that updating.

[stanisc, 2016/11/29 21:38:44]

I tried binding SetDisplayVSyncParameters directly but it expects a regular,
non-weak pointer to ui::Compositor. The code that creates the callback has a
weak pointer and it doesn't seem to be OK to resolve WeakPtr<ui::Compositor> to
ui::Compositor* at that time - it is a weak pointer for a reason, right?

So the main reason for this function is to resolve the weak pointer. As for the
name I am open for suggestions.

+  compositor->vsync_manager()->UpdateVSyncParameters(timebase, interval);
+}
+
 void GpuProcessTransportFactory::SetOutputIsSecure(ui::Compositor* compositor,
                                                    bool secure) {
   PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor);