Reland r206537 - cc: Don't return mailboxes if the TextureLayer is removed from the tree.

cc/layers/texture_layer.cc

Index: cc/layers/texture_layer.cc
diff --git a/cc/layers/texture_layer.cc b/cc/layers/texture_layer.cc
index d09963139101e955de76cb914513e31d51777b6f..0b19fdcc5e64d1f729ddd3ba17a396644e0f672a 100644
--- a/cc/layers/texture_layer.cc
+++ b/cc/layers/texture_layer.cc
@@ -4,6 +4,9 @@
 
 #include "cc/layers/texture_layer.h"
 
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "cc/base/thread.h"
 #include "cc/layers/texture_layer_client.h"
 #include "cc/layers/texture_layer_impl.h"
@@ -12,28 +15,6 @@
 
 namespace cc {
 
-namespace {
-
-void RunCallback(
-    const TextureMailbox::ReleaseCallback& callback,
-    unsigned sync_point,
-    bool lost_resource) {
-  callback.Run(sync_point, lost_resource);
-}
-
-void PostCallbackToThread(
-    Thread* thread,
-    const TextureMailbox::ReleaseCallback& callback,
-    unsigned sync_point,
-    bool lost_resource) {
-  if (!callback.is_null()) {
-    thread->PostTask(base::Bind(&RunCallback, callback,
-                                sync_point, lost_resource));
-  }
-}
-
-}  // namespace
-
 scoped_refptr<TextureLayer> TextureLayer::Create(TextureLayerClient* client) {
   return scoped_refptr<TextureLayer>(new TextureLayer(client, false));
 }
@@ -55,7 +36,7 @@ TextureLayer::TextureLayer(TextureLayerClient* client, bool uses_mailbox)
       context_lost_(false),
       content_committed_(false),
       texture_id_(0),
-      own_mailbox_(false) {
+      needs_set_mailbox_(false) {
   vertex_opacity_[0] = 1.0f;
   vertex_opacity_[1] = 1.0f;
   vertex_opacity_[2] = 1.0f;
@@ -69,8 +50,6 @@ TextureLayer::~TextureLayer() {
     if (rate_limit_context_ && client_)
       layer_tree_host()->StopRateLimiter(client_->Context3d());
   }
-  if (own_mailbox_)
-    texture_mailbox_.RunReleaseCallback(texture_mailbox_.sync_point(), false);
 }
 
 void TextureLayer::ClearClient() {
@@ -147,13 +126,14 @@ void TextureLayer::SetTextureId(unsigned id) {
 
 void TextureLayer::SetTextureMailbox(const TextureMailbox& mailbox) {
   DCHECK(uses_mailbox_);
-  if (own_mailbox_)
-    DCHECK(!mailbox.IsValid() || !mailbox.Equals(texture_mailbox_));
-  // If we never commited the mailbox, we need to release it here
-  if (own_mailbox_)
-    texture_mailbox_.RunReleaseCallback(texture_mailbox_.sync_point(), false);
-  texture_mailbox_ = mailbox;
-  own_mailbox_ = true;
+  DCHECK(!mailbox.IsValid() || !holder_ref_ ||
+         !mailbox.Equals(holder_ref_->holder()->mailbox()));
+  // If we never commited the mailbox, we need to release it here.
+  if (mailbox.IsValid())
+    holder_ref_ = MailboxHolder::Create(mailbox);
+  else
+    holder_ref_.reset();
+  needs_set_mailbox_ = true;
   SetNeedsCommit();
 }
 
@@ -174,11 +154,16 @@ void TextureLayer::SetNeedsDisplayRect(const gfx::RectF& dirty_rect) {
 void TextureLayer::SetLayerTreeHost(LayerTreeHost* host) {
   if (texture_id_ && layer_tree_host() && host != layer_tree_host())
     layer_tree_host()->AcquireLayerTextures();
+  // If we're removed from the tree, the TextureLayerImpl will be destroyed, and
+  // we will need to set the mailbox again on a new TextureLayerImpl the next
+  // time we push.
+  if (!host && uses_mailbox_ && holder_ref_.get())

[danakj, 2013/06/18 02:20:20]

nit: don't need .get() for scoped_ptr

[piman, 2013/06/18 02:53:01]

Done.

+    needs_set_mailbox_ = true;
   Layer::SetLayerTreeHost(host);
 }
 
 bool TextureLayer::DrawsContent() const {
-  return (client_ || texture_id_ || texture_mailbox_.IsValid()) &&
+  return (client_ || texture_id_ || holder_ref_.get()) &&

[danakj, 2013/06/18 02:20:20]

nit: don't need .get() for scoped_ptr

[piman, 2013/06/18 02:53:01]

Done.

          !context_lost_ && Layer::DrawsContent();
 }
 
@@ -213,13 +198,16 @@ void TextureLayer::PushPropertiesTo(LayerImpl* layer) {
   texture_layer->set_uv_bottom_right(uv_bottom_right_);
   texture_layer->set_vertex_opacity(vertex_opacity_);
   texture_layer->set_premultiplied_alpha(premultiplied_alpha_);
-  if (uses_mailbox_ && own_mailbox_) {
-    Thread* main_thread = layer_tree_host()->proxy()->MainThread();
-    TextureMailbox::ReleaseCallback callback = base::Bind(
-        &PostCallbackToThread, main_thread, texture_mailbox_.callback());
-    texture_layer->SetTextureMailbox(
-        texture_mailbox_.CopyWithNewCallback(callback));
-    own_mailbox_ = false;
+  if (uses_mailbox_ && needs_set_mailbox_) {
+    TextureMailbox texture_mailbox;
+    if (holder_ref_) {
+      MailboxHolder* holder = holder_ref_->holder();
+      TextureMailbox::ReleaseCallback callback =
+          holder->GetCallbackForImplThread();
+      texture_mailbox = holder->mailbox().CopyWithNewCallback(callback);
+    }
+    texture_layer->SetTextureMailbox(texture_mailbox);
+    needs_set_mailbox_ = false;
   } else {
     texture_layer->set_texture_id(texture_id_);
   }
@@ -237,4 +225,71 @@ bool TextureLayer::CanClipSelf() const {
   return true;
 }
 
+TextureLayer::MailboxHolder::MainThreadReference::MainThreadReference(
+    MailboxHolder* holder)
+    : holder_(holder) {
+}
+
+TextureLayer::MailboxHolder::MainThreadReference::~MainThreadReference() {
+  holder_->InternalRelease();
+}
+
+TextureLayer::MailboxHolder::MailboxHolder(const TextureMailbox& mailbox)
+    : message_loop_(base::MessageLoopProxy::current()),
+      internal_references_(1),

[danakj, 2013/06/18 02:20:20]

nit: start with 0 here, and have MainThreadRef constructor do InternalAddRef()?
Or add comment here that this is for the MainThreadReference.

[piman, 2013/06/18 02:53:01]

Done.

+      mailbox_(mailbox),
+      sync_point_(mailbox.sync_point()),
+      is_lost_(false) {
+}
+
+TextureLayer::MailboxHolder::~MailboxHolder() {
+  DCHECK_EQ(0u, internal_references_);
+}
+
+scoped_ptr<TextureLayer::MailboxHolder::MainThreadReference>
+TextureLayer::MailboxHolder::Create(const TextureMailbox& mailbox) {
+  return scoped_ptr<MainThreadReference>(new MainThreadReference(
+      new MailboxHolder(mailbox)));
+}
+
+void TextureLayer::MailboxHolder::Return(unsigned sync_point, bool is_lost) {
+  sync_point_ = sync_point;
+  is_lost_ = is_lost;
+}
+
+TextureMailbox::ReleaseCallback
+TextureLayer::MailboxHolder::GetCallbackForImplThread() {
+  // We can't call GetCallbackForImplThread if we released the main thread
+  // reference.
+  DCHECK(internal_references_);

[danakj, 2013/06/18 02:20:20]

nit: DCHECK_GT or DCHECK_NE 0?

[piman, 2013/06/18 02:53:01]

Done.

+  InternalAddRef();
+  return base::Bind(&MailboxHolder::ReturnAndReleaseOnImplThread, this);
+}
+
+void TextureLayer::MailboxHolder::InternalAddRef() {
+  ++internal_references_;
+}
+
+void TextureLayer::MailboxHolder::InternalRelease() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  if (!--internal_references_) {
+    mailbox_.RunReleaseCallback(sync_point_, is_lost_);
+    mailbox_ = TextureMailbox();
+  }
+}
+
+void TextureLayer::MailboxHolder::ReturnAndReleaseOnMainThread(
+    unsigned sync_point, bool is_lost) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  Return(sync_point, is_lost);
+  InternalRelease();
+}
+
+void TextureLayer::MailboxHolder::ReturnAndReleaseOnImplThread(
+    unsigned sync_point, bool is_lost) {
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &MailboxHolder::ReturnAndReleaseOnMainThread,
+      this, sync_point, is_lost));
+}
+
 }  // namespace cc