Skip to content

Commit

Permalink
Revert "Add gpu::MailboxHolder to hold state for a gpu::Mailbox"
Browse files Browse the repository at this point in the history
This reverts commit 9ee2343.

This patch is the source of the top crash in canary. See the bug
for more details.

TBR=sheu
NOTREECHECKS=true
BUG=336040

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245959 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
danakj@chromium.org committed Jan 21, 2014
1 parent 48ed261 commit a11e5c2
Show file tree
Hide file tree
Showing 54 changed files with 561 additions and 588 deletions.
1 change: 0 additions & 1 deletion cc/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ include_rules = [
"+gpu/command_buffer/common/gpu_memory_allocation.h",
"+gpu/command_buffer/common/capabilities.h",
"+gpu/command_buffer/common/mailbox.h",
"+gpu/command_buffer/common/mailbox_holder.h",
"+media",
"+skia/ext",
"+third_party/skia/include",
Expand Down
2 changes: 1 addition & 1 deletion cc/layers/delegated_frame_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class DelegatedFrameProviderTest
ResourceProvider::ResourceId resource_id) {
TransferableResource resource;
resource.id = resource_id;
resource.mailbox_holder.texture_target = GL_TEXTURE_2D;
resource.target = GL_TEXTURE_2D;
frame->resource_list.push_back(resource);
}

Expand Down
50 changes: 23 additions & 27 deletions cc/layers/texture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,10 @@ void TextureLayer::SetTextureMailboxInternal(
DCHECK_EQ(mailbox.IsValid(), !!release_callback);

// If we never commited the mailbox, we need to release it here.
if (mailbox.IsValid()) {
holder_ref_ =
TextureMailboxHolder::Create(mailbox, release_callback.Pass());
} else {
if (mailbox.IsValid())
holder_ref_ = MailboxHolder::Create(mailbox, release_callback.Pass());
else
holder_ref_.reset();
}
needs_set_mailbox_ = true;
// If we are within a commit, no need to do it again immediately after.
if (requires_commit)
Expand Down Expand Up @@ -264,7 +262,7 @@ void TextureLayer::PushPropertiesTo(LayerImpl* layer) {
TextureMailbox texture_mailbox;
scoped_ptr<SingleReleaseCallback> release_callback;
if (holder_ref_) {
TextureMailboxHolder* holder = holder_ref_->holder();
MailboxHolder* holder = holder_ref_->holder();
texture_mailbox = holder->mailbox();
release_callback = holder->GetCallbackForImplThread();
}
Expand All @@ -286,61 +284,60 @@ Region TextureLayer::VisibleContentOpaqueRegion() const {
return Region();
}

TextureLayer::TextureMailboxHolder::MainThreadReference::MainThreadReference(
TextureMailboxHolder* holder)
TextureLayer::MailboxHolder::MainThreadReference::MainThreadReference(
MailboxHolder* holder)
: holder_(holder) {
holder_->InternalAddRef();
}

TextureLayer::TextureMailboxHolder::MainThreadReference::
~MainThreadReference() {
TextureLayer::MailboxHolder::MainThreadReference::~MainThreadReference() {
holder_->InternalRelease();
}

TextureLayer::TextureMailboxHolder::TextureMailboxHolder(
TextureLayer::MailboxHolder::MailboxHolder(
const TextureMailbox& mailbox,
scoped_ptr<SingleReleaseCallback> release_callback)
: message_loop_(BlockingTaskRunner::current()),
internal_references_(0),
mailbox_(mailbox),
release_callback_(release_callback.Pass()),
sync_point_(mailbox.sync_point()),
is_lost_(false) {}
is_lost_(false) {
}

TextureLayer::TextureMailboxHolder::~TextureMailboxHolder() {
TextureLayer::MailboxHolder::~MailboxHolder() {
DCHECK_EQ(0u, internal_references_);
}

scoped_ptr<TextureLayer::TextureMailboxHolder::MainThreadReference>
TextureLayer::TextureMailboxHolder::Create(
scoped_ptr<TextureLayer::MailboxHolder::MainThreadReference>
TextureLayer::MailboxHolder::Create(
const TextureMailbox& mailbox,
scoped_ptr<SingleReleaseCallback> release_callback) {
return scoped_ptr<MainThreadReference>(new MainThreadReference(
new TextureMailboxHolder(mailbox, release_callback.Pass())));
new MailboxHolder(mailbox, release_callback.Pass())));
}

void TextureLayer::TextureMailboxHolder::Return(uint32 sync_point,
bool is_lost) {
void TextureLayer::MailboxHolder::Return(unsigned sync_point, bool is_lost) {
base::AutoLock lock(arguments_lock_);
sync_point_ = sync_point;
is_lost_ = is_lost;
}

scoped_ptr<SingleReleaseCallback>
TextureLayer::TextureMailboxHolder::GetCallbackForImplThread() {
TextureLayer::MailboxHolder::GetCallbackForImplThread() {
// We can't call GetCallbackForImplThread if we released the main thread
// reference.
DCHECK_GT(internal_references_, 0u);
InternalAddRef();
return SingleReleaseCallback::Create(
base::Bind(&TextureMailboxHolder::ReturnAndReleaseOnImplThread, this));
base::Bind(&MailboxHolder::ReturnAndReleaseOnImplThread, this));
}

void TextureLayer::TextureMailboxHolder::InternalAddRef() {
void TextureLayer::MailboxHolder::InternalAddRef() {
++internal_references_;
}

void TextureLayer::TextureMailboxHolder::InternalRelease() {
void TextureLayer::MailboxHolder::InternalRelease() {
DCHECK(message_loop_->BelongsToCurrentThread());
if (!--internal_references_) {
release_callback_->Run(sync_point_, is_lost_);
Expand All @@ -349,12 +346,11 @@ void TextureLayer::TextureMailboxHolder::InternalRelease() {
}
}

void TextureLayer::TextureMailboxHolder::ReturnAndReleaseOnImplThread(
uint32 sync_point,
bool is_lost) {
void TextureLayer::MailboxHolder::ReturnAndReleaseOnImplThread(
unsigned sync_point, bool is_lost) {
Return(sync_point, is_lost);
message_loop_->PostTask(
FROM_HERE, base::Bind(&TextureMailboxHolder::InternalRelease, this));
message_loop_->PostTask(FROM_HERE,
base::Bind(&MailboxHolder::InternalRelease, this));
}

} // namespace cc
29 changes: 14 additions & 15 deletions cc/layers/texture_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ class TextureLayerClient;
// A Layer containing a the rendered output of a plugin instance.
class CC_EXPORT TextureLayer : public Layer {
public:
class CC_EXPORT TextureMailboxHolder
: public base::RefCountedThreadSafe<TextureMailboxHolder> {
class CC_EXPORT MailboxHolder
: public base::RefCountedThreadSafe<MailboxHolder> {
public:
class CC_EXPORT MainThreadReference {
public:
explicit MainThreadReference(TextureMailboxHolder* holder);
explicit MainThreadReference(MailboxHolder* holder);
~MainThreadReference();
TextureMailboxHolder* holder() { return holder_.get(); }
MailboxHolder* holder() { return holder_.get(); }

private:
scoped_refptr<TextureMailboxHolder> holder_;
scoped_refptr<MailboxHolder> holder_;
DISALLOW_COPY_AND_ASSIGN(MainThreadReference);
};

const TextureMailbox& mailbox() const { return mailbox_; }
void Return(uint32 sync_point, bool is_lost);
void Return(unsigned sync_point, bool is_lost);

// Gets a ReleaseCallback that can be called from another thread. Note: the
// caller must ensure the callback is called.
Expand All @@ -49,18 +49,17 @@ class CC_EXPORT TextureLayer : public Layer {
static scoped_ptr<MainThreadReference> Create(
const TextureMailbox& mailbox,
scoped_ptr<SingleReleaseCallback> release_callback);
virtual ~TextureMailboxHolder();
virtual ~MailboxHolder();

private:
friend class base::RefCountedThreadSafe<TextureMailboxHolder>;
friend class base::RefCountedThreadSafe<MailboxHolder>;
friend class MainThreadReference;
explicit TextureMailboxHolder(
const TextureMailbox& mailbox,
scoped_ptr<SingleReleaseCallback> release_callback);
explicit MailboxHolder(const TextureMailbox& mailbox,
scoped_ptr<SingleReleaseCallback> release_callback);

void InternalAddRef();
void InternalRelease();
void ReturnAndReleaseOnImplThread(uint32 sync_point, bool is_lost);
void ReturnAndReleaseOnImplThread(unsigned sync_point, bool is_lost);

// This member is thread safe, and is accessed on main and impl threads.
const scoped_refptr<BlockingTaskRunner> message_loop_;
Expand All @@ -76,9 +75,9 @@ class CC_EXPORT TextureLayer : public Layer {
// values of these fields are well-ordered such that the last call to
// ReturnAndReleaseOnImplThread() defines their values.
base::Lock arguments_lock_;
uint32 sync_point_;
unsigned sync_point_;
bool is_lost_;
DISALLOW_COPY_AND_ASSIGN(TextureMailboxHolder);
DISALLOW_COPY_AND_ASSIGN(MailboxHolder);
};

// If this texture layer requires special preparation logic for each frame
Expand Down Expand Up @@ -166,7 +165,7 @@ class CC_EXPORT TextureLayer : public Layer {
bool content_committed_;

unsigned texture_id_;
scoped_ptr<TextureMailboxHolder::MainThreadReference> holder_ref_;
scoped_ptr<MailboxHolder::MainThreadReference> holder_ref_;
bool needs_set_mailbox_;

DISALLOW_COPY_AND_ASSIGN(TextureLayer);
Expand Down
Loading

0 comments on commit a11e5c2

Please sign in to comment.