Add a glBegin/End pair to make glBindFramebuffer work

It appears that this fixes the issue where rendered contents wouldn't
appear on the screen. I say "it appears" because this bug is very
difficult to repro, and so I can never say it's actually gone, just
that I wasn't able to make it happen anymore.

I can come up with a line of reasoning that the glBegin/End causes
the driver to go and validate its internal state. Perhaps sometimes the
dirty bit for the FBO is missed (and doing the glBegin/End immediately
after changing the FBO ensure it's picked up). Ultimately it is just a
guess.

This workaround was discovered by accident through the following
sequence of steps when debugging:
1. Draw a triangle to the CAOpenGLLayer after drawing the texture, to
   ensure that we are actually getting draw calls
   - This appeared and kept updating even when the texture stopped
     updating
2. Draw a triangle to the FBO's texture from the CAOpenGLLayer's
   context, to make sure changes to the texture would go through
   - This appeared and kept updating even when the textures topped
     updating (so it's probably a problem with the command buffer
     context).
3. Draw a triangle to the FBO's texture from the command buffer's
   context just before glSwapBuffers
   - This triangle never appeared, so I tried the next experiment.
4. Draw a triangle to the FBO's texture right after it is bound using
   glBindFramebufferEXT
   - Suddenly the bug went away (and I never saw the triangle, because
     it was drawn over).
5. Just do a glBegin/End with program 0, since we found this was enough to
   work around driver bugs in the past.
   - Still couldn't repro the bug with this.

BUG=435786

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

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

7 files changed, 66 insertions, 1 deletions

diff --git a/content/common/gpu/image_transport_surface_fbo_mac.h b/content/common/gpu/image_transport_surface_fbo_mac.h
index dbcdc83..f9f8b0e 100644
--- a/content/common/gpu/image_transport_surface_fbo_mac.h
+++ b/content/common/gpu/image_transport_surface_fbo_mac.h
@@ -74,6 +74,7 @@ class ImageTransportSurfaceFBO
   void* GetHandle() override;
   void* GetDisplay() override;
   bool OnMakeCurrent(gfx::GLContext* context) override;
+  void NotifyWasBound() override;
   unsigned int GetBackingFrameBufferObject() override;
   bool SetBackbufferAllocation(bool allocated) override;
   void SetFrontbufferAllocation(bool allocated) override;
diff --git a/content/common/gpu/image_transport_surface_fbo_mac.mm b/content/common/gpu/image_transport_surface_fbo_mac.mm
index 6af794d..e081633 100644
--- a/content/common/gpu/image_transport_surface_fbo_mac.mm
+++ b/content/common/gpu/image_transport_surface_fbo_mac.mm
@@ -81,6 +81,42 @@ bool ImageTransportSurfaceFBO::OnMakeCurrent(gfx::GLContext* context) {
   return true;
 }
 
+void ImageTransportSurfaceFBO::NotifyWasBound() {
+  // Sometimes calling glBindFramebuffer doesn't seem to be enough to get
+  // rendered contents to show up in the color attachment. It appears that doing
+  // a glBegin/End pair with program 0 is enough to tickle the driver into
+  // actually effecting the binding.
+  // http://crbug.com/435786
+  DCHECK(has_complete_framebuffer_);
+
+  // We will restore the current program after the dummy glBegin/End pair.
+  // Ensure that we will be able to restore this state before attempting to
+  // change it.
+  GLint old_program_signed = 0;
+  glGetIntegerv(GL_CURRENT_PROGRAM, &old_program_signed);
+  GLuint old_program = static_cast<GLuint>(old_program_signed);
+  if (old_program && glIsProgram(old_program)) {
+    // A deleted program cannot be re-bound.
+    GLint delete_status = GL_FALSE;
+    glGetProgramiv(old_program, GL_DELETE_STATUS, &delete_status);
+    if (delete_status == GL_TRUE)
+      return;
+    // A program which has had the most recent link fail cannot be re-bound.
+    GLint link_status = GL_FALSE;
+    glGetProgramiv(old_program, GL_LINK_STATUS, &link_status);
+    if (link_status != GL_TRUE)
+      return;
+  }
+
+  // Issue the dummy call and then restore the state.
+  glUseProgram(0);
+  GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
+  DCHECK(status == GL_FRAMEBUFFER_COMPLETE);
+  glBegin(GL_TRIANGLES);
+  glEnd();
+  glUseProgram(old_program);
+}
+
 unsigned int ImageTransportSurfaceFBO::GetBackingFrameBufferObject() {
   return fbo_id_;
 }
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 6d37175..7ea1a06 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -4278,6 +4278,14 @@ void GLES2DecoderImpl::SetIgnoreCachedStateForTest(bool ignore) {
 void GLES2DecoderImpl::OnFboChanged() const {
   if (workarounds().restore_scissor_on_fbo_change)
     state_.fbo_binding_for_scissor_workaround_dirty_ = true;
+
+  if (workarounds().gl_begin_gl_end_on_fbo_change_to_backbuffer) {
+    GLint bound_fbo_unsigned = -1;
+    glGetIntegerv(GL_FRAMEBUFFER_BINDING_EXT, &bound_fbo_unsigned);
+    GLuint bound_fbo = static_cast<GLuint>(bound_fbo_unsigned);
+    if (surface_ && surface_->GetBackingFrameBufferObject() == bound_fbo)
+      surface_->NotifyWasBound();
+  }
 }
 
 // Called after the FBO is checked for completeness.
diff --git a/gpu/config/gpu_driver_bug_list_json.cc b/gpu/config/gpu_driver_bug_list_json.cc
index b447c1a..80147469 100644
--- a/gpu/config/gpu_driver_bug_list_json.cc
+++ b/gpu/config/gpu_driver_bug_list_json.cc
@@ -19,7 +19,7 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST(
 {
   "name": "gpu driver bug list",
   // Please update the version number whenever you change this file.
-  "version": "7.11",
+  "version": "7.12",
   "entries": [
     {
       "id": 1,
@@ -1081,6 +1081,17 @@ LONG_STRING_CONST(
       "features": [
         "gl_clear_broken"
       ]
+    },
+    {
+      "id": 96,
+      "description": "glBindFramebuffer sometimes requires a glBegin/End to take effect",
+      "cr_bugs": [435786],
+      "os": {
+        "type": "macosx"
+      },
+      "features": [
+        "gl_begin_gl_end_on_fbo_change_to_backbuffer"
+      ]
     }
   ]
 }
diff --git a/gpu/config/gpu_driver_bug_workaround_type.h b/gpu/config/gpu_driver_bug_workaround_type.h
index 841a470..77d721b 100644
--- a/gpu/config/gpu_driver_bug_workaround_type.h
+++ b/gpu/config/gpu_driver_bug_workaround_type.h
@@ -54,6 +54,8 @@
          force_gl_finish_after_compositing)                  \
   GPU_OP(FORCE_INTEGRATED_GPU,                               \
          force_integrated_gpu)                               \
+  GPU_OP(GL_BEGIN_GL_END_ON_FBO_CHANGE_TO_BACKBUFFER,        \
+         gl_begin_gl_end_on_fbo_change_to_backbuffer)        \
   GPU_OP(GL_CLEAR_BROKEN,                                    \
          gl_clear_broken)                                    \
   GPU_OP(INIT_GL_POSITION_IN_VERTEX_SHADER,                  \
diff --git a/ui/gl/gl_surface.cc b/ui/gl/gl_surface.cc
index e2f1717..3de560b 100644
--- a/ui/gl/gl_surface.cc
+++ b/ui/gl/gl_surface.cc
@@ -199,6 +199,9 @@ bool GLSurface::OnMakeCurrent(GLContext* context) {
   return true;
 }
 
+void GLSurface::NotifyWasBound() {
+}
+
 bool GLSurface::SetBackbufferAllocation(bool allocated) {
   return true;
 }
diff --git a/ui/gl/gl_surface.h b/ui/gl/gl_surface.h
index 1302cce..ec04c96 100644
--- a/ui/gl/gl_surface.h
+++ b/ui/gl/gl_surface.h
@@ -88,6 +88,10 @@ class GL_EXPORT GLSurface : public base::RefCounted<GLSurface> {
   // on error.
   virtual bool OnMakeCurrent(GLContext* context);
 
+  // Called when the surface is bound as the current framebuffer for the
+  // current context.
+  virtual void NotifyWasBound();
+
   // Used for explicit buffer management.
   virtual bool SetBackbufferAllocation(bool allocated);
   virtual void SetFrontbufferAllocation(bool allocated);