Delete framebuffers when no longer referenced.

TEST=unit tests
BUG=122007


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

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

4 files changed, 56 insertions, 21 deletions

diff --git a/gpu/command_buffer/service/framebuffer_manager.cc b/gpu/command_buffer/service/framebuffer_manager.cc
index a8f5a57..4680a37 100644
--- a/gpu/command_buffer/service/framebuffer_manager.cc
+++ b/gpu/command_buffer/service/framebuffer_manager.cc
@@ -168,15 +168,20 @@ class TextureAttachment
 };
 
 FramebufferManager::FramebufferManager()
-    : framebuffer_state_change_count_(1) {
+    : framebuffer_state_change_count_(1),
+      framebuffer_info_count_(0),
+      have_context_(true) {
 }
 
 FramebufferManager::~FramebufferManager() {
   DCHECK(framebuffer_infos_.empty());
+  // If this triggers, that means something is keeping a reference to a
+  // FramebufferInfo belonging to this.
+  CHECK_EQ(framebuffer_info_count_, 0u);
 }
 
 void FramebufferManager::FramebufferInfo::MarkAsDeleted() {
-  service_id_ = 0;
+  deleted_ = true;
   while (!attachments_.empty()) {
     Attachment* attachment = attachments_.begin()->second.get();
     attachment->DetachFromFramebuffer();
@@ -185,17 +190,18 @@ void FramebufferManager::FramebufferInfo::MarkAsDeleted() {
 }
 
 void FramebufferManager::Destroy(bool have_context) {
-  while (!framebuffer_infos_.empty()) {
-    FramebufferInfo* info = framebuffer_infos_.begin()->second;
-    if (!info->IsDeleted()) {
-      if (have_context) {
-        GLuint service_id = info->service_id();
-        glDeleteFramebuffersEXT(1, &service_id);
-      }
-      info->MarkAsDeleted();
-    }
-    framebuffer_infos_.erase(framebuffer_infos_.begin());
-  }
+  have_context_ = have_context;
+  framebuffer_infos_.clear();
+}
+
+void FramebufferManager::StartTracking(
+    FramebufferManager::FramebufferInfo* /* framebuffer */) {
+  ++framebuffer_info_count_;
+}
+
+void FramebufferManager::StopTracking(
+    FramebufferManager::FramebufferInfo* /* framebuffer */) {
+  --framebuffer_info_count_;
 }
 
 void FramebufferManager::CreateFramebufferInfo(
@@ -204,17 +210,30 @@ void FramebufferManager::CreateFramebufferInfo(
       framebuffer_infos_.insert(
           std::make_pair(
               client_id,
-              FramebufferInfo::Ref(new FramebufferInfo(service_id))));
+              FramebufferInfo::Ref(new FramebufferInfo(this, service_id))));
   DCHECK(result.second);
 }
 
-FramebufferManager::FramebufferInfo::FramebufferInfo(GLuint service_id)
-    : service_id_(service_id),
+FramebufferManager::FramebufferInfo::FramebufferInfo(
+    FramebufferManager* manager, GLuint service_id)
+    : manager_(manager),
+      deleted_(false),
+      service_id_(service_id),
       has_been_bound_(false),
       framebuffer_complete_state_count_id_(0) {
+  manager->StartTracking(this);
 }
 
-FramebufferManager::FramebufferInfo::~FramebufferInfo() {}
+FramebufferManager::FramebufferInfo::~FramebufferInfo() {
+  if (manager_) {
+    if (manager_->have_context_) {
+      GLuint id = service_id();
+      glDeleteFramebuffersEXT(1, &id);
+    }
+    manager_->StopTracking(this);
+    manager_ = NULL;
+  }
+}
 
 bool FramebufferManager::FramebufferInfo::HasUnclearedAttachment(
     GLenum attachment) const {
diff --git a/gpu/command_buffer/service/framebuffer_manager.h b/gpu/command_buffer/service/framebuffer_manager.h
index a21fa8e..d2dcee5 100644
--- a/gpu/command_buffer/service/framebuffer_manager.h
+++ b/gpu/command_buffer/service/framebuffer_manager.h
@@ -47,7 +47,7 @@ class GPU_EXPORT FramebufferManager {
       virtual bool ValidForAttachmentType(GLenum attachment_type) = 0;
     };
 
-    explicit FramebufferInfo(GLuint service_id);
+    FramebufferInfo(FramebufferManager* manager, GLuint service_id);
 
     GLuint service_id() const {
       return service_id_;
@@ -76,7 +76,7 @@ class GPU_EXPORT FramebufferManager {
     const Attachment* GetAttachment(GLenum attachment) const;
 
     bool IsDeleted() const {
-      return service_id_ == 0;
+      return deleted_;
     }
 
     void MarkAsValid() {
@@ -123,6 +123,11 @@ class GPU_EXPORT FramebufferManager {
       return framebuffer_complete_state_count_id_;
     }
 
+    // The managers that owns this.
+    FramebufferManager* manager_;
+
+    bool deleted_;
+
     // Service side framebuffer id.
     GLuint service_id_;
 
@@ -173,6 +178,9 @@ class GPU_EXPORT FramebufferManager {
   }
 
  private:
+  void StartTracking(FramebufferInfo* info);
+  void StopTracking(FramebufferInfo* info);
+
   // Info for each framebuffer in the system.
   typedef base::hash_map<GLuint, FramebufferInfo::Ref> FramebufferInfoMap;
   FramebufferInfoMap framebuffer_infos_;
@@ -181,6 +189,12 @@ class GPU_EXPORT FramebufferManager {
   // state.
   unsigned framebuffer_state_change_count_;
 
+  // Counts the number of FramebufferInfo allocated with 'this' as its manager.
+  // Allows to check no FramebufferInfo will outlive this.
+  unsigned int framebuffer_info_count_;
+
+  bool have_context_;
+
   DISALLOW_COPY_AND_ASSIGN(FramebufferManager);
 };
 
diff --git a/gpu/command_buffer/service/framebuffer_manager_unittest.cc b/gpu/command_buffer/service/framebuffer_manager_unittest.cc
index 6f361b3..de34126 100644
--- a/gpu/command_buffer/service/framebuffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/framebuffer_manager_unittest.cc
@@ -73,6 +73,10 @@ TEST_F(FramebufferManagerTest, Basic) {
   EXPECT_TRUE(manager_.GetFramebufferInfo(kClient2Id) == NULL);
   // Check trying to a remove non-existent framebuffers does not crash.
   manager_.RemoveFramebufferInfo(kClient2Id);
+  // Check framebuffer gets deleted when last reference is released.
+  EXPECT_CALL(*gl_, DeleteFramebuffersEXT(1, ::testing::Pointee(kService1Id)))
+      .Times(1)
+      .RetiresOnSaturation();
   // Check we can't get the framebuffer after we remove it.
   manager_.RemoveFramebufferInfo(kClient1Id);
   EXPECT_TRUE(manager_.GetFramebufferInfo(kClient1Id) == NULL);
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 3f23bb2..955ae6e 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2352,8 +2352,6 @@ void GLES2DecoderImpl::DeleteFramebuffersHelper(
             GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
         glBindFramebufferEXT(target, GetBackbufferServiceId());
       }
-      GLuint service_id = framebuffer->service_id();
-      glDeleteFramebuffersEXT(1, &service_id);
       RemoveFramebufferInfo(client_ids[ii]);
     }
   }