Commit 83cb753f authored by piman@chromium.org's avatar piman@chromium.org

Delay GpuCommandBufferStub::AddDestructionObserver until we need it.

In certain failure cases at initialization time (caused by a race) we used to call
AddDestructionObserver before we would make the surface current, causing us to
destroy the surface before OnWillDestroyStub had a chance to be called, so we
wouldn't RemoveDestructionObserver and we'd call into a dead pointer.

This fixes it. By the time MakeCurrent is called, we know we have a ref to the
surface and it won't destroy it until GpuCommandBufferStub::Destroy, after
OnWillDestroyStub is called.

BUG=140502
TEST=attach debugger to renderer, add a breakpoint to CommandBufferProxyImpl::Initialize, right-click and "view page source" (breakpoint should hit), kill new tab, and only then continue breakpoint. Observe no GPU process crash.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150227 0039d316-1c4b-4281-b951-d872f2087c98
parent b28603d6
...@@ -131,6 +131,7 @@ class ImageTransportHelper : public IPC::Listener { ...@@ -131,6 +131,7 @@ class ImageTransportHelper : public IPC::Listener {
void Suspend(); void Suspend();
GpuChannelManager* manager() const { return manager_; } GpuChannelManager* manager() const { return manager_; }
GpuCommandBufferStub* stub() const { return stub_.get(); }
private: private:
gpu::GpuScheduler* Scheduler(); gpu::GpuScheduler* Scheduler();
......
...@@ -47,8 +47,6 @@ TextureImageTransportSurface::TextureImageTransportSurface( ...@@ -47,8 +47,6 @@ TextureImageTransportSurface::TextureImageTransportSurface(
manager, manager,
stub, stub,
gfx::kNullPluginWindow)); gfx::kNullPluginWindow));
stub->AddDestructionObserver(this);
} }
TextureImageTransportSurface::~TextureImageTransportSurface() { TextureImageTransportSurface::~TextureImageTransportSurface() {
...@@ -134,9 +132,13 @@ bool TextureImageTransportSurface::OnMakeCurrent(gfx::GLContext* context) { ...@@ -134,9 +132,13 @@ bool TextureImageTransportSurface::OnMakeCurrent(gfx::GLContext* context) {
GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER); GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER);
if (status != GL_FRAMEBUFFER_COMPLETE) { if (status != GL_FRAMEBUFFER_COMPLETE) {
DLOG(ERROR) << "Framebuffer incomplete."; DLOG(ERROR) << "Framebuffer incomplete.";
glDeleteFramebuffersEXT(1, &fbo_id_);
fbo_id_ = 0;
return false; return false;
} }
#endif #endif
DCHECK(helper_->stub());
helper_->stub()->AddDestructionObserver(this);
} }
return true; return true;
...@@ -208,6 +210,7 @@ void TextureImageTransportSurface::OnWillDestroyStub( ...@@ -208,6 +210,7 @@ void TextureImageTransportSurface::OnWillDestroyStub(
ReleaseParentStub(); ReleaseParentStub();
helper_->SetPreemptByCounter(NULL); helper_->SetPreemptByCounter(NULL);
} else { } else {
DCHECK(stub == helper_->stub());
stub->RemoveDestructionObserver(this); stub->RemoveDestructionObserver(this);
// We are losing the stub owning us, this is our last chance to clean up the // We are losing the stub owning us, this is our last chance to clean up the
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment