Use base::ScopedTypeRef for CGL types.

Use gfx::ScopedCGLSetCurrentContext for setting the current
context in the browser process. When calling CGLSetCurrentContext,
the pre-existing code inconsitently did either of restoring the
context, setting the context to NULL, or leaving the context set.
This makes the behavior consistent. It also makes error checking
pervasive.

BUG=245900

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

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

7 files changed, 117 insertions, 110 deletions

diff --git a/content/browser/renderer_host/compositing_iosurface_context_mac.h b/content/browser/renderer_host/compositing_iosurface_context_mac.h
index 4abb008..357925b 100644
--- a/content/browser/renderer_host/compositing_iosurface_context_mac.h
+++ b/content/browser/renderer_host/compositing_iosurface_context_mac.h
@@ -15,6 +15,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/browser/renderer_host/display_link_mac.h"
+#include "ui/gl/scoped_cgl.h"
 
 namespace content {
 
@@ -60,7 +61,7 @@ class CompositingIOSurfaceContext
   CompositingIOSurfaceContext(
       int window_number,
       NSOpenGLContext* nsgl_context,
-      CGLContextObj clg_context_strong,
+      base::ScopedTypeRef<CGLContextObj> clg_context_strong,
       CGLContextObj clg_context,
       bool is_vsync_disabled_,
       scoped_refptr<DisplayLinkMac> display_link,
@@ -69,7 +70,7 @@ class CompositingIOSurfaceContext
 
   int window_number_;
   base::scoped_nsobject<NSOpenGLContext> nsgl_context_;
-  CGLContextObj cgl_context_strong_;
+  base::ScopedTypeRef<CGLContextObj> cgl_context_strong_;
   // Weak, backed by |nsgl_context_| or |cgl_context_strong_|.
   CGLContextObj cgl_context_;
 
diff --git a/content/browser/renderer_host/compositing_iosurface_context_mac.mm b/content/browser/renderer_host/compositing_iosurface_context_mac.mm
index 4699392..2daef5d 100644
--- a/content/browser/renderer_host/compositing_iosurface_context_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_context_mac.mm
@@ -16,43 +16,6 @@
 #include "ui/gl/gl_switches.h"
 #include "ui/gl/gpu_switching_manager.h"
 
-namespace {
-
-template<typename T, void Release(T)>
-class ScopedCGLTypeRef {
- public:
-  ScopedCGLTypeRef() : object_(NULL) {}
-
-  ~ScopedCGLTypeRef() {
-    if (object_)
-      Release(object_);
-    object_ = NULL;
-  }
-
-  // Only to be used for pass-by-pointer initialization. The object must have
-  // been reset to NULL prior to calling.
-  T* operator&() {
-    DCHECK(object_ == NULL);
-    return &object_;
-  }
-
-  operator T() const {
-    return object_;
-  }
-
-  T release() WARN_UNUSED_RESULT {
-    T object = object_;
-    object_ = NULL;
-    return object;
-  }
-
- private:
-  T object_;
-  DISALLOW_COPY_AND_ASSIGN(ScopedCGLTypeRef);
-};
-
-}
-
 namespace content {
 
 CoreAnimationStatus GetCoreAnimationStatus() {
@@ -78,7 +41,7 @@ CompositingIOSurfaceContext::Get(int window_number) {
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableGpuVsync);
 
   base::scoped_nsobject<NSOpenGLContext> nsgl_context;
-  ScopedCGLTypeRef<CGLContextObj, CGLReleaseContext> cgl_context_strong;
+  base::ScopedTypeRef<CGLContextObj> cgl_context_strong;
   CGLContextObj cgl_context = NULL;
   if (GetCoreAnimationStatus() == CORE_ANIMATION_DISABLED) {
     std::vector<NSOpenGLPixelFormatAttribute> attributes;
@@ -134,9 +97,10 @@ CompositingIOSurfaceContext::Get(int window_number) {
     }
     attribs.push_back(static_cast<CGLPixelFormatAttribute>(0));
     GLint number_virtual_screens = 0;
-    ScopedCGLTypeRef<CGLPixelFormatObj, CGLReleasePixelFormat> pixel_format;
-    error = CGLChoosePixelFormat(
-        &attribs.front(), &pixel_format, &number_virtual_screens);
+    base::ScopedTypeRef<CGLPixelFormatObj> pixel_format;
+    error = CGLChoosePixelFormat(&attribs.front(),
+                                 pixel_format.InitializeInto(),
+                                 &number_virtual_screens);
     if (error != kCGLNoError) {
       LOG(ERROR) << "Failed to create pixel format object.";
       return NULL;
@@ -148,7 +112,7 @@ CompositingIOSurfaceContext::Get(int window_number) {
     if (!window_map()->empty())
       share_context = window_map()->begin()->second->cgl_context();
     error = CGLCreateContext(
-        pixel_format, share_context, &cgl_context_strong);
+        pixel_format, share_context, cgl_context_strong.InitializeInto());
     if (error != kCGLNoError) {
       LOG(ERROR) << "Failed to create context object.";
       return NULL;
@@ -161,19 +125,20 @@ CompositingIOSurfaceContext::Get(int window_number) {
 
   // Prepare the shader program cache. Precompile the shader programs
   // needed to draw the IO Surface for non-offscreen contexts.
-  CGLSetCurrentContext(cgl_context);
-  scoped_ptr<CompositingIOSurfaceShaderPrograms> shader_program_cache(
-      new CompositingIOSurfaceShaderPrograms());
   bool prepared = false;
-  if (window_number == kOffscreenContextWindowNumber) {
-    prepared = true;
-  } else {
-    prepared = (
-        shader_program_cache->UseBlitProgram() &&
-        shader_program_cache->UseSolidWhiteProgram());
+  scoped_ptr<CompositingIOSurfaceShaderPrograms> shader_program_cache;
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(cgl_context);
+    shader_program_cache.reset(new CompositingIOSurfaceShaderPrograms());
+    if (window_number == kOffscreenContextWindowNumber) {
+      prepared = true;
+    } else {
+      prepared = (
+          shader_program_cache->UseBlitProgram() &&
+          shader_program_cache->UseSolidWhiteProgram());
+    }
+    glUseProgram(0u);
   }
-  glUseProgram(0u);
-  CGLSetCurrentContext(0);
   if (!prepared) {
     LOG(ERROR) << "IOSurface failed to compile/link required shader programs.";
     return NULL;
@@ -188,7 +153,7 @@ CompositingIOSurfaceContext::Get(int window_number) {
   return new CompositingIOSurfaceContext(
       window_number,
       nsgl_context.release(),
-      cgl_context_strong.release(),
+      cgl_context_strong,
       cgl_context,
       is_vsync_disabled,
       display_link,
@@ -208,7 +173,7 @@ void CompositingIOSurfaceContext::MarkExistingContextsAsNotShareable() {
 CompositingIOSurfaceContext::CompositingIOSurfaceContext(
     int window_number,
     NSOpenGLContext* nsgl_context,
-    CGLContextObj cgl_context_strong,
+    base::ScopedTypeRef<CGLContextObj> cgl_context_strong,
     CGLContextObj cgl_context,
     bool is_vsync_disabled,
     scoped_refptr<DisplayLinkMac> display_link,
@@ -229,9 +194,10 @@ CompositingIOSurfaceContext::CompositingIOSurfaceContext(
 }
 
 CompositingIOSurfaceContext::~CompositingIOSurfaceContext() {
-  CGLSetCurrentContext(cgl_context_);
-  shader_program_cache_->Reset();
-  CGLSetCurrentContext(0);
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(cgl_context_);
+    shader_program_cache_->Reset();
+  }
   if (can_be_shared_) {
     DCHECK(window_map()->find(window_number_) != window_map()->end());
     DCHECK(window_map()->find(window_number_)->second == this);
@@ -241,8 +207,6 @@ CompositingIOSurfaceContext::~CompositingIOSurfaceContext() {
     if (found != window_map()->end())
       DCHECK(found->second != this);
   }
-  if (cgl_context_strong_)
-    CGLReleaseContext(cgl_context_strong_);
 }
 
 NSOpenGLContext* CompositingIOSurfaceContext::nsgl_context() const {
diff --git a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
index b05ffe2..38752b4 100644
--- a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
@@ -102,10 +102,26 @@
   // This makes the window content not lag behind the resize (at the cost of
   // blocking on the browser's main thread).
   if (cached_view->render_widget_host_) {
-    cached_view->about_to_validate_and_paint_ = true;
-    (void)cached_view->render_widget_host_->GetBackingStore(true);
-    cached_view->about_to_validate_and_paint_ = false;
-    CGLSetCurrentContext(glContext);
+    // Note that GetBackingStore can potentially spawn a nested run loop, which
+    // may change the current GL context, or, because the GL contexts are
+    // shared, may change the currently-bound FBO. Ensure that, when the run
+    // loop returns, the original GL context remain current, and the original
+    // FBO remain bound.
+    // TODO(ccameron): This is far too fragile a mechanism to rely on. Find
+    // a way to avoid doing this.
+    GLuint previous_framebuffer = 0;
+    glGetIntegerv(GL_FRAMEBUFFER_BINDING,
+                  reinterpret_cast<GLint*>(&previous_framebuffer));
+    {
+      gfx::ScopedCGLSetCurrentContext scoped_set_current_context(NULL);
+      cached_view->about_to_validate_and_paint_ = true;
+      (void)cached_view->render_widget_host_->GetBackingStore(true);
+      cached_view->about_to_validate_and_paint_ = false;
+    }
+    CHECK_EQ(CGLGetCurrentContext(), glContext)
+        << "original GL context failed to re-bind after nested run loop, "
+        << "browser crash is imminent.";
+    glBindFramebuffer(GL_FRAMEBUFFER, previous_framebuffer);
   }
 
   // If a transition to software mode has occurred, this layer should be
diff --git a/content/browser/renderer_host/compositing_iosurface_mac.mm b/content/browser/renderer_host/compositing_iosurface_mac.mm
index b7b6e86..31e4f22 100644
--- a/content/browser/renderer_host/compositing_iosurface_mac.mm
+++ b/content/browser/renderer_host/compositing_iosurface_mac.mm
@@ -257,10 +257,12 @@ CompositingIOSurfaceMac::CompositingIOSurfaceMac(
 
 CompositingIOSurfaceMac::~CompositingIOSurfaceMac() {
   FailAllCopies();
-  CGLSetCurrentContext(offscreen_context_->cgl_context());
-  DestroyAllCopyContextsWithinContext();
-  UnrefIOSurfaceWithContextCurrent();
-  CGLSetCurrentContext(0);
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        offscreen_context_->cgl_context());
+    DestroyAllCopyContextsWithinContext();
+    UnrefIOSurfaceWithContextCurrent();
+  }
   offscreen_context_ = NULL;
 }
 
@@ -440,12 +442,15 @@ void CompositingIOSurfaceMac::CopyTo(
   DCHECK_EQ(output->rowBytesAsPixels(), dst_pixel_size.width())
       << "Stride is required to be equal to width for GPU readback.";
 
-  CGLSetCurrentContext(offscreen_context_->cgl_context());
-  const base::Closure copy_done_callback = CopyToSelectedOutputWithinContext(
-      src_pixel_subrect, gfx::Rect(dst_pixel_size), false,
-      output.get(), NULL,
-      base::Bind(&ReverseArgumentOrder, callback, base::Passed(&output)));
-  CGLSetCurrentContext(0);
+  base::Closure copy_done_callback;
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        offscreen_context_->cgl_context());
+    copy_done_callback = CopyToSelectedOutputWithinContext(
+        src_pixel_subrect, gfx::Rect(dst_pixel_size), false,
+        output.get(), NULL,
+        base::Bind(&ReverseArgumentOrder, callback, base::Passed(&output)));
+  }
   if (!copy_done_callback.is_null())
     copy_done_callback.Run();
 }
@@ -454,10 +459,13 @@ void CompositingIOSurfaceMac::CopyToVideoFrame(
     const gfx::Rect& src_pixel_subrect,
     const scoped_refptr<media::VideoFrame>& target,
     const base::Callback<void(bool)>& callback) {
-  CGLSetCurrentContext(offscreen_context_->cgl_context());
-  const base::Closure copy_done_callback = CopyToVideoFrameWithinContext(
-      src_pixel_subrect, false, target, callback);
-  CGLSetCurrentContext(0);
+  base::Closure copy_done_callback;
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        offscreen_context_->cgl_context());
+    copy_done_callback = CopyToVideoFrameWithinContext(
+        src_pixel_subrect, false, target, callback);
+  }
   if (!copy_done_callback.is_null())
     copy_done_callback.Run();
 }
@@ -542,9 +550,9 @@ bool CompositingIOSurfaceMac::MapIOSurfaceToTextureWithContextCurrent(
 }
 
 void CompositingIOSurfaceMac::UnrefIOSurface() {
-  CGLSetCurrentContext(offscreen_context_->cgl_context());
+  gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+      offscreen_context_->cgl_context());
   UnrefIOSurfaceWithContextCurrent();
-  CGLSetCurrentContext(0);
 }
 
 void CompositingIOSurfaceMac::DrawQuad(const SurfaceQuad& quad) {
@@ -747,11 +755,12 @@ void CompositingIOSurfaceMac::CheckIfAllCopiesAreFinished(
     return;
 
   std::vector<base::Closure> done_callbacks;
-  CGLContextObj previous_context = CGLGetCurrentContext();
-  CGLSetCurrentContext(offscreen_context_->cgl_context());
-  CheckIfAllCopiesAreFinishedWithinContext(
-      block_until_finished, &done_callbacks);
-  CGLSetCurrentContext(previous_context);
+  {
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+        offscreen_context_->cgl_context());
+    CheckIfAllCopiesAreFinishedWithinContext(
+        block_until_finished, &done_callbacks);
+  }
   for (size_t i = 0; i < done_callbacks.size(); ++i)
     done_callbacks[i].Run();
 }
diff --git a/content/browser/renderer_host/display_link_mac.cc b/content/browser/renderer_host/display_link_mac.cc
index b7239c0..8d26a6d 100644
--- a/content/browser/renderer_host/display_link_mac.cc
+++ b/content/browser/renderer_host/display_link_mac.cc
@@ -7,23 +7,36 @@
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 
+namespace base {
+
+template<>
+struct ScopedTypeRefTraits<CVDisplayLinkRef> {
+  static void Retain(CVDisplayLinkRef object) {
+    CVDisplayLinkRetain(object);
+  }
+  static void Release(CVDisplayLinkRef object) {
+    CVDisplayLinkRelease(object);
+  }
+};
+
+}  // namespace base
+
 namespace content {
 
 // static
 scoped_refptr<DisplayLinkMac> DisplayLinkMac::Create() {
   CVReturn ret = kCVReturnSuccess;
 
-  scoped_refptr<DisplayLinkMac> display_link_mac;
-  {
-    CVDisplayLinkRef display_link = NULL;
-    ret = CVDisplayLinkCreateWithActiveCGDisplays(&display_link);
-    if (ret != kCVReturnSuccess) {
-      LOG(ERROR) << "CVDisplayLinkCreateWithActiveCGDisplays failed: " << ret;
-      return NULL;
-    }
-    display_link_mac = new DisplayLinkMac(display_link);
+  base::ScopedTypeRef<CVDisplayLinkRef> display_link;
+  ret = CVDisplayLinkCreateWithActiveCGDisplays(display_link.InitializeInto());
+  if (ret != kCVReturnSuccess) {
+    LOG(ERROR) << "CVDisplayLinkCreateWithActiveCGDisplays failed: " << ret;
+    return NULL;
   }
 
+  scoped_refptr<DisplayLinkMac> display_link_mac;
+  display_link_mac = new DisplayLinkMac(display_link);
+
   ret = CVDisplayLinkSetOutputCallback(
       display_link_mac->display_link_,
       &DisplayLinkCallback,
@@ -36,18 +49,18 @@ scoped_refptr<DisplayLinkMac> DisplayLinkMac::Create() {
   return display_link_mac;
 }
 
-DisplayLinkMac::DisplayLinkMac(CVDisplayLinkRef display_link)
-  : display_link_(display_link),
-    stop_timer_(
-        FROM_HERE, base::TimeDelta::FromSeconds(1),
-        this, &DisplayLinkMac::StopDisplayLink),
-    timebase_and_interval_valid_(false) {
+DisplayLinkMac::DisplayLinkMac(
+    base::ScopedTypeRef<CVDisplayLinkRef> display_link)
+      : display_link_(display_link),
+        stop_timer_(
+            FROM_HERE, base::TimeDelta::FromSeconds(1),
+            this, &DisplayLinkMac::StopDisplayLink),
+        timebase_and_interval_valid_(false) {
 }
 
 DisplayLinkMac::~DisplayLinkMac() {
   if (CVDisplayLinkIsRunning(display_link_))
     CVDisplayLinkStop(display_link_);
-  CVDisplayLinkRelease(display_link_);
 }
 
 bool DisplayLinkMac::GetVSyncParameters(
diff --git a/content/browser/renderer_host/display_link_mac.h b/content/browser/renderer_host/display_link_mac.h
index 9398015..c708608 100644
--- a/content/browser/renderer_host/display_link_mac.h
+++ b/content/browser/renderer_host/display_link_mac.h
@@ -5,8 +5,9 @@
 #ifndef CONTENT_BROWSER_RENDERER_HOST_DISPLAY_LINK_MAC_H_
 #define CONTENT_BROWSER_RENDERER_HOST_DISPLAY_LINK_MAC_H_
 
-#import <QuartzCore/CVDisplayLink.h>
+#include <QuartzCore/CVDisplayLink.h>
 
+#include "base/mac/scoped_typeref.h"
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/lock.h"
 #include "base/time/time.h"
@@ -26,7 +27,7 @@ class DisplayLinkMac : public base::RefCounted<DisplayLinkMac> {
  private:
   friend class base::RefCounted<DisplayLinkMac>;
 
-  DisplayLinkMac(CVDisplayLinkRef display_link);
+  DisplayLinkMac(base::ScopedTypeRef<CVDisplayLinkRef> display_link);
   virtual ~DisplayLinkMac();
 
   void StartOrContinueDisplayLink();
@@ -42,7 +43,7 @@ class DisplayLinkMac : public base::RefCounted<DisplayLinkMac> {
       void* context);
 
   // CVDisplayLink for querying VSync timing info.
-  CVDisplayLinkRef display_link_;
+  base::ScopedTypeRef<CVDisplayLinkRef> display_link_;
 
   // Timer for stopping the display link if it has not been queried in
   // the last second.
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 9130ced..bf1c800 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -1310,7 +1310,8 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
       RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback callback;
       if (frame_subscriber_->ShouldCaptureFrame(present_time,
                                                 &frame, &callback)) {
-        CGLSetCurrentContext(compositing_iosurface_context_->cgl_context());
+        gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+            compositing_iosurface_context_->cgl_context());
         compositing_iosurface_->SetIOSurfaceWithContextCurrent(
             compositing_iosurface_context_, surface_handle, size,
             surface_scale_factor);
@@ -1352,7 +1353,8 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
 
   // Make the context current and update the IOSurface with the handle
   // passed in by the swap command.
-  CGLSetCurrentContext(compositing_iosurface_context_->cgl_context());
+  gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
+      compositing_iosurface_context_->cgl_context());
   if (!compositing_iosurface_->SetIOSurfaceWithContextCurrent(
           compositing_iosurface_context_, surface_handle, size,
           surface_scale_factor)) {
@@ -1376,7 +1378,8 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
       compositing_iosurface_->CopyToVideoFrame(
           gfx::Rect(size), frame,
           base::Bind(callback, present_time));
-      CGLSetCurrentContext(compositing_iosurface_context_->cgl_context());
+      DCHECK_EQ(CGLGetCurrentContext(),
+                compositing_iosurface_context_->cgl_context());
     }
   }
 
@@ -2914,7 +2917,7 @@ void RenderWidgetHostViewMac::TickPendingLatencyInfoDelay() {
       NSRectFill(dirtyRect);
     }
 
-    CGLSetCurrentContext(
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
         renderWidgetHostView_->compositing_iosurface_context_->cgl_context());
     if (renderWidgetHostView_->DrawIOSurfaceWithoutCoreAnimation())
       return;