Workaround to prevent crashes when destroying CGL contexts

Observation 1: It was discovered in crbug.com/410402 that deleting
the GL textures bound to IOSurfaces before releasing the CGL context
from the CAOpenGLLayer can result in crashes.

Observation 2: It was discovered in crbug.com/411782 that not deleting
the GL textures bounds to IOSurfaces before destroying the CGL context
can result in crashes.

As way speculative workaround for this, retain the CGL context when the
texture is created (when it is known to be valid), and the post a task to
- make that CGL context current
- delete the GL texture
- make no CGL context current
- release the CGL context
- release the IOSurface used by the GL texture

The theory here is that by doing this in a posted task, having a fresh stack
and having manually retained the CGL context when it was known-valid
will work around the crash in Observation 1, and manually destroying the
texture while its IOSurface is still retained and before the CGL context is
destroyed will work around the crash in Observation 2.

BUG=411782

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

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

content/browser/compositor/io_surface_layer_mac.h

@@ -12,6 +12,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/timer/timer.h"
 #include "ui/gfx/size.h"
+#include "ui/gl/scoped_cgl.h"
 
 @class IOSurfaceLayer;
 
@@ -66,10 +67,12 @@ class IOSurfaceLayerClient {
   // size of |io_surface_|.
   gfx::Size frame_pixel_size_;
 
-  // The GL texture that is bound to |io_surface_|. If |io_surface_| changes,
-  // then this is marked as dirty by setting |io_surface_texture_dirty_|.
+  // The GL texture that was bound to |io_surface_| during the last draw call,
+  // along with the context that was done in, and the value of |io_surface_| at
+  // the time of the bind.
   GLuint io_surface_texture_;
-  bool io_surface_texture_dirty_;
+  base::ScopedTypeRef<CGLContextObj> io_surface_texture_context_;
+  base::ScopedCFTypeRef<IOSurfaceRef> io_surface_texture_io_surface_;
 
   // The CGL renderer ID, captured at draw time.
   GLint cgl_renderer_id_;

content/browser/compositor/io_surface_layer_mac.mm

@@ -27,20 +27,42 @@
     LOG_IF(ERROR, gl_error != GL_NO_ERROR) << "GL Error: " << gl_error; \
   } while (0)
 
-// Helper function for logging error codes.
 namespace {
+
+// Helper function for logging error codes.
 template<typename T>
 std::string to_string(T value) {
   std::ostringstream stream;
   stream << value;
   return stream.str();
 }
+
+// Helper function posted to destroy textures on a clean stack.
+void DestroyTexture(
+    base::ScopedTypeRef<CGLContextObj> context,
+    base::ScopedCFTypeRef<IOSurfaceRef> io_surface,
+    GLuint texture) {
+  // Destroy the texture before tearing down the context, and while the
+  // IOSurface is still being kept around.
+  CGLSetCurrentContext(context);
+  glDeleteTextures(1, &texture);
+  CGLSetCurrentContext(NULL);
+
+  // Release (and likely destroy) the context.
+  context.reset();
+
+  // Finally release the IOSurface.
+  io_surface.reset();
+}
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // IOSurfaceLayer(Private)
 
 @interface IOSurfaceLayer(Private)
+// Reset the texture and post a task to clean it up.
+- (void)resetTextureAndPostDestroy;
+
 // Force a draw immediately, but only if one was requested.
 - (void)displayIfNeededAndAck;
 
@@ -113,7 +135,6 @@ void IOSurfaceLayerHelper::TimerFired() {
     did_not_draw_counter_ = 0;
     is_pumping_frames_ = false;
     io_surface_texture_ = 0;
-    io_surface_texture_dirty_ = false;
     cgl_renderer_id_ = 0;
 
     [self setBackgroundColor:CGColorGetConstantColor(kCGColorWhite)];
@@ -170,8 +191,8 @@ void IOSurfaceLayerHelper::TimerFired() {
   // GL texture needs to bind to the new surface.
   if (!io_surface_ || io_surface_id != IOSurfaceGetID(io_surface_)) {
     io_surface_.reset(IOSurfaceLookup(io_surface_id));
-    io_surface_texture_dirty_ = true;
     if (!io_surface_) {
+      [self resetTextureAndPostDestroy];
       content::GpuDataManagerImpl::GetInstance()->AddLogMessage(
           logging::LOG_ERROR,
           "IOSurfaceLayer",
@@ -306,9 +327,7 @@ void IOSurfaceLayerHelper::TimerFired() {
 - (void)releaseCGLContext:(CGLContextObj)glContext {
   // The GL context is being destroyed, so mark the resources as needing to be
   // recreated.
-  io_surface_texture_ = 0;
-  io_surface_texture_dirty_ = true;
-  cgl_renderer_id_ = 0;
+  [self resetTextureAndPostDestroy];
   [super releaseCGLContext:glContext];
 }
 
@@ -325,6 +344,8 @@ void IOSurfaceLayerHelper::TimerFired() {
 
   // Create the texture if it has not been created in this context yet.
   if (!io_surface_texture_) {
+    DCHECK(!io_surface_texture_io_surface_);
+    DCHECK(!io_surface_texture_context_);
     glGenTextures(1, &io_surface_texture_);
     glBindTexture(GL_TEXTURE_RECTANGLE_ARB, io_surface_texture_);
     glTexParameteri(
@@ -332,12 +353,13 @@ void IOSurfaceLayerHelper::TimerFired() {
     glTexParameteri(
         GL_TEXTURE_RECTANGLE_ARB, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
     glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
-    io_surface_texture_dirty_ = true;
+    io_surface_texture_context_.reset(glContext, base::scoped_policy::RETAIN);
   }
+  DCHECK(io_surface_texture_context_ == glContext);
 
   // Associate the IOSurface with this texture, if the underlying IOSurface has
   // been changed.
-  if (io_surface_texture_dirty_ && io_surface_) {
+  if (io_surface_ != io_surface_texture_io_surface_) {
     glBindTexture(GL_TEXTURE_RECTANGLE_ARB, io_surface_texture_);
     CGLError cgl_error = CGLTexImageIOSurface2D(
         glContext,
@@ -350,20 +372,20 @@ void IOSurfaceLayerHelper::TimerFired() {
         io_surface_.get(),
         0 /* plane */);
     glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
-    if (cgl_error != kCGLNoError) {
+    if (cgl_error == kCGLNoError) {
+      io_surface_texture_io_surface_ = io_surface_;
+    } else {
+      [self resetTextureAndPostDestroy];
       content::GpuDataManagerImpl::GetInstance()->AddLogMessage(
           logging::LOG_ERROR,
           "IOSurfaceLayer",
           std::string("CGLTexImageIOSurface2D failed with CGL error ") +
               to_string(cgl_error));
-      glDeleteTextures(1, &io_surface_texture_);
-      io_surface_texture_ = 0;
       if (client_)
         client_->IOSurfaceLayerHitError();
     }
-  } else if (io_surface_texture_) {
-    glDeleteTextures(1, &io_surface_texture_);
-    io_surface_texture_ = 0;
+  } else if (!io_surface_) {
+    [self resetTextureAndPostDestroy];
   }
 
   // Fill the viewport with the texture. The viewport must be smaller or equal
@@ -452,4 +474,26 @@ void IOSurfaceLayerHelper::TimerFired() {
   [self ackPendingFrame];
 }
 
+- (void)resetTextureAndPostDestroy {
+  if (!io_surface_texture_) {
+    DCHECK(!io_surface_texture_context_);
+    DCHECK(!io_surface_texture_io_surface_);
+    return;
+  }
+
+  // Post a task to clean up the texture on a clean stack.
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&DestroyTexture,
+                 io_surface_texture_context_,
+                 io_surface_texture_io_surface_,
+                 io_surface_texture_));
+
+  // Reset the texture state.
+  io_surface_texture_ = 0;
+  io_surface_texture_context_.reset();
+  io_surface_texture_io_surface_.reset();
+  cgl_renderer_id_ = 0;
+}
+
 @end