Fix use-after-free in ~TextureInfo

In some paths, the TextureInfo (refcounted) may outlive the TextureManager,
accessing a stale pointer in its destructor. This fixes it.

BUG=chromium-os:25634
TEST=pepper flash, reload videos many times.


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

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

3 files changed, 33 insertions, 0 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 34c6916..c440108 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1519,6 +1519,9 @@ class GLES2DecoderImpl : public base::SupportsWeakPtr<GLES2DecoderImpl>,
   TextureToIOSurfaceMap texture_to_io_surface_map_;
 #endif
 
+  typedef std::vector<GLES2DecoderImpl*> ChildList;
+  ChildList children_;
+
   DISALLOW_COPY_AND_ASSIGN(GLES2DecoderImpl);
 };
 
@@ -2699,6 +2702,10 @@ bool GLES2DecoderImpl::GetServiceTextureId(uint32 client_texture_id,
 void GLES2DecoderImpl::Destroy() {
   bool have_context = context_.get() && MakeCurrent();
 
+  ChildList children = children_;
+  for (ChildList::iterator it = children.begin(); it != children.end(); ++it)
+    (*it)->SetParent(NULL, 0);
+  DCHECK(children_.empty());
   SetParent(NULL, 0);
 
   // Unbind everything.
@@ -2801,6 +2808,12 @@ bool GLES2DecoderImpl::SetParent(GLES2Decoder* new_parent,
   // parent pointer is a weak pointer so it will be null if the parent has
   // already been destroyed.
   if (parent_) {
+    ChildList::iterator it = std::find(
+        parent_->children_.begin(),
+        parent_->children_.end(),
+        this);
+    DCHECK(it != parent_->children_.end());
+    parent_->children_.erase(it);
     // First check the texture has been mapped into the parent. This might not
     // be the case if initialization failed midway through.
     GLuint service_id = offscreen_saved_color_texture_->id();
@@ -2813,6 +2826,14 @@ bool GLES2DecoderImpl::SetParent(GLES2Decoder* new_parent,
   GLES2DecoderImpl* new_parent_impl = static_cast<GLES2DecoderImpl*>(
       new_parent);
   if (new_parent_impl) {
+#ifndef NDEBUG
+    ChildList::iterator it = std::find(
+        new_parent_impl->children_.begin(),
+        new_parent_impl->children_.end(),
+        this);
+    DCHECK(it == new_parent_impl->children_.end());
+#endif
+    new_parent_impl->children_.push_back(this);
     // Map the ID of the saved offscreen texture into the parent so that
     // it can reference it.
     GLuint service_id = offscreen_saved_color_texture_->id();
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index cb04dd0..652d313 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -58,6 +58,10 @@ static size_t FaceIndexToGLTarget(size_t index) {
 
 TextureManager::~TextureManager() {
   DCHECK(texture_infos_.empty());
+
+  // If this triggers, that means something is keeping a reference to
+  // a TextureInfo belonging to this.
+  CHECK_EQ(texture_info_count_, 0u);
 }
 
 void TextureManager::Destroy(bool have_context) {
@@ -91,6 +95,7 @@ void TextureManager::Destroy(bool have_context) {
 TextureManager::TextureInfo::~TextureInfo() {
   if (manager_) {
     manager_->StopTracking(this);
+    --manager_->texture_info_count_;
     manager_ = NULL;
   }
 }
@@ -565,6 +570,7 @@ TextureManager::TextureManager(
       num_unrenderable_textures_(0),
       num_unsafe_textures_(0),
       num_uncleared_mips_(0),
+      texture_info_count_(0),
       mem_represented_(0),
       last_reported_mem_represented_(1) {
   for (int ii = 0; ii < kNumDefaultTextures; ++ii) {
diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h
index 81801e1..3fdc258 100644
--- a/gpu/command_buffer/service/texture_manager.h
+++ b/gpu/command_buffer/service/texture_manager.h
@@ -60,6 +60,8 @@ class TextureManager {
           stream_texture_(false),
           immutable_(false),
           estimated_size_(0) {
+      if (manager_)
+        ++manager_->texture_info_count_;
     }
 
     GLenum min_filter() const {
@@ -533,6 +535,10 @@ class TextureManager {
   int num_unsafe_textures_;
   int num_uncleared_mips_;
 
+  // Counts the number of TextureInfo allocated with 'this' as its manager.
+  // Allows to check no TextureInfo will outlive this.
+  unsigned int texture_info_count_;
+
   uint32 mem_represented_;
   uint32 last_reported_mem_represented_;