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

In some cases, the UI may destroy the textures the renderer is rendering into
before the renderer has a chance to finish.
This patch makes it so that we avoid generating a lost context event in the
page that is closing, because we can't recover correctly in the other pages in
the same renderer process.

This also adds logging so that we can audit all cases of lost context.

BUG=122868
TEST=run aura chrome, open and close tabs (from the same process), observe no
flashing and no mention of a lost context.


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

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

2 files changed, 75 insertions, 55 deletions

diff --git a/content/common/gpu/texture_image_transport_surface.cc b/content/common/gpu/texture_image_transport_surface.cc
index e9649f5..67d5637 100644
--- a/content/common/gpu/texture_image_transport_surface.cc
+++ b/content/common/gpu/texture_image_transport_surface.cc
@@ -50,19 +50,27 @@ class ScopedTextureBinder {
 
 }  // 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);
@@ -70,14 +78,18 @@ TextureImageTransportSurface::TextureImageTransportSurface(
   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,
@@ -98,6 +110,10 @@ bool TextureImageTransportSurface::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)
@@ -105,6 +121,7 @@ void TextureImageTransportSurface::Destroy() {
     GpuHostMsg_AcceleratedSurfaceRelease_Params params;
     params.identifier = texture.client_id;
     helper_->SendAcceleratedSurfaceRelease(params);
+    texture.info = NULL;
   }
 
   helper_->Destroy();
@@ -167,11 +184,11 @@ void* TextureImageTransportSurface::GetShareHandle() {
 }
 
 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) {
@@ -180,15 +197,30 @@ void TextureImageTransportSurface::OnResize(gfx::Size size) {
 
 void TextureImageTransportSurface::OnWillDestroyStub(
     GpuCommandBufferStub* stub) {
-  glDeleteFramebuffersEXT(1, &fbo_id_);
-  CHECK_GL_ERROR();
-  fbo_id_ = 0;
-
   stub->RemoveDestructionObserver(this);
-  stub_destroyed_ = true;
+  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;
+    parent_stub_ = NULL;
+  } 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_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);
@@ -204,18 +236,16 @@ bool TextureImageTransportSurface::SwapBuffers() {
 
 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;
@@ -279,7 +309,7 @@ gfx::Size TextureImageTransportSurface::GetSize() {
 }
 
 void* TextureImageTransportSurface::GetHandle() {
-  return parent_stub_.get() ? parent_stub_->surface()->GetHandle() : NULL;
+  return parent_stub_ ? parent_stub_->surface()->GetHandle() : NULL;
 }
 
 
@@ -310,11 +340,10 @@ void TextureImageTransportSurface::OnResizeViewACK() {
 }
 
 void TextureImageTransportSurface::ReleaseBackTexture() {
-  if (!parent_stub_.get())
-    return;
-  TextureInfo* info = GetParentInfo(textures_[back()].client_id);
-  if (!info)
+  if (!parent_stub_)
     return;
+  TextureInfo* info = textures_[back()].info;
+  DCHECK(info);
 
   GLuint service_id = info->service_id();
   if (!service_id)
@@ -330,12 +359,11 @@ void TextureImageTransportSurface::ReleaseBackTexture() {
 }
 
 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();
 
@@ -388,17 +416,15 @@ void TextureImageTransportSurface::CreateBackTexture(const gfx::Size& size) {
 }
 
 void TextureImageTransportSurface::AttachBackTextureToFBO() {
-  if (!parent_stub_.get())
-    return;
-  TextureInfo* info = GetParentInfo(textures_[back()].client_id);
-  if (!info)
+  if (!parent_stub_)
     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();
@@ -410,11 +436,3 @@ void TextureImageTransportSurface::AttachBackTextureToFBO() {
   }
 #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;
-}
diff --git a/content/common/gpu/texture_image_transport_surface.h b/content/common/gpu/texture_image_transport_surface.h
index 5fcd5d3..6dc5dd2 100644
--- a/content/common/gpu/texture_image_transport_surface.h
+++ b/content/common/gpu/texture_image_transport_surface.h
@@ -7,7 +7,6 @@
 #pragma once
 
 #include "base/basictypes.h"
-#include "base/memory/weak_ptr.h"
 #include "content/common/gpu/gpu_command_buffer_stub.h"
 #include "content/common/gpu/image_transport_surface.h"
 #include "gpu/command_buffer/service/texture_manager.h"
@@ -56,7 +55,8 @@ class TextureImageTransportSurface :
  private:
   // A texture backing the front/back buffer in the parent stub.
   struct Texture {
-    Texture() : client_id(0), sent_to_client(false) {}
+    Texture();
+    ~Texture();
 
     // The client-side id in the parent stub.
     uint32 client_id;
@@ -66,13 +66,15 @@ class TextureImageTransportSurface :
 
     // Whether or not that texture has been sent to the client yet.
     bool sent_to_client;
+
+    // The texture info in the parent stub.
+    gpu::gles2::TextureManager::TextureInfo::Ref info;
   };
 
   virtual ~TextureImageTransportSurface();
   void CreateBackTexture(const gfx::Size& size);
   void ReleaseBackTexture();
   void AttachBackTextureToFBO();
-  gpu::gles2::TextureManager::TextureInfo* GetParentInfo(uint32 client_id);
   int back() const { return 1 - front_; }
 
   // The framebuffer that represents this surface (service id). Allocated lazily
@@ -91,7 +93,7 @@ class TextureImageTransportSurface :
   bool stub_destroyed_;
 
   scoped_ptr<ImageTransportHelper> helper_;
-  base::WeakPtr<GpuCommandBufferStub> parent_stub_;
+  GpuCommandBufferStub* parent_stub_;
 
   DISALLOW_COPY_AND_ASSIGN(TextureImageTransportSurface);
 };