Implements index validation for DrawElements.

(note: I also forgot to check in the changes to 
build_gles2_cmd_buffer.py from my last CL so
that's in here as well)

TEST=various unit tests
BUG=26101

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

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

8 files changed, 407 insertions, 117 deletions

diff --git a/gpu/command_buffer/build_gles2_cmd_buffer.py b/gpu/command_buffer/build_gles2_cmd_buffer.py
index 36dd9dd..af2f9b0 100755
--- a/gpu/command_buffer/build_gles2_cmd_buffer.py
+++ b/gpu/command_buffer/build_gles2_cmd_buffer.py
@@ -849,7 +849,7 @@ _FUNCTION_INFO = {
   'BindRenderbuffer': {'decoder_func': 'glBindRenderbufferEXT'},
   'BindTexture': {'decoder_func': 'DoBindTexture'},
   'BufferData': {'type': 'Manual', 'immediate': True},
-  'BufferSubData': {'type': 'Data'},
+  'BufferSubData': {'type': 'Data', 'decoder_func': 'DoBufferSubData'},
   'CheckFramebufferStatus': {'decoder_func': 'glCheckFramebufferStatusEXT'},
   'ClearDepthf': {'decoder_func': 'glClearDepth'},
   'CompileShader': {'decoder_func': 'DoCompileShader', 'unit_test': False},
@@ -1715,20 +1715,19 @@ class DataHandler(TypeHandler):
     name = func.name
     if name.endswith("Immediate"):
       name = name[0:-9]
-    if name == 'BufferData':
+    if name == 'BufferData' or name == 'BufferSubData':
       file.Write("  uint32 data_size = size;\n")
-    elif name == 'BufferSubData':
-      file.Write("  uint32 data_size = size;\n")
-    elif name == 'CompressedTexImage2D':
-      file.Write("  uint32 data_size = imageSize;\n")
-    elif name == 'CompressedTexSubImage2D':
+    elif (name == 'CompressedTexImage2D' or
+          name == 'CompressedTexSubImage2D'):
       file.Write("  uint32 data_size = imageSize;\n")
-    elif name == 'TexImage2D':
-      file.Write("  uint32 data_size = GLES2Util::ComputeImageDataSize(\n")
-      file.Write("      width, height, format, type, unpack_alignment_);\n")
-    elif name == 'TexSubImage2D':
-      file.Write("  uint32 data_size = GLES2Util::ComputeImageDataSize(\n")
-      file.Write("      width, height, format, type, unpack_alignment_);\n")
+    elif name == 'TexImage2D' or name == 'TexSubImage2D':
+      code = """  uint32 data_size;
+  if (!GLES2Util::ComputeImageDataSize(
+      width, height, format, type, unpack_alignment_, &data_size)) {
+    return error::kOutOfBounds;
+  }
+"""
+      file.Write(code)
     else:
       file.Write("// uint32 data_size = 0;  // TODO(gman): get correct size!\n")
 
@@ -1819,7 +1818,12 @@ class GENnHandler(TypeHandler):
 
   def WriteGetDataSizeCode(self, func, file):
     """Overrriden from TypeHandler."""
-    file.Write("  uint32 data_size = n * sizeof(GLuint);\n")
+    code = """  uint32 data_size;
+  if (!SafeMultiplyUint32(n, sizeof(GLuint), &data_size)) {
+    return error::kOutOfBounds;
+  }
+"""
+    file.Write(code)
 
   def WriteHandlerImplementation (self, func, file):
     """Overrriden from TypeHandler."""
@@ -2072,7 +2076,12 @@ class DELnHandler(TypeHandler):
 
   def WriteGetDataSizeCode(self, func, file):
     """Overrriden from TypeHandler."""
-    file.Write("  uint32 data_size = n * sizeof(GLuint);\n")
+    code = """  uint32 data_size;
+  if (!SafeMultiplyUint32(n, sizeof(GLuint), &data_size)) {
+    return error::kOutOfBounds;
+  }
+"""
+    file.Write(code)
 
   def WriteServiceUnitTest(self, func, file):
     """Overrriden from TypeHandler."""
@@ -2283,11 +2292,17 @@ class GETnHandler(TypeHandler):
     for arg in all_but_last_args:
       arg.WriteGetCode(file)
 
-    file.Write("  %s params;\n" % last_arg.type)
-    file.Write("  GLsizei num_values = util_.GLGetNumValuesReturned(pname);\n")
-    file.Write("  uint32 params_size = num_values * sizeof(*params);\n")
-    file.Write("  params = GetSharedMemoryAs<%s>(\n" % last_arg.type)
-    file.Write("      c.params_shm_id, c.params_shm_offset, params_size);\n")
+    code = """
+  %(last_arg_type)s params;
+  GLsizei num_values = util_.GLGetNumValuesReturned(pname);
+  uint32 params_size;
+  if (!SafeMultiplyUint32(num_values, sizeof(*params), &params_size)) {
+    return error::kOutOfBounds;
+  }
+  params = GetSharedMemoryAs<%(last_arg_type)s>(
+      c.params_shm_id, c.params_shm_offset, params_size);
+"""
+    file.Write(code % {'last_arg_type': last_arg.type})
     func.WriteHandlerValidation(file)
     func.WriteHandlerImplementation(file)
     file.Write("  return error::kNoError;\n")
@@ -2366,9 +2381,16 @@ TEST_F(%(test_name)s, %(name)sInvalidArgs%(arg_index)d_%(value_index)d) {
 
   def WriteGetDataSizeCode(self, func, file):
     """Overrriden from TypeHandler."""
-    file.Write("  uint32 data_size = ComputeImmediateDataSize("
-               "immediate_data_size, 1, sizeof(%s), %d);\n" %
-               (func.info.data_type, func.info.count))
+    code = """  uint32 data_size;
+  if (!ComputeDataSize(1, sizeof(%s), %d, &data_size)) {
+    return error::kOutOfBounds;
+  }
+"""
+    file.Write(code % (func.info.data_type, func.info.count))
+    if func.is_immediate:
+      file.Write("  if (data_size > immediate_data_size) {\n")
+      file.Write("    return error::kOutOfBounds;\n")
+      file.Write("  }\n")
 
   def WriteGLES2ImplementationHeader(self, func, file):
     """Overrriden from TypeHandler."""
@@ -2537,9 +2559,16 @@ TEST_F(%(test_name)s, %(name)sInvalidArgs%(arg_index)d_%(value_index)d) {
 
   def WriteGetDataSizeCode(self, func, file):
     """Overrriden from TypeHandler."""
-    file.Write("  uint32 data_size = ComputeImmediateDataSize("
-               "immediate_data_size, 1, sizeof(%s), %d);\n" %
-               (func.info.data_type, func.info.count))
+    code = """  uint32 data_size;
+  if (!ComputeDataSize(1, sizeof(%s), %d, &data_size)) {
+    return error::kOutOfBounds;
+  }
+"""
+    file.Write(code % (func.info.data_type, func.info.count))
+    if func.is_immediate:
+      file.Write("  if (data_size > immediate_data_size) {\n")
+      file.Write("    return error::kOutOfBounds;\n")
+      file.Write("  }\n")
 
   def WriteGLES2ImplementationHeader(self, func, file):
     """Overrriden from TypeHandler."""
@@ -3125,15 +3154,17 @@ class STRnHandler(TypeHandler):
     code = """%(return_type)s %(func_name)s(%(args)s) {
   helper_->SetBucketSize(kResultBucketId, 0);
   helper_->%(func_name)s(%(id_name)s, kResultBucketId);
-  std::string str;
-  if (GetBucketAsString(kResultBucketId, &str)) {
-    GLsizei max_size =
-        std::min(static_cast<size_t>(%(bufsize_name)s) - 1, str.size());
-    if (%(length_name)s != NULL) {
-      *%(length_name)s = max_size;
+  if (bufsize > 0) {
+    std::string str;
+    if (GetBucketAsString(kResultBucketId, &str)) {
+      GLsizei max_size =
+          std::min(static_cast<size_t>(%(bufsize_name)s) - 1, str.size());
+      if (%(length_name)s != NULL) {
+        *%(length_name)s = max_size;
+      }
+      memcpy(%(dest_name)s, str.c_str(), max_size);
+      %(dest_name)s[max_size] = '\\0';
     }
-    memcpy(%(dest_name)s, str.c_str(), max_size);
-    %(dest_name)s[max_size] = '\\0';
   }
 }
 """
diff --git a/gpu/command_buffer/common/gles2_cmd_utils.h b/gpu/command_buffer/common/gles2_cmd_utils.h
index c5cd792..318d2d8 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils.h
+++ b/gpu/command_buffer/common/gles2_cmd_utils.h
@@ -18,12 +18,13 @@ namespace gles2 {
 // returns true.
 template <typename T>
 inline bool SafeMultiply(T a, T b, T* dst) {
-  *dst = 0;
   if (b == 0) {
+    *dst = 0;
     return true;
   }
   T v = a * b;
   if (v / b != a) {
+    *dst = 0;
     return false;
   }
   *dst = v;
@@ -38,8 +39,8 @@ inline bool SafeMultiplyUint32(uint32 a, uint32 b, uint32* dst) {
 // Does an add checking for overflow.  If there was no overflow returns true.
 template <typename T>
 inline bool SafeAdd(T a, T b, T* dst) {
-  *dst = 0;
   if (a + b < a) {
+    *dst = 0;
     return false;
   }
   *dst = a + b;
diff --git a/gpu/command_buffer/service/buffer_manager.cc b/gpu/command_buffer/service/buffer_manager.cc
index 2dfbc15..9b58e42 100644
--- a/gpu/command_buffer/service/buffer_manager.cc
+++ b/gpu/command_buffer/service/buffer_manager.cc
@@ -4,6 +4,7 @@
 
 #include "gpu/command_buffer/service/buffer_manager.h"
 #include "base/logging.h"
+#include "gpu/command_buffer/common/gles2_cmd_utils.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 namespace gpu {
@@ -31,10 +32,96 @@ void BufferManager::RemoveBufferInfo(GLuint buffer_id) {
   }
 }
 
-GLuint BufferManager::BufferInfo::GetMaxValueForRange(
-    GLuint offset, GLsizei count, GLenum type) {
-  // TODO(gman): Scan the values in the given range and cache their results.
-  return 0u;
+void BufferManager::BufferInfo::SetSize(GLsizeiptr size) {
+  DCHECK(!IsDeleted());
+  if (size != size_) {
+    size_ = size;
+    ClearCache();
+    if (target_ == GL_ELEMENT_ARRAY_BUFFER) {
+      shadow_.reset(new int8[size]);
+      memset(shadow_.get(), 0, size);
+    }
+  }
+}
+
+bool BufferManager::BufferInfo::SetRange(
+    GLintptr offset, GLsizeiptr size, const GLvoid * data) {
+  DCHECK(!IsDeleted());
+  if (offset + size < offset ||
+        offset + size > size_) {
+    return false;
+  }
+  if (target_ == GL_ELEMENT_ARRAY_BUFFER) {
+    memcpy(shadow_.get() + offset, data, size);
+    ClearCache();
+  }
+  return true;
+}
+
+void BufferManager::BufferInfo::ClearCache() {
+  range_set_.clear();
+}
+
+template <typename T>
+GLuint GetMaxValue(const void* data, GLuint offset, GLsizei count) {
+  GLuint max_value = 0;
+  const T* element = reinterpret_cast<const T*>(
+      static_cast<const int8*>(data) + offset);
+  const T* end = element + count;
+  for (; element < end; ++element) {
+    if (*element > max_value) {
+      max_value = *element;
+    }
+  }
+  return max_value;
+}
+
+bool BufferManager::BufferInfo::GetMaxValueForRange(
+    GLuint offset, GLsizei count, GLenum type, GLuint* max_value) {
+  DCHECK_EQ(target_, static_cast<GLenum>(GL_ELEMENT_ARRAY_BUFFER));
+  DCHECK(!IsDeleted());
+  Range range(offset, count, type);
+  RangeToMaxValueMap::iterator it = range_set_.find(range);
+  if (it != range_set_.end()) {
+    *max_value = it->second;
+    return true;
+  }
+
+  uint32 size;
+  if (!SafeMultiplyUint32(
+      count, GLES2Util::GetGLTypeSizeForTexturesAndBuffers(type), &size)) {
+    return false;
+  }
+
+  if (!SafeAddUint32(offset, size, &size)) {
+    return false;
+  }
+
+  if (size > static_cast<uint32>(size_)) {
+    return false;
+  }
+
+  // Scan the range for the max value and store
+  GLuint max_v = 0;
+  switch (type) {
+    case GL_UNSIGNED_BYTE:
+      max_v = GetMaxValue<uint8>(shadow_.get(), offset, count);
+      break;
+    case GL_UNSIGNED_SHORT:
+      // Check we are not accessing an odd byte for a 2 byte value.
+      if ((offset & 1) != 0) {
+        return false;
+      }
+      max_v = GetMaxValue<uint16>(shadow_.get(), offset, count);
+      break;
+    default:
+      NOTREACHED();  // should never get here by validation.
+      break;
+  }
+  std::pair<RangeToMaxValueMap::iterator, bool> result =
+      range_set_.insert(std::make_pair(range, max_v));
+  *max_value = max_v;
+  return true;
 }
 
 }  // namespace gles2
diff --git a/gpu/command_buffer/service/buffer_manager.h b/gpu/command_buffer/service/buffer_manager.h
index e278a85..4afdaa0 100644
--- a/gpu/command_buffer/service/buffer_manager.h
+++ b/gpu/command_buffer/service/buffer_manager.h
@@ -7,7 +7,9 @@
 
 #include <map>
 #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"
 
 namespace gpu {
@@ -27,6 +29,7 @@ class BufferManager {
 
     explicit BufferInfo(GLuint buffer_id)
         : buffer_id_(buffer_id),
+          target_(0),
           size_(0) {
     }
 
@@ -34,17 +37,32 @@ class BufferManager {
       return buffer_id_;
     }
 
+    GLenum target() const {
+      return target_;
+    }
+
+    void set_target(GLenum target) {
+      DCHECK_EQ(target_, 0u);  // you can only set this once.
+      target_ = target;
+    }
+
     GLsizeiptr size() const {
       return size_;
     }
 
-    void set_size(GLsizeiptr size) {
-      size_ = size;
-    }
+    void SetSize(GLsizeiptr size);
+
+    // Sets a range of data for this buffer. Returns false if the offset or size
+    // is out of range.
+    bool SetRange(
+      GLintptr offset, GLsizeiptr size, const GLvoid * data);
 
-    // Returns the maximum value in the buffer for the given range
-    // interpreted as the given type.
-    GLuint GetMaxValueForRange(GLuint offset, GLsizei count, GLenum type);
+    // Gets the maximum value in the buffer for the given range interpreted as
+    // the given type. Returns false if offset and count are out of range.
+    // offset is in bytes.
+    // count is in elements of type.
+    bool GetMaxValueForRange(GLuint offset, GLsizei count, GLenum type,
+                             GLuint* max_value);
 
     bool IsDeleted() {
       return buffer_id_ == 0;
@@ -54,14 +72,63 @@ class BufferManager {
     friend class BufferManager;
     friend class base::RefCounted<BufferInfo>;
 
+    // Represents a range in a buffer.
+    class Range {
+     public:
+      Range(GLuint offset, GLsizei count, GLenum type)
+          : offset_(offset),
+            count_(count),
+            type_(type) {
+      }
+
+      // A less functor provided for std::map so it can find ranges.
+      struct Less {
+        bool operator() (const Range& lhs, const Range& rhs) {
+          if (lhs.offset_ != rhs.offset_) {
+            return lhs.offset_ < rhs.offset_;
+          }
+          if (lhs.count_ != rhs.count_) {
+            return lhs.count_ < rhs.count_;
+          }
+          return lhs.type_ < rhs.type_;
+        }
+      };
+
+     private:
+      GLuint offset_;
+      GLsizei count_;
+      GLenum type_;
+    };
+
     ~BufferInfo() { }
 
     void MarkAsDeleted() {
       buffer_id_ = 0;
+      shadow_.reset();
+      ClearCache();
     }
 
+    // Clears any cache of index ranges.
+    void ClearCache();
+
+    // Service side buffer id.
     GLuint buffer_id_;
+
+    // The type of buffer. 0 = unset, GL_BUFFER_ARRAY = vertex data,
+    // GL_ELEMENT_BUFFER_ARRAY = index data.
+    // Once set a buffer can not be used for something else.
+    GLenum target_;
+
+    // Size of buffer.
     GLsizeiptr size_;
+
+    // A copy of the data in the buffer. This data is only kept if the target
+    // is GL_ELEMENT_BUFFER_ARRAY
+    scoped_array<int8> shadow_;
+
+    // A map of ranges to the highest value in that range of a certain type.
+    typedef std::map<Range, GLuint, Range::Less> RangeToMaxValueMap;
+    RangeToMaxValueMap range_set_;
   };
 
   BufferManager() { }
diff --git a/gpu/command_buffer/service/buffer_manager_unittest.cc b/gpu/command_buffer/service/buffer_manager_unittest.cc
index 2b121d8..df58915 100644
--- a/gpu/command_buffer/service/buffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/buffer_manager_unittest.cc
@@ -32,8 +32,14 @@ TEST_F(BufferManagerTest, Basic) {
   // Check buffer got created.
   BufferManager::BufferInfo* info1 = manager_.GetBufferInfo(kBuffer1Id);
   ASSERT_TRUE(info1 != NULL);
+  EXPECT_EQ(0u, info1->target());
+  EXPECT_EQ(0, info1->size());
+  EXPECT_FALSE(info1->IsDeleted());
+  EXPECT_EQ(kBuffer1Id, info1->buffer_id());
+  info1->set_target(GL_ELEMENT_ARRAY_BUFFER);
+  EXPECT_EQ(GL_ELEMENT_ARRAY_BUFFER, info1->target());
   // Check we and set its size.
-  info1->set_size(kBuffer1Size);
+  info1->SetSize(kBuffer1Size);
   EXPECT_EQ(kBuffer1Size, info1->size());
   // Check we get nothing for a non-existent buffer.
   EXPECT_TRUE(manager_.GetBufferInfo(kBuffer2Id) == NULL);
@@ -44,7 +50,80 @@ TEST_F(BufferManagerTest, Basic) {
   EXPECT_TRUE(manager_.GetBufferInfo(kBuffer1Id) == NULL);
 }
 
-// TODO(gman): Test GetMaxValueForRange.
+TEST_F(BufferManagerTest, SetRange) {
+  const GLuint kBufferId = 1;
+  const uint8 data[] = {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
+  const uint8 new_data[] = {100, 120, 110};
+  manager_.CreateBufferInfo(kBufferId);
+  BufferManager::BufferInfo* info = manager_.GetBufferInfo(kBufferId);
+  ASSERT_TRUE(info != NULL);
+  info->set_target(GL_ELEMENT_ARRAY_BUFFER);
+  info->SetSize(sizeof(data));
+  EXPECT_TRUE(info->SetRange(0, sizeof(data), data));
+  EXPECT_TRUE(info->SetRange(sizeof(data), 0, data));
+  EXPECT_FALSE(info->SetRange(sizeof(data), 1, data));
+  EXPECT_FALSE(info->SetRange(0, sizeof(data) + 1, data));
+}
+
+TEST_F(BufferManagerTest, GetMaxValueForRangeUint8) {
+  const GLuint kBufferId = 1;
+  const uint8 data[] = {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
+  const uint8 new_data[] = {100, 120, 110};
+  manager_.CreateBufferInfo(kBufferId);
+  BufferManager::BufferInfo* info = manager_.GetBufferInfo(kBufferId);
+  ASSERT_TRUE(info != NULL);
+  info->set_target(GL_ELEMENT_ARRAY_BUFFER);
+  info->SetSize(sizeof(data));
+  EXPECT_TRUE(info->SetRange(0, sizeof(data), data));
+  GLuint max_value;
+  // Check entire range succeeds.
+  EXPECT_TRUE(info->GetMaxValueForRange(0, 10, GL_UNSIGNED_BYTE, &max_value));
+  EXPECT_EQ(10u, max_value);
+  // Check sub range succeeds.
+  EXPECT_TRUE(info->GetMaxValueForRange(4, 3, GL_UNSIGNED_BYTE, &max_value));
+  EXPECT_EQ(6u, max_value);
+  // Check changing sub range succeeds.
+  EXPECT_TRUE(info->SetRange(4, sizeof(new_data), new_data));
+  EXPECT_TRUE(info->GetMaxValueForRange(4, 3, GL_UNSIGNED_BYTE, &max_value));
+  EXPECT_EQ(120u, max_value);
+  max_value = 0;
+  EXPECT_TRUE(info->GetMaxValueForRange(0, 10, GL_UNSIGNED_BYTE, &max_value));
+  EXPECT_EQ(120u, max_value);
+  // Check out of range fails.
+  EXPECT_FALSE(info->GetMaxValueForRange(0, 11, GL_UNSIGNED_BYTE, &max_value));
+  EXPECT_FALSE(info->GetMaxValueForRange(10, 1, GL_UNSIGNED_BYTE, &max_value));
+}
+
+TEST_F(BufferManagerTest, GetMaxValueForRangeUint16) {
+  const GLuint kBufferId = 1;
+  const uint16 data[] = {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
+  const uint16 new_data[] = {100, 120, 110};
+  manager_.CreateBufferInfo(kBufferId);
+  BufferManager::BufferInfo* info = manager_.GetBufferInfo(kBufferId);
+  ASSERT_TRUE(info != NULL);
+  info->set_target(GL_ELEMENT_ARRAY_BUFFER);
+  info->SetSize(sizeof(data));
+  EXPECT_TRUE(info->SetRange(0, sizeof(data), data));
+  GLuint max_value;
+  // Check entire range succeeds.
+  EXPECT_TRUE(info->GetMaxValueForRange(0, 10, GL_UNSIGNED_SHORT, &max_value));
+  EXPECT_EQ(10u, max_value);
+  // Check odd offset fails for GL_UNSIGNED_SHORT.
+  EXPECT_FALSE(info->GetMaxValueForRange(1, 10, GL_UNSIGNED_SHORT, &max_value));
+  // Check sub range succeeds.
+  EXPECT_TRUE(info->GetMaxValueForRange(8, 3, GL_UNSIGNED_SHORT, &max_value));
+  EXPECT_EQ(6u, max_value);
+  // Check changing sub range succeeds.
+  EXPECT_TRUE(info->SetRange(8, sizeof(new_data), new_data));
+  EXPECT_TRUE(info->GetMaxValueForRange(8, 3, GL_UNSIGNED_SHORT, &max_value));
+  EXPECT_EQ(120u, max_value);
+  max_value = 0;
+  EXPECT_TRUE(info->GetMaxValueForRange(0, 10, GL_UNSIGNED_SHORT, &max_value));
+  EXPECT_EQ(120u, max_value);
+  // Check out of range fails.
+  EXPECT_FALSE(info->GetMaxValueForRange(0, 11, GL_UNSIGNED_SHORT, &max_value));
+  EXPECT_FALSE(info->GetMaxValueForRange(20, 1, GL_UNSIGNED_SHORT, &max_value));
+}
 
 }  // namespace gles2
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 54cef89..dbb49e9 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -434,6 +434,14 @@ class GLES2DecoderImpl : public GLES2Decoder {
   // Wrapper for glBindTexture since we need to track the current targets.
   void DoBindTexture(GLenum target, GLuint texture);
 
+  // Wrapper for BufferData.
+  void DoBufferData(
+    GLenum target, GLsizeiptr size, const GLvoid * data, GLenum usage);
+
+  // Wrapper for BufferSubData.
+  void DoBufferSubData(
+    GLenum target, GLintptr offset, GLsizeiptr size, const GLvoid * data);
+
   // Wrapper for glCompileShader.
   void DoCompileShader(GLuint shader);
 
@@ -1503,10 +1511,15 @@ void GLES2DecoderImpl::DoBindBuffer(GLenum target, GLuint buffer) {
   BufferManager::BufferInfo* info = NULL;
   if (buffer) {
     info = GetBufferInfo(buffer);
-    if (!info) {
+    // Check the buffer exists
+    // Check that we are not trying to bind it to a different target.
+    if (!info || (info->target() != 0 && info->target() != target)) {
       SetGLError(GL_INVALID_OPERATION);
       return;
     }
+    if (info->target() == 0) {
+      info->set_target(target);
+    }
   }
   switch (target) {
     case GL_ARRAY_BUFFER:
@@ -1919,28 +1932,19 @@ error::Error GLES2DecoderImpl::HandleDrawElements(
                !ValidateGLenumIndexType(type)) {
       SetGLError(GL_INVALID_ENUM);
     } else {
-      GLsizeiptr buffer_size = bound_element_array_buffer_->size();
-      if (offset > buffer_size) {
+      GLuint max_vertex_accessed;
+      if (!bound_element_array_buffer_->GetMaxValueForRange(
+          offset, count, type, &max_vertex_accessed)) {
         SetGLError(GL_INVALID_OPERATION);
       } else {
-        GLsizei usable_size = buffer_size - offset;
-        GLsizei num_elements =
-            usable_size / GLES2Util::GetGLTypeSizeForTexturesAndBuffers(type);
-        if (count > num_elements) {
-          SetGLError(GL_INVALID_OPERATION);
-        } else {
+        if (IsDrawValid(max_vertex_accessed)) {
+          bool has_non_renderable_textures;
+          SetBlackTextureForNonRenderableTextures(
+              &has_non_renderable_textures);
           const GLvoid* indices = reinterpret_cast<const GLvoid*>(offset);
-          GLuint max_vertex_accessed =
-              bound_element_array_buffer_->GetMaxValueForRange(
-                  offset, count, type);
-          if (IsDrawValid(max_vertex_accessed)) {
-            bool has_non_renderable_textures;
-            SetBlackTextureForNonRenderableTextures(
-                &has_non_renderable_textures);
-            glDrawElements(mode, count, type, indices);
-            if (has_non_renderable_textures) {
-              RestoreStateForNonRenderableTextures();
-            }
+          glDrawElements(mode, count, type, indices);
+          if (has_non_renderable_textures) {
+            RestoreStateForNonRenderableTextures();
           }
         }
       }
@@ -2272,33 +2276,21 @@ error::Error GLES2DecoderImpl::HandleGetString(
   return error::kNoError;
 }
 
-error::Error GLES2DecoderImpl::HandleBufferData(
-    uint32 immediate_data_size, const gles2::BufferData& c) {
-  GLenum target = static_cast<GLenum>(c.target);
-  GLsizeiptr size = static_cast<GLsizeiptr>(c.size);
-  uint32 data_shm_id = static_cast<uint32>(c.data_shm_id);
-  uint32 data_shm_offset = static_cast<uint32>(c.data_shm_offset);
-  GLenum usage = static_cast<GLenum>(c.usage);
-  const void* data = NULL;
-  if (data_shm_id != 0 || data_shm_offset != 0) {
-    data = GetSharedMemoryAs<const void*>(data_shm_id, data_shm_offset, size);
-    if (!data) {
-      return error::kOutOfBounds;
-    }
-  }
+void GLES2DecoderImpl::DoBufferData(
+  GLenum target, GLsizeiptr size, const GLvoid * data, GLenum usage) {
   if (!ValidateGLenumBufferTarget(target) ||
       !ValidateGLenumBufferUsage(usage)) {
     SetGLError(GL_INVALID_ENUM);
-    return error::kNoError;
+    return;
   }
   if (size < 0) {
     SetGLError(GL_INVALID_VALUE);
-    return error::kNoError;
+    DoBufferData(target, size, data, usage);
   }
   BufferManager::BufferInfo* info = GetBufferInfoForTarget(target);
   if (!info) {
     SetGLError(GL_INVALID_OPERATION);
-    return error::kNoError;
+    DoBufferData(target, size, data, usage);
   }
   // Clear the buffer to 0 if no initial data was passed in.
   scoped_array<int8> zero;
@@ -2313,8 +2305,26 @@ error::Error GLES2DecoderImpl::HandleBufferData(
   if (error != GL_NO_ERROR) {
     SetGLError(error);
   } else {
-    info->set_size(size);
+    info->SetSize(size);
+    info->SetRange(0, size, data);
   }
+}
+
+error::Error GLES2DecoderImpl::HandleBufferData(
+    uint32 immediate_data_size, const gles2::BufferData& c) {
+  GLenum target = static_cast<GLenum>(c.target);
+  GLsizeiptr size = static_cast<GLsizeiptr>(c.size);
+  uint32 data_shm_id = static_cast<uint32>(c.data_shm_id);
+  uint32 data_shm_offset = static_cast<uint32>(c.data_shm_offset);
+  GLenum usage = static_cast<GLenum>(c.usage);
+  const void* data = NULL;
+  if (data_shm_id != 0 || data_shm_offset != 0) {
+    data = GetSharedMemoryAs<const void*>(data_shm_id, data_shm_offset, size);
+    if (!data) {
+      return error::kOutOfBounds;
+    }
+  }
+  DoBufferData(target, size, data, usage);
   return error::kNoError;
 }
 
@@ -2328,29 +2338,21 @@ error::Error GLES2DecoderImpl::HandleBufferDataImmediate(
     return error::kOutOfBounds;
   }
   GLenum usage = static_cast<GLenum>(c.usage);
-  if (!ValidateGLenumBufferTarget(target) ||
-      !ValidateGLenumBufferUsage(usage)) {
-    SetGLError(GL_INVALID_ENUM);
-    return error::kNoError;
-  }
-  if (size < 0) {
-    SetGLError(GL_INVALID_VALUE);
-    return error::kNoError;
-  }
+  DoBufferData(target, size, data, usage);
+  return error::kNoError;
+}
+
+void GLES2DecoderImpl::DoBufferSubData(
+  GLenum target, GLintptr offset, GLsizeiptr size, const GLvoid * data) {
   BufferManager::BufferInfo* info = GetBufferInfoForTarget(target);
   if (!info) {
     SetGLError(GL_INVALID_OPERATION);
-    return error::kNoError;
   }
-  CopyRealGLErrorsToWrapper();
-  glBufferData(target, size, data, usage);
-  GLenum error = glGetError();
-  if (error != GL_NO_ERROR) {
-    SetGLError(error);
+  if (!info->SetRange(offset, size, data)) {
+    SetGLError(GL_INVALID_VALUE);
   } else {
-    info->set_size(size);
+    glBufferSubData(target, offset, size, data);
   }
-  return error::kNoError;
 }
 
 error::Error GLES2DecoderImpl::DoCompressedTexImage2D(
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h b/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
index 025e0b3..eb66e7f 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
@@ -231,7 +231,7 @@ error::Error GLES2DecoderImpl::HandleBufferSubData(
   if (data == NULL) {
     return error::kOutOfBounds;
   }
-  glBufferSubData(target, offset, size, data);
+  DoBufferSubData(target, offset, size, data);
   return error::kNoError;
 }
 
@@ -254,7 +254,7 @@ error::Error GLES2DecoderImpl::HandleBufferSubDataImmediate(
   if (data == NULL) {
     return error::kOutOfBounds;
   }
-  glBufferSubData(target, offset, size, data);
+  DoBufferSubData(target, offset, size, data);
   return error::kNoError;
 }
 
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index 5f96cd5..9228165 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -194,12 +194,12 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsNoAttributesSucceeds) {
   SetupIndexBuffer();
   EXPECT_CALL(*gl_, DrawElements(GL_TRIANGLES, kValidIndexRangeCount,
                                  GL_UNSIGNED_SHORT,
-                                 BufferOffset(kValidIndexRangeStart)))
+                                 BufferOffset(kValidIndexRangeStart * 2)))
       .Times(1)
       .RetiresOnSaturation();
   DrawElements cmd;
   cmd.Init(GL_TRIANGLES, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kValidIndexRangeStart);
+           kValidIndexRangeStart * 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
@@ -212,7 +212,7 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsMissingAttributesFails) {
       .Times(0);
   DrawElements cmd;
   cmd.Init(GL_TRIANGLES, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kValidIndexRangeStart);
+           kValidIndexRangeStart * 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
 }
@@ -225,12 +225,12 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsValidAttributesSucceeds) {
 
   EXPECT_CALL(*gl_, DrawElements(GL_TRIANGLES, kValidIndexRangeCount,
                                  GL_UNSIGNED_SHORT,
-                                 BufferOffset(kValidIndexRangeStart)))
+                                 BufferOffset(kValidIndexRangeStart * 2)))
       .Times(1)
       .RetiresOnSaturation();
   DrawElements cmd;
   cmd.Init(GL_TRIANGLES, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kValidIndexRangeStart);
+           kValidIndexRangeStart * 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
@@ -245,7 +245,7 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsDeletedBufferFails) {
       .Times(0);
   DrawElements cmd;
   cmd.Init(GL_TRIANGLES, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kValidIndexRangeStart);
+           kValidIndexRangeStart * 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
 }
@@ -260,7 +260,7 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsDeletedProgramSucceedsNoGLCall) {
       .Times(0);
   DrawElements cmd;
   cmd.Init(GL_TRIANGLES, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kValidIndexRangeStart);
+           kValidIndexRangeStart * 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
@@ -274,7 +274,7 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsWithInvalidModeFails) {
       .Times(0);
   DrawElements cmd;
   cmd.Init(GL_QUADS, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kValidIndexRangeStart);
+           kValidIndexRangeStart * 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_ENUM, GetGLError());
   cmd.Init(GL_POLYGON, kValidIndexRangeCount, GL_UNSIGNED_SHORT,
@@ -291,7 +291,7 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsInvalidCountFails) {
   // Try start > 0
   EXPECT_CALL(*gl_, DrawElements(_, _, _, _)).Times(0);
   DrawElements cmd;
-  cmd.Init(GL_TRIANGLES, kNumIndices, GL_UNSIGNED_SHORT, 1);
+  cmd.Init(GL_TRIANGLES, kNumIndices, GL_UNSIGNED_SHORT, 2);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
@@ -303,7 +303,6 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsInvalidCountFails) {
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
 
-#if 0  // TODO(gman): Turn on this test once buffer validation is in
 TEST_F(GLES2DecoderWithShaderTest, DrawElementsOutOfRangeIndicesFails) {
   SetupVertexBuffer();
   SetupIndexBuffer();
@@ -312,12 +311,24 @@ TEST_F(GLES2DecoderWithShaderTest, DrawElementsOutOfRangeIndicesFails) {
   EXPECT_CALL(*gl_, DrawElements(_, _, _, _)).Times(0);
   DrawElements cmd;
   cmd.Init(GL_TRIANGLES, kInvalidIndexRangeCount, GL_UNSIGNED_SHORT,
-           kInvalidIndexRangeStart);
+           kInvalidIndexRangeStart * 2);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
+TEST_F(GLES2DecoderWithShaderTest, DrawElementsOddOffsetForUint16Fails) {
+  SetupVertexBuffer();
+  SetupIndexBuffer();
+  DoVertexAttribPointer(1, 2, GL_FLOAT, 0, 0);
+
+  EXPECT_CALL(*gl_, DrawElements(_, _, _, _)).Times(0);
+  DrawElements cmd;
+  cmd.Init(GL_TRIANGLES, kInvalidIndexRangeCount, GL_UNSIGNED_SHORT, 1);
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
   EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
-#endif
 
 TEST_F(GLES2DecoderWithShaderTest, GetVertexAttribPointervSucceeds) {
   const float dummy = 0;
@@ -990,6 +1001,18 @@ TEST_F(GLES2DecoderWithShaderTest, Uniform1ivImmediateValidArgs) {
             ExecuteImmediateCmd(cmd, sizeof(temp)));
 }
 
+TEST_F(GLES2DecoderWithShaderTest, BindBufferToDifferentTargetFails) {
+  // Bind the buffer to GL_ARRAY_BUFFER
+  DoBindBuffer(GL_ARRAY_BUFFER, client_buffer_id_, kServiceBufferId);
+  // Attempt to rebind to GL_ELEMENT_ARRAY_BUFFER
+  // NOTE: Real GLES2 does not have this restriction but WebGL and we do.
+  EXPECT_CALL(*gl_, BindBuffer(_, _))
+      .Times(0);
+  BindBuffer cmd;
+  cmd.Init(GL_ELEMENT_ARRAY_BUFFER, client_buffer_id_);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
+}
 
 // TODO(gman): BindAttribLocation