Reduce the scope of CGL current contexts

content/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/render_widget_host_view_mac.h"
 
 #import <objc/runtime.h>
 #include <QuartzCore/QuartzCore.h>
 
 #include "base/basictypes.h"
@@ skipping 1319 matching lines @@
     render_widget_host_->Send(new ViewMsg_PluginImeCompositionCompleted(
         render_widget_host_->GetRoutingID(), text, plugin_id));
   }
 }
 
 void RenderWidgetHostViewMac::CompositorSwapBuffers(
     uint64 surface_handle,
     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(
       base::Bind(&RenderWidgetHostViewMac::SendPendingSwapAck,
                  weak_factory_.GetWeakPtr()));
 
   if (render_widget_host_->is_hidden())
     return;
 
   // Ensure that if this function exits before the frame is set up (but not
   // necessarily drawn) then it is treated as an error.
   base::ScopedClosureRunner scoped_error(
       base::Bind(&RenderWidgetHostViewMac::GotAcceleratedCompositingError,
                  weak_factory_.GetWeakPtr()));
 
   AddPendingLatencyInfo(latency_info);
 
   // Ensure compositing_iosurface_ and compositing_iosurface_context_ be
   // allocated.
   if (!EnsureCompositedIOSurface()) {
     LOG(ERROR) << "Failed EnsureCompositingIOSurface";
     return;
   }
 
   // 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
   // will be done in an offscreen context, so it is necessary to re-set the
   // current context afterward.
   bool frame_was_captured = false;
   if (frame_subscriber_) {
     const base::TimeTicks present_time = base::TimeTicks::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();
+      {

[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;
     }
   }
 
   // At this point the surface, its context, and its layer have been set up, so
   // don't generate an error (one may be generated when drawing).
   ignore_result(scoped_error.Release());
 
   GotAcceleratedFrame();
 
@@ skipping 37 matching lines @@
   }
 
   // If we reach here, then the frame will be displayed by a future draw
   // call, so don't make the callback.
   ignore_result(scoped_ack.Release());
   if (use_core_animation_) {
     DCHECK(compositing_iosurface_layer_);
     compositing_iosurface_layer_async_timer_.Reset();
     [compositing_iosurface_layer_ gotNewFrame];
   } else {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        compositing_iosurface_context_->cgl_context());
     DrawIOSurfaceWithoutCoreAnimation();
   }
 
   // The IOSurface's size may have changed, so re-layout the layers to take
   // this into account. This may force an immediate draw.
   LayoutLayers();
 }
 
 void RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
   CHECK(!use_core_animation_);
@@ skipping 2827 matching lines @@
 }
 
 - (void)disableRendering {
   // Disable the fade-out animation as the layer is removed.
   ScopedCAActionDisabler disabler;
   [self removeFromSuperlayer];
   renderWidgetHostView_ = nil;
 }
 
 @end  // implementation SoftwareLayer