gpu: Bind dummy GL API when no context is current

Also make platform behavior consistent in always releasing
any previously current context when MakeCurrent() fails.

This catches GL call sites with no context current.
It also avoids problems with GL implementations potentially not liking
this (and crashing) or even us ending up calling into the wrong context
(for example accidentally deleting a resource in the wrong context).

BUG=355275
R=piman@chromium.org

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

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

content/common/gpu/texture_image_transport_surface.cc

@@ -30,8 +30,7 @@ namespace content {
 namespace {
 
 bool IsContextValid(ImageTransportHelper* helper) {
-  return helper->stub()->decoder()->GetGLContext()->IsCurrent(NULL) ||
-      helper->stub()->decoder()->WasContextLost();
+  return helper->stub()->decoder()->GetGLContext()->IsCurrent(NULL);
 }
 
 }  // namespace
@@ -175,19 +174,26 @@ void TextureImageTransportSurface::OnResize(gfx::Size size,
 }
 
 void TextureImageTransportSurface::OnWillDestroyStub() {
-  DCHECK(IsContextValid(helper_.get()));
+  bool have_context = IsContextValid(helper_.get());
   helper_->stub()->RemoveDestructionObserver(this);
 
   // We are losing the stub owning us, this is our last chance to clean up the
   // resources we allocated in the stub's context.
-  ReleaseBackTexture();
-  ReleaseFrontTexture();
+  if (have_context) {
+    ReleaseBackTexture();
+    ReleaseFrontTexture();
+  } else {
+    backbuffer_ = NULL;
+    back_mailbox_ = Mailbox();
+    frontbuffer_ = NULL;
+    front_mailbox_ = Mailbox();
+  }
 
-  if (fbo_id_) {
+  if (fbo_id_ && have_context) {
     glDeleteFramebuffersEXT(1, &fbo_id_);
     CHECK_GL_ERROR();
-    fbo_id_ = 0;
   }
+  fbo_id_ = 0;
 
   stub_destroyed_ = true;
 }

gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc

@@ -6622,6 +6622,11 @@ void GLES2DecoderWithShaderTest::CheckRenderbufferChangesMarkFBOAsNotComplete(
   } else {
     EXPECT_TRUE(framebuffer_manager->IsComplete(framebuffer));
   }
+  // Cleanup
+  DoDeleteFramebuffer(
+      client_framebuffer_id_, kServiceFramebufferId,
+      bound_fbo, GL_FRAMEBUFFER, 0,
+      bound_fbo, GL_FRAMEBUFFER, 0);
 }
 
 TEST_F(GLES2DecoderWithShaderTest,
@@ -6711,6 +6716,11 @@ void GLES2DecoderWithShaderTest::CheckTextureChangesMarkFBOAsNotComplete(
   } else {
     EXPECT_TRUE(framebuffer_manager->IsComplete(framebuffer));
   }
+  // Cleanup
+  DoDeleteFramebuffer(
+      client_framebuffer_id_, kServiceFramebufferId,
+      bound_fbo, GL_FRAMEBUFFER, 0,
+      bound_fbo, GL_FRAMEBUFFER, 0);
 }
 
 TEST_F(GLES2DecoderWithShaderTest, TextureChangesMarkFBOAsNotCompleteBoundFBO) {

ui/gl/generate_bindings.py

@@ -1652,6 +1652,15 @@ def WriteConditionalFuncBinding(file, func):
 }
 """ % set_name.upper())
 
+  def MakeArgNames(arguments):
+    argument_names = re.sub(
+        r'(const )?[a-zA-Z0-9_]+\** ([a-zA-Z0-9_]+)', r'\2', arguments)
+    argument_names = re.sub(
+        r'(const )?[a-zA-Z0-9_]+\** ([a-zA-Z0-9_]+)', r'\2', argument_names)
+    if argument_names == 'void' or argument_names == '':
+      argument_names = ''
+    return argument_names
+
   # Write GLApiBase functions
   for func in functions:
     function_name = func['known_as']
@@ -1660,12 +1669,7 @@ def WriteConditionalFuncBinding(file, func):
     file.write('\n')
     file.write('%s %sApiBase::%sFn(%s) {\n' %
         (return_type, set_name.upper(), function_name, arguments))
-    argument_names = re.sub(
-        r'(const )?[a-zA-Z0-9_]+\** ([a-zA-Z0-9_]+)', r'\2', arguments)
-    argument_names = re.sub(
-        r'(const )?[a-zA-Z0-9_]+\** ([a-zA-Z0-9_]+)', r'\2', argument_names)
-    if argument_names == 'void' or argument_names == '':
-      argument_names = ''
+    argument_names = MakeArgNames(arguments)
     if return_type == 'void':
       file.write('  driver_->fn.%sFn(%s);\n' %
           (function_name, argument_names))
@@ -1682,12 +1686,7 @@ def WriteConditionalFuncBinding(file, func):
     file.write('\n')
     file.write('%s Trace%sApi::%sFn(%s) {\n' %
         (return_type, set_name.upper(), function_name, arguments))
-    argument_names = re.sub(
-        r'(const )?[a-zA-Z0-9_]+\** ([a-zA-Z0-9_]+)', r'\2', arguments)
-    argument_names = re.sub(
-        r'(const )?[a-zA-Z0-9_]+\** ([a-zA-Z0-9_]+)', r'\2', argument_names)
-    if argument_names == 'void' or argument_names == '':
-      argument_names = ''
+    argument_names = MakeArgNames(arguments)
     file.write('  TRACE_EVENT_BINARY_EFFICIENT0("gpu", "TraceGLAPI::%s")\n' %
                function_name)
     if return_type == 'void':
@@ -1698,6 +1697,37 @@ def WriteConditionalFuncBinding(file, func):
           (set_name.lower(), function_name, argument_names))
     file.write('}\n')
 
+  # Write NoContextGLApi functions
+  if set_name.upper() == "GL":
+    for func in functions:
+      function_name = func['known_as']
+      return_type = func['return_type']
+      arguments = func['arguments']
+      file.write('\n')
+      file.write('%s NoContextGLApi::%sFn(%s) {\n' %
+          (return_type, function_name, arguments))
+      argument_names = MakeArgNames(arguments)
+      no_context_error = "Trying to call %s() without current GL context" % function_name
+      file.write('  NOTREACHED() <<  "%s";\n' % no_context_error)
+      file.write('  LOG(ERROR) <<  "%s";\n' % no_context_error)
+      default_value = { 'GLenum': 'static_cast<GLenum>(0)',
+                        'GLuint': '0U',
+                        'GLint': '0',
+                        'GLboolean': 'GL_FALSE',
+                        'GLbyte': '0',
+                        'GLubyte': '0',
+                        'GLbutfield': '0',
+                        'GLushort': '0',
+                        'GLsizei': '0',
+                        'GLfloat': '0.0f',
+                        'GLdouble': '0.0',
+                        'GLsync': 'NULL'}
+      if return_type.endswith('*'):
+        file.write('  return NULL;\n')
+      elif return_type != 'void':
+        file.write('  return %s;\n' % default_value[return_type])
+      file.write('}\n')
+
   file.write('\n')
   file.write('}  // namespace gfx\n')
 

ui/gl/gl_context.cc

@@ -26,6 +26,18 @@ base::LazyInstance<base::ThreadLocalPointer<GLContext> >::Leaky
     current_real_context_ = LAZY_INSTANCE_INITIALIZER;
 }  // namespace
 
+GLContext::ScopedReleaseCurrent::ScopedReleaseCurrent() : canceled_(false) {}
+
+GLContext::ScopedReleaseCurrent::~ScopedReleaseCurrent() {
+  if (!canceled_ && GetCurrent()) {
+    GetCurrent()->ReleaseCurrent(NULL);
+  }
+}
+
+void GLContext::ScopedReleaseCurrent::Cancel() {
+  canceled_ = true;
+}
+
 GLContext::GLContext(GLShareGroup* share_group) : share_group_(share_group) {
   if (!share_group_.get())
     share_group_ = new GLShareGroup;
@@ -125,6 +137,8 @@ GLContext* GLContext::GetRealCurrent() {
 void GLContext::SetCurrent(GLSurface* surface) {
   current_context_.Pointer()->Set(surface ? this : NULL);
   GLSurface::SetCurrent(surface);
+  if (!surface)
+    SetGLApiToNoContext();
 }
 
 GLStateRestorer* GLContext::GetGLStateRestorer() {

ui/gl/gl_context.h

@@ -117,6 +117,18 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
  protected:
   virtual ~GLContext();
 
+  // Will release the current context when going out of scope, unless canceled.
+  class ScopedReleaseCurrent {
+   public:
+    ScopedReleaseCurrent();
+    ~ScopedReleaseCurrent();
+
+    void Cancel();
+
+   private:
+    bool canceled_;
+  };
+
   // Sets the GL api to the real hardware API (vs the VirtualAPI)
   static void SetRealGLApi();
   virtual void SetCurrent(GLSurface* surface);

ui/gl/gl_context_cgl.cc

@@ -178,6 +178,7 @@ bool GLContextCGL::MakeCurrent(GLSurface* surface) {
   if (IsCurrent(surface))
     return true;
 
+  ScopedReleaseCurrent release_current;
   TRACE_EVENT0("gpu", "GLContextCGL::MakeCurrent");
 
   if (CGLSetCurrentContext(
@@ -191,7 +192,6 @@ bool GLContextCGL::MakeCurrent(GLSurface* surface) {
 
   SetCurrent(surface);
   if (!InitializeDynamicBindings()) {
-    ReleaseCurrent(surface);
     return false;
   }
 
@@ -200,6 +200,7 @@ bool GLContextCGL::MakeCurrent(GLSurface* surface) {
     return false;
   }
 
+  release_current.Cancel();
   return true;
 }
 

ui/gl/gl_context_egl.cc

@@ -93,6 +93,7 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
   if (IsCurrent(surface))
       return true;
 
+  ScopedReleaseCurrent release_current;
   TRACE_EVENT2("gpu", "GLContextEGL::MakeCurrent",
                "context", context_,
                "surface", surface);
@@ -116,7 +117,6 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
 
   SetCurrent(surface);
   if (!InitializeDynamicBindings()) {
-    ReleaseCurrent(surface);
     return false;
   }
 
@@ -125,6 +125,7 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
     return false;
   }
 
+  release_current.Cancel();
   return true;
 }
 

ui/gl/gl_context_glx.cc

@@ -98,6 +98,7 @@ bool GLContextGLX::MakeCurrent(GLSurface* surface) {
   if (IsCurrent(surface))
     return true;
 
+  ScopedReleaseCurrent release_current;
   TRACE_EVENT0("gpu", "GLContextGLX::MakeCurrent");
   if (!glXMakeContextCurrent(
       display_,
@@ -114,18 +115,17 @@ bool GLContextGLX::MakeCurrent(GLSurface* surface) {
 
   SetCurrent(surface);
   if (!InitializeDynamicBindings()) {
-    ReleaseCurrent(surface);
     Destroy();
     return false;
   }
 
   if (!surface->OnMakeCurrent(this)) {
     LOG(ERROR) << "Could not make current.";
-    ReleaseCurrent(surface);
     Destroy();
     return false;
   }
 
+  release_current.Cancel();
   return true;
 }
 

ui/gl/gl_context_nsview.mm

@@ -55,6 +55,7 @@
 }
 
 bool GLContextNSView::MakeCurrent(GLSurface* surface) {
+  ScopedReleaseCurrent release_current;
   TRACE_EVENT0("gpu", "GLContextNSView::MakeCurrent");
   AcceleratedWidget view =
       static_cast<AcceleratedWidget>(surface->GetHandle());
@@ -67,7 +68,6 @@
   SetRealGLApi();
   SetCurrent(surface);
   if (!InitializeDynamicBindings()) {
-    ReleaseCurrent(surface);
     return false;
   }
 
@@ -76,6 +76,7 @@
     return false;
   }
 
+  release_current.Cancel();
   return true;
 }
 

ui/gl/gl_context_osmesa.cc

@@ -52,6 +52,7 @@ bool GLContextOSMesa::MakeCurrent(GLSurface* surface) {
 
   gfx::Size size = surface->GetSize();
 
+  ScopedReleaseCurrent release_current;
   if (!OSMesaMakeCurrent(context_,
                          surface->GetHandle(),
                          GL_UNSIGNED_BYTE,
@@ -70,7 +71,6 @@ bool GLContextOSMesa::MakeCurrent(GLSurface* surface) {
 
   SetCurrent(surface);
   if (!InitializeDynamicBindings()) {
-    ReleaseCurrent(surface);
     return false;
   }
 
@@ -79,6 +79,7 @@ bool GLContextOSMesa::MakeCurrent(GLSurface* surface) {
     return false;
   }
 
+  release_current.Cancel();
   return true;
 }
 
@@ -87,6 +88,7 @@ void GLContextOSMesa::ReleaseCurrent(GLSurface* surface) {
     return;
 
   SetCurrent(NULL);
+  // TODO: Calling with NULL here does not work to release the context.
   OSMesaMakeCurrent(NULL, NULL, GL_UNSIGNED_BYTE, 0, 0);
 }
 

ui/gl/gl_context_wgl.cc

@@ -74,6 +74,7 @@ bool GLContextWGL::MakeCurrent(GLSurface* surface) {
   if (IsCurrent(surface))
     return true;
 
+  ScopedReleaseCurrent release_current;
   TRACE_EVENT0("gpu", "GLContextWGL::MakeCurrent");
 
   if (!wglMakeCurrent(static_cast<HDC>(surface->GetHandle()), context_)) {
@@ -86,7 +87,6 @@ bool GLContextWGL::MakeCurrent(GLSurface* surface) {
 
   SetCurrent(surface);
   if (!InitializeDynamicBindings()) {
-    ReleaseCurrent(surface);
     return false;
   }
 
@@ -95,6 +95,7 @@ bool GLContextWGL::MakeCurrent(GLSurface* surface) {
     return false;
   }
 
+  release_current.Cancel();
   return true;
 }
 

ui/gl/gl_gl_api_implementation.cc

@@ -22,6 +22,9 @@ namespace gfx {
 static GLApi* g_gl;
 // A GL Api that calls directly into the driver.
 static RealGLApi* g_real_gl;
+// A GL Api that does nothing but warn about illegal GL calls without a context
+// current.
+static NoContextGLApi* g_no_context_gl;
 // A GL Api that calls TRACE and then calls another GL api.
 static TraceGLApi* g_trace_gl;
 // GL version used when initializing dynamic bindings.
@@ -258,6 +261,7 @@ void InitializeStaticGLBindingsGL() {
   if (!g_real_gl) {
     g_real_gl = new RealGLApi();
     g_trace_gl = new TraceGLApi(g_real_gl);
+    g_no_context_gl = new NoContextGLApi();
   }
   g_real_gl->Initialize(&g_driver_gl);
   g_gl = g_real_gl;
@@ -280,6 +284,10 @@ void SetGLToRealGLApi() {
   SetGLApi(g_gl);
 }
 
+void SetGLApiToNoContext() {
+  SetGLApi(g_no_context_gl);
+}
+
 void InitializeDynamicGLBindingsGL(GLContext* context) {
   g_driver_gl.InitializeCustomDynamicBindings(context);
   DCHECK(context && context->IsCurrent(NULL) && !g_version_info);
@@ -356,6 +364,12 @@ void RealGLApi::Initialize(DriverGL* driver) {
 TraceGLApi::~TraceGLApi() {
 }
 
+NoContextGLApi::NoContextGLApi() {
+}
+
+NoContextGLApi::~NoContextGLApi() {
+}
+
 VirtualGLApi::VirtualGLApi()
     : real_context_(NULL),
       current_context_(NULL) {