Route D3D VSync signal to Compositor

content/browser/compositor/gpu_browser_compositor_output_surface.h

Index: content/browser/compositor/gpu_browser_compositor_output_surface.h
diff --git a/content/browser/compositor/gpu_browser_compositor_output_surface.h b/content/browser/compositor/gpu_browser_compositor_output_surface.h
index 6bf62dbce91b5a28f09e27b32cecf07801845f6c..df5d28ca427fe0880c25d34a6be21c26c43b6116 100644
--- a/content/browser/compositor/gpu_browser_compositor_output_surface.h
+++ b/content/browser/compositor/gpu_browser_compositor_output_surface.h
@@ -11,6 +11,7 @@
 #include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
 #include "content/browser/compositor/browser_compositor_output_surface.h"
+#include "content/browser/compositor/gpu_vsync_begin_frame_source.h"
 #include "ui/gfx/swap_result.h"
 
 namespace display_compositor {
@@ -33,8 +34,8 @@ class ReflectorTexture;
 // Adapts a WebGraphicsContext3DCommandBufferImpl into a
 // cc::OutputSurface that also handles vsync parameter updates
 // arriving from the GPU process.
-class GpuBrowserCompositorOutputSurface
-    : public BrowserCompositorOutputSurface {
+class GpuBrowserCompositorOutputSurface : public BrowserCompositorOutputSurface,
+                                          public GpuVSyncControl {
  public:
   GpuBrowserCompositorOutputSurface(
       scoped_refptr<ui::ContextProviderCommandBuffer> context,
@@ -75,6 +76,11 @@ class GpuBrowserCompositorOutputSurface
   unsigned GetOverlayTextureId() const override;
   bool SurfaceIsSuspendForRecycle() const override;
 
+  GpuVSyncControl* GetVSyncControl() override;

[enne (OOO), 2017/01/13 21:14:11]

I don't know that this really needs to be part of the interface here.  It seems
like you can derive from GpuVSyncControl and then just grab this inside of
GpuProcessTransportFactory when the GpuBrowserCompositorOutputSurface is
created.

[stanisc, 2017/01/14 00:32:20]

This was my attempt to simplify the logic in the factory code which creates 7
different types of surface derived from BrowserCompositorOutputSurface. Some of
them would support this new VSync mechanism, and only on OS_WIN, and some won't.
And to make it more complex all newly created instances are immediately assigned
to unique_ptr<BrowserCompositorOutputSurface>. The alternative that I've tried
is to have some code that will assign GpuVSyncControl* in some (3?) of those 7
cases and then the outcome would have to be different depending on whether it
was on OS_WIN or not. And because on creation these instances are assigned to
the base class unique_ptr I needed to either downcast it or use an intermediate
GpuBrowserCompositorOutputSurface* pointer. So the code didn't look pretty. It
is far more straightforward this way. I'll give it another shot.

[stanisc, 2017/01/24 21:23:25]

OK, I've removed this function.

+
+  // GpuVSyncControl
+  void EnableVSync(bool enabled) override;

[enne (OOO), 2017/01/13 21:14:11]

Similarly, EnableVSync doesn't need to be virtual either.  I don't think that's
something any other BrowserCompositorOutputSurface will handle.  Also, is there
some better name than EnableVSync? It's not like we're ignoring vsync in other
cases?

[stanisc, 2017/01/14 00:32:20]

Well, I guess if GpuBrowserCompositorOutputSurface* was passed to
GpuVSyncBeginFrameSource I could avoid  having this method virtua (and needing
GpuVsyncControl interface). I thought that decoupling BFS from the actual
implementation of sending Enable/Disable VSync was a cleaner approach.

Regarding the name, I agree, I've already realized that that would conflict with
a notion of disabling VSync completely via --disable-vsync flag. How about
SetNeedsVSync which is consistent with other similar named methods
(SetNeedsCommit, SetNeedsRedraw, etc)?

[stanisc, 2017/01/24 21:23:25]

I've kept this as virtual for now, mostly for the purpose of decoupling
GpuVSyncControl facet of functionality from the rest of
GpuBrowserCompositorOutputSurface. I could get rid of GpuVSyncControl if I made
GpuVsyncBeginFrameSource depend directly on this class.

+
  protected:
   gpu::CommandBufferProxyImpl* GetCommandBufferProxy();