Fix BufferManager's handling of deleted buffers.

The BufferManager was originally designed assuming
you can't use a deleted buffer. Now that you can
there were some bugs.

TEST=unit tests
BUG=110198

R=apatrick@chromium.org


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

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

5 files changed, 112 insertions, 71 deletions

diff --git a/gpu/command_buffer/service/buffer_manager.cc b/gpu/command_buffer/service/buffer_manager.cc
index 9670766..0ab46c9 100644
--- a/gpu/command_buffer/service/buffer_manager.cc
+++ b/gpu/command_buffer/service/buffer_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -25,7 +25,6 @@ BufferManager::~BufferManager() {
 void BufferManager::Destroy(bool have_context) {
   while (!buffer_infos_.empty()) {
     BufferInfo* info = buffer_infos_.begin()->second;
-    mem_represented_ -= info->size();
     if (have_context) {
       if (!info->IsDeleted()) {
         GLuint service_id = info->service_id();
@@ -51,7 +50,7 @@ void BufferManager::CreateBufferInfo(GLuint client_id, GLuint service_id) {
   std::pair<BufferInfoMap::iterator, bool> result =
       buffer_infos_.insert(
           std::make_pair(client_id,
-                         BufferInfo::Ref(new BufferInfo(service_id))));
+                         BufferInfo::Ref(new BufferInfo(this, service_id))));
   DCHECK(result.second);
 }
 
@@ -66,21 +65,30 @@ void BufferManager::RemoveBufferInfo(GLuint client_id) {
   if (it != buffer_infos_.end()) {
     BufferInfo* buffer = it->second;
     buffer->MarkAsDeleted();
-    mem_represented_ -= buffer->size();
-    UpdateMemRepresented();
     buffer_infos_.erase(it);
   }
 }
 
-BufferManager::BufferInfo::BufferInfo(GLuint service_id)
-    : service_id_(service_id),
+void BufferManager::StopTracking(BufferManager::BufferInfo* buffer) {
+  mem_represented_ -= buffer->size();
+  UpdateMemRepresented();
+}
+
+BufferManager::BufferInfo::BufferInfo(BufferManager* manager, GLuint service_id)
+    : manager_(manager),
+      service_id_(service_id),
       target_(0),
       size_(0),
       usage_(GL_STATIC_DRAW),
       shadowed_(false) {
 }
 
-BufferManager::BufferInfo::~BufferInfo() { }
+BufferManager::BufferInfo::~BufferInfo() {
+  if (manager_) {
+    manager_->StopTracking(this);
+    manager_ = NULL;
+  }
+}
 
 void BufferManager::BufferInfo::SetInfo(
     GLsizeiptr size, GLenum usage, bool shadow) {
diff --git a/gpu/command_buffer/service/buffer_manager.h b/gpu/command_buffer/service/buffer_manager.h
index d59f755..32be176 100644
--- a/gpu/command_buffer/service/buffer_manager.h
+++ b/gpu/command_buffer/service/buffer_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -28,7 +28,7 @@ class BufferManager {
    public:
     typedef scoped_refptr<BufferInfo> Ref;
 
-    explicit BufferInfo(GLuint service_id);
+    BufferInfo(BufferManager* manager, GLuint service_id);
 
     GLuint service_id() const {
       return service_id_;
@@ -122,6 +122,9 @@ class BufferManager {
     // Clears any cache of index ranges.
     void ClearCache();
 
+    // The manager that owns this BufferInfo.
+    BufferManager* manager_;
+
     // Service side buffer id.
     GLuint service_id_;
 
@@ -179,6 +182,8 @@ class BufferManager {
  private:
   void UpdateMemRepresented();
 
+  void StopTracking(BufferInfo* info);
+
   // Info for each buffer in the system.
   typedef base::hash_map<GLuint, BufferInfo::Ref> BufferInfoMap;
   BufferInfoMap buffer_infos_;
diff --git a/gpu/command_buffer/service/buffer_manager_unittest.cc b/gpu/command_buffer/service/buffer_manager_unittest.cc
index 1e4258b..ec46915 100644
--- a/gpu/command_buffer/service/buffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/buffer_manager_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -221,6 +221,20 @@ TEST_F(BufferManagerTest, GetMaxValueForRangeUint32) {
   EXPECT_FALSE(info->GetMaxValueForRange(40, 1, GL_UNSIGNED_INT, &max_value));
 }
 
+TEST_F(BufferManagerTest, UseDeletedBuffer) {
+  const GLuint kClientBufferId = 1;
+  const GLuint kServiceBufferId = 11;
+  const uint32 data[] = {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
+  manager_.CreateBufferInfo(kClientBufferId, kServiceBufferId);
+  BufferManager::BufferInfo::Ref info = manager_.GetBufferInfo(kClientBufferId);
+  ASSERT_TRUE(info != NULL);
+  manager_.SetTarget(info, GL_ARRAY_BUFFER);
+  // Remove buffer
+  manager_.RemoveBufferInfo(kClientBufferId);
+  // Use it after removing
+  manager_.SetInfo(info, sizeof(data), GL_STATIC_DRAW);
+}
+
 }  // 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 f544e54..6b51105 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1366,7 +1366,7 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>,
   BufferManager::BufferInfo::Ref bound_element_array_buffer_;
 
   // Class that manages vertex attribs.
-  VertexAttribManager vertex_attrib_manager_;
+  scoped_ptr<VertexAttribManager> vertex_attrib_manager_;
 
   // The buffer we bind to attrib 0 since OpenGL requires it (ES does not).
   GLuint attrib_0_buffer_id_;
@@ -1972,7 +1972,8 @@ bool GLES2DecoderImpl::Initialize(
   CHECK_GL_ERROR();
   disallowed_features_ = disallowed_features;
 
-  vertex_attrib_manager_.Initialize(group_->max_vertex_attribs());
+  vertex_attrib_manager_.reset(new VertexAttribManager());
+  vertex_attrib_manager_->Initialize(group_->max_vertex_attribs());
 
   util_.set_num_compressed_texture_formats(
       validators_->compressed_texture_format.GetValues().size());
@@ -2320,7 +2321,7 @@ void GLES2DecoderImpl::DeleteBuffersHelper(
   for (GLsizei ii = 0; ii < n; ++ii) {
     BufferManager::BufferInfo* buffer = GetBufferInfo(client_ids[ii]);
     if (buffer && !buffer->IsDeleted()) {
-      vertex_attrib_manager_.Unbind(buffer);
+      vertex_attrib_manager_->Unbind(buffer);
       if (bound_array_buffer_ == buffer) {
         bound_array_buffer_ = NULL;
       }
@@ -2690,6 +2691,7 @@ void GLES2DecoderImpl::Destroy() {
   SetParent(NULL, 0);
 
   // Unbind everything.
+  vertex_attrib_manager_.reset();
   texture_units_.reset();
   bound_array_buffer_ = NULL;
   bound_element_array_buffer_ = NULL;
@@ -3332,7 +3334,7 @@ void GLES2DecoderImpl::DoBindTexture(GLenum target, GLuint client_id) {
 }
 
 void GLES2DecoderImpl::DoDisableVertexAttribArray(GLuint index) {
-  if (vertex_attrib_manager_.Enable(index, false)) {
+  if (vertex_attrib_manager_->Enable(index, false)) {
     if (index != 0 ||
         gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2) {
       glDisableVertexAttribArray(index);
@@ -3344,7 +3346,7 @@ void GLES2DecoderImpl::DoDisableVertexAttribArray(GLuint index) {
 }
 
 void GLES2DecoderImpl::DoEnableVertexAttribArray(GLuint index) {
-  if (vertex_attrib_manager_.Enable(index, true)) {
+  if (vertex_attrib_manager_->Enable(index, true)) {
     glEnableVertexAttribArray(index);
   } else {
     SetGLError(GL_INVALID_VALUE,
@@ -4830,7 +4832,7 @@ bool GLES2DecoderImpl::IsDrawValid(GLuint max_vertex_accessed) {
   // If they are not used by the current program check that they have a buffer
   // assigned.
   const VertexAttribManager::VertexAttribInfoList& infos =
-      vertex_attrib_manager_.GetEnabledVertexAttribInfos();
+      vertex_attrib_manager_->GetEnabledVertexAttribInfos();
   for (VertexAttribManager::VertexAttribInfoList::const_iterator it =
        infos.begin(); it != infos.end(); ++it) {
     const VertexAttribManager::VertexAttribInfo* info = *it;
@@ -4866,7 +4868,7 @@ bool GLES2DecoderImpl::SimulateAttrib0(
     return true;
 
   const VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(0);
+      vertex_attrib_manager_->GetVertexAttribInfo(0);
   // If it's enabled or it's not used then we don't need to do anything.
   bool attrib_0_used = current_program_->GetAttribInfoByLocation(0) != NULL;
   if (info->enabled() && attrib_0_used) {
@@ -4922,7 +4924,7 @@ bool GLES2DecoderImpl::SimulateAttrib0(
 
 void GLES2DecoderImpl::RestoreStateForSimulatedAttrib0() {
   const VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(0);
+      vertex_attrib_manager_->GetVertexAttribInfo(0);
   const void* ptr = reinterpret_cast<const void*>(info->offset());
   BufferManager::BufferInfo* buffer_info = info->buffer();
   glBindBuffer(GL_ARRAY_BUFFER, buffer_info ? buffer_info->service_id() : 0);
@@ -4940,7 +4942,7 @@ bool GLES2DecoderImpl::SimulateFixedAttribs(
   if (gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2)
     return true;
 
-  if (!vertex_attrib_manager_.HaveFixedAttribs()) {
+  if (!vertex_attrib_manager_->HaveFixedAttribs()) {
     return true;
   }
 
@@ -4959,7 +4961,7 @@ bool GLES2DecoderImpl::SimulateFixedAttribs(
 
   GLuint elements_needed = 0;
   const VertexAttribManager::VertexAttribInfoList& infos =
-      vertex_attrib_manager_.GetEnabledVertexAttribInfos();
+      vertex_attrib_manager_->GetEnabledVertexAttribInfos();
   for (VertexAttribManager::VertexAttribInfoList::const_iterator it =
        infos.begin(); it != infos.end(); ++it) {
     const VertexAttribManager::VertexAttribInfo* info = *it;
@@ -5485,7 +5487,7 @@ void GLES2DecoderImpl::DoValidateProgram(GLuint program_client_id) {
 void GLES2DecoderImpl::DoGetVertexAttribfv(
     GLuint index, GLenum pname, GLfloat* params) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glGetVertexAttribfv: index out of range");
     return;
@@ -5530,7 +5532,7 @@ void GLES2DecoderImpl::DoGetVertexAttribfv(
 void GLES2DecoderImpl::DoGetVertexAttribiv(
     GLuint index, GLenum pname, GLint* params) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glGetVertexAttribiv: index out of range");
     return;
@@ -5574,7 +5576,7 @@ void GLES2DecoderImpl::DoGetVertexAttribiv(
 
 void GLES2DecoderImpl::DoVertexAttrib1f(GLuint index, GLfloat v0) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib1f: index out of range");
     return;
@@ -5590,7 +5592,7 @@ void GLES2DecoderImpl::DoVertexAttrib1f(GLuint index, GLfloat v0) {
 
 void GLES2DecoderImpl::DoVertexAttrib2f(GLuint index, GLfloat v0, GLfloat v1) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib2f: index out of range");
     return;
@@ -5607,7 +5609,7 @@ void GLES2DecoderImpl::DoVertexAttrib2f(GLuint index, GLfloat v0, GLfloat v1) {
 void GLES2DecoderImpl::DoVertexAttrib3f(
     GLuint index, GLfloat v0, GLfloat v1, GLfloat v2) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib3f: index out of range");
     return;
@@ -5624,7 +5626,7 @@ void GLES2DecoderImpl::DoVertexAttrib3f(
 void GLES2DecoderImpl::DoVertexAttrib4f(
     GLuint index, GLfloat v0, GLfloat v1, GLfloat v2, GLfloat v3) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib4f: index out of range");
     return;
@@ -5640,7 +5642,7 @@ void GLES2DecoderImpl::DoVertexAttrib4f(
 
 void GLES2DecoderImpl::DoVertexAttrib1fv(GLuint index, const GLfloat* v) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib1fv: index out of range");
     return;
@@ -5656,7 +5658,7 @@ void GLES2DecoderImpl::DoVertexAttrib1fv(GLuint index, const GLfloat* v) {
 
 void GLES2DecoderImpl::DoVertexAttrib2fv(GLuint index, const GLfloat* v) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib2fv: index out of range");
     return;
@@ -5672,7 +5674,7 @@ void GLES2DecoderImpl::DoVertexAttrib2fv(GLuint index, const GLfloat* v) {
 
 void GLES2DecoderImpl::DoVertexAttrib3fv(GLuint index, const GLfloat* v) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib3fv: index out of range");
     return;
@@ -5688,7 +5690,7 @@ void GLES2DecoderImpl::DoVertexAttrib3fv(GLuint index, const GLfloat* v) {
 
 void GLES2DecoderImpl::DoVertexAttrib4fv(GLuint index, const GLfloat* v) {
   VertexAttribManager::VertexAttribInfo* info =
-      vertex_attrib_manager_.GetVertexAttribInfo(index);
+      vertex_attrib_manager_->GetVertexAttribInfo(index);
   if (!info) {
     SetGLError(GL_INVALID_VALUE, "glVertexAttrib4fv: index out of range");
     return;
@@ -5758,7 +5760,7 @@ error::Error GLES2DecoderImpl::HandleVertexAttribPointer(
                "glVertexAttribPointer: stride not valid for type");
     return error::kNoError;
   }
-  vertex_attrib_manager_.SetAttribInfo(
+  vertex_attrib_manager_->SetAttribInfo(
       indx,
       bound_array_buffer_,
       size,
@@ -7057,7 +7059,7 @@ error::Error GLES2DecoderImpl::HandleGetVertexAttribPointerv(
   }
   result->SetNumResults(1);
   *result->GetData() =
-      vertex_attrib_manager_.GetVertexAttribInfo(index)->offset();
+      vertex_attrib_manager_->GetVertexAttribInfo(index)->offset();
   return error::kNoError;
 }
 
diff --git a/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc b/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc
index ab51f83..74ed4de 100644
--- a/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc
+++ b/gpu/command_buffer/service/vertex_attrib_manager_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -31,17 +31,19 @@ class VertexAttribManagerTest : public testing::Test {
     gl_.reset(new ::testing::StrictMock< ::gfx::MockGLInterface>());
     ::gfx::GLInterface::SetGLInterface(gl_.get());
 
-    manager_.Initialize(kNumVertexAttribs);
+    manager_.reset(new VertexAttribManager());
+    manager_->Initialize(kNumVertexAttribs);
   }
 
   virtual void TearDown() {
+    manager_.reset();
     ::gfx::GLInterface::SetGLInterface(NULL);
     gl_.reset();
   }
 
   // Use StrictMock to make 100% sure we know how GL will be called.
   scoped_ptr< ::testing::StrictMock< ::gfx::MockGLInterface> > gl_;
-  VertexAttribManager manager_;
+  scoped_ptr<VertexAttribManager> manager_;
 };
 
 // GCC requires these declarations, but MSVC requires they not be present
@@ -50,16 +52,16 @@ const uint32 VertexAttribManagerTest::kNumVertexAttribs;
 #endif
 
 TEST_F(VertexAttribManagerTest, Basic) {
-  EXPECT_TRUE(manager_.GetVertexAttribInfo(kNumVertexAttribs) == NULL);
-  EXPECT_FALSE(manager_.HaveFixedAttribs());
+  EXPECT_TRUE(manager_->GetVertexAttribInfo(kNumVertexAttribs) == NULL);
+  EXPECT_FALSE(manager_->HaveFixedAttribs());
 
   const VertexAttribManager::VertexAttribInfoList& infos =
-      manager_.GetEnabledVertexAttribInfos();
+      manager_->GetEnabledVertexAttribInfos();
   EXPECT_EQ(0u, infos.size());
 
   for (uint32 ii = 0; ii < kNumVertexAttribs; ii += kNumVertexAttribs - 1) {
     VertexAttribManager::VertexAttribInfo* info =
-        manager_.GetVertexAttribInfo(ii);
+        manager_->GetVertexAttribInfo(ii);
     ASSERT_TRUE(info != NULL);
     EXPECT_EQ(ii, info->index());
     EXPECT_TRUE(info->buffer() == NULL);
@@ -73,32 +75,32 @@ TEST_F(VertexAttribManagerTest, Basic) {
     EXPECT_EQ(0.0f, info->value().v[1]);
     EXPECT_EQ(0.0f, info->value().v[2]);
     EXPECT_EQ(1.0f, info->value().v[3]);
-    manager_.Enable(ii, true);
+    manager_->Enable(ii, true);
     EXPECT_TRUE(info->enabled());
   }
 }
 
 TEST_F(VertexAttribManagerTest, Enable) {
   const VertexAttribManager::VertexAttribInfoList& infos =
-      manager_.GetEnabledVertexAttribInfos();
+      manager_->GetEnabledVertexAttribInfos();
 
   VertexAttribManager::VertexAttribInfo* info1 =
-      manager_.GetVertexAttribInfo(1);
+      manager_->GetVertexAttribInfo(1);
   VertexAttribManager::VertexAttribInfo* info2 =
-      manager_.GetVertexAttribInfo(3);
+      manager_->GetVertexAttribInfo(3);
 
-  manager_.Enable(1, true);
+  manager_->Enable(1, true);
   ASSERT_EQ(1u, infos.size());
   EXPECT_TRUE(info1->enabled());
-  manager_.Enable(3, true);
+  manager_->Enable(3, true);
   ASSERT_EQ(2u, infos.size());
   EXPECT_TRUE(info2->enabled());
 
-  manager_.Enable(1, false);
+  manager_->Enable(1, false);
   ASSERT_EQ(1u, infos.size());
   EXPECT_FALSE(info1->enabled());
 
-  manager_.Enable(3, false);
+  manager_->Enable(3, false);
   ASSERT_EQ(0u, infos.size());
   EXPECT_FALSE(info2->enabled());
 }
@@ -110,9 +112,9 @@ TEST_F(VertexAttribManagerTest, SetAttribInfo) {
   ASSERT_TRUE(buffer != NULL);
 
   VertexAttribManager::VertexAttribInfo* info =
-      manager_.GetVertexAttribInfo(1);
+      manager_->GetVertexAttribInfo(1);
 
-  manager_.SetAttribInfo(1, buffer, 3, GL_SHORT, GL_TRUE, 32, 32, 4);
+  manager_->SetAttribInfo(1, buffer, 3, GL_SHORT, GL_TRUE, 32, 32, 4);
 
   EXPECT_EQ(buffer, info->buffer());
   EXPECT_EQ(4, info->offset());
@@ -120,19 +122,23 @@ TEST_F(VertexAttribManagerTest, SetAttribInfo) {
   EXPECT_EQ(static_cast<GLenum>(GL_SHORT), info->type());
   EXPECT_EQ(GL_TRUE, info->normalized());
   EXPECT_EQ(32, info->gl_stride());
+
+  // The VertexAttribManager must be destroyed before the BufferManager
+  // so it releases its buffers.
+  manager_.reset();
   buffer_manager.Destroy(false);
 }
 
 TEST_F(VertexAttribManagerTest, HaveFixedAttribs) {
-  EXPECT_FALSE(manager_.HaveFixedAttribs());
-  manager_.SetAttribInfo(1, NULL, 4, GL_FIXED, GL_FALSE, 0, 16, 0);
-  EXPECT_TRUE(manager_.HaveFixedAttribs());
-  manager_.SetAttribInfo(3, NULL, 4, GL_FIXED, GL_FALSE, 0, 16, 0);
-  EXPECT_TRUE(manager_.HaveFixedAttribs());
-  manager_.SetAttribInfo(1, NULL, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
-  EXPECT_TRUE(manager_.HaveFixedAttribs());
-  manager_.SetAttribInfo(3, NULL, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
-  EXPECT_FALSE(manager_.HaveFixedAttribs());
+  EXPECT_FALSE(manager_->HaveFixedAttribs());
+  manager_->SetAttribInfo(1, NULL, 4, GL_FIXED, GL_FALSE, 0, 16, 0);
+  EXPECT_TRUE(manager_->HaveFixedAttribs());
+  manager_->SetAttribInfo(3, NULL, 4, GL_FIXED, GL_FALSE, 0, 16, 0);
+  EXPECT_TRUE(manager_->HaveFixedAttribs());
+  manager_->SetAttribInfo(1, NULL, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
+  EXPECT_TRUE(manager_->HaveFixedAttribs());
+  manager_->SetAttribInfo(3, NULL, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
+  EXPECT_FALSE(manager_->HaveFixedAttribs());
 }
 
 TEST_F(VertexAttribManagerTest, CanAccess) {
@@ -142,13 +148,13 @@ TEST_F(VertexAttribManagerTest, CanAccess) {
   ASSERT_TRUE(buffer != NULL);
 
   VertexAttribManager::VertexAttribInfo* info =
-      manager_.GetVertexAttribInfo(1);
+      manager_->GetVertexAttribInfo(1);
 
   EXPECT_TRUE(info->CanAccess(0));
-  manager_.Enable(1, true);
+  manager_->Enable(1, true);
   EXPECT_FALSE(info->CanAccess(0));
 
-  manager_.SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
+  manager_->SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
   EXPECT_FALSE(info->CanAccess(0));
 
   EXPECT_TRUE(buffer_manager.SetTarget(buffer, GL_ARRAY_BUFFER));
@@ -159,18 +165,21 @@ TEST_F(VertexAttribManagerTest, CanAccess) {
   EXPECT_TRUE(info->CanAccess(0));
   EXPECT_FALSE(info->CanAccess(1));
 
-  manager_.SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 16, 1);
+  manager_->SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 16, 1);
   EXPECT_FALSE(info->CanAccess(0));
 
   buffer_manager.SetInfo(buffer, 32, GL_STATIC_DRAW);
   EXPECT_TRUE(info->CanAccess(0));
   EXPECT_FALSE(info->CanAccess(1));
-  manager_.SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
+  manager_->SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 16, 0);
   EXPECT_TRUE(info->CanAccess(1));
-  manager_.SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 20, 0);
+  manager_->SetAttribInfo(1, buffer, 4, GL_FLOAT, GL_FALSE, 0, 20, 0);
   EXPECT_TRUE(info->CanAccess(0));
   EXPECT_FALSE(info->CanAccess(1));
 
+  // The VertexAttribManager must be destroyed before the BufferManager
+  // so it releases its buffers.
+  manager_.reset();
   buffer_manager.Destroy(false);
 }
 
@@ -184,27 +193,30 @@ TEST_F(VertexAttribManagerTest, Unbind) {
   ASSERT_TRUE(buffer2 != NULL);
 
   VertexAttribManager::VertexAttribInfo* info1 =
-      manager_.GetVertexAttribInfo(1);
+      manager_->GetVertexAttribInfo(1);
   VertexAttribManager::VertexAttribInfo* info3 =
-      manager_.GetVertexAttribInfo(3);
+      manager_->GetVertexAttribInfo(3);
 
   // Attach to 2 buffers.
-  manager_.SetAttribInfo(1, buffer1, 3, GL_SHORT, GL_TRUE, 32, 32, 4);
-  manager_.SetAttribInfo(3, buffer1, 3, GL_SHORT, GL_TRUE, 32, 32, 4);
+  manager_->SetAttribInfo(1, buffer1, 3, GL_SHORT, GL_TRUE, 32, 32, 4);
+  manager_->SetAttribInfo(3, buffer1, 3, GL_SHORT, GL_TRUE, 32, 32, 4);
   // Check they were attached.
   EXPECT_EQ(buffer1, info1->buffer());
   EXPECT_EQ(buffer1, info3->buffer());
   // Unbind unattached buffer.
-  manager_.Unbind(buffer2);
+  manager_->Unbind(buffer2);
   // Should be no-op.
   EXPECT_EQ(buffer1, info1->buffer());
   EXPECT_EQ(buffer1, info3->buffer());
   // Unbind buffer.
-  manager_.Unbind(buffer1);
+  manager_->Unbind(buffer1);
   // Check they were detached
   EXPECT_TRUE(NULL == info1->buffer());
   EXPECT_TRUE(NULL == info3->buffer());
 
+  // The VertexAttribManager must be destroyed before the BufferManager
+  // so it releases its buffers.
+  manager_.reset();
   buffer_manager.Destroy(false);
 }