Use vsync manager regardless of begin frame settings

There was some hope that the ui::CompositorVSyncManager could be removed
once --enable-begin-frame-scheduling was turned on.  Unfortunately, the
vsync manager is used by two additional systems that can't be easily
removed: DelegatedFrameHost::AttemptFrameSubscriberCapture as well as
components/exo/wayland/server.cc.  The latter may go away eventually.
If so, then the frame capture could then be changed to more simple
polling as needed instead of vsync manager and observer system that
needs to be plumbed everywhere.

In the medium term, the vsync manager needs to live on in all paths.
This means that BrowserCompositorOutputSurface and ui::Compositor always
need to update the vsync manager with changes.  Conditionals for
whether or not to send vsync information to renderers just get moved
down the pipeline from the output surface to DelegatedFrameHost.

Review-Url: https://codereview.chromium.org/2277883002
Cr-Commit-Position: refs/heads/master@{#414186}

content/browser/compositor/browser_compositor_output_surface.cc

@@ -11,7 +11,6 @@
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
-#include "cc/base/switches.h"
 #include "cc/output/output_surface_client.h"
 #include "cc/scheduler/begin_frame_source.h"
 #include "components/display_compositor/compositor_overlay_candidate_validator.h"
@@ -29,10 +28,7 @@ BrowserCompositorOutputSurface::BrowserCompositorOutputSurface(
     : OutputSurface(std::move(context_provider), nullptr, nullptr),
       vsync_manager_(std::move(vsync_manager)),
       synthetic_begin_frame_source_(begin_frame_source),
-      reflector_(nullptr),
-      use_begin_frame_scheduling_(
-          !base::CommandLine::ForCurrentProcess()->HasSwitch(
-              cc::switches::kDisableBeginFrameScheduling)) {
+      reflector_(nullptr) {
   overlay_candidate_validator_ = std::move(overlay_candidate_validator);
   Initialize();
 }
@@ -44,10 +40,7 @@ BrowserCompositorOutputSurface::BrowserCompositorOutputSurface(
     : OutputSurface(nullptr, nullptr, std::move(software_device)),
       vsync_manager_(vsync_manager),
       synthetic_begin_frame_source_(begin_frame_source),
-      reflector_(nullptr),
-      use_begin_frame_scheduling_(
-          !base::CommandLine::ForCurrentProcess()->HasSwitch(
-              cc::switches::kDisableBeginFrameScheduling)) {
+      reflector_(nullptr) {
   Initialize();
 }
 
@@ -66,57 +59,21 @@ BrowserCompositorOutputSurface::~BrowserCompositorOutputSurface() {
   if (reflector_)
     reflector_->DetachFromOutputSurface();
   DCHECK(!reflector_);
-  if (!HasClient())
-    return;
-
-  // When BeginFrame scheduling is enabled, vsync info is not routed to renderer
-  // by using |vsync_manager_|. Instead, BeginFrame message is used.
-  if (!use_begin_frame_scheduling_)
-    vsync_manager_->RemoveObserver(this);
 }
 
 void BrowserCompositorOutputSurface::Initialize() {
   capabilities_.adjust_deadline_for_parent = false;
 }
 
-bool BrowserCompositorOutputSurface::BindToClient(
-    cc::OutputSurfaceClient* client) {
-  if (!OutputSurface::BindToClient(client))
-    return false;
-
-  // Don't want vsync notifications until there is a client.
-  if (!use_begin_frame_scheduling_)
-    vsync_manager_->AddObserver(this);
-  return true;
-}
-
-void BrowserCompositorOutputSurface::UpdateVSyncParametersInternal(
+void BrowserCompositorOutputSurface::OnUpdateVSyncParametersFromGpu(
     base::TimeTicks timebase,
     base::TimeDelta interval) {
+  DCHECK(HasClient());
   if (interval.is_zero()) {
     // TODO(brianderson): We should not be receiving 0 intervals.
     interval = cc::BeginFrameArgs::DefaultInterval();
   }
   synthetic_begin_frame_source_->OnUpdateVSyncParameters(timebase, interval);
-}
-
-void BrowserCompositorOutputSurface::OnUpdateVSyncParameters(
-    base::TimeTicks timebase,
-    base::TimeDelta interval) {
-  DCHECK(HasClient());
-  DCHECK(!use_begin_frame_scheduling_);
-  UpdateVSyncParametersInternal(timebase, interval);
-}
-
-void BrowserCompositorOutputSurface::OnUpdateVSyncParametersFromGpu(
-    base::TimeTicks timebase,
-    base::TimeDelta interval) {
-  DCHECK(HasClient());
-  if (use_begin_frame_scheduling_) {
-    UpdateVSyncParametersInternal(timebase, interval);
-    return;
-  }
-
   vsync_manager_->UpdateVSyncParameters(timebase, interval);
 }
 

content/browser/compositor/browser_compositor_output_surface.h

@@ -35,19 +35,13 @@ class ReflectorImpl;
 class WebGraphicsContext3DCommandBufferImpl;
 
 class CONTENT_EXPORT BrowserCompositorOutputSurface
-    : public cc::OutputSurface,
-      public ui::CompositorVSyncManager::Observer {
+    : public cc::OutputSurface {
  public:
   ~BrowserCompositorOutputSurface() override;
 
   // cc::OutputSurface implementation.
-  bool BindToClient(cc::OutputSurfaceClient* client) override;
   cc::OverlayCandidateValidator* GetOverlayCandidateValidator() const override;
 
-  // ui::CompositorVSyncManager::Observer implementation.
-  void OnUpdateVSyncParameters(base::TimeTicks timebase,
-                               base::TimeDelta interval) override;
-
   void OnUpdateVSyncParametersFromGpu(base::TimeTicks timebase,
                                       base::TimeDelta interval);
 
@@ -95,15 +89,9 @@ class CONTENT_EXPORT BrowserCompositorOutputSurface
   cc::SyntheticBeginFrameSource* synthetic_begin_frame_source_;
   ReflectorImpl* reflector_;
 
-  // True when BeginFrame scheduling is enabled.
-  bool use_begin_frame_scheduling_;
-
  private:
   void Initialize();
 
-  void UpdateVSyncParametersInternal(base::TimeTicks timebase,
-                                     base::TimeDelta interval);
-
   std::unique_ptr<display_compositor::CompositorOverlayCandidateValidator>
       overlay_candidate_validator_;
 

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -300,13 +300,8 @@ void BrowserCompositorMac::UpdateVSyncParameters(
     const base::TimeTicks& timebase,
     const base::TimeDelta& interval) {
   if (recyclable_compositor_) {
-    recyclable_compositor_->compositor()
-        ->vsync_manager()
-        ->UpdateVSyncParameters(timebase, interval);
-    recyclable_compositor_->compositor()
-        ->context_factory()
-        ->SetDisplayVSyncParameters(recyclable_compositor_->compositor(),
-                                    timebase, interval);
+    recyclable_compositor_->compositor()->SetDisplayVSyncParameters(
+        timebase, interval);
   }
 }
 
@@ -474,7 +469,6 @@ bool BrowserCompositorMac::IsAutoResizeEnabled() const {
 // cc::BeginFrameSourceBase, public:
 
 void BrowserCompositorMac::OnBeginFrame(const cc::BeginFrameArgs& args) {
-  delegated_frame_host_->SetVSyncParameters(args.frame_time, args.interval);
   client_->BrowserCompositorMacSendBeginFrame(args);
   last_begin_frame_args_ = args;
 }

content/browser/renderer_host/delegated_frame_host.cc

@@ -13,6 +13,7 @@
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/time/default_tick_clock.h"
+#include "cc/base/switches.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/resources/single_release_callback.h"
@@ -65,6 +66,9 @@ void RequireCallback(cc::SurfaceManager* manager,
 DelegatedFrameHost::DelegatedFrameHost(DelegatedFrameHostClient* client)
     : client_(client),
       compositor_(nullptr),
+      using_begin_frame_scheduling_(
+          !base::CommandLine::ForCurrentProcess()->HasSwitch(
+              cc::switches::kDisableBeginFrameScheduling)),
       tick_clock_(new base::DefaultTickClock()),
       last_output_surface_id_(0),
       pending_delegated_ack_count_(0),
@@ -797,8 +801,9 @@ void DelegatedFrameHost::OnCompositingShuttingDown(ui::Compositor* compositor) {
 
 void DelegatedFrameHost::OnUpdateVSyncParameters(base::TimeTicks timebase,
                                                  base::TimeDelta interval) {
-  SetVSyncParameters(timebase, interval);
-  if (client_->DelegatedFrameHostIsVisible())
+  vsync_timebase_ = timebase;
+  vsync_interval_ = interval;
+  if (!using_begin_frame_scheduling_ && client_->DelegatedFrameHostIsVisible())
     client_->DelegatedFrameHostUpdateVSyncParameters(timebase, interval);
 }
 
@@ -854,21 +859,15 @@ void DelegatedFrameHost::ResetCompositor() {
   }
   if (compositor_->HasObserver(this))
     compositor_->RemoveObserver(this);
-  if (vsync_manager_.get()) {
+  if (vsync_manager_) {
     vsync_manager_->RemoveObserver(this);
-    vsync_manager_ = NULL;
+    vsync_manager_ = nullptr;
   }
 
   compositor_->RemoveSurfaceClient(id_allocator_->client_id());
   compositor_ = nullptr;
 }
 
-void DelegatedFrameHost::SetVSyncParameters(const base::TimeTicks& timebase,
-                                            const base::TimeDelta& interval) {
-  vsync_timebase_ = timebase;
-  vsync_interval_ = interval;
-}
-
 void DelegatedFrameHost::LockResources() {
   DCHECK(!surface_id_.is_null());
   delegated_frame_evictor_->LockFrame();

content/browser/renderer_host/delegated_frame_host.h

@@ -139,8 +139,6 @@ class CONTENT_EXPORT DelegatedFrameHost
   gfx::Size GetRequestedRendererSize() const;
   void SetCompositor(ui::Compositor* compositor);
   void ResetCompositor();
-  void SetVSyncParameters(const base::TimeTicks& timebase,
-                          const base::TimeDelta& interval);
   // Note: |src_subset| is specified in DIP dimensions while |output_size|
   // expects pixels.
   void CopyFromCompositingSurface(const gfx::Rect& src_subrect,
@@ -257,6 +255,7 @@ class CONTENT_EXPORT DelegatedFrameHost
 
   // The vsync manager we are observing for changes, if any.
   scoped_refptr<ui::CompositorVSyncManager> vsync_manager_;
+  bool using_begin_frame_scheduling_;
 
   // The current VSync timebase and interval. These are zero until the first
   // call to SetVSyncParameters().

content/browser/renderer_host/render_widget_host_view_aura.cc

@@ -723,7 +723,6 @@ void RenderWidgetHostViewAura::SetNeedsBeginFrames(bool needs_begin_frames) {
 
 void RenderWidgetHostViewAura::OnBeginFrame(
     const cc::BeginFrameArgs& args) {
-  delegated_frame_host_->SetVSyncParameters(args.frame_time, args.interval);
   host_->Send(new ViewMsg_BeginFrame(host_->GetRoutingID(), args));
   last_begin_frame_args_ = args;
 }

ui/compositor/compositor.cc

@@ -398,10 +398,13 @@ bool Compositor::GetScrollOffsetForLayer(int layer_id,
 void Compositor::SetAuthoritativeVSyncInterval(
     const base::TimeDelta& interval) {
   context_factory_->SetAuthoritativeVSyncInterval(this, interval);
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-         cc::switches::kDisableBeginFrameScheduling)) {
-    vsync_manager_->SetAuthoritativeVSyncInterval(interval);
-  }
+  vsync_manager_->SetAuthoritativeVSyncInterval(interval);
+}
+
+void Compositor::SetDisplayVSyncParameters(base::TimeTicks timebase,
+                                           base::TimeDelta interval) {
+  context_factory_->SetDisplayVSyncParameters(this, timebase, interval);
+  vsync_manager_->UpdateVSyncParameters(timebase, interval);
 }
 
 void Compositor::SetAcceleratedWidget(gfx::AcceleratedWidget widget) {

ui/compositor/compositor.h

@@ -287,6 +287,12 @@ class COMPOSITOR_EXPORT Compositor
   // context.
   void SetAuthoritativeVSyncInterval(const base::TimeDelta& interval);
 
+  // Most platforms set their vsync info via BrowerCompositorOutputSurface's
+  // OnUpdateVSyncParametersFromGpu, but Mac routes vsync info via the
+  // browser compositor instead through this path.
+  void SetDisplayVSyncParameters(base::TimeTicks timebase,
+                                 base::TimeDelta interval);
+
   // Sets the widget for the compositor to render into.
   void SetAcceleratedWidget(gfx::AcceleratedWidget widget);
   // Releases the widget previously set through SetAcceleratedWidget().