Fix TexSubImage2D and CompressedTexSubImage2D to check that format and type

match the texture they are going to update.

Also fix the info_log and source stuff so they handle NULL strings
vs empty strings.

TEST=unit tests and ran gles2 conformance tests
BUG=71135,71073,71109

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

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

10 files changed, 80 insertions, 57 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index d71f764..4f3d920 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -4218,7 +4218,7 @@ error::Error GLES2DecoderImpl::ShaderSourceHelper(
   }
   // Note: We don't actually call glShaderSource here. We wait until
   // the call to glCompileShader.
-  info->Update(std::string(data, data + data_size));
+  info->Update(std::string(data, data + data_size).c_str());
   return error::kNoError;
 }
 
@@ -4263,7 +4263,7 @@ void GLES2DecoderImpl::DoCompileShader(GLuint client_id) {
   }
   // Translate GL ES 2.0 shader to Desktop GL shader and pass that to
   // glShaderSource and then glCompileShader.
-  const char* shader_src = info->source().c_str();
+  const char* shader_src = info->source() ? info->source()->c_str() : "";
   ShaderTranslator* translator = NULL;
   if (use_shader_translator_) {
     translator = info->shader_type() == GL_VERTEX_SHADER ?
@@ -4295,7 +4295,7 @@ void GLES2DecoderImpl::DoCompileShader(GLuint client_id) {
     glGetShaderInfoLog(info->service_id(), max_len, &len, temp.get());
     DCHECK(max_len == 0 || len < max_len);
     DCHECK(len ==0 || temp[len] == '\0');
-    info->SetStatus(false, std::string(temp.get(), len), NULL);
+    info->SetStatus(false, std::string(temp.get(), len).c_str(), NULL);
   }
 };
 
@@ -4308,13 +4308,13 @@ void GLES2DecoderImpl::DoGetShaderiv(
   }
   switch (pname) {
     case GL_SHADER_SOURCE_LENGTH:
-      *params = info->source().size() + 1;
+      *params = info->source() ? info->source()->size() + 1 : 0;
       return;
     case GL_COMPILE_STATUS:
       *params = info->IsValid();
       return;
     case GL_INFO_LOG_LENGTH:
-      *params = info->log_info().size() + 1;
+      *params = info->log_info() ? info->log_info()->size() + 1 : 0;
       return;
     default:
       break;
@@ -4329,11 +4329,11 @@ error::Error GLES2DecoderImpl::HandleGetShaderSource(
   Bucket* bucket = CreateBucket(bucket_id);
   ShaderManager::ShaderInfo* info = GetShaderInfoNotProgram(
       shader, "glGetShaderSource");
-  if (!info) {
+  if (!info || !info->source()) {
     bucket->SetSize(0);
     return error::kNoError;
   }
-  bucket->SetFromString(info->source().c_str());
+  bucket->SetFromString(info->source()->c_str());
   return error::kNoError;
 }
 
@@ -4344,10 +4344,11 @@ error::Error GLES2DecoderImpl::HandleGetProgramInfoLog(
   Bucket* bucket = CreateBucket(bucket_id);
   ProgramManager::ProgramInfo* info = GetProgramInfoNotShader(
       program, "glGetProgramInfoLog");
-  if (!info) {
+  if (!info || !info->log_info()) {
+    bucket->SetSize(0);
     return error::kNoError;
   }
-  bucket->SetFromString(info->log_info().c_str());
+  bucket->SetFromString(info->log_info()->c_str());
   return error::kNoError;
 }
 
@@ -4358,11 +4359,11 @@ error::Error GLES2DecoderImpl::HandleGetShaderInfoLog(
   Bucket* bucket = CreateBucket(bucket_id);
   ShaderManager::ShaderInfo* info = GetShaderInfoNotProgram(
       shader, "glGetShaderInfoLog");
-  if (!info) {
+  if (!info || !info->log_info()) {
     bucket->SetSize(0);
     return error::kNoError;
   }
-  bucket->SetFromString(info->log_info().c_str());
+  bucket->SetFromString(info->log_info()->c_str());
   return error::kNoError;
 }
 
@@ -5428,10 +5429,21 @@ void GLES2DecoderImpl::DoCompressedTexSubImage2D(
     return;
   }
   GLenum type = 0;
-  GLenum dummy = 0;
-  if (!info->GetLevelType(target, level, &type, &dummy) ||
-      !info->ValidForTexture(
-          target, level, xoffset, yoffset, width, height, format, type)) {
+  GLenum internal_format = 0;
+  if (!info->GetLevelType(target, level, &type, &internal_format)) {
+    SetGLError(
+        GL_INVALID_OPERATION,
+        "glCompressdTexSubImage2D: level does not exist.");
+    return;
+  }
+  if (internal_format != format) {
+    SetGLError(
+        GL_INVALID_OPERATION,
+        "glCompressdTexSubImage2D: format does not match internal format.");
+    return;
+  }
+  if (!info->ValidForTexture(
+      target, level, xoffset, yoffset, width, height, format, type)) {
     SetGLError(GL_INVALID_VALUE,
                "glCompressdTexSubImage2D: bad dimensions.");
     return;
@@ -5627,6 +5639,25 @@ void GLES2DecoderImpl::DoTexSubImage2D(
                "glTexSubImage2D: unknown texture for target");
     return;
   }
+  GLenum current_type = 0;
+  GLenum internal_format = 0;
+  if (!info->GetLevelType(target, level, &current_type, &internal_format)) {
+    SetGLError(
+        GL_INVALID_OPERATION,
+        "glTexSubImage2D: level does not exist.");
+    return;
+  }
+  if (format != internal_format) {
+    SetGLError(GL_INVALID_OPERATION,
+               "glTexSubImage2D: format does not match internal format.");
+    return;
+  }
+  if (type != current_type) {
+    SetGLError(GL_INVALID_OPERATION,
+               "glTexSubImage2D: type does not match type of texture.");
+    return;
+  }
+
   if (!info->ValidForTexture(
           target, level, xoffset, yoffset, width, height, format, type)) {
     SetGLError(GL_INVALID_VALUE,
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index 4099e33..aaed425 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -2369,11 +2369,11 @@ TEST_F(GLES2DecoderTest, TexSubImage2DBadArgs) {
   cmd.Init(GL_TEXTURE_2D, 1, 0, 0, kWidth, kHeight, GL_RGB, GL_UNSIGNED_BYTE,
            kSharedMemoryId, kSharedMemoryOffset);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
-  EXPECT_EQ(GL_INVALID_VALUE, GetGLError());
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
   cmd.Init(GL_TEXTURE_2D, 1, 0, 0, kWidth, kHeight, GL_RGBA,
            GL_UNSIGNED_SHORT_4_4_4_4, kSharedMemoryId, kSharedMemoryOffset);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
-  EXPECT_EQ(GL_INVALID_VALUE, GetGLError());
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
   cmd.Init(GL_TEXTURE_2D, 1, 0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE,
            kInvalidSharedMemoryId, kSharedMemoryOffset);
   EXPECT_NE(error::kNoError, ExecuteCmd(cmd));
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_2.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_2.cc
index 452857e..2823379 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_2.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_2.cc
@@ -61,10 +61,6 @@ void GLES2DecoderTestBase::SpecializedSetup<LinkProgram, 0>(bool /* valid */) {
       GetProgramiv(kServiceProgramId, GL_INFO_LOG_LENGTH, _))
       .WillOnce(SetArgumentPointee<2>(0))
       .RetiresOnSaturation();
-  EXPECT_CALL(*gl_,
-      GetProgramInfoLog(kServiceProgramId, _, _, _))
-      .Times(1)
-      .RetiresOnSaturation();
   EXPECT_CALL(*gl_, GetProgramiv(kServiceProgramId, GL_ACTIVE_ATTRIBUTES, _))
       .WillOnce(SetArgumentPointee<2>(0));
   EXPECT_CALL(
@@ -104,10 +100,6 @@ void GLES2DecoderTestBase::SpecializedSetup<ValidateProgram, 0>(
       GetProgramiv(kServiceProgramId, GL_INFO_LOG_LENGTH, _))
       .WillOnce(SetArgumentPointee<2>(0))
       .RetiresOnSaturation();
-  EXPECT_CALL(*gl_,
-      GetProgramInfoLog(kServiceProgramId, _, _, _))
-      .Times(1)
-      .RetiresOnSaturation();
 };
 
 template <>
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
index e936276..bf108cc 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
@@ -573,10 +573,6 @@ void GLES2DecoderTestBase::SetupShader(
         .WillOnce(SetArgumentPointee<2>(0))
         .RetiresOnSaturation();
     EXPECT_CALL(*gl_,
-        GetProgramInfoLog(program_service_id, _, _, _))
-        .Times(1)
-        .RetiresOnSaturation();
-    EXPECT_CALL(*gl_,
         GetProgramiv(program_service_id, GL_ACTIVE_ATTRIBUTES, _))
         .WillOnce(SetArgumentPointee<2>(num_attribs))
         .RetiresOnSaturation();
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index a88a678..2b9ed0b 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -68,12 +68,16 @@ void ProgramManager::ProgramInfo::Reset() {
 void ProgramManager::ProgramInfo::UpdateLogInfo() {
   GLint max_len = 0;
   glGetProgramiv(service_id_, GL_INFO_LOG_LENGTH, &max_len);
+  if (max_len == 0) {
+    set_log_info(NULL);
+    return;
+  }
   scoped_array<char> temp(new char[max_len]);
   GLint len = 0;
   glGetProgramInfoLog(service_id_, max_len, &len, temp.get());
   DCHECK(max_len == 0 || len < max_len);
   DCHECK(len == 0 || temp[len] == '\0');
-  set_log_info(std::string(temp.get(), len));
+  set_log_info(std::string(temp.get(), len).c_str());
 }
 
 void ProgramManager::ProgramInfo::Update() {
@@ -344,7 +348,7 @@ void ProgramManager::ProgramInfo::GetProgramiv(GLenum pname, GLint* params) {
       break;
     case GL_INFO_LOG_LENGTH:
       // Notice +1 to accomodate NULL terminator.
-      *params = log_info_.size() + 1;
+      *params = log_info_.get() ? (log_info_->size() + 1) : 0;
       break;
     case GL_VALIDATE_STATUS:
       if (!CanLink()) {
diff --git a/gpu/command_buffer/service/program_manager.h b/gpu/command_buffer/service/program_manager.h
index ab51bf1..f3088d6 100644
--- a/gpu/command_buffer/service/program_manager.h
+++ b/gpu/command_buffer/service/program_manager.h
@@ -11,6 +11,7 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "gpu/command_buffer/service/gl_utils.h"
 #include "gpu/command_buffer/service/shader_manager.h"
 
@@ -137,12 +138,12 @@ class ProgramManager {
 
     bool CanLink() const;
 
-    const std::string& log_info() const {
-      return log_info_;
+    const std::string* log_info() const {
+      return log_info_.get();
     }
 
-    void set_log_info(const std::string& str) {
-      log_info_ = str;
+    void set_log_info(const char* str) {
+      log_info_.reset(str ? new std::string(str) : NULL);
     }
 
     bool InUse() const {
@@ -216,7 +217,7 @@ class ProgramManager {
     bool link_status_;
 
     // Log info
-    std::string log_info_;
+    scoped_ptr<std::string> log_info_;
   };
 
   ProgramManager();
diff --git a/gpu/command_buffer/service/program_manager_unittest.cc b/gpu/command_buffer/service/program_manager_unittest.cc
index 788e18c..8e08051 100644
--- a/gpu/command_buffer/service/program_manager_unittest.cc
+++ b/gpu/command_buffer/service/program_manager_unittest.cc
@@ -99,7 +99,7 @@ TEST_F(ProgramManagerTest, ProgramInfo) {
   EXPECT_FALSE(info1->IsValid());
   EXPECT_FALSE(info1->IsDeleted());
   EXPECT_FALSE(info1->CanLink());
-  EXPECT_STREQ("", info1->log_info().c_str());
+  EXPECT_TRUE(info1->log_info() == NULL);
 }
 
 class ProgramManagerWithShaderTest : public testing::Test {
@@ -186,10 +186,6 @@ class ProgramManagerWithShaderTest : public testing::Test {
         .WillOnce(SetArgumentPointee<2>(0))
         .RetiresOnSaturation();
     EXPECT_CALL(*gl_,
-        GetProgramInfoLog(service_id, _, _, _))
-        .Times(1)
-        .RetiresOnSaturation();
-    EXPECT_CALL(*gl_,
         GetProgramiv(service_id, GL_ACTIVE_ATTRIBUTES, _))
         .WillOnce(SetArgumentPointee<2>(num_attribs))
         .RetiresOnSaturation();
diff --git a/gpu/command_buffer/service/shader_manager.cc b/gpu/command_buffer/service/shader_manager.cc
index 59fad72..61cb3f2 100644
--- a/gpu/command_buffer/service/shader_manager.cc
+++ b/gpu/command_buffer/service/shader_manager.cc
@@ -9,9 +9,9 @@ namespace gpu {
 namespace gles2 {
 
 void ShaderManager::ShaderInfo::SetStatus(
-    bool valid, const std::string& log, ShaderTranslatorInterface* translator) {
+    bool valid, const char* log, ShaderTranslatorInterface* translator) {
   valid_ = valid;
-  log_info_ = log;
+  log_info_.reset(log ? new std::string(log) : NULL);
   if (translator && valid) {
     attrib_map_ = translator->attrib_map();
     uniform_map_ = translator->uniform_map();
diff --git a/gpu/command_buffer/service/shader_manager.h b/gpu/command_buffer/service/shader_manager.h
index 60aca6c..f826a3c 100644
--- a/gpu/command_buffer/service/shader_manager.h
+++ b/gpu/command_buffer/service/shader_manager.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "gpu/command_buffer/service/gl_utils.h"
 #include "gpu/command_buffer/service/shader_translator.h"
 
@@ -38,8 +39,8 @@ class ShaderManager {
           valid_(false) {
     }
 
-    void Update(const std::string& source) {
-      source_ = source;
+    void Update(const char* source) {
+      source_.reset(source ? new std::string(source) : NULL);
     }
 
     GLuint service_id() const {
@@ -50,19 +51,19 @@ class ShaderManager {
       return shader_type_;
     }
 
-    const std::string& source() const {
-      return source_;
+    const std::string* source() const {
+      return source_.get();
     }
 
     void SetStatus(
-        bool valid, const std::string& log,
+        bool valid, const char* log,
         ShaderTranslatorInterface* translator);
 
     const VariableInfo* GetAttribInfo(const std::string& name) const;
     const VariableInfo* GetUniformInfo(const std::string& name) const;
 
-    const std::string& log_info() const {
-      return log_info_;
+    const std::string* log_info() const {
+      return log_info_.get();
     }
 
     bool IsValid() const {
@@ -110,10 +111,10 @@ class ShaderManager {
     bool valid_;
 
     // The shader source as passed to glShaderSource.
-    std::string source_;
+    scoped_ptr<std::string> source_;
 
     // The shader translation log.
-    std::string log_info_;
+    scoped_ptr<std::string> log_info_;
 
     // The type info when the shader was last compiled.
     VariableMap attrib_map_;
diff --git a/gpu/command_buffer/service/shader_manager_unittest.cc b/gpu/command_buffer/service/shader_manager_unittest.cc
index 344cefd..c3c084d 100644
--- a/gpu/command_buffer/service/shader_manager_unittest.cc
+++ b/gpu/command_buffer/service/shader_manager_unittest.cc
@@ -79,7 +79,7 @@ TEST_F(ShaderManagerTest, ShaderInfo) {
   const GLuint kClient1Id = 1;
   const GLuint kService1Id = 11;
   const GLenum kShader1Type = GL_VERTEX_SHADER;
-  const std::string kClient1Source("hello world");
+  const char* kClient1Source = "hello world";
   // Check we can create shader.
   manager_.CreateShaderInfo(kClient1Id, kService1Id, kShader1Type);
   // Check shader got created.
@@ -90,14 +90,15 @@ TEST_F(ShaderManagerTest, ShaderInfo) {
   EXPECT_EQ(kShader1Type, info1->shader_type());
   EXPECT_FALSE(info1->IsValid());
   EXPECT_FALSE(info1->InUse());
-  EXPECT_STREQ("", info1->log_info().c_str());
+  EXPECT_TRUE(info1->source() == NULL);
+  EXPECT_TRUE(info1->log_info() == NULL);
   const char* kLog = "foo";
   info1->SetStatus(true, kLog, NULL);
   EXPECT_TRUE(info1->IsValid());
-  EXPECT_STREQ(kLog, info1->log_info().c_str());
+  EXPECT_STREQ(kLog, info1->log_info()->c_str());
   // Check we can set its source.
   info1->Update(kClient1Source);
-  EXPECT_STREQ(kClient1Source.c_str(), info1->source().c_str());
+  EXPECT_STREQ(kClient1Source, info1->source()->c_str());
 }
 
 TEST_F(ShaderManagerTest, GetInfo) {
@@ -155,7 +156,8 @@ TEST_F(ShaderManagerTest, GetInfo) {
     EXPECT_EQ(it->second.size, variable_info->size);
   }
   // Check attrib and uniform get cleared.
-  info1->SetStatus(true, "", NULL);
+  info1->SetStatus(true, NULL, NULL);
+  EXPECT_TRUE(info1->log_info() == NULL);
   for (ShaderTranslator::VariableMap::const_iterator it = attrib_map.begin();
        it != attrib_map.end(); ++it) {
     const ShaderManager::ShaderInfo::VariableInfo* variable_info =