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

cc/layers/texture_layer.cc

 // Copyright 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "cc/trees/layer_tree_host.h"
 #include "third_party/WebKit/public/platform/WebGraphicsContext3D.h"
 
 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));
 }
 
 scoped_refptr<TextureLayer> TextureLayer::CreateForMailbox(
     TextureLayerClient* client) {
   return scoped_refptr<TextureLayer>(new TextureLayer(client, true));
 }
 
 TextureLayer::TextureLayer(TextureLayerClient* client, bool uses_mailbox)
     : Layer(),
       client_(client),
       uses_mailbox_(uses_mailbox),
       flipped_(true),
       uv_top_left_(0.f, 0.f),
       uv_bottom_right_(1.f, 1.f),
       premultiplied_alpha_(true),
       rate_limit_context_(false),
       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;
   vertex_opacity_[3] = 1.0f;
 }
 
 TextureLayer::~TextureLayer() {
   if (layer_tree_host()) {
     if (texture_id_)
       layer_tree_host()->AcquireLayerTextures();
     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() {
   client_ = NULL;
   if (uses_mailbox_)
     SetTextureMailbox(TextureMailbox());
   else
     SetTextureId(0);
 }
 
@@ skipping 56 matching lines @@
   if (texture_id_ == id)
     return;
   if (texture_id_ && layer_tree_host())
     layer_tree_host()->AcquireLayerTextures();
   texture_id_ = id;
   SetNeedsCommit();
 }
 
 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_ || !mailbox.Equals(holder_->mailbox()));
+  // If we never commited the mailbox, we need to release it here.
+  holder_ = mailbox.IsValid() ? new MailboxHolder(mailbox) : NULL;
+  needs_set_mailbox_ = true;
   SetNeedsCommit();
 }
 
 void TextureLayer::WillModifyTexture() {
   if (layer_tree_host() && (DrawsContent() || content_committed_)) {
     layer_tree_host()->AcquireLayerTextures();
     content_committed_ = false;
   }
 }
 
 void TextureLayer::SetNeedsDisplayRect(const gfx::RectF& dirty_rect) {
   Layer::SetNeedsDisplayRect(dirty_rect);
 
   if (rate_limit_context_ && client_ && layer_tree_host() && DrawsContent())
     layer_tree_host()->StartRateLimiter(client_->Context3d());
 }
 
 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_)
+    needs_set_mailbox_ = true;
   Layer::SetLayerTreeHost(host);
 }
 
 bool TextureLayer::DrawsContent() const {
-  return (client_ || texture_id_ || texture_mailbox_.IsValid()) &&
+  return (client_ || texture_id_ || holder_.get()) &&
          !context_lost_ && Layer::DrawsContent();
 }
 
 void TextureLayer::Update(ResourceUpdateQueue* queue,
                           const OcclusionTracker* occlusion,
                           RenderingStats* stats) {
   if (client_) {
     if (uses_mailbox_) {
       TextureMailbox mailbox;
       if (client_->PrepareTextureMailbox(&mailbox)) {
@@ skipping 14 matching lines @@
 
 void TextureLayer::PushPropertiesTo(LayerImpl* layer) {
   Layer::PushPropertiesTo(layer);
 
   TextureLayerImpl* texture_layer = static_cast<TextureLayerImpl*>(layer);
   texture_layer->set_flipped(flipped_);
   texture_layer->set_uv_top_left(uv_top_left_);
   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_) {
+      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_);
   }
   content_committed_ = DrawsContent();
 }
 
 bool TextureLayer::BlocksPendingCommit() const {
   // Double-buffered texture layers need to be blocked until they can be made
   // triple-buffered.  Single-buffered layers already prevent draws, so
   // can block too for simplicity.
   return DrawsContent();
 }
 
 bool TextureLayer::CanClipSelf() const {
   return true;
 }
 
+TextureLayer::MailboxHolder::MailboxHolder(const TextureMailbox& mailbox)
+    : message_loop_(base::MessageLoopProxy::current()),
+      mailbox_(mailbox),
+      sync_point_(mailbox.sync_point()),
+      is_lost_(false) {
+}
+
+TextureLayer::MailboxHolder::~MailboxHolder() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  if (mailbox_.callback().is_null())
+    return;
+  mailbox_.RunReleaseCallback(sync_point_, is_lost_);
+}
+
+void TextureLayer::MailboxHolder::Return(unsigned sync_point, bool is_lost) {
+  sync_point_ = sync_point;
+  is_lost_ = is_lost;
+}
+
+TextureMailbox::ReleaseCallback
+TextureLayer::MailboxHolder::GetCallbackForImplThread() {
+  // Note: we refcount manually here and pass Unretained, because otherwise the
+  // callback would keep a reference on the MailboxHolder, which may be released
+  // when the callback is destroyed on the impl thread.
+  AddRef();
+  return base::Bind(&MailboxHolder::ReturnAndReleaseOnImplThread,
+                    Unretained(this));
+}
+
+void TextureLayer::MailboxHolder::ReturnAndReleaseOnMainThread(
+    unsigned sync_point, bool is_lost) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  Return(sync_point, is_lost);
+  Release();
+}
+
+void TextureLayer::MailboxHolder::ReturnAndReleaseOnImplThread(
+    unsigned sync_point, bool is_lost) {
+  message_loop_->PostTask(FROM_HERE, base::Bind(

[danakj, 2013/06/18 00:53:24]

I guess the downside here is we can't DCHECK that we called this function, we
would just leak the MailboxHolder if we failed to call it.

Maybe we could get around that by keeping track in here if the impl thread has a
ref on the object, and if it does when we get to the ~ then we post task in the
~ to the main thread, otherwise run the callback immediately.
GetCallbackForImplThread() would just have to set a bool ref_on_impl_thread_ =
true;

Is that idea crazy or helpful, I'm not sure?

[piman, 2013/06/18 02:10:58]

Following our discussion, see the new version. Not entirely sure whether the
MainThreadReference is overkill or not...

+      &MailboxHolder::ReturnAndReleaseOnMainThread,
+      Unretained(this), sync_point, is_lost));
+}
+
 }  // namespace cc