Fix RenderbufferManager to handle deleted objects

TEST=unit tests
BUG=110198

R=apatrick@chromium.org


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

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

3 files changed, 48 insertions, 13 deletions

diff --git a/gpu/command_buffer/service/renderbuffer_manager.cc b/gpu/command_buffer/service/renderbuffer_manager.cc
index 225ac26..017f2b5 100644
--- a/gpu/command_buffer/service/renderbuffer_manager.cc
+++ b/gpu/command_buffer/service/renderbuffer_manager.cc
@@ -29,6 +29,13 @@ size_t RenderbufferManager::RenderbufferInfo::EstimatedSize() {
          GLES2Util::RenderbufferBytesPerPixel(internal_format_);
 }
 
+RenderbufferManager::RenderbufferInfo::~RenderbufferInfo() {
+  if (manager_) {
+    manager_->StopTracking(this);
+    manager_ = NULL;
+  }
+}
+
 void RenderbufferManager::UpdateMemRepresented() {
   TRACE_COUNTER_ID1(
       "RenderbufferManager", "RenderbufferMemory", this, mem_represented_);
@@ -37,7 +44,6 @@ void RenderbufferManager::UpdateMemRepresented() {
 void RenderbufferManager::Destroy(bool have_context) {
   while (!renderbuffer_infos_.empty()) {
     RenderbufferInfo* info = renderbuffer_infos_.begin()->second;
-    mem_represented_ -= info->EstimatedSize();
     if (have_context) {
       if (!info->IsDeleted()) {
         GLuint service_id = info->service_id();
@@ -51,9 +57,16 @@ void RenderbufferManager::Destroy(bool have_context) {
   UpdateMemRepresented();
 }
 
+void RenderbufferManager::StopTracking(RenderbufferInfo* renderbuffer) {
+  if (!renderbuffer->cleared()) {
+    --num_uncleared_renderbuffers_;
+  }
+  mem_represented_ -= renderbuffer->EstimatedSize();
+}
+
 void RenderbufferManager::SetInfo(
-  RenderbufferInfo* renderbuffer,
-  GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height) {
+    RenderbufferInfo* renderbuffer,
+    GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height) {
   DCHECK(renderbuffer);
   if (!renderbuffer->cleared()) {
     --num_uncleared_renderbuffers_;
@@ -80,7 +93,7 @@ void RenderbufferManager::SetCleared(RenderbufferInfo* renderbuffer) {
 
 void RenderbufferManager::CreateRenderbufferInfo(
     GLuint client_id, GLuint service_id) {
-  RenderbufferInfo::Ref info(new RenderbufferInfo(service_id));
+  RenderbufferInfo::Ref info(new RenderbufferInfo(this, service_id));
   std::pair<RenderbufferInfoMap::iterator, bool> result =
       renderbuffer_infos_.insert(std::make_pair(client_id, info));
   DCHECK(result.second);
@@ -99,11 +112,6 @@ void RenderbufferManager::RemoveRenderbufferInfo(GLuint client_id) {
   RenderbufferInfoMap::iterator it = renderbuffer_infos_.find(client_id);
   if (it != renderbuffer_infos_.end()) {
     RenderbufferInfo* info = it->second;
-    if (!info->cleared()) {
-      --num_uncleared_renderbuffers_;
-    }
-    mem_represented_ -= info->EstimatedSize();
-    UpdateMemRepresented();
     info->MarkAsDeleted();
     renderbuffer_infos_.erase(it);
   }
diff --git a/gpu/command_buffer/service/renderbuffer_manager.h b/gpu/command_buffer/service/renderbuffer_manager.h
index cae19f3..8cfd993 100644
--- a/gpu/command_buffer/service/renderbuffer_manager.h
+++ b/gpu/command_buffer/service/renderbuffer_manager.h
@@ -23,8 +23,9 @@ class RenderbufferManager {
    public:
     typedef scoped_refptr<RenderbufferInfo> Ref;
 
-    explicit RenderbufferInfo(GLuint service_id)
-        : service_id_(service_id),
+    RenderbufferInfo(RenderbufferManager* manager, GLuint service_id)
+        : manager_(manager),
+          service_id_(service_id),
           cleared_(true),
           has_been_bound_(false),
           samples_(0),
@@ -75,7 +76,7 @@ class RenderbufferManager {
     friend class RenderbufferManager;
     friend class base::RefCounted<RenderbufferInfo>;
 
-    ~RenderbufferInfo() { }
+    ~RenderbufferInfo();
 
     void set_cleared() {
       cleared_ = true;
@@ -94,6 +95,9 @@ class RenderbufferManager {
       service_id_ = 0;
     }
 
+    // RenderbufferManager that owns this RenderbufferInfo.
+    RenderbufferManager* manager_;
+
     // Service side renderbuffer id.
     GLuint service_id_;
 
@@ -153,6 +157,8 @@ class RenderbufferManager {
  private:
   void UpdateMemRepresented();
 
+  void StopTracking(RenderbufferInfo* renderbuffer);
+
   GLint max_renderbuffer_size_;
   GLint max_samples_;
 
diff --git a/gpu/command_buffer/service/renderbuffer_manager_unittest.cc b/gpu/command_buffer/service/renderbuffer_manager_unittest.cc
index 0384785..6f34c750 100644
--- a/gpu/command_buffer/service/renderbuffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/renderbuffer_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.
 
@@ -131,6 +131,27 @@ TEST_F(RenderbufferManagerTest, RenderbufferInfo) {
   EXPECT_FALSE(manager_.HaveUnclearedRenderbuffers());
 }
 
+TEST_F(RenderbufferManagerTest, UseDeletedRenderbufferInfo) {
+  const GLuint kClient1Id = 1;
+  const GLuint kService1Id = 11;
+  manager_.CreateRenderbufferInfo(kClient1Id, kService1Id);
+  RenderbufferManager::RenderbufferInfo::Ref info1(
+      manager_.GetRenderbufferInfo(kClient1Id));
+  ASSERT_TRUE(info1 != NULL);
+  // Remove it.
+  manager_.RemoveRenderbufferInfo(kClient1Id);
+  // Use after removing.
+  const GLsizei kSamples = 4;
+  const GLenum kFormat = GL_RGBA4;
+  const GLsizei kWidth = 128;
+  const GLsizei kHeight = 64;
+  manager_.SetInfo(info1, kSamples, kFormat, kWidth, kHeight);
+  // See that it still affects manager.
+  EXPECT_TRUE(manager_.HaveUnclearedRenderbuffers());
+  manager_.SetCleared(info1);
+  EXPECT_FALSE(manager_.HaveUnclearedRenderbuffers());
+}
+
 }  // namespace gles2
 }  // namespace gpu