Revert 169641 - Cache resource bindings on client

This is the rest of a CL that was already approved plus the fix for one ChromeOS instance.
https://chromiumcodereview.appspot.com/11415003

BUG=160370
TBR=apatrick@chromium.org,brettw@chromium.org

Reverting due to test failing on all configs: GLES2ImplementationTest.BindTexture

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

TBR=gman@chromium.org
Review URL: https://codereview.chromium.org/11299199

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

5 files changed, 50 insertions, 147 deletions

diff --git a/gpu/command_buffer/build_gles2_cmd_buffer.py b/gpu/command_buffer/build_gles2_cmd_buffer.py
index 5e49ada..88eaa4e 100755
--- a/gpu/command_buffer/build_gles2_cmd_buffer.py
+++ b/gpu/command_buffer/build_gles2_cmd_buffer.py
@@ -1159,6 +1159,7 @@ _FUNCTION_INFO = {
     'type': 'Bind',
     'decoder_func': 'DoBindBuffer',
     'gen_func': 'GenBuffersARB',
+    'impl_func': False,
   },
   'BindFramebuffer': {
     'type': 'Bind',
@@ -1975,11 +1976,7 @@ _FUNCTION_INFO = {
     'client_test': False,
     'pepper_interface': 'ChromiumMapSub',
   },
-  'UseProgram': {
-    'decoder_func': 'DoUseProgram',
-    'impl_func': False,
-    'unit_test': False,
-  },
+  'UseProgram': {'decoder_func': 'DoUseProgram', 'unit_test': False},
   'ValidateProgram': {'decoder_func': 'DoValidateProgram'},
   'VertexAttrib1f': {'decoder_func': 'DoVertexAttrib1f'},
   'VertexAttrib1fv': {
@@ -3466,9 +3463,8 @@ TEST_F(%(test_name)s, %(name)sInvalidArgs%(arg_index)d_%(value_index)d) {
     SetGLError(GL_INVALID_OPERATION, "%(name)s\", \"%(id)s reserved id");
     return;
   }
-  if (Bind%(type)sHelper(%(arg_string)s)) {
-    helper_->%(name)s(%(arg_string)s);
-  }
+  Bind%(type)sHelper(%(arg_string)s);
+  helper_->%(name)s(%(arg_string)s);
 }
 
 """
@@ -3488,36 +3484,6 @@ TEST_F(%(test_name)s, %(name)sInvalidArgs%(arg_index)d_%(value_index)d) {
           'lc_type': name_arg.resource_type.lower(),
         })
 
-  def WriteGLES2ImplementationUnitTest(self, func, file):
-    """Overrriden from TypeHandler."""
-    code = """
-TEST_F(GLES2ImplementationTest, %(name)s) {
-  struct Cmds {
-    %(name)s cmd;
-  };
-  Cmds expected;
-  expected.cmd.Init(%(cmd_args)s);
-
-  gl_->%(name)s(%(args)s);
-  EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
-  ClearCommands();
-  gl_->%(name)s(%(args)s);
-  EXPECT_TRUE(NoCommandsWritten());
-}
-"""
-    cmd_arg_strings = []
-    for count, arg in enumerate(func.GetCmdArgs()):
-      cmd_arg_strings.append(arg.GetValidClientSideCmdArg(func, count, 0))
-      count += 1
-    gl_arg_strings = []
-    for count, arg in enumerate(func.GetOriginalArgs()):
-      gl_arg_strings.append(arg.GetValidClientSideArg(func, count, 0))
-    file.Write(code % {
-          'name': func.name,
-          'args': ", ".join(gl_arg_strings),
-          'cmd_args': ", ".join(cmd_arg_strings),
-        })
-
 
 class GENnHandler(TypeHandler):
   """Handler for glGen___ type functions."""
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 8fc6e75..e45a4bc 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -438,7 +438,6 @@ GLES2Implementation::GLES2Implementation(
       active_texture_unit_(0),
       bound_framebuffer_(0),
       bound_renderbuffer_(0),
-      current_program_(0),
       bound_array_buffer_id_(0),
       bound_element_array_buffer_id_(0),
       bound_pixel_unpack_transfer_buffer_id_(0),
@@ -1227,6 +1226,20 @@ void GLES2Implementation::BindUniformLocationCHROMIUM(
   helper_->SetBucketSize(kResultBucketId, 0);
 }
 
+void GLES2Implementation::BindBuffer(GLenum target, GLuint buffer) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glBindBuffer("
+                 << GLES2Util::GetStringBufferTarget(target) << ", "
+                 << buffer << ")");
+  if (IsBufferReservedId(buffer)) {
+    SetGLError(GL_INVALID_OPERATION, "BindBuffer", "buffer reserved id");
+    return;
+  }
+  BindBufferHelper(target, buffer);
+  if (target != GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM)
+    helper_->BindBuffer(target, buffer);
+}
+
 void GLES2Implementation::GetVertexAttribPointerv(
     GLuint index, GLenum pname, void** ptr) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
@@ -1267,9 +1280,6 @@ bool GLES2Implementation::DeleteProgramHelper(GLuint program) {
         "glDeleteProgram", "id not created by this context.");
     return false;
   }
-  if (program == current_program_) {
-    current_program_ = 0;
-  }
   return true;
 }
 
@@ -1355,15 +1365,6 @@ GLint GLES2Implementation::GetUniformLocation(
   return loc;
 }
 
-void GLES2Implementation::UseProgram(GLuint program) {
-  GPU_CLIENT_SINGLE_THREAD_CHECK();
-  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glUseProgram(" << program << ")");
-  if (current_program_ != program) {
-    current_program_ = program;
-    helper_->UseProgram(program);
-  }
-}
-
 bool GLES2Implementation::GetProgramivHelper(
     GLuint program, GLenum pname, GLint* params) {
   bool got_value = share_group_->program_info_manager()->GetProgramiv(
@@ -2483,116 +2484,80 @@ void GLES2Implementation::ActiveTexture(GLenum texture) {
 // the old model but possibly not true in the new model if another context has
 // deleted the resource.
 
-bool GLES2Implementation::BindBufferHelper(
+void GLES2Implementation::BindBufferHelper(
     GLenum target, GLuint buffer) {
   // TODO(gman): See note #1 above.
-  bool changed = false;
   switch (target) {
     case GL_ARRAY_BUFFER:
-      if (bound_array_buffer_id_ != buffer) {
-        bound_array_buffer_id_ = buffer;
-        changed = true;
-      }
+      bound_array_buffer_id_ = buffer;
       break;
     case GL_ELEMENT_ARRAY_BUFFER:
-      if (bound_element_array_buffer_id_ != buffer) {
-        bound_element_array_buffer_id_ = buffer;
-        changed = true;
-      }
+      bound_element_array_buffer_id_ = buffer;
       break;
     case GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM:
       bound_pixel_unpack_transfer_buffer_id_ = buffer;
       break;
     default:
-      changed = true;
       break;
   }
   // TODO(gman): There's a bug here. If the target is invalid the ID will not be
   // used even though it's marked it as used here.
   GetIdHandler(id_namespaces::kBuffers)->MarkAsUsedForBind(buffer);
-  return changed;
 }
 
-bool GLES2Implementation::BindFramebufferHelper(
+void GLES2Implementation::BindFramebufferHelper(
     GLenum target, GLuint framebuffer) {
   // TODO(gman): See note #1 above.
-  bool changed = false;
   switch (target) {
     case GL_FRAMEBUFFER:
-      if (bound_framebuffer_ != framebuffer) {
-        bound_framebuffer_ = framebuffer;
-        changed = true;
-      }
+      bound_framebuffer_ = framebuffer;
       break;
     default:
-      changed = true;
       break;
   }
   // TODO(gman): There's a bug here. If the target is invalid the ID will not be
   // used even though it's marked it as used here.
   GetIdHandler(id_namespaces::kFramebuffers)->MarkAsUsedForBind(framebuffer);
-  return changed;
 }
 
-bool GLES2Implementation::BindRenderbufferHelper(
+void GLES2Implementation::BindRenderbufferHelper(
     GLenum target, GLuint renderbuffer) {
   // TODO(gman): See note #1 above.
-  bool changed = false;
   switch (target) {
     case GL_RENDERBUFFER:
-      if (bound_renderbuffer_ != renderbuffer) {
-        bound_renderbuffer_ = renderbuffer;
-        changed = true;
-      }
+      bound_renderbuffer_ = renderbuffer;
       break;
     default:
-      changed = true;
       break;
   }
   // TODO(gman): There's a bug here. If the target is invalid the ID will not be
   // used even though it's marked it as used here.
   GetIdHandler(id_namespaces::kRenderbuffers)->MarkAsUsedForBind(renderbuffer);
-  return changed;
 }
 
-bool GLES2Implementation::BindTextureHelper(GLenum target, GLuint texture) {
+void GLES2Implementation::BindTextureHelper(GLenum target, GLuint texture) {
   // TODO(gman): See note #1 above.
-  // TODO(gman): Change this to false once we figure out why it's failing
-  //     on daisy.
-  bool changed = true;
   TextureUnit& unit = texture_units_[active_texture_unit_];
   switch (target) {
     case GL_TEXTURE_2D:
-      if (unit.bound_texture_2d != texture) {
-        unit.bound_texture_2d = texture;
-        changed = true;
-      }
+      unit.bound_texture_2d = texture;
       break;
     case GL_TEXTURE_CUBE_MAP:
-      if (unit.bound_texture_cube_map != texture) {
-        unit.bound_texture_cube_map = texture;
-        changed = true;
-      }
+      unit.bound_texture_cube_map = texture;
       break;
     default:
-      changed = true;
       break;
   }
   // TODO(gman): There's a bug here. If the target is invalid the ID will not be
   // used. even though it's marked it as used here.
   GetIdHandler(id_namespaces::kTextures)->MarkAsUsedForBind(texture);
-  return changed;
 }
 
-bool GLES2Implementation::BindVertexArrayHelper(GLuint array) {
+void GLES2Implementation::BindVertexArrayHelper(GLuint array) {
   // TODO(gman): See note #1 above.
-  bool changed = false;
-  if (bound_vertex_array_id_ != array) {
-    bound_vertex_array_id_ = array;
-    changed = true;
-  }
+  bound_vertex_array_id_ = array;
+
   GetIdHandler(id_namespaces::kVertexArrays)->MarkAsUsedForBind(array);
-  return changed;
 }
 
 #if defined(GLES2_SUPPORT_CLIENT_SIDE_ARRAYS)
diff --git a/gpu/command_buffer/client/gles2_implementation.h b/gpu/command_buffer/client/gles2_implementation.h
index 0f9b765..016c73a 100644
--- a/gpu/command_buffer/client/gles2_implementation.h
+++ b/gpu/command_buffer/client/gles2_implementation.h
@@ -383,11 +383,11 @@ class GLES2_IMPL_EXPORT GLES2Implementation : public GLES2Interface {
   bool IsTextureReservedId(GLuint id) { return false; }
   bool IsVertexArrayReservedId(GLuint id) { return false; }
 
-  bool BindBufferHelper(GLenum target, GLuint texture);
-  bool BindFramebufferHelper(GLenum target, GLuint texture);
-  bool BindRenderbufferHelper(GLenum target, GLuint texture);
-  bool BindTextureHelper(GLenum target, GLuint texture);
-  bool BindVertexArrayHelper(GLuint array);
+  void BindBufferHelper(GLenum target, GLuint texture);
+  void BindFramebufferHelper(GLenum target, GLuint texture);
+  void BindRenderbufferHelper(GLenum target, GLuint texture);
+  void BindTextureHelper(GLenum target, GLuint texture);
+  void BindVertexArrayHelper(GLuint array);
 
   void DeleteBuffersHelper(GLsizei n, const GLuint* buffers);
   void DeleteFramebuffersHelper(GLsizei n, const GLuint* framebuffers);
@@ -509,9 +509,6 @@ class GLES2_IMPL_EXPORT GLES2Implementation : public GLES2Interface {
   GLuint bound_framebuffer_;
   GLuint bound_renderbuffer_;
 
-  // The program in use by glUseProgram
-  GLuint current_program_;
-
   // The currently bound array buffer.
   GLuint bound_array_buffer_id_;
 
diff --git a/gpu/command_buffer/client/gles2_implementation_impl_autogen.h b/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
index 4ce044e..10a76a6 100644
--- a/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
+++ b/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
@@ -17,18 +17,6 @@ void GLES2Implementation::AttachShader(GLuint program, GLuint shader) {
   helper_->AttachShader(program, shader);
 }
 
-void GLES2Implementation::BindBuffer(GLenum target, GLuint buffer) {
-  GPU_CLIENT_SINGLE_THREAD_CHECK();
-  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glBindBuffer(" << GLES2Util::GetStringBufferTarget(target) << ", " << buffer << ")");  // NOLINT
-  if (IsBufferReservedId(buffer)) {
-    SetGLError(GL_INVALID_OPERATION, "BindBuffer", "buffer reserved id");
-    return;
-  }
-  if (BindBufferHelper(target, buffer)) {
-    helper_->BindBuffer(target, buffer);
-  }
-}
-
 void GLES2Implementation::BindFramebuffer(GLenum target, GLuint framebuffer) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glBindFramebuffer(" << GLES2Util::GetStringFrameBufferTarget(target) << ", " << framebuffer << ")");  // NOLINT
@@ -37,9 +25,8 @@ void GLES2Implementation::BindFramebuffer(GLenum target, GLuint framebuffer) {
         GL_INVALID_OPERATION, "BindFramebuffer", "framebuffer reserved id");
     return;
   }
-  if (BindFramebufferHelper(target, framebuffer)) {
-    helper_->BindFramebuffer(target, framebuffer);
-  }
+  BindFramebufferHelper(target, framebuffer);
+  helper_->BindFramebuffer(target, framebuffer);
 }
 
 void GLES2Implementation::BindRenderbuffer(
@@ -51,9 +38,8 @@ void GLES2Implementation::BindRenderbuffer(
         GL_INVALID_OPERATION, "BindRenderbuffer", "renderbuffer reserved id");
     return;
   }
-  if (BindRenderbufferHelper(target, renderbuffer)) {
-    helper_->BindRenderbuffer(target, renderbuffer);
-  }
+  BindRenderbufferHelper(target, renderbuffer);
+  helper_->BindRenderbuffer(target, renderbuffer);
 }
 
 void GLES2Implementation::BindTexture(GLenum target, GLuint texture) {
@@ -63,9 +49,8 @@ void GLES2Implementation::BindTexture(GLenum target, GLuint texture) {
     SetGLError(GL_INVALID_OPERATION, "BindTexture", "texture reserved id");
     return;
   }
-  if (BindTextureHelper(target, texture)) {
-    helper_->BindTexture(target, texture);
-  }
+  BindTextureHelper(target, texture);
+  helper_->BindTexture(target, texture);
 }
 
 void GLES2Implementation::BlendColor(
@@ -1196,6 +1181,12 @@ void GLES2Implementation::UniformMatrix4fv(
   helper_->UniformMatrix4fvImmediate(location, count, transpose, value);
 }
 
+void GLES2Implementation::UseProgram(GLuint program) {
+  GPU_CLIENT_SINGLE_THREAD_CHECK();
+  GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glUseProgram(" << program << ")");
+  helper_->UseProgram(program);
+}
+
 void GLES2Implementation::ValidateProgram(GLuint program) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glValidateProgram(" << program << ")");  // NOLINT
@@ -1424,9 +1415,8 @@ void GLES2Implementation::BindVertexArrayOES(GLuint array) {
         GL_INVALID_OPERATION, "BindVertexArrayOES", "array reserved id");
     return;
   }
-  if (BindVertexArrayHelper(array)) {
-    helper_->BindVertexArrayOES(array);
-  }
+  BindVertexArrayHelper(array);
+  helper_->BindVertexArrayOES(array);
 }
 
 void GLES2Implementation::GetTranslatedShaderSourceANGLE(
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h b/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
index 027d6e17..2366a42 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
+++ b/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
@@ -33,9 +33,6 @@ TEST_F(GLES2ImplementationTest, BindBuffer) {
 
   gl_->BindBuffer(GL_ARRAY_BUFFER, 2);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
-  ClearCommands();
-  gl_->BindBuffer(GL_ARRAY_BUFFER, 2);
-  EXPECT_TRUE(NoCommandsWritten());
 }
 
 TEST_F(GLES2ImplementationTest, BindFramebuffer) {
@@ -47,9 +44,6 @@ TEST_F(GLES2ImplementationTest, BindFramebuffer) {
 
   gl_->BindFramebuffer(GL_FRAMEBUFFER, 2);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
-  ClearCommands();
-  gl_->BindFramebuffer(GL_FRAMEBUFFER, 2);
-  EXPECT_TRUE(NoCommandsWritten());
 }
 
 TEST_F(GLES2ImplementationTest, BindRenderbuffer) {
@@ -61,9 +55,6 @@ TEST_F(GLES2ImplementationTest, BindRenderbuffer) {
 
   gl_->BindRenderbuffer(GL_RENDERBUFFER, 2);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
-  ClearCommands();
-  gl_->BindRenderbuffer(GL_RENDERBUFFER, 2);
-  EXPECT_TRUE(NoCommandsWritten());
 }
 
 TEST_F(GLES2ImplementationTest, BindTexture) {
@@ -75,9 +66,6 @@ TEST_F(GLES2ImplementationTest, BindTexture) {
 
   gl_->BindTexture(GL_TEXTURE_2D, 2);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
-  ClearCommands();
-  gl_->BindTexture(GL_TEXTURE_2D, 2);
-  EXPECT_TRUE(NoCommandsWritten());
 }
 
 TEST_F(GLES2ImplementationTest, BlendColor) {
@@ -1657,9 +1645,6 @@ TEST_F(GLES2ImplementationTest, BindVertexArrayOES) {
 
   gl_->BindVertexArrayOES(1);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
-  ClearCommands();
-  gl_->BindVertexArrayOES(1);
-  EXPECT_TRUE(NoCommandsWritten());
 }
 // TODO: Implement unit test for GenSharedIdsCHROMIUM
 // TODO: Implement unit test for DeleteSharedIdsCHROMIUM