Skip to content

Commit

Permalink
Fixes crash in ViewManager during shutdown
Browse files Browse the repository at this point in the history
The crash happens because the CommandBufferImpl can be destroyed on
another thread. This is because the CommandBufferImpl uses a
strongbinding that is created on another thread. This means if there
is a connection error the CommandBufferImpl is destroyed on the
background thread. CommandBufferImpl has references to a bunch of
things that need to be destroyed on the main thread.

BUG=none
TEST=none
R=jam@chromium.org

Review URL: https://codereview.chromium.org/1179933003

Cr-Commit-Position: refs/heads/master@{#334082}
  • Loading branch information
sky authored and Commit bot committed Jun 11, 2015
1 parent 11f73b2 commit b773743
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
22 changes: 15 additions & 7 deletions components/view_manager/gles2/command_buffer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,6 @@ CommandBufferImpl::CommandBufferImpl(
base::Unretained(this), base::Passed(&request)));
}

CommandBufferImpl::~CommandBufferImpl() {
if (observer_)
observer_->OnCommandBufferImplDestroyed();
driver_task_runner_->PostTask(
FROM_HERE, base::Bind(&DestroyDriver, base::Passed(&driver_)));
}

void CommandBufferImpl::Initialize(
mojo::CommandBufferSyncClientPtr sync_client,
mojo::CommandBufferSyncPointClientPtr sync_point_client,
Expand Down Expand Up @@ -144,9 +137,24 @@ void CommandBufferImpl::Echo(const mojo::Callback<void()>& callback) {
base::Bind(&RunCallback, callback));
}

CommandBufferImpl::~CommandBufferImpl() {
if (observer_)
observer_->OnCommandBufferImplDestroyed();
driver_task_runner_->PostTask(
FROM_HERE, base::Bind(&DestroyDriver, base::Passed(&driver_)));
}

void CommandBufferImpl::BindToRequest(
mojo::InterfaceRequest<mojo::CommandBuffer> request) {
binding_.Bind(request.Pass());
binding_.set_error_handler(this);
}

void CommandBufferImpl::OnConnectionError() {
// OnConnectionError() is called on the background thread
// |control_task_runner| but objects we own (such as CommandBufferDriver)
// need to be destroyed on the thread we were created on.
driver_task_runner_->DeleteSoon(FROM_HERE, this);
}

void CommandBufferImpl::DidLoseContext() {
Expand Down
15 changes: 11 additions & 4 deletions components/view_manager/gles2/command_buffer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/single_thread_task_runner.h"
#include "components/view_manager/public/interfaces/command_buffer.mojom.h"
#include "components/view_manager/public/interfaces/viewport_parameter_listener.mojom.h"
#include "third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h"
#include "third_party/mojo/src/mojo/public/cpp/bindings/binding.h"

namespace gpu {
class SyncPointManager;
Expand All @@ -24,15 +24,15 @@ class CommandBufferImplObserver;
// so that we can insert sync points without blocking on the GL driver. It
// forwards most method calls to the CommandBufferDriver, which runs on the
// same thread as the native viewport.
class CommandBufferImpl : public mojo::CommandBuffer {
class CommandBufferImpl : public mojo::CommandBuffer,
public mojo::ErrorHandler {
public:
CommandBufferImpl(
mojo::InterfaceRequest<CommandBuffer> request,
mojo::ViewportParameterListenerPtr listener,
scoped_refptr<base::SingleThreadTaskRunner> control_task_runner,
gpu::SyncPointManager* sync_point_manager,
scoped_ptr<CommandBufferDriver> driver);
~CommandBufferImpl() override;

void Initialize(mojo::CommandBufferSyncClientPtr sync_client,
mojo::CommandBufferSyncPointClientPtr sync_point_client,
Expand All @@ -58,14 +58,21 @@ class CommandBufferImpl : public mojo::CommandBuffer {
}

private:
friend class base::DeleteHelper<CommandBufferImpl>;

~CommandBufferImpl() override;

void BindToRequest(mojo::InterfaceRequest<CommandBuffer> request);

// mojo::ErrorHandler:
void OnConnectionError() override;

scoped_refptr<gpu::SyncPointManager> sync_point_manager_;
scoped_refptr<base::SingleThreadTaskRunner> driver_task_runner_;
scoped_ptr<CommandBufferDriver> driver_;
mojo::CommandBufferSyncPointClientPtr sync_point_client_;
mojo::ViewportParameterListenerPtr viewport_parameter_listener_;
mojo::StrongBinding<CommandBuffer> binding_;
mojo::Binding<CommandBuffer> binding_;
CommandBufferImplObserver* observer_;

base::WeakPtrFactory<CommandBufferImpl> weak_factory_;
Expand Down

0 comments on commit b773743

Please sign in to comment.