Merge 246077 "Fix bug where GL fence objects were shared between..."

> Fix bug where GL fence objects were shared between contexts.
> 
> These objects were allocated during DrawIOSurface when copying
> the IOSurface to a video frame.
> 
> This pulls the copy to the video frame up to CompositorSwapBuffers,
> and performs the copy in a separate context. Using the separate
> context makes a call to glFlush before the copy necessary (to be sure
> that the copy gets the correct contents), and after, to kick off the async
> copy.
> 
> Moving the copy up to the swap make it so that we won't send more
> frames just because of window damage.
> 
> BUG=334742
> 
> Review URL: https://codereview.chromium.org/137893022



git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@246451 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 59 insertions, 58 deletions

diff --git a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
index fce38b8..0a06d78 100644
--- a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
@@ -125,7 +125,6 @@
         context_,
         window_rect,
         window_scale_factor,
-        renderWidgetHostView_->frame_subscriber(),
         false)) {
     renderWidgetHostView_->GotAcceleratedCompositingError();
   }
diff --git a/content/browser/renderer_host/compositing_iosurface_mac.h b/content/browser/renderer_host/compositing_iosurface_mac.h
index 0649351..315ddc5 100644
--- a/content/browser/renderer_host/compositing_iosurface_mac.h
+++ b/content/browser/renderer_host/compositing_iosurface_mac.h
@@ -50,10 +50,12 @@ class CompositingIOSurfaceMac {
   ~CompositingIOSurfaceMac();
 
   // Set IOSurface that will be drawn on the next NSView drawRect.
-  bool SetIOSurface(uint64 io_surface_handle,
-                    const gfx::Size& size,
-                    float scale_factor,
-                    const ui::LatencyInfo& latency_info);
+  bool SetIOSurfaceWithContextCurrent(
+      scoped_refptr<CompositingIOSurfaceContext> current_context,
+      uint64 io_surface_handle,
+      const gfx::Size& size,
+      float scale_factor,
+      const ui::LatencyInfo& latency_info);
 
   // Get the CGL renderer ID currently associated with this context.
   int GetRendererID();
@@ -62,13 +64,10 @@ class CompositingIOSurfaceMac {
   // with the origin in the lower left corner. If the window rect's size is
   // larger than the IOSurface, the remaining right and bottom edges will be
   // white. |window_scale_factor| is 1 in normal views, 2 in HiDPI views.
-  // |frame_subscriber| listens to this draw event and provides output buffer
-  // for copying this frame into.
   bool DrawIOSurface(
       scoped_refptr<CompositingIOSurfaceContext> drawing_context,
       const gfx::Rect& window_rect,
       float window_scale_factor,
-      RenderWidgetHostViewFrameSubscriber* frame_subscriber,
       bool flush_drawable);
 
   // Copy the data of the "live" OpenGL texture referring to this IOSurfaceRef
@@ -223,7 +222,9 @@ class CompositingIOSurfaceMac {
   bool IsVendorIntel();
 
   // Returns true if IOSurface is ready to render. False otherwise.
-  bool MapIOSurfaceToTexture(uint64 io_surface_handle);
+  bool MapIOSurfaceToTextureWithContextCurrent(
+      const scoped_refptr<CompositingIOSurfaceContext>& current_context,
+      uint64 io_surface_handle);
 
   void UnrefIOSurfaceWithContextCurrent();
 
diff --git a/content/browser/renderer_host/compositing_iosurface_mac.mm b/content/browser/renderer_host/compositing_iosurface_mac.mm
index b5759ed..e948863 100644
--- a/content/browser/renderer_host/compositing_iosurface_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_mac.mm
@@ -328,7 +328,8 @@ CompositingIOSurfaceMac::~CompositingIOSurfaceMac() {
   offscreen_context_ = NULL;
 }
 
-bool CompositingIOSurfaceMac::SetIOSurface(
+bool CompositingIOSurfaceMac::SetIOSurfaceWithContextCurrent(
+    scoped_refptr<CompositingIOSurfaceContext> current_context,
     uint64 io_surface_handle,
     const gfx::Size& size,
     float scale_factor,
@@ -337,14 +338,8 @@ bool CompositingIOSurfaceMac::SetIOSurface(
   scale_factor_ = scale_factor;
   dip_io_surface_size_ = gfx::ToFlooredSize(
       gfx::ScaleSize(pixel_io_surface_size_, 1.0 / scale_factor_));
-
-  CGLError cgl_error = CGLSetCurrentContext(offscreen_context_->cgl_context());
-  if (cgl_error != kCGLNoError) {
-    LOG(ERROR) << "CGLSetCurrentContext error in SetIOSurface: " << cgl_error;
-    return false;
-  }
-  bool result = MapIOSurfaceToTexture(io_surface_handle);
-  CGLSetCurrentContext(0);
+  bool result = MapIOSurfaceToTextureWithContextCurrent(
+      current_context, io_surface_handle);
   latency_info_.MergeWith(latency_info);
   return result;
 }
@@ -362,7 +357,6 @@ bool CompositingIOSurfaceMac::DrawIOSurface(
     scoped_refptr<CompositingIOSurfaceContext> drawing_context,
     const gfx::Rect& window_rect,
     float window_scale_factor,
-    RenderWidgetHostViewFrameSubscriber* frame_subscriber,
     bool flush_drawable) {
   DCHECK_EQ(CGLGetCurrentContext(), drawing_context->cgl_context());
 
@@ -467,18 +461,6 @@ bool CompositingIOSurfaceMac::DrawIOSurface(
     glFinish();
   }
 
-  base::Closure copy_done_callback;
-  if (frame_subscriber) {
-    const base::Time present_time = base::Time::Now();
-    scoped_refptr<media::VideoFrame> frame;
-    RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback callback;
-    if (frame_subscriber->ShouldCaptureFrame(present_time, &frame, &callback)) {
-      copy_done_callback = CopyToVideoFrameWithinContext(
-          gfx::Rect(pixel_io_surface_size_), true, frame,
-          base::Bind(callback, present_time));
-    }
-  }
-
   bool result = true;
   if (flush_drawable) {
     CGLError cgl_error =  CGLFlushDrawable(drawing_context->cgl_context());
@@ -571,7 +553,8 @@ base::Closure CompositingIOSurfaceMac::CopyToVideoFrameWithinContext(
       NULL, target, callback);
 }
 
-bool CompositingIOSurfaceMac::MapIOSurfaceToTexture(
+bool CompositingIOSurfaceMac::MapIOSurfaceToTextureWithContextCurrent(
+    const scoped_refptr<CompositingIOSurfaceContext>& current_context,
     uint64 io_surface_handle) {
   if (io_surface_.get() && io_surface_handle == io_surface_handle_)
     return true;
@@ -603,7 +586,7 @@ bool CompositingIOSurfaceMac::MapIOSurfaceToTexture(
   CHECK_AND_SAVE_GL_ERROR();
   GLuint plane = 0;
   CGLError cgl_error = io_surface_support_->CGLTexImageIOSurface2D(
-      offscreen_context_->cgl_context(),
+      current_context->cgl_context(),
       GL_TEXTURE_RECTANGLE_ARB,
       GL_RGBA,
       rounded_size.width(),
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 9e1c6fd..9897f44 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -845,9 +845,9 @@ bool RenderWidgetHostViewMac::HasFocus() const {
 }
 
 bool RenderWidgetHostViewMac::IsSurfaceAvailableForCopy() const {
-  return !!render_widget_host_->GetBackingStore(false) ||
-      software_frame_manager_->HasCurrentFrame() ||
-      (compositing_iosurface_ && compositing_iosurface_->HasIOSurface());
+  return software_frame_manager_->HasCurrentFrame() ||
+         (compositing_iosurface_ && compositing_iosurface_->HasIOSurface()) ||
+         !!render_widget_host_->GetBackingStore(false);
 }
 
 void RenderWidgetHostViewMac::Show() {
@@ -1308,8 +1308,10 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
       RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback callback;
       if (frame_subscriber_->ShouldCaptureFrame(present_time,
                                                 &frame, &callback)) {
-        compositing_iosurface_->SetIOSurface(
-            surface_handle, size, surface_scale_factor, latency_info);
+        CGLSetCurrentContext(compositing_iosurface_context_->cgl_context());
+        compositing_iosurface_->SetIOSurfaceWithContextCurrent(
+            compositing_iosurface_context_, surface_handle, size,
+            surface_scale_factor, latency_info);
         compositing_iosurface_->CopyToVideoFrame(
             gfx::Rect(size), frame,
             base::Bind(callback, present_time));
@@ -1338,19 +1340,44 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
     return;
   }
 
+  // Ensure compositing_iosurface_ and compositing_iosurface_context_ be
+  // allocated.
   if (!CreateCompositedIOSurface()) {
     LOG(ERROR) << "Failed to create CompositingIOSurface";
     GotAcceleratedCompositingError();
     return;
   }
 
-  if (!compositing_iosurface_->SetIOSurface(
-          surface_handle, size, surface_scale_factor, latency_info)) {
+  // Make the context current and update the IOSurface with the handle
+  // passed in by the swap command.
+  CGLSetCurrentContext(compositing_iosurface_context_->cgl_context());
+  if (!compositing_iosurface_->SetIOSurfaceWithContextCurrent(
+          compositing_iosurface_context_, surface_handle, size,
+          surface_scale_factor, latency_info)) {
     LOG(ERROR) << "Failed SetIOSurface on CompositingIOSurfaceMac";
     GotAcceleratedCompositingError();
     return;
   }
 
+  // Grab video frames now that the IOSurface has been set up. Note that this
+  // will be done in an offscreen context, so it is necessary to re-set the
+  // current context afterward.
+  if (frame_subscriber_) {
+    const base::Time present_time = base::Time::Now();
+    scoped_refptr<media::VideoFrame> frame;
+    RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback callback;
+    if (frame_subscriber_->ShouldCaptureFrame(present_time,
+                                              &frame, &callback)) {
+      // Flush the context that updated the IOSurface, to ensure that the
+      // context that does the copy picks up the correct version.
+      glFlush();
+      compositing_iosurface_->CopyToVideoFrame(
+          gfx::Rect(size), frame,
+          base::Bind(callback, present_time));
+      CGLSetCurrentContext(compositing_iosurface_context_->cgl_context());
+    }
+  }
+
   // Create the layer for the composited content only after the IOSurface has
   // been initialized.
   if (!CreateCompositedIOSurfaceLayer()) {
@@ -1464,23 +1491,13 @@ bool RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
     return true;
   }
 
-  CGLError cgl_error = CGLSetCurrentContext(
-      compositing_iosurface_context_->cgl_context());
-  if (cgl_error != kCGLNoError) {
-    LOG(ERROR) << "CGLSetCurrentContext error in DrawIOSurface: " << cgl_error;
-    return false;
-  }
-
   [compositing_iosurface_context_->nsgl_context() setView:cocoa_view_];
   bool has_overlay = overlay_view_ && overlay_view_->compositing_iosurface_;
 
   gfx::Rect view_rect(NSRectToCGRect([cocoa_view_ frame]));
   if (!compositing_iosurface_->DrawIOSurface(
-      compositing_iosurface_context_,
-      view_rect,
-      scale_factor(),
-      frame_subscriber(),
-      !has_overlay)) {
+          compositing_iosurface_context_, view_rect,
+          scale_factor(), !has_overlay)) {
     return false;
   }
 
@@ -1492,12 +1509,11 @@ bool RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
     overlay_view_rect.set_y(view_rect.height() -
                             overlay_view_rect.height() -
                             overlay_view_offset_.y());
-    return overlay_view_->compositing_iosurface_->DrawIOSurface(
-        compositing_iosurface_context_,
-        overlay_view_rect,
-        overlay_view_->scale_factor(),
-        overlay_view_->frame_subscriber(),
-        true);
+    if (!overlay_view_->compositing_iosurface_->DrawIOSurface(
+            compositing_iosurface_context_, overlay_view_rect,
+            overlay_view_->scale_factor(), true)) {
+      return false;
+    }
   }
 
   return true;
@@ -2904,6 +2920,8 @@ void RenderWidgetHostViewMac::FrameSwapped() {
       NSRectFill(dirtyRect);
     }
 
+    CGLSetCurrentContext(
+        renderWidgetHostView_->compositing_iosurface_context_->cgl_context());
     if (renderWidgetHostView_->DrawIOSurfaceWithoutCoreAnimation())
       return;