Fix assorted issues with remote CoreAnimation

These issues came up while running for a few days with the
flag --enable-remote-core-animation.

Fix flashes of old frames by hooking up the DiscardBackbuffer (which
happens when being made non-visible) to re-set the CAContext and
CALayer (so the browser gets a new one with new content for the next
frame).

Add support for disabling vsync by using setNeedsDisplay to draw.

Change the backpressure mechanism to rely on the browser to apply
backpressure in its compositor via the cc:: output surface.

Add a ScopedSetGLToRealGLApi structure to ensure that we are talking
to the real GL API while in the CoreAnimation callback.

BUG=312462

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

Cr-Commit-Position: refs/heads/master@{#292293}

content/browser/compositor/browser_compositor_view_private_mac.mm

@@ -61,6 +61,7 @@ BrowserCompositorViewMacInternal::BrowserCompositorViewMacInternal()
       native_widget_,
       content::GetContextFactory(),
       RenderWidgetResizeHelper::Get()->task_runner()));
+  compositor_->SetVisible(false);
 }
 
 BrowserCompositorViewMacInternal::~BrowserCompositorViewMacInternal() {
@@ -81,6 +82,7 @@ void BrowserCompositorViewMacInternal::SetClient(
   DCHECK(background_layer);
   [flipped_layer_ setBounds:[background_layer bounds]];
   [background_layer addSublayer:flipped_layer_];
+  compositor_->SetVisible(true);
 }
 
 void BrowserCompositorViewMacInternal::ResetClient() {
@@ -98,6 +100,7 @@ void BrowserCompositorViewMacInternal::ResetClient() {
   accelerated_output_surface_id_ = 0;
   last_swap_size_dip_ = gfx::Size();
 
+  compositor_->SetVisible(false);
   compositor_->SetScaleAndSize(1.0, gfx::Size(0, 0));
   compositor_->SetRootLayer(NULL);
   client_ = NULL;

content/common/gpu/image_transport_surface_calayer_mac.h

@@ -28,18 +28,27 @@ class CALayerStorageProvider
       CGLContextObj context, GLuint texture,
       gfx::Size pixel_size, float scale_factor) OVERRIDE;
   virtual void FreeColorBufferStorage() OVERRIDE;
-  virtual uint64 GetSurfaceHandle() const OVERRIDE;
-  virtual void WillSwapBuffers() OVERRIDE;
-  virtual void CanFreeSwappedBuffer() OVERRIDE;
+  virtual void SwapBuffers(const gfx::Size& size, float scale_factor) OVERRIDE;
+  virtual void WillWriteToBackbuffer() OVERRIDE;
+  virtual void DiscardBackbuffer() OVERRIDE;
+  virtual void SwapBuffersAckedByBrowser() OVERRIDE;
 
   // Interface to ImageTransportLayer:
   CGLContextObj LayerShareGroupContext();
   bool LayerCanDraw();
   void LayerDoDraw();
+  void LayerResetStorageProvider();
 
  private:
+  void DrawWithVsyncDisabled();
+  void SendPendingSwapToBrowserAfterFrameDrawn();
+
   ImageTransportSurfaceFBO* transport_surface_;
 
+  // Used to determine if we should use setNeedsDisplay or setAsynchronous to
+  // animate.
+  const bool gpu_vsync_disabled_;
+
   // Set when a new swap occurs, and un-set when |layer_| draws that frame.
   bool has_pending_draw_;
 
@@ -53,11 +62,13 @@ class CALayerStorageProvider
   base::ScopedTypeRef<CGLContextObj> share_group_context_;
   GLuint fbo_texture_;
   gfx::Size fbo_pixel_size_;
+  float fbo_scale_factor_;
 
   // The CALayer that the current frame is being drawn into.
   base::scoped_nsobject<CAContext> context_;
   base::scoped_nsobject<ImageTransportLayer> layer_;
 
+  base::WeakPtrFactory<CALayerStorageProvider> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(CALayerStorageProvider);
 };
 

content/common/gpu/image_transport_surface_calayer_mac.mm

@@ -4,10 +4,13 @@
 
 #include "content/common/gpu/image_transport_surface_calayer_mac.h"
 
+#include "base/command_line.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "content/common/gpu/surface_handle_types_mac.h"
 #include "ui/base/cocoa/animation_utils.h"
 #include "ui/gfx/geometry/size_conversions.h"
+#include "ui/gl/gl_gl_api_implementation.h"
+#include "ui/gl/gl_switches.h"
 
 @interface ImageTransportLayer : CAOpenGLLayer {
   content::CALayerStorageProvider* storageProvider_;
@@ -26,6 +29,8 @@
 }
 
 - (void)resetStorageProvider {
+  if (storageProvider_)
+    storageProvider_->LayerResetStorageProvider();
   storageProvider_ = NULL;
 }
 
@@ -60,6 +65,10 @@
              pixelFormat:(CGLPixelFormatObj)pixelFormat
             forLayerTime:(CFTimeInterval)timeInterval
              displayTime:(const CVTimeStamp*)timeStamp {
+  // While in this callback, CoreAnimation has set |glContext| to be current.
+  // Ensure that the GL calls that we make are made against the native GL API.
+  gfx::ScopedSetGLToRealGLApi scoped_set_gl_api;
+
   if (storageProvider_) {
     storageProvider_->LayerDoDraw();
   } else {
@@ -79,17 +88,13 @@ namespace content {
 CALayerStorageProvider::CALayerStorageProvider(
     ImageTransportSurfaceFBO* transport_surface)
         : transport_surface_(transport_surface),
+          gpu_vsync_disabled_(CommandLine::ForCurrentProcess()->HasSwitch(
+              switches::kDisableGpuVsync)),
           has_pending_draw_(false),
           can_draw_returned_false_count_(0),
-          fbo_texture_(0) {
-  // Allocate a CAContext to use to transport the CALayer to the browser
-  // process.
-  base::scoped_nsobject<NSDictionary> dict([[NSDictionary alloc] init]);
-  CGSConnectionID connection_id = CGSMainConnectionID();
-  context_.reset([CAContext contextWithCGSConnection:connection_id
-                                             options:dict]);
-  [context_ retain];
-}
+          fbo_texture_(0),
+          fbo_scale_factor_(1),
+          weak_factory_(this) {}
 
 CALayerStorageProvider::~CALayerStorageProvider() {
 }
@@ -126,15 +131,12 @@ bool CALayerStorageProvider::AllocateColorBufferStorage(
   // Disable the fade-in animation as the layer is changed.
   ScopedCAActionDisabler disabler;
 
-  // Allocate a CALayer to draw texture into.
+  // Set the parameters that will be used to allocate the CALayer to draw the
+  // texture into.
   share_group_context_.reset(CGLRetainContext(context));
   fbo_texture_ = texture;
   fbo_pixel_size_ = pixel_size;
-  layer_.reset([[ImageTransportLayer alloc] initWithStorageProvider:this]);
-  gfx::Size dip_size(gfx::ToFlooredSize(gfx::ScaleSize(
-      fbo_pixel_size_, 1.0f / scale_factor)));
-  [layer_ setContentsScale:scale_factor];
-  [layer_ setFrame:CGRectMake(0, 0, dip_size.width(), dip_size.height())];
+  fbo_scale_factor_ = scale_factor;
   return true;
 }
 
@@ -155,25 +157,78 @@ void CALayerStorageProvider::FreeColorBufferStorage() {
   fbo_pixel_size_ = gfx::Size();
 }
 
-uint64 CALayerStorageProvider::GetSurfaceHandle() const {
-  return SurfaceHandleFromCAContextID([context_ contextId]);
-}
-
-void CALayerStorageProvider::WillSwapBuffers() {
+void CALayerStorageProvider::SwapBuffers(
+    const gfx::Size& size, float scale_factor) {
   DCHECK(!has_pending_draw_);
   has_pending_draw_ = true;
 
-  // Don't add the layer to the CAContext until a SwapBuffers is going to be
-  // called, because the texture does not have any content until the
-  // SwapBuffers call is about to be made.
-  if ([context_ layer] != layer_.get())
+  // Allocate a CAContext to use to transport the CALayer to the browser
+  // process.
+  if (!context_) {
+    base::scoped_nsobject<NSDictionary> dict([[NSDictionary alloc] init]);
+    CGSConnectionID connection_id = CGSMainConnectionID();
+    context_.reset([CAContext contextWithCGSConnection:connection_id
+                                               options:dict]);
+    [context_ retain];
+  }
+
+  // Allocate a CALayer to use to draw the content.
+  if (!layer_) {
+    layer_.reset([[ImageTransportLayer alloc] initWithStorageProvider:this]);
+    gfx::Size dip_size(gfx::ToFlooredSize(gfx::ScaleSize(
+        fbo_pixel_size_, 1.0f / fbo_scale_factor_)));
+    [layer_ setContentsScale:fbo_scale_factor_];
+    [layer_ setFrame:CGRectMake(0, 0, dip_size.width(), dip_size.height())];
+
+    // Make the CALayer current to the CAContext and display its contents
+    // immediately.
     [context_ setLayer:layer_];
+  }
 
-  if (![layer_ isAsynchronous])
-    [layer_ setAsynchronous:YES];
+  // Tell CoreAnimation to draw our frame. We will send the IPC to the browser
+  // when CoreAnimation has drawn our frame.
+  if (gpu_vsync_disabled_) {
+    DrawWithVsyncDisabled();
+  } else {
+    if (![layer_ isAsynchronous])
+      [layer_ setAsynchronous:YES];
+  }
 }
 
-void CALayerStorageProvider::CanFreeSwappedBuffer() {
+void CALayerStorageProvider::DrawWithVsyncDisabled() {
+  DCHECK(has_pending_draw_);
+  [layer_ setNeedsDisplay];
+
+  // Sometimes, setNeedsDisplay calls are dropped on the floor. Make this not
+  // hang the renderer by re-issuing the call if the draw has not yet
+  // happened.
+  if (has_pending_draw_) {
+    // Delay sending another draw immediately to avoid starving the run loop.
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&CALayerStorageProvider::DrawWithVsyncDisabled,
+                   weak_factory_.GetWeakPtr()),
+        base::TimeDelta::FromMilliseconds(5));
+  }
+}
+
+void CALayerStorageProvider::WillWriteToBackbuffer() {
+  // TODO(ccameron): The browser may need to continue issuing swaps even when
+  // they do not draw. In these cases it is necessary to either double-buffer
+  // the resulting texture, or to drop frames.
+}
+
+void CALayerStorageProvider::DiscardBackbuffer() {
+  // If this surface's backbuffer is discarded, it is because this surface has
+  // been made non-visible. Ensure that the previous contents are not briefly
+  // flashed when this is made visible by creating a new CALayer and CAContext
+  // at the next swap.
+  [layer_ resetStorageProvider];
+  layer_.reset();
+  context_.reset();
+}
+
+void CALayerStorageProvider::SwapBuffersAckedByBrowser() {
 }
 
 CGLContextObj CALayerStorageProvider::LayerShareGroupContext() {
@@ -185,20 +240,22 @@ bool CALayerStorageProvider::LayerCanDraw() {
     can_draw_returned_false_count_ = 0;
     return true;
   } else {
-    if (can_draw_returned_false_count_ == 30) {
-      if ([layer_ isAsynchronous])
+    if ([layer_ isAsynchronous]) {
+      DCHECK(!gpu_vsync_disabled_);
+      // If we are in asynchronous mode, we will be getting callbacks at every
+      // vsync, asking us if we have anything to draw. If we get 30 of these in
+      // a row, ask that we stop getting these callback for now, so that we
+      // don't waste CPU cycles.
+      if (can_draw_returned_false_count_ == 30)
         [layer_ setAsynchronous:NO];
-    } else {
-      can_draw_returned_false_count_ += 1;
+      else
+        can_draw_returned_false_count_ += 1;
     }
     return false;
   }
 }
 
 void CALayerStorageProvider::LayerDoDraw() {
-  DCHECK(has_pending_draw_);
-  has_pending_draw_ = false;
-
   GLint viewport[4] = {0, 0, 0, 0};
   glGetIntegerv(GL_VIEWPORT, viewport);
   gfx::Size viewport_size(viewport[2], viewport[3]);
@@ -233,7 +290,25 @@ void CALayerStorageProvider::LayerDoDraw() {
   glDisable(GL_TEXTURE_RECTANGLE_ARB);
 
   // Allow forward progress in the context now that the swap is complete.
-  transport_surface_->UnblockContextAfterPendingSwap();
+  DCHECK(has_pending_draw_);
+  SendPendingSwapToBrowserAfterFrameDrawn();
+}
+
+void CALayerStorageProvider::LayerResetStorageProvider() {
+  // If we are providing back-pressure by waiting for a draw, that draw will
+  // now never come, so release the pressure now.
+  SendPendingSwapToBrowserAfterFrameDrawn();
+}
+
+void CALayerStorageProvider::SendPendingSwapToBrowserAfterFrameDrawn() {
+  if (!has_pending_draw_)
+    return;
+  weak_factory_.InvalidateWeakPtrs();
+  has_pending_draw_ = false;
+  transport_surface_->SendSwapBuffers(
+      SurfaceHandleFromCAContextID([context_ contextId]),
+      fbo_pixel_size_,
+      fbo_scale_factor_);
 }
 
 }  //  namespace content

content/common/gpu/image_transport_surface_fbo_mac.h

@@ -41,20 +41,20 @@ class ImageTransportSurfaceFBO
     // GL texture that was bound has already been deleted by the caller.
     virtual void FreeColorBufferStorage() = 0;
 
-    // Retrieve the handle for the surface to send to the browser process to
-    // display.
-    virtual uint64 GetSurfaceHandle() const = 0;
-
-    // Called when a new frame has been rendered into the texture, and the
-    // browser is about to be sent the surface to display.
-    virtual void WillSwapBuffers() = 0;
-
-    // Called once for every WillSwapBuffers call when the buffer that was sent
-    // to the browser may be released by the GPU process (this may be because
-    // the browser is holding a reference, in which case this will come
-    // quickly, or it may be because the browser is done with the surface, in
-    // which case it will come much later).
-    virtual void CanFreeSwappedBuffer() = 0;
+    // Swap buffers and return the handle for the surface to send to the browser
+    // process to display.
+    virtual void SwapBuffers(const gfx::Size& size, float scale_factor) = 0;
+
+    // Indicate that the backbuffer will be written to.
+    virtual void WillWriteToBackbuffer() = 0;
+
+    // Indicate that the backbuffer has been discarded and should not be seen
+    // again.
+    virtual void DiscardBackbuffer() = 0;
+
+    // Called once for every SwapBuffers call when the IPC for the present has
+    // been processed by the browser.
+    virtual void SwapBuffersAckedByBrowser() = 0;
   };
 
   ImageTransportSurfaceFBO(GpuChannelManager* manager,
@@ -78,7 +78,9 @@ class ImageTransportSurfaceFBO
   virtual void SetFrontbufferAllocation(bool allocated) OVERRIDE;
 
   // Called when the context may continue to make forward progress after a swap.
-  void UnblockContextAfterPendingSwap();
+  void SendSwapBuffers(uint64 surface_handle,
+                       const gfx::Size pixel_size,
+                       float scale_factor);
 
  protected:
   // ImageTransportSurface implementation
@@ -120,12 +122,8 @@ class ImageTransportSurfaceFBO
   // Whether or not we've successfully made the surface current once.
   bool made_current_;
 
-  // Whether a SwapBuffers is pending.
-  bool is_swap_buffers_pending_;
-
-  // Whether we unscheduled command buffer because of pending SwapBuffers.
-  bool did_unschedule_;
-
+  // Whether a SwapBuffers IPC needs to be sent to the browser.
+  bool is_swap_buffers_send_pending_;
   std::vector<ui::LatencyInfo> latency_info_;
 
   scoped_ptr<ImageTransportHelper> helper_;

content/common/gpu/image_transport_surface_fbo_mac.mm

@@ -28,8 +28,7 @@ ImageTransportSurfaceFBO::ImageTransportSurfaceFBO(
       context_(NULL),
       scale_factor_(1.f),
       made_current_(false),
-      is_swap_buffers_pending_(false),
-      did_unschedule_(false) {
+      is_swap_buffers_send_pending_(false) {
   if (ui::RemoteLayerAPISupported())
     storage_provider_.reset(new CALayerStorageProvider(this));
   else
@@ -62,17 +61,9 @@ void ImageTransportSurfaceFBO::Destroy() {
 }
 
 bool ImageTransportSurfaceFBO::DeferDraws() {
-  // The command buffer hit a draw/clear command that could clobber the
-  // IOSurface in use by an earlier SwapBuffers. If a Swap is pending, abort
-  // processing of the command by returning true and unschedule until the Swap
-  // Ack arrives.
-  if(did_unschedule_)
-    return true;  // Still unscheduled, so just return true.
-  if (is_swap_buffers_pending_) {
-    did_unschedule_ = true;
-    helper_->SetScheduled(false);
-    return true;
-  }
+  storage_provider_->WillWriteToBackbuffer();
+  // We should not have a pending send when we are drawing the next frame.
+  DCHECK(!is_swap_buffers_send_pending_);
   return false;
 }
 
@@ -101,6 +92,8 @@ bool ImageTransportSurfaceFBO::SetBackbufferAllocation(bool allocation) {
     return true;
   backbuffer_suggested_allocation_ = allocation;
   AdjustBufferAllocation();
+  if (!allocation)
+    storage_provider_->DiscardBackbuffer();
   return true;
 }
 
@@ -130,18 +123,22 @@ bool ImageTransportSurfaceFBO::SwapBuffers() {
     return true;
   glFlush();
 
+  // It is the responsibility of the storage provider to send the swap IPC.
+  is_swap_buffers_send_pending_ = true;
+  storage_provider_->SwapBuffers(size_, scale_factor_);
+  return true;
+}
+
+void ImageTransportSurfaceFBO::SendSwapBuffers(uint64 surface_handle,
+                                               const gfx::Size pixel_size,
+                                               float scale_factor) {
   GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
-  params.surface_handle = storage_provider_->GetSurfaceHandle();
-  params.size = GetSize();
-  params.scale_factor = scale_factor_;
+  params.surface_handle = surface_handle;
+  params.size = pixel_size;
+  params.scale_factor = scale_factor;
   params.latency_info.swap(latency_info_);
   helper_->SendAcceleratedSurfaceBuffersSwapped(params);
-
-  DCHECK(!is_swap_buffers_pending_);
-  is_swap_buffers_pending_ = true;
-
-  storage_provider_->WillSwapBuffers();
-  return true;
+  is_swap_buffers_send_pending_ = false;
 }
 
 bool ImageTransportSurfaceFBO::PostSubBuffer(
@@ -170,16 +167,7 @@ void* ImageTransportSurfaceFBO::GetDisplay() {
 void ImageTransportSurfaceFBO::OnBufferPresented(
     const AcceleratedSurfaceMsg_BufferPresented_Params& params) {
   context_->share_group()->SetRendererID(params.renderer_id);
-  storage_provider_->CanFreeSwappedBuffer();
-}
-
-void ImageTransportSurfaceFBO::UnblockContextAfterPendingSwap() {
-  DCHECK(is_swap_buffers_pending_);
-  is_swap_buffers_pending_ = false;
-  if (did_unschedule_) {
-    did_unschedule_ = false;
-    helper_->SetScheduled(true);
-  }
+  storage_provider_->SwapBuffersAckedByBrowser();
 }
 
 void ImageTransportSurfaceFBO::OnResize(gfx::Size size,

content/common/gpu/image_transport_surface_iosurface_mac.cc

@@ -105,21 +105,26 @@ void IOSurfaceStorageProvider::FreeColorBufferStorage() {
   io_surface_id_ = 0;
 }
 
-uint64 IOSurfaceStorageProvider::GetSurfaceHandle() const {
-  return SurfaceHandleFromIOSurfaceID(io_surface_id_);
-}
-
-void IOSurfaceStorageProvider::WillSwapBuffers() {
+void IOSurfaceStorageProvider::SwapBuffers(
+    const gfx::Size& size, float scale_factor) {
   // The browser compositor will throttle itself, so we are free to unblock the
   // context immediately. Make sure that the browser is doing its throttling
   // appropriately by ensuring that the previous swap was acknowledged before
   // we get another swap.
   DCHECK(pending_swapped_surfaces_.empty());
   pending_swapped_surfaces_.push_back(io_surface_);
-  transport_surface_->UnblockContextAfterPendingSwap();
+
+  transport_surface_->SendSwapBuffers(
+      SurfaceHandleFromIOSurfaceID(io_surface_id_), size, scale_factor);
+}
+
+void IOSurfaceStorageProvider::WillWriteToBackbuffer() {
+}
+
+void IOSurfaceStorageProvider::DiscardBackbuffer() {
 }
 
-void IOSurfaceStorageProvider::CanFreeSwappedBuffer() {
+void IOSurfaceStorageProvider::SwapBuffersAckedByBrowser() {
   DCHECK(!pending_swapped_surfaces_.empty());
   pending_swapped_surfaces_.pop_front();
 }

content/common/gpu/image_transport_surface_iosurface_mac.h

@@ -28,9 +28,10 @@ class IOSurfaceStorageProvider
       CGLContextObj context, GLuint texture,
       gfx::Size pixel_size, float scale_factor) OVERRIDE;
   virtual void FreeColorBufferStorage() OVERRIDE;
-  virtual uint64 GetSurfaceHandle() const OVERRIDE;
-  virtual void WillSwapBuffers() OVERRIDE;
-  virtual void CanFreeSwappedBuffer() OVERRIDE;
+  virtual void SwapBuffers(const gfx::Size& size, float scale_factor) OVERRIDE;
+  virtual void WillWriteToBackbuffer() OVERRIDE;
+  virtual void DiscardBackbuffer() OVERRIDE;
+  virtual void SwapBuffersAckedByBrowser() OVERRIDE;
 
  private:
   ImageTransportSurfaceFBO* transport_surface_;

ui/compositor/compositor.cc

@@ -276,6 +276,10 @@ void Compositor::SetBackgroundColor(SkColor color) {
   ScheduleDraw();
 }
 
+void Compositor::SetVisible(bool visible) {
+  host_->SetVisible(visible);
+}
+
 scoped_refptr<CompositorVSyncManager> Compositor::vsync_manager() const {
   return vsync_manager_;
 }

ui/compositor/compositor.h

@@ -186,6 +186,9 @@ class COMPOSITOR_EXPORT Compositor
   // the |root_layer|.
   void SetBackgroundColor(SkColor color);
 
+  // Set the visibility of the underlying compositor.
+  void SetVisible(bool visible);
+
   // Returns the widget for this compositor.
   gfx::AcceleratedWidget widget() const { return widget_; }
 

ui/gl/gl_gl_api_implementation.cc

@@ -492,4 +492,13 @@ void VirtualGLApi::glFinishFn() {
   GLApiBase::SignalFlush();
 }
 
+ScopedSetGLToRealGLApi::ScopedSetGLToRealGLApi()
+    : old_gl_api_(GetCurrentGLApi()) {
+  SetGLToRealGLApi();
+}
+
+ScopedSetGLToRealGLApi::~ScopedSetGLToRealGLApi() {
+  SetGLApi(old_gl_api_);
+}
+
 }  // namespace gfx

ui/gl/gl_gl_api_implementation.h

@@ -7,6 +7,7 @@
 
 #include "base/compiler_specific.h"
 #include "ui/gl/gl_bindings.h"
+#include "ui/gl/gl_export.h"
 
 namespace gpu {
 namespace gles2 {
@@ -117,6 +118,15 @@ private:
   std::string extensions_;
 };
 
+class GL_EXPORT ScopedSetGLToRealGLApi {
+ public:
+  ScopedSetGLToRealGLApi();
+  ~ScopedSetGLToRealGLApi();
+
+ private:
+  GLApi* old_gl_api_;
+};
+
 }  // namespace gfx
 
 #endif  // UI_GL_GL_GL_API_IMPLEMENTATION_H_