Decouple BrowserCompositorOutputSurface from BeginFrameSource.

content/browser/compositor/gpu_process_transport_factory.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/compositor/gpu_process_transport_factory.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 34 matching lines @@
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "gpu/command_buffer/client/shared_memory_limits.h"
 #include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "services/service_manager/runner/common/client_util.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #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"
 
 #if defined(USE_AURA)
 #include "content/browser/compositor/mus_browser_compositor_output_surface.h"
 #include "content/public/common/service_manager_connection.h"
 #include "ui/aura/window_tree_host.h"
 #endif
 
@@ skipping 375 matching lines @@
   if (!compositor->GetRendererSettings().disable_display_vsync) {
     begin_frame_source.reset(new cc::DelayBasedBeginFrameSource(
         base::MakeUnique<cc::DelayBasedTimeSource>(
             compositor->task_runner().get())));
   } else {
     begin_frame_source.reset(new cc::BackToBackBeginFrameSource(
         base::MakeUnique<cc::DelayBasedTimeSource>(
             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();
     } else {
       display_output_surface = std::move(vulkan_surface);
     }
   }
 #endif
 
   if (!display_output_surface) {
     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);
       const auto& capabilities = context_provider->ContextCapabilities();
       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());
 #endif
       } else {
         std::unique_ptr<display_compositor::CompositorOverlayCandidateValidator>
             validator;
         const bool use_mus = IsUsingMus();
 #if !defined(OS_MACOSX)
         // Overlays are only supported on surfaceless output surfaces on Mac.
         if (!use_mus)
           validator = CreateOverlayCandidateValidator(compositor->widget());
 #endif
         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()) {
             // TODO(mfomitchev): Remove this clause once we complete the switch
             // to Aura-Mus.
             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(
                     compositor->widget());
             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();
 #endif
         }
       }
     }
   }
 
   data->display_output_surface = display_output_surface.get();
@@ skipping 65 matching lines @@
 void GpuProcessTransportFactory::RemoveCompositor(ui::Compositor* compositor) {
   PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor);
   if (it == per_compositor_data_.end())
     return;
   PerCompositorData* data = it->second.get();
   DCHECK(data);
 #if !defined(GPU_SURFACE_HANDLE_IS_ACCELERATED_WINDOW)
   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,
     // causing things to request a new GLHelper. Due to crbug.com/176091 the
     // GLHelper created in this case would be lost/leaked if we just reset()
     // on the |gl_helper_| variable directly. So instead we call reset() on a
     // local std::unique_ptr.
     std::unique_ptr<display_compositor::GLHelper> helper =
         std::move(gl_helper_);
 
@@ skipping 93 matching lines @@
     base::TimeDelta interval) {
   PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor);
   if (it == per_compositor_data_.end())
     return;
   PerCompositorData* data = it->second.get();
   DCHECK(data);
   if (data->begin_frame_source)
     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);
   if (it == per_compositor_data_.end())
     return;
   PerCompositorData* data = it->second.get();
   DCHECK(data);
   data->output_is_secure = secure;
   if (data->display)
     data->display->SetOutputIsSecure(secure);
@@ skipping 141 matching lines @@
       shared_vulkan_context_provider_ =
           cc::VulkanInProcessContextProvider::Create();
     }
 
     shared_vulkan_context_provider_initialized_ = true;
   }
   return shared_vulkan_context_provider_;
 }
 
 }  // namespace content