Make --disable-gpu-vsync work with CoreAnimation

CoreAnimation provides GPU backpressure from the browser process to the
renderer process by only acknowledging frames when they are displayed.
This mechanism has the side-effect of throttling the renderer to about
vsync, because the layer will not request to be displayed much more
than once per vsync.

Relieve this pressure by not going into isAsynchronous mode when
receiving new frames when vsync is disabled.

This exposed an issue where unluckily-timed setNeedsDisplay calls could
be dropped on the floor, hanging the renderer because it would never
get its ack. Work around this by calling setNeedsDisplay and
displayIfNeeded explicitly, and, if the draw is not triggered by that,
sending the ack explicitly. This bug has never triggered without vsync
disabled, but apply the workaround in both situations, to be sure.

Use a ScopedClosureRunner to ensure that the swap ack be issued in
CompositorSwapBuffers, rather than sprinkling ack calls all over the
function. Remove the TODOs because the scoped runner should solve
the problem they were working around.

BUG=245900

Review URL: https://codereview.chromium.org/171513012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252784 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 41 insertions, 18 deletions

diff --git a/content/browser/renderer_host/compositing_iosurface_context_mac.mm b/content/browser/renderer_host/compositing_iosurface_context_mac.mm
index 2daef5d..899703b 100644
--- a/content/browser/renderer_host/compositing_iosurface_context_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_context_mac.mm
@@ -144,10 +144,13 @@ CompositingIOSurfaceContext::Get(int window_number) {
     return NULL;
   }
 
-  scoped_refptr<DisplayLinkMac> display_link = DisplayLinkMac::Create();
-  if (!display_link) {
-    LOG(ERROR) << "Failed to create display link for GL context.";
-    return NULL;
+  scoped_refptr<DisplayLinkMac> display_link;
+  if (!is_vsync_disabled) {
+    display_link = DisplayLinkMac::Create();
+    if (!display_link) {
+      LOG(ERROR) << "Failed to create display link for GL context.";
+      return NULL;
+    }
   }
 
   return new CompositingIOSurfaceContext(
diff --git a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
index fcab2b0..4a222f0 100644
--- a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
@@ -57,11 +57,22 @@
 }
 
 - (void)gotNewFrame {
-  if (![self isAsynchronous]) {
+  if (context_ && context_->is_vsync_disabled()) {
+    // If vsync is disabled, draw immediately and don't bother trying to use
+    // the isAsynchronous property to ensure smooth animation.
     [self setNeedsDisplay];
-    [self setAsynchronous:YES];
+    [self displayIfNeeded];
+
+    // Calls to setNeedsDisplay can sometimes be ignored, especially if issued
+    // rapidly (e.g, with vsync off). This is unacceptable because the failure
+    // to ack a single frame will hang the renderer. Ensure that the renderer
+    // not be blocked.
+    if (needsDisplay_)
+      renderWidgetHostView_->SendPendingSwapAck();
   } else {
     needsDisplay_ = YES;
+    if (![self isAsynchronous])
+      [self setAsynchronous:YES];
   }
 }
 
@@ -70,8 +81,15 @@
     return;
 
   [self setAsynchronous:NO];
-  if (needsDisplay_)
+
+  // If there was a pending frame, ensure that it goes through.
+  if (needsDisplay_) {
     [self setNeedsDisplay];
+    [self displayIfNeeded];
+  }
+  // If that fails then ensure that, at a minimum, the renderer is not blocked.
+  if (needsDisplay_)
+    renderWidgetHostView_->SendPendingSwapAck();
 }
 
 - (void)waitForResizedFrameInContext:(CGLContextObj)glContext {
@@ -178,6 +196,11 @@
 
   needsDisplay_ = NO;
   renderWidgetHostView_->SendPendingLatencyInfoToHost();
+
+  [super drawInCGLContext:glContext
+              pixelFormat:pixelFormat
+             forLayerTime:timeInterval
+              displayTime:timeStamp];
 }
 
 @end
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm
index 31f286e..f241c8f 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -1331,6 +1331,12 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
     const gfx::Size& size,
     float surface_scale_factor,
     const std::vector<ui::LatencyInfo>& latency_info) {
+  // Ensure that the frame be acked unless it is explicitly passed to a
+  // display function.
+  base::ScopedClosureRunner scoped_ack(
+      base::Bind(&RenderWidgetHostViewMac::SendPendingSwapAck,
+                 weak_factory_.GetWeakPtr()));
+
   if (render_widget_host_->is_hidden())
     return;
 
@@ -1354,7 +1360,6 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
         compositing_iosurface_->CopyToVideoFrame(
             gfx::Rect(size), frame,
             base::Bind(callback, present_time));
-        SendPendingSwapAck();
         return;
       }
     }
@@ -1440,6 +1445,7 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
   // No need to draw the surface if we are inside a drawRect. It will be done
   // later.
   if (!about_to_validate_and_paint_) {
+    scoped_ack.Reset();
     if (use_core_animation_) {
       DCHECK(compositing_iosurface_layer_);
       compositing_iosurface_layer_async_timer_.Reset();
@@ -1696,9 +1702,6 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
                         params.size,
                         params.scale_factor,
                         params.latency_info);
-  // TODO(ccameron): This ack should not be needed, and will inappropriately
-  // remove GPU back-pressure. Remove this.
-  SendPendingSwapAck();
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
@@ -1716,9 +1719,6 @@ void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
                         params.surface_size,
                         params.surface_scale_factor,
                         params.latency_info);
-  // TODO(ccameron): This ack should not be needed, and will inappropriately
-  // remove GPU back-pressure. Remove this.
-  SendPendingSwapAck();
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceSuspend() {
@@ -1891,6 +1891,7 @@ void RenderWidgetHostViewMac::GotAcceleratedFrame() {
   base::TimeTicks timebase;
   base::TimeDelta interval;
   if (compositing_iosurface_context_ &&
+      compositing_iosurface_context_->display_link() &&
       compositing_iosurface_context_->display_link()->GetVSyncParameters(
           &timebase, &interval)) {
     render_widget_host_->UpdateVSyncParameters(timebase, interval);
@@ -2094,10 +2095,6 @@ void RenderWidgetHostViewMac::AddPendingSwapAck(
   // loss. Drop the old acks.
   pending_swap_ack_.reset(new PendingSwapAck(
       route_id, gpu_host_id, renderer_id));
-  // Ack this immediately if we're already inside a paint call, or if we're
-  // hidden and this may not paint for a long time.
-  if (about_to_validate_and_paint_ || render_widget_host_->is_hidden())
-    SendPendingSwapAck();
 }
 
 void RenderWidgetHostViewMac::SendPendingSwapAck() {