Fixed bug in DetachShader.

TEST=unit tests
BUG=none

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

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

6 files changed, 22 insertions, 11 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 0d9f326..23a9bac 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -4455,8 +4455,12 @@ void GLES2DecoderImpl::DoDetachShader(
   if (!shader_info) {
     return;
   }
+  if (!program_info->DetachShader(shader_manager(), shader_info)) {
+    SetGLError(GL_INVALID_OPERATION,
+               "glDetachShader: shader not attached to program");
+    return;
+  }
   glDetachShader(program_info->service_id(), shader_info->service_id());
-  program_info->DetachShader(shader_manager(), shader_info);
 }
 
 void GLES2DecoderImpl::DoValidateProgram(GLuint program_client_id) {
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index 2b9ed0b..683449a 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -377,13 +377,17 @@ bool ProgramManager::ProgramInfo::AttachShader(
   return true;
 }
 
-void ProgramManager::ProgramInfo::DetachShader(
+bool ProgramManager::ProgramInfo::DetachShader(
     ShaderManager* shader_manager,
     ShaderManager::ShaderInfo* info) {
   DCHECK(shader_manager);
   DCHECK(info);
+  if (attached_shaders_[ShaderTypeToIndex(info->shader_type())].get() != info) {
+    return false;
+  }
   attached_shaders_[ShaderTypeToIndex(info->shader_type())] = NULL;
   shader_manager->UnuseShader(info);
+  return true;
 }
 
 void ProgramManager::ProgramInfo::DetachShaders(ShaderManager* shader_manager) {
diff --git a/gpu/command_buffer/service/program_manager.h b/gpu/command_buffer/service/program_manager.h
index f3088d6..5c0070f 100644
--- a/gpu/command_buffer/service/program_manager.h
+++ b/gpu/command_buffer/service/program_manager.h
@@ -134,7 +134,7 @@ class ProgramManager {
     }
 
     bool AttachShader(ShaderManager* manager, ShaderManager::ShaderInfo* info);
-    void DetachShader(ShaderManager* manager, ShaderManager::ShaderInfo* info);
+    bool DetachShader(ShaderManager* manager, 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 8e08051..2f37cfd 100644
--- a/gpu/command_buffer/service/program_manager_unittest.cc
+++ b/gpu/command_buffer/service/program_manager_unittest.cc
@@ -457,6 +457,8 @@ TEST_F(ProgramManagerWithShaderTest, AttachDetachShader) {
   EXPECT_FALSE(program_info->CanLink());
   fshader->SetStatus(true, "", NULL);
   EXPECT_TRUE(program_info->CanLink());
+  EXPECT_TRUE(program_info->DetachShader(&shader_manager, fshader));
+  EXPECT_FALSE(program_info->DetachShader(&shader_manager, fshader));
   shader_manager.Destroy(false);
 }
 
diff --git a/gpu/command_buffer/service/shader_manager.h b/gpu/command_buffer/service/shader_manager.h
index f826a3c..548a965 100644
--- a/gpu/command_buffer/service/shader_manager.h
+++ b/gpu/command_buffer/service/shader_manager.h
@@ -32,13 +32,6 @@ class ShaderManager {
     typedef scoped_refptr<ShaderInfo> Ref;
     typedef ShaderTranslator::VariableInfo VariableInfo;
 
-    explicit ShaderInfo(GLuint service_id, GLenum shader_type)
-        : use_count_(0),
-          service_id_(service_id),
-          shader_type_(shader_type),
-          valid_(false) {
-    }
-
     void Update(const char* source) {
       source_.reset(source ? new std::string(source) : NULL);
     }
@@ -84,6 +77,14 @@ class ShaderManager {
 
     friend class base::RefCounted<ShaderInfo>;
     friend class ShaderManager;
+
+    ShaderInfo(GLuint service_id, GLenum shader_type)
+        : use_count_(0),
+          service_id_(service_id),
+          shader_type_(shader_type),
+          valid_(false) {
+    }
+
     ~ShaderInfo() { }
 
     void IncUseCount() {
diff --git a/gpu/command_buffer/service/shader_translator.h b/gpu/command_buffer/service/shader_translator.h
index 9e834ea..948a00d 100644
--- a/gpu/command_buffer/service/shader_translator.h
+++ b/gpu/command_buffer/service/shader_translator.h
@@ -62,7 +62,7 @@ class ShaderTranslatorInterface {
 class ShaderTranslator : public ShaderTranslatorInterface {
  public:
   ShaderTranslator();
-  ~ShaderTranslator();
+  virtual ~ShaderTranslator();
 
   // Overridden from ShaderTranslatorInterface.
   virtual bool Init(