Make buffer not get deleted until the last reference is released.

TEST=unit tests
BUG=122007

R=apatrick@chromium.org


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

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

4 files changed, 40 insertions, 19 deletions

diff --git a/gpu/command_buffer/service/buffer_manager.cc b/gpu/command_buffer/service/buffer_manager.cc
index 0ab46c9..864027d 100644
--- a/gpu/command_buffer/service/buffer_manager.cc
+++ b/gpu/command_buffer/service/buffer_manager.cc
@@ -14,26 +14,20 @@ namespace gles2 {
 BufferManager::BufferManager()
     : allow_buffers_on_multiple_targets_(false),
       mem_represented_(0),
-      last_reported_mem_represented_(1) {
+      last_reported_mem_represented_(1),
+      buffer_info_count_(0),
+      have_context_(true) {
   UpdateMemRepresented();
 }
 
 BufferManager::~BufferManager() {
   DCHECK(buffer_infos_.empty());
+  CHECK_EQ(buffer_info_count_, 0u);
 }
 
 void BufferManager::Destroy(bool have_context) {
-  while (!buffer_infos_.empty()) {
-    BufferInfo* info = buffer_infos_.begin()->second;
-    if (have_context) {
-      if (!info->IsDeleted()) {
-        GLuint service_id = info->service_id();
-        glDeleteBuffersARB(1, &service_id);
-        info->MarkAsDeleted();
-      }
-    }
-    buffer_infos_.erase(buffer_infos_.begin());
-  }
+  have_context_ = have_context;
+  buffer_infos_.clear();
   DCHECK_EQ(0u, mem_represented_);
   UpdateMemRepresented();
 }
@@ -47,10 +41,9 @@ void BufferManager::UpdateMemRepresented() {
 }
 
 void BufferManager::CreateBufferInfo(GLuint client_id, GLuint service_id) {
+  BufferInfo::Ref buffer(new BufferInfo(this, service_id));
   std::pair<BufferInfoMap::iterator, bool> result =
-      buffer_infos_.insert(
-          std::make_pair(client_id,
-                         BufferInfo::Ref(new BufferInfo(this, service_id))));
+      buffer_infos_.insert(std::make_pair(client_id, buffer));
   DCHECK(result.second);
 }
 
@@ -69,22 +62,33 @@ void BufferManager::RemoveBufferInfo(GLuint client_id) {
   }
 }
 
+void BufferManager::StartTracking(BufferManager::BufferInfo* /* buffer */) {
+  ++buffer_info_count_;
+}
+
 void BufferManager::StopTracking(BufferManager::BufferInfo* buffer) {
   mem_represented_ -= buffer->size();
+  --buffer_info_count_;
   UpdateMemRepresented();
 }
 
 BufferManager::BufferInfo::BufferInfo(BufferManager* manager, GLuint service_id)
     : manager_(manager),
+      deleted_(false),
       service_id_(service_id),
       target_(0),
       size_(0),
       usage_(GL_STATIC_DRAW),
       shadowed_(false) {
+  manager_->StartTracking(this);
 }
 
 BufferManager::BufferInfo::~BufferInfo() {
   if (manager_) {
+    if (manager_->have_context_) {
+      GLuint id = service_id();
+      glDeleteBuffersARB(1, &id);
+    }
     manager_->StopTracking(this);
     manager_ = NULL;
   }
diff --git a/gpu/command_buffer/service/buffer_manager.h b/gpu/command_buffer/service/buffer_manager.h
index f2db2f9..b3b4ede 100644
--- a/gpu/command_buffer/service/buffer_manager.h
+++ b/gpu/command_buffer/service/buffer_manager.h
@@ -59,7 +59,7 @@ class GPU_EXPORT BufferManager {
     const void* GetRange(GLintptr offset, GLsizeiptr size) const;
 
     bool IsDeleted() const {
-      return service_id_ == 0;
+      return deleted_;
     }
 
     bool IsValid() const {
@@ -115,7 +115,7 @@ class GPU_EXPORT BufferManager {
     }
 
     void MarkAsDeleted() {
-      service_id_ = 0;
+      deleted_ = true;
     }
 
     void SetInfo(GLsizeiptr size, GLenum usage, bool shadow);
@@ -126,6 +126,9 @@ class GPU_EXPORT BufferManager {
     // The manager that owns this BufferInfo.
     BufferManager* manager_;
 
+    // True if deleted.
+    bool deleted_;
+
     // Service side buffer id.
     GLuint service_id_;
 
@@ -183,6 +186,7 @@ class GPU_EXPORT BufferManager {
  private:
   void UpdateMemRepresented();
 
+  void StartTracking(BufferInfo* info);
   void StopTracking(BufferInfo* info);
 
   // Info for each buffer in the system.
@@ -195,6 +199,12 @@ class GPU_EXPORT BufferManager {
   size_t mem_represented_;
   size_t last_reported_mem_represented_;
 
+  // Counts the number of BufferInfo allocated with 'this' as its manager.
+  // Allows to check no BufferInfo will outlive this.
+  unsigned int buffer_info_count_;
+
+  bool have_context_;
+
   DISALLOW_COPY_AND_ASSIGN(BufferManager);
 };
 
diff --git a/gpu/command_buffer/service/buffer_manager_unittest.cc b/gpu/command_buffer/service/buffer_manager_unittest.cc
index ec46915..7a84b05 100644
--- a/gpu/command_buffer/service/buffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/buffer_manager_unittest.cc
@@ -65,6 +65,10 @@ TEST_F(BufferManagerTest, Basic) {
   EXPECT_TRUE(manager_.GetBufferInfo(kClientBuffer2Id) == NULL);
   // Check trying to a remove non-existent buffers does not crash.
   manager_.RemoveBufferInfo(kClientBuffer2Id);
+  // Check that it gets deleted when the last reference is released.
+  EXPECT_CALL(*gl_, DeleteBuffersARB(1, ::testing::Pointee(kServiceBuffer1Id)))
+      .Times(1)
+      .RetiresOnSaturation();
   // Check we can't get the buffer after we remove it.
   manager_.RemoveBufferInfo(kClientBuffer1Id);
   EXPECT_TRUE(manager_.GetBufferInfo(kClientBuffer1Id) == NULL);
@@ -233,6 +237,11 @@ TEST_F(BufferManagerTest, UseDeletedBuffer) {
   manager_.RemoveBufferInfo(kClientBufferId);
   // Use it after removing
   manager_.SetInfo(info, sizeof(data), GL_STATIC_DRAW);
+  // Check that it gets deleted when the last reference is released.
+  EXPECT_CALL(*gl_, DeleteBuffersARB(1, ::testing::Pointee(kServiceBufferId)))
+      .Times(1)
+      .RetiresOnSaturation();
+  info = NULL;
 }
 
 }  // namespace gles2
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 90fc797..ff703ed 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2325,8 +2325,6 @@ void GLES2DecoderImpl::DeleteBuffersHelper(
       if (bound_element_array_buffer_ == buffer) {
         bound_element_array_buffer_ = NULL;
       }
-      GLuint service_id = buffer->service_id();
-      glDeleteBuffersARB(1, &service_id);
       RemoveBufferInfo(client_ids[ii]);
     }
   }