Reduce the scope of CGL current contexts

content/browser/renderer_host/render_widget_host_view_mac.mm

Index: content/browser/renderer_host/render_widget_host_view_mac.mm
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 31d9caf5bca0d1cd206dbed13859cbc65499da46..1a44204ced9013be4f6a121b8e29fa68e699803a 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -1337,6 +1337,8 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
     const gfx::Size& size,
     float surface_scale_factor,
     const std::vector<ui::LatencyInfo>& latency_info) {
+  DCHECK(!CGLGetCurrentContext());

[Ken Russell (switch to Gerrit), 2014/03/28 16:18:41]

Have you run a debug build with this DCHECK in place for some period of time to
see whether it's triggered?

[ccameron, 2014/03/28 16:22:49]

Yes, but, you bring up a good point -- I haven't run a build long enough. I'll
want to add a bunch of CHECKs to ensure this and some other things (like that
the expected context is current when ~ScopedCGLSetCurrentContext) are true. I'll
put those in a separate patch.

+
   // Ensure that the frame be acked unless it is explicitly passed to a
   // display function.
   base::ScopedClosureRunner scoped_ack(
@@ -1363,13 +1365,15 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
 
   // Make the context current and update the IOSurface with the handle
   // passed in by the swap command.
-  gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
-      compositing_iosurface_context_->cgl_context());
-  if (!compositing_iosurface_->SetIOSurfaceWithContextCurrent(
-          compositing_iosurface_context_, surface_handle, size,
-          surface_scale_factor)) {
-    LOG(ERROR) << "Failed SetIOSurface on CompositingIOSurfaceMac";
-    return;
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        compositing_iosurface_context_->cgl_context());
+    if (!compositing_iosurface_->SetIOSurfaceWithContextCurrent(
+            compositing_iosurface_context_, surface_handle, size,
+            surface_scale_factor)) {
+      LOG(ERROR) << "Failed SetIOSurface on CompositingIOSurfaceMac";
+      return;
+    }
   }
 
   // Grab video frames now that the IOSurface has been set up. Note that this
@@ -1384,12 +1388,14 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
                                               &frame, &callback)) {
       // Flush the context that updated the IOSurface, to ensure that the
       // context that does the copy picks up the correct version.
-      glFlush();
+      {

[ccameron, 2014/03/28 10:01:48]

Of note is that this should potentially be moved up to the
SetIOSurfaceWithContextCurrent block unconditionally. I'm leaving this as-is to
minimize altered behavior.

+        gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+            compositing_iosurface_context_->cgl_context());
+        glFlush();
+      }
       compositing_iosurface_->CopyToVideoFrame(
           gfx::Rect(size), frame,
           base::Bind(callback, present_time));
-      DCHECK_EQ(CGLGetCurrentContext(),
-                compositing_iosurface_context_->cgl_context());
       frame_was_captured = true;
     }
   }
@@ -1447,6 +1453,8 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
     compositing_iosurface_layer_async_timer_.Reset();
     [compositing_iosurface_layer_ gotNewFrame];
   } else {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        compositing_iosurface_context_->cgl_context());
     DrawIOSurfaceWithoutCoreAnimation();
   }