Allow TextureImageTransportSurface to fail initialization

There's a very rare race, where the UI may destroy the textures before the
TextureImageTransportSurface has a chance to use them for initialization,
causing a crash. This fixes the crash by moving initialization to Initialize(),
allowing it to fail.

BUG=None
TEST=manual (with debugger).


Review URL: https://chromiumcodereview.appspot.com/10447137

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

3 files changed, 37 insertions, 23 deletions

diff --git a/content/common/gpu/image_transport_surface.h b/content/common/gpu/image_transport_surface.h
index 307a11e..fa4620d 100644
--- a/content/common/gpu/image_transport_surface.h
+++ b/content/common/gpu/image_transport_surface.h
@@ -124,6 +124,8 @@ class ImageTransportHelper : public IPC::Channel::Listener {
 
   void Suspend();
 
+  GpuChannelManager* manager() const { return manager_; }
+
  private:
   gpu::GpuScheduler* Scheduler();
   gpu::gles2::GLES2Decoder* Decoder();
diff --git a/content/common/gpu/texture_image_transport_surface.cc b/content/common/gpu/texture_image_transport_surface.cc
index 7e22113..37ed491 100644
--- a/content/common/gpu/texture_image_transport_surface.cc
+++ b/content/common/gpu/texture_image_transport_surface.cc
@@ -67,11 +67,31 @@ TextureImageTransportSurface::TextureImageTransportSurface(
         stub_destroyed_(false),
         backbuffer_suggested_allocation_(true),
         frontbuffer_suggested_allocation_(true),
+        handle_(handle),
         parent_stub_(NULL) {
-  GpuChannel* parent_channel = manager->LookupChannel(handle.parent_client_id);
-  DCHECK(parent_channel);
-  parent_stub_ = parent_channel->LookupCommandBuffer(handle.parent_context_id);
-  DCHECK(parent_stub_);
+  helper_.reset(new ImageTransportHelper(this,
+                                         manager,
+                                         stub,
+                                         gfx::kNullPluginWindow));
+
+  stub->AddDestructionObserver(this);
+}
+
+TextureImageTransportSurface::~TextureImageTransportSurface() {
+  DCHECK(stub_destroyed_);
+  Destroy();
+}
+
+bool TextureImageTransportSurface::Initialize() {
+  GpuChannelManager* manager = helper_->manager();
+  GpuChannel* parent_channel = manager->LookupChannel(handle_.parent_client_id);
+  if (!parent_channel)
+    return false;
+
+  parent_stub_ = parent_channel->LookupCommandBuffer(handle_.parent_context_id);
+  if (!parent_stub_)
+    return false;
+
   parent_stub_->AddDestructionObserver(this);
   TextureManager* texture_manager =
       parent_stub_->decoder()->GetContextGroup()->texture_manager();
@@ -79,9 +99,11 @@ TextureImageTransportSurface::TextureImageTransportSurface(
 
   for (int i = 0; i < 2; ++i) {
     Texture& texture = textures_[i];
-    texture.client_id = handle.parent_texture_id[i];
+    texture.client_id = handle_.parent_texture_id[i];
     texture.info = texture_manager->GetTextureInfo(texture.client_id);
-    DCHECK(texture.info);
+    if (!texture.info)
+      return false;
+
     if (!texture.info->target())
       texture_manager->SetInfoTarget(texture.info, GL_TEXTURE_2D);
     texture_manager->SetParameter(
@@ -94,20 +116,6 @@ TextureImageTransportSurface::TextureImageTransportSurface(
         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();
 }
 
@@ -199,11 +207,14 @@ void TextureImageTransportSurface::OnWillDestroyStub(
     ReleaseParentStub();
   } else {
     stub->RemoveDestructionObserver(this);
+
     // 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;
+    if (fbo_id_) {
+      glDeleteFramebuffersEXT(1, &fbo_id_);
+      CHECK_GL_ERROR();
+      fbo_id_ = 0;
+    }
 
     stub_destroyed_ = true;
   }
diff --git a/content/common/gpu/texture_image_transport_surface.h b/content/common/gpu/texture_image_transport_surface.h
index 394da75..907bfa4 100644
--- a/content/common/gpu/texture_image_transport_surface.h
+++ b/content/common/gpu/texture_image_transport_surface.h
@@ -98,6 +98,7 @@ class TextureImageTransportSurface :
   bool frontbuffer_suggested_allocation_;
 
   scoped_ptr<ImageTransportHelper> helper_;
+  gfx::GLSurfaceHandle handle_;
   GpuCommandBufferStub* parent_stub_;
 
   DISALLOW_COPY_AND_ASSIGN(TextureImageTransportSurface);