Fixes glAttachShader to be OpenGL ES 2.0 spec compliant.

You're not supposed to be able to attach more than
one shader of the same type unlike OpenGL.

TEST=unit tests
BUG=none

Review URL: http://codereview.chromium.org/3107007

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

4 files changed, 20 insertions, 11 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index a2966ff..0450a5c 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2034,7 +2034,7 @@ void GLES2DecoderImpl::Destroy() {
   // TODO(alokp): Move ShInitialize/ShFinalize where they are called only
   // once per process. Currently they get called out-of-order leading to
   // crashes.
-  //ShFinalize();
+  // ShFinalize();
 #endif  // GLES2_GPU_SERVICE_TRANSLATE_SHADER)
 
   if (context_.get()) {
@@ -3714,7 +3714,12 @@ void GLES2DecoderImpl::DoAttachShader(
   if (!shader_info) {
     return;
   }
-  program_info->AttachShader(shader_info);
+  if (!program_info->AttachShader(shader_info)) {
+    SetGLError(GL_INVALID_OPERATION,
+               "glAttachShader: can not attach more than"
+               " one shader of the same type.");
+    return;
+  }
   glAttachShader(program_info->service_id(), shader_info->service_id());
 }
 
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index 7210bdb..3460019 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -296,10 +296,14 @@ void ProgramManager::ProgramInfo::GetProgramiv(GLenum pname, GLint* params) {
   }
 }
 
-void ProgramManager::ProgramInfo::AttachShader(
+bool ProgramManager::ProgramInfo::AttachShader(
     ShaderManager::ShaderInfo* info) {
-  attached_shaders_[ShaderTypeToIndex(info->shader_type())] =
-      ShaderManager::ShaderInfo::Ref(info);
+  int index = ShaderTypeToIndex(info->shader_type());
+  if (attached_shaders_[index] != NULL) {
+    return false;
+  }
+  attached_shaders_[index] = ShaderManager::ShaderInfo::Ref(info);
+  return true;
 }
 
 void ProgramManager::ProgramInfo::DetachShader(
diff --git a/gpu/command_buffer/service/program_manager.h b/gpu/command_buffer/service/program_manager.h
index 66c09f2..66ff795 100644
--- a/gpu/command_buffer/service/program_manager.h
+++ b/gpu/command_buffer/service/program_manager.h
@@ -138,7 +138,7 @@ class ProgramManager {
       return valid_;
     }
 
-    void AttachShader(ShaderManager::ShaderInfo* info);
+    bool AttachShader(ShaderManager::ShaderInfo* info);
     void DetachShader(ShaderManager::ShaderInfo* info);
 
     bool CanLink() const;
diff --git a/gpu/command_buffer/service/program_manager_unittest.cc b/gpu/command_buffer/service/program_manager_unittest.cc
index a6b5614..fa159de 100644
--- a/gpu/command_buffer/service/program_manager_unittest.cc
+++ b/gpu/command_buffer/service/program_manager_unittest.cc
@@ -428,19 +428,19 @@ TEST_F(ProgramManagerWithShaderTest, AttachDetachShader) {
   ShaderManager::ShaderInfo* fshader = shader_manager.GetShaderInfo(
       kFShaderClientId);
   fshader->SetStatus(true, "");
-  program_info->AttachShader(vshader);
+  EXPECT_TRUE(program_info->AttachShader(vshader));
   EXPECT_FALSE(program_info->CanLink());
-  program_info->AttachShader(fshader);
+  EXPECT_TRUE(program_info->AttachShader(fshader));
   EXPECT_TRUE(program_info->CanLink());
   program_info->DetachShader(vshader);
   EXPECT_FALSE(program_info->CanLink());
-  program_info->AttachShader(vshader);
+  EXPECT_TRUE(program_info->AttachShader(vshader));
   EXPECT_TRUE(program_info->CanLink());
   program_info->DetachShader(fshader);
   EXPECT_FALSE(program_info->CanLink());
-  program_info->AttachShader(vshader);
+  EXPECT_FALSE(program_info->AttachShader(vshader));
   EXPECT_FALSE(program_info->CanLink());
-  program_info->AttachShader(fshader);
+  EXPECT_TRUE(program_info->AttachShader(fshader));
   EXPECT_TRUE(program_info->CanLink());
   vshader->SetStatus(false, "");
   EXPECT_FALSE(program_info->CanLink());