Fixed crashes when running on top of Apple's software OpenGL renderer.

This configuration is seen on the reliability bots, which always use
the software GL renderer because they are headless. These fixes also
make --use-gl=apple work with Chrome's on-screen rendering path.

 - Renamed the pbuffer GLSurface class to NoOpGLSurfaceCGL and removed
   its unnecessary pbuffer allocation, as well as the requirement that
   the context be pbuffer capable.

 - Removed the accelerated requirement from the context in
   accelerated_plugin_view_mac.mm. The system automatically prefers
   accelerated contexts where possible.

 - Supported --use-gl=apple in accelerated_surface_mac.cc and
   image_transport_surface_mac.cc.

 - Removed the depth and stencil renderbuffers from
   accelerated_surface_mac.cc. The only consequence of this change is
   that when a Core Animation plugin's layer is rendered via our
   CARenderer, the destination surface will not have a depth buffer.
   Firefox already has this behavior, and, as expected, this change
   does not affect the correctness of any test case.

There is still a remaining problem on the reliability bots where
composited content does not redraw often enough, but this seems to be
an orthogonal problem and is not reproducible when running Chrome
locally with --use-gl=apple.

Ran previously crashing content on reliability bot to verify fix.
Also tested manually on 10.6.8, both with and without --use-gl=apple, with:
 - http://unity3d.com/gallery/demos/live-demos (Unity 3D)
 - http://www.nissan-stagejuk3d.com/
    (Stage 3D in Flash 11, live resizing; web site is flaky, sometimes
    fails to start)
 - http://www.vimeo.com/

BUG=106891
TEST=manual testing with above test cases
Review URL: http://codereview.chromium.org/9006045

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

2 files changed, 14 insertions, 38 deletions

diff --git a/ui/gfx/surface/accelerated_surface_mac.cc b/ui/gfx/surface/accelerated_surface_mac.cc
index 1a0fd31..6978c57 100644
--- a/ui/gfx/surface/accelerated_surface_mac.cc
+++ b/ui/gfx/surface/accelerated_surface_mac.cc
@@ -18,8 +18,7 @@ AcceleratedSurface::AcceleratedSurface()
     : io_surface_id_(0),
       allocate_fbo_(false),
       texture_(0),
-      fbo_(0),
-      depth_stencil_renderbuffer_(0) {
+      fbo_(0) {
 }
 
 AcceleratedSurface::~AcceleratedSurface() {}
@@ -34,9 +33,10 @@ bool AcceleratedSurface::Initialize(
   if (!gfx::GLSurface::InitializeOneOff())
     return false;
 
-  // Drawing to IOSurfaces via OpenGL only works with desktop GL and
+  // Drawing to IOSurfaces via OpenGL only works with Apple's GL and
   // not with the OSMesa software renderer.
-  if (gfx::GetGLImplementation() != gfx::kGLImplementationDesktopGL)
+  if (gfx::GetGLImplementation() != gfx::kGLImplementationDesktopGL &&
+      gfx::GetGLImplementation() != gfx::kGLImplementationAppleGL)
     return false;
 
   gl_surface_ = gfx::GLSurface::CreateOffscreenGLSurface(
@@ -167,21 +167,8 @@ void AcceleratedSurface::AllocateRenderBuffers(GLenum target,
     // Generate and bind the framebuffer object.
     glGenFramebuffersEXT(1, &fbo_);
     glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fbo_);
-    // Generate (but don't bind) the depth buffer -- we don't need
-    // this bound in order to do offscreen rendering.
-    glGenRenderbuffersEXT(1, &depth_stencil_renderbuffer_);
   }
 
-  // Reallocate the depth buffer.
-  glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, depth_stencil_renderbuffer_);
-  glRenderbufferStorageEXT(GL_RENDERBUFFER_EXT,
-                           GL_DEPTH24_STENCIL8_EXT,
-                           size.width(),
-                           size.height());
-
-  // Unbind the renderbuffers.
-  glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, 0);
-
   // Make sure that subsequent set-up code affects the render texture.
   glBindTexture(target, texture_);
 }
@@ -195,21 +182,6 @@ bool AcceleratedSurface::SetupFrameBufferObject(GLenum target) {
                             texture_,
                             0);
   fbo_status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
-  if (fbo_status == GL_FRAMEBUFFER_COMPLETE_EXT) {
-    glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT,
-                                 GL_DEPTH_ATTACHMENT_EXT,
-                                 GL_RENDERBUFFER_EXT,
-                                 depth_stencil_renderbuffer_);
-    fbo_status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
-  }
-  // Attach the depth and stencil buffer.
-  if (fbo_status == GL_FRAMEBUFFER_COMPLETE_EXT) {
-    glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT,
-                                 0x8D20,  // GL_STENCIL_ATTACHMENT,
-                                 GL_RENDERBUFFER_EXT,
-                                 depth_stencil_renderbuffer_);
-    fbo_status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
-  }
   return fbo_status == GL_FRAMEBUFFER_COMPLETE_EXT;
 }
 
@@ -290,7 +262,7 @@ uint32 AcceleratedSurface::SetSurfaceSize(const gfx::Size& size) {
 
   // Don't think we need to identify a plane.
   GLuint plane = 0;
-  io_surface_support->CGLTexImageIOSurface2D(
+  CGLError error = io_surface_support->CGLTexImageIOSurface2D(
       static_cast<CGLContextObj>(gl_context_->GetHandle()),
       target,
       GL_RGBA,
@@ -300,9 +272,14 @@ uint32 AcceleratedSurface::SetSurfaceSize(const gfx::Size& size) {
       GL_UNSIGNED_INT_8_8_8_8_REV,
       io_surface_.get(),
       plane);
+  if (error != kCGLNoError) {
+    DLOG(ERROR) << "CGL error " << error << " during CGLTexImageIOSurface2D";
+  }
   if (allocate_fbo_) {
     // Set up the frame buffer object.
-    SetupFrameBufferObject(target);
+    if (!SetupFrameBufferObject(target)) {
+      DLOG(ERROR) << "Failed to set up frame buffer object";
+    }
   }
   surface_size_ = size;
   real_surface_size_ = clamped_size;
diff --git a/ui/gfx/surface/accelerated_surface_mac.h b/ui/gfx/surface/accelerated_surface_mac.h
index 37f50a7..e63154b 100644
--- a/ui/gfx/surface/accelerated_surface_mac.h
+++ b/ui/gfx/surface/accelerated_surface_mac.h
@@ -119,9 +119,9 @@ class SURFACE_EXPORT AcceleratedSurface {
   gfx::Size GetSize() const { return surface_size_; }
 
  private:
-  // Helper function to generate names for the backing texture, render buffers
-  // and FBO.  On return, the resulting buffer names can be attached to |fbo_|.
-  // |target| is the target type for the color buffer.
+  // Helper function to generate names for the backing texture and FBO.  On
+  // return, the resulting names can be attached to |fbo_|.  |target| is
+  // the target type for the color buffer.
   void AllocateRenderBuffers(GLenum target, const gfx::Size& size);
 
   // Helper function to attach the buffers previously allocated by a call to
@@ -173,7 +173,6 @@ class SURFACE_EXPORT AcceleratedSurface {
   // The FBO and renderbuffer are only allocated if allocate_fbo_ is
   // true.
   GLuint fbo_;
-  GLuint depth_stencil_renderbuffer_;
   // Allocate a TransportDIB in the renderer.
   base::Callback<void(size_t, TransportDIB::Handle*)> dib_alloc_callback_;
   base::Callback<void(TransportDIB::Id)> dib_free_callback_;