aura/texture transport: Keep ref to texture infos to avoid races with the UI

content/common/gpu/texture_image_transport_surface.cc

 // 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.
 
 #include "content/common/gpu/texture_image_transport_surface.h"
 
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/gpu_channel_manager.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "gpu/command_buffer/service/context_group.h"
@@ skipping 32 matching lines @@
   ~ScopedTextureBinder() {
     glBindTexture(GL_TEXTURE_2D, old_id_);
   }
 
  private:
   int old_id_;
 };
 
 }  // anonymous namespace
 
+TextureImageTransportSurface::Texture::Texture()
+    : client_id(0),
+      sent_to_client(false) {
+}
+
+TextureImageTransportSurface::Texture::~Texture() {
+}
+
 TextureImageTransportSurface::TextureImageTransportSurface(
     GpuChannelManager* manager,
     GpuCommandBufferStub* stub,
     const gfx::GLSurfaceHandle& handle)
       : fbo_id_(0),
         front_(0),
-        stub_destroyed_(false) {
+        stub_destroyed_(false),
+        parent_stub_(NULL) {
   GpuChannel* parent_channel = manager->LookupChannel(handle.parent_client_id);
   DCHECK(parent_channel);
-  GpuCommandBufferStub* parent_stub = parent_channel->LookupCommandBuffer(
-      handle.parent_context_id);
-  DCHECK(parent_stub);
-  parent_stub_ = parent_stub->AsWeakPtr();
+  parent_stub_ = parent_channel->LookupCommandBuffer(handle.parent_context_id);
+  DCHECK(parent_stub_);
+  parent_stub_->AddDestructionObserver(this);
   TextureManager* texture_manager =
       parent_stub_->decoder()->GetContextGroup()->texture_manager();
   DCHECK(texture_manager);
 
   for (int i = 0; i < 2; ++i) {
     Texture& texture = textures_[i];
     texture.client_id = handle.parent_texture_id[i];
-    TextureInfo* info = texture_manager->GetTextureInfo(texture.client_id);
-    DCHECK(info);
-    if (!info->target())
-      texture_manager->SetInfoTarget(info, GL_TEXTURE_2D);
-    texture_manager->SetParameter(info, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
-    texture_manager->SetParameter(info, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
-    texture_manager->SetParameter(info, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-    texture_manager->SetParameter(info, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    texture.info = texture_manager->GetTextureInfo(texture.client_id);
+    DCHECK(texture.info);
+    if (!texture.info->target())
+      texture_manager->SetInfoTarget(texture.info, GL_TEXTURE_2D);
+    texture_manager->SetParameter(
+        texture.info, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    texture_manager->SetParameter(
+        texture.info, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    texture_manager->SetParameter(
+        texture.info, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    texture_manager->SetParameter(
+        texture.info, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
   }
 
   helper_.reset(new ImageTransportHelper(this,
                                          manager,
                                          stub,
                                          gfx::kNullPluginWindow));
 
   stub->AddDestructionObserver(this);
 }
 
 TextureImageTransportSurface::~TextureImageTransportSurface() {
   DCHECK(stub_destroyed_);
   Destroy();
 }
 
 bool TextureImageTransportSurface::Initialize() {
   return helper_->Initialize();
 }
 
 void TextureImageTransportSurface::Destroy() {
+  if (parent_stub_) {
+    parent_stub_->RemoveDestructionObserver(this);
+    parent_stub_ = NULL;
+  }
   for (int i = 0; i < 2; ++i) {
     Texture& texture = textures_[i];
     if (!texture.sent_to_client)
       continue;
     GpuHostMsg_AcceleratedSurfaceRelease_Params params;
     params.identifier = texture.client_id;
     helper_->SendAcceleratedSurfaceRelease(params);
+    texture.info = NULL;
   }
 
   helper_->Destroy();
 }
 
 bool TextureImageTransportSurface::Resize(const gfx::Size&) {
   return true;
 }
 
 bool TextureImageTransportSurface::IsOffscreen() {
@@ skipping 42 matching lines @@
       ReleaseBackTexture();
       break;
   };
 }
 
 void* TextureImageTransportSurface::GetShareHandle() {
   return GetHandle();
 }
 
 void* TextureImageTransportSurface::GetDisplay() {
-  return parent_stub_.get() ? parent_stub_->surface()->GetDisplay() : NULL;
+  return parent_stub_ ? parent_stub_->surface()->GetDisplay() : NULL;
 }
 
 void* TextureImageTransportSurface::GetConfig() {
-  return parent_stub_.get() ? parent_stub_->surface()->GetConfig() : NULL;
+  return parent_stub_ ? parent_stub_->surface()->GetConfig() : NULL;
 }
 
 void TextureImageTransportSurface::OnResize(gfx::Size size) {
   CreateBackTexture(size);
 }
 
 void TextureImageTransportSurface::OnWillDestroyStub(
     GpuCommandBufferStub* stub) {
-  glDeleteFramebuffersEXT(1, &fbo_id_);
-  CHECK_GL_ERROR();
-  fbo_id_ = 0;
+  stub->RemoveDestructionObserver(this);
+  if (stub == parent_stub_) {
+    // We are losing the parent stub, we need to clear the reference on the
+    // infos (they are not allowed to outlive the stub).
+    textures_[0].info = NULL;
+    textures_[1].info = NULL;

[jonathan.backer, 2012/04/17 17:21:50]

parent_stub_ = NULL so that we don't access a stale pointer in
TextureImageTransportSurface::Destroy

[piman, 2012/04/17 17:37:24]

Done.

+  } else {
+    // We are losing the stub owning us, this is our last chance to clean up the
+    // resources we allocated in the stub's context.
+    glDeleteFramebuffersEXT(1, &fbo_id_);
+    CHECK_GL_ERROR();
+    fbo_id_ = 0;
 
-  stub->RemoveDestructionObserver(this);
-  stub_destroyed_ = true;
+    stub_destroyed_ = true;
+  }
 }
 
 bool TextureImageTransportSurface::SwapBuffers() {
+  if (!parent_stub_) {
+    LOG(ERROR) << "SwapBuffers failed because no parent stub.";
+    return false;
+  }
+
   glFlush();
   front_ = back();
   previous_damage_rect_ = gfx::Rect(textures_[front_].size);
 
   DCHECK(textures_[front_].client_id != 0);
 
   GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
   params.surface_handle = textures_[front_].client_id;
   helper_->SendAcceleratedSurfaceBuffersSwapped(params);
   helper_->SetScheduled(false);
   return true;
 }
 
 bool TextureImageTransportSurface::PostSubBuffer(
     int x, int y, int width, int height) {
-  if (!parent_stub_.get())
+  if (!parent_stub_) {
+    LOG(ERROR) << "PostSubBuffer failed because no parent stub.";
     return false;
+  }
 
-  TextureInfo* info = GetParentInfo(textures_[back()].client_id);
-  if (!info)
-    return false;
-  int back_texture_service_id = info->service_id();
+  DCHECK(textures_[back()].info);
+  int back_texture_service_id = textures_[back()].info->service_id();
 
-  info = GetParentInfo(textures_[front_].client_id);
-  if (!info)
-    return false;
-  int front_texture_service_id = info->service_id();
+  DCHECK(textures_[front_].info);
+  int front_texture_service_id = textures_[front_].info->service_id();
 
   gfx::Size expected_size = textures_[back()].size;
   bool surfaces_same_size = textures_[front_].size == expected_size;
 
   const gfx::Rect new_damage_rect(x, y, width, height);
 
   // An empty damage rect is a successful no-op.
   if (new_damage_rect.IsEmpty())
     return true;
 
@@ skipping 43 matching lines @@
   extensions += "GL_CHROMIUM_front_buffer_cached ";
   extensions += "GL_CHROMIUM_post_sub_buffer";
   return extensions;
 }
 
 gfx::Size TextureImageTransportSurface::GetSize() {
   return textures_[back()].size;
 }
 
 void* TextureImageTransportSurface::GetHandle() {
-  return parent_stub_.get() ? parent_stub_->surface()->GetHandle() : NULL;
+  return parent_stub_ ? parent_stub_->surface()->GetHandle() : NULL;
 }
 
 
 void TextureImageTransportSurface::OnNewSurfaceACK(
     uint64 surface_handle, TransportDIB::Handle /*shm_handle*/) {
 }
 
 void TextureImageTransportSurface::OnBuffersSwappedACK() {
   if (helper_->MakeCurrent()) {
     if (textures_[front_].size != textures_[back()].size) {
@@ skipping 10 matching lines @@
 
 void TextureImageTransportSurface::OnPostSubBufferACK() {
   OnBuffersSwappedACK();
 }
 
 void TextureImageTransportSurface::OnResizeViewACK() {
   NOTREACHED();
 }
 
 void TextureImageTransportSurface::ReleaseBackTexture() {
-  if (!parent_stub_.get())
+  if (!parent_stub_)
     return;
-  TextureInfo* info = GetParentInfo(textures_[back()].client_id);
-  if (!info)
-    return;
+  TextureInfo* info = textures_[back()].info;
+  DCHECK(info);
 
   GLuint service_id = info->service_id();
   if (!service_id)
     return;
   info->SetServiceId(0);
 
   {
     ScopedFrameBufferBinder fbo_binder(fbo_id_);
     glDeleteTextures(1, &service_id);
   }
   glFlush();
   CHECK_GL_ERROR();
 }
 
 void TextureImageTransportSurface::CreateBackTexture(const gfx::Size& size) {
-  if (!parent_stub_.get())
+  if (!parent_stub_)
     return;
   Texture& texture = textures_[back()];
-  TextureInfo* info = GetParentInfo(texture.client_id);
-  if (!info)
-    return;
+  TextureInfo* info = texture.info;
+  DCHECK(info);
 
   GLuint service_id = info->service_id();
 
   if (service_id && texture.size == size)
     return;
 
   if (!service_id) {
     glGenTextures(1, &service_id);
     info->SetServiceId(service_id);
   }
@@ skipping 32 matching lines @@
 
   GpuHostMsg_AcceleratedSurfaceNew_Params params;
   params.width = size.width();
   params.height = size.height();
   params.surface_handle = texture.client_id;
   helper_->SendAcceleratedSurfaceNew(params);
   texture.sent_to_client = true;
 }
 
 void TextureImageTransportSurface::AttachBackTextureToFBO() {
-  if (!parent_stub_.get())
+  if (!parent_stub_)
     return;
-  TextureInfo* info = GetParentInfo(textures_[back()].client_id);
-  if (!info)
-    return;
+  DCHECK(textures_[back()].info);
 
   ScopedFrameBufferBinder fbo_binder(fbo_id_);
   glFramebufferTexture2DEXT(GL_FRAMEBUFFER,
       GL_COLOR_ATTACHMENT0,
       GL_TEXTURE_2D,
-      info->service_id(),
+      textures_[back()].info->service_id(),
       0);
   glFlush();
   CHECK_GL_ERROR();
 
 #ifndef NDEBUG
   GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER);
   if (status != GL_FRAMEBUFFER_COMPLETE) {
     DLOG(ERROR) << "Framebuffer incomplete.";
   }
 #endif
 }
-
-TextureInfo* TextureImageTransportSurface::GetParentInfo(uint32 client_id) {
-  DCHECK(parent_stub_.get());
-  TextureManager* texture_manager =
-      parent_stub_->decoder()->GetContextGroup()->texture_manager();
-  TextureInfo* info = texture_manager->GetTextureInfo(client_id);
-  return info;
-}