Replace context parenting by sharing through mailboxes

Replace context parenting by sharing through mailboxes

Instead of tying contexts together through the parenting mechanism, we can allow
the client to produce the front buffer of an offscreen context into a mailbox,
and use that in the "parent" context.
It simplifies the code and the semantics wrt lost contexts, and we want to use
mailboxes anyway.

BUG=164095
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=204194

[piman, 2013-06-01 01:33:55]

(follow-up is at https://codereview.chromium.org/16290005 for the pepper
changes).

[piman, 2013-06-01 01:34:58]

oh, and +jschuh for gpu_messages.h

[piman, 2013-06-04 02:40:34]

PTAL. I uploaded a new version that actually removed the "SetNotOwned" business.
The Texture that is created on the ProduceFrontBuffer keeps ownership of the gl
texture after the context is destroyed, which is even better.
I also added a test.

[apatrick_chromium, 2013-06-04 20:37:51]

lgtm

https://codereview.chromium.org/15798014/diff/11001/content/common/gpu/gpu_co...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

https://codereview.chromium.org/15798014/diff/11001/content/common/gpu/gpu_co...
content/common/gpu/gpu_command_buffer_stub.cc:583: TRACE_EVENT0("gpu",
"GpuCommandBufferStub::OnSetParent");
GpuCommandBufferStub::OnSetParent -> GpuCommandBufferStub::OnProduceFrontBuffe

[apatrick_chromium, 2013-06-04 20:38:24]

Oh and the "From Dust" game would be a good test if you haven't tried it
already.

[jschuh, 2013-06-04 20:40:00]

ipc security lgtm. custom id to mailbox change.

[greggman, 2013-06-04 23:14:47]

lgtm

[piman, 2013-06-04 23:55:29]

https://codereview.chromium.org/15798014/diff/11001/content/common/gpu/gpu_co...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

https://codereview.chromium.org/15798014/diff/11001/content/common/gpu/gpu_co...
content/common/gpu/gpu_command_buffer_stub.cc:583: TRACE_EVENT0("gpu",
"GpuCommandBufferStub::OnSetParent");
On 2013/06/04 20:37:51, apatrick_chromium wrote:
> GpuCommandBufferStub::OnSetParent -> GpuCommandBufferStub::OnProduceFrontBuffe

Done.

[commit-bot: I haz the power, 2013-06-04 23:55:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/15798014/27001

[commit-bot: I haz the power, 2013-06-04 23:55:58]

Failed to apply patch for gpu/command_buffer/service/gles2_cmd_decoder.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file gpu/command_buffer/service/gles2_cmd_decoder.cc
  Hunk #2 succeeded at 514 (offset 1 line).
  Hunk #3 succeeded at 1557 (offset 1 line).
  Hunk #4 succeeded at 1685 (offset 2 lines).
  Hunk #5 FAILED at 2969.
  Hunk #6 succeeded at 3103 (offset 4 lines).
  Hunk #7 succeeded at 3114 (offset 4 lines).
  Hunk #8 FAILED at 3227.
  Hunk #9 FAILED at 9018.
  3 out of 9 hunks FAILED -- saving rejects to file
gpu/command_buffer/service/gles2_cmd_decoder.cc.rej

Patch:       gpu/command_buffer/service/gles2_cmd_decoder.cc
Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc
b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index
1642fe9be4d445fff20f539db9af4ab2ff203282..1e54069138e5f2272f2a421c601b139e366d1a2a
100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -29,6 +29,7 @@
 #include "gpu/command_buffer/common/gles2_cmd_utils.h"
 #include "gpu/command_buffer/common/debug_marker_manager.h"
 #include "gpu/command_buffer/common/id_allocator.h"
+#include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/command_buffer/service/buffer_manager.h"
 #include "gpu/command_buffer/service/cmd_buffer_engine.h"
 #include "gpu/command_buffer/service/context_group.h"
@@ -512,8 +513,7 @@ class GLES2DecoderImpl : public GLES2Decoder {
   virtual void Destroy(bool have_context) OVERRIDE;
   virtual void SetSurface(
       const scoped_refptr<gfx::GLSurface>& surface) OVERRIDE;
-  virtual bool SetParent(GLES2Decoder* parent_decoder,
-                         uint32 parent_texture_id) OVERRIDE;
+  virtual bool ProduceFrontBuffer(const Mailbox& mailbox) OVERRIDE;
   virtual bool ResizeOffscreenFrameBuffer(const gfx::Size& size) OVERRIDE;
   void UpdateParentTextureInfo();
   virtual bool MakeCurrent() OVERRIDE;
@@ -1556,10 +1556,6 @@ class GLES2DecoderImpl : public GLES2Decoder {
   // All the state for this context.
   ContextState state_;
 
-  // A parent decoder can access this decoders saved offscreen frame buffer.
-  // The parent pointer is reset if the parent is destroyed.
-  base::WeakPtr<GLES2DecoderImpl> parent_;
-
   // Current width and height of the offscreen frame buffer.
   gfx::Size offscreen_size_;
 
@@ -1687,9 +1683,6 @@ class GLES2DecoderImpl : public GLES2Decoder {
   TextureToIOSurfaceMap texture_to_io_surface_map_;
 #endif
 
-  typedef std::vector<GLES2DecoderImpl*> ChildList;
-  ChildList children_;
-
   scoped_ptr<CopyTextureCHROMIUMResourceManager> copy_texture_CHROMIUM_;
 
   // Cached values of the currently assigned viewport dimensions.
@@ -2976,54 +2969,48 @@ GLenum
GLES2DecoderImpl::GetBoundDrawFrameBufferInternalFormat() {
 }
 
 void GLES2DecoderImpl::UpdateParentTextureInfo() {
-  if (parent_) {
-    // Update the info about the offscreen saved color texture in the parent.
-    // The reference to the parent is a weak pointer and will become null if
the
-    // parent is later destroyed.
-    GLenum target = offscreen_saved_color_texture_info_->texture()->target();
-    TextureManager* parent_texture_manager = parent_->texture_manager();
-    glBindTexture(target, offscreen_saved_color_texture_info_->service_id());
-    parent_texture_manager->SetLevelInfo(
-        offscreen_saved_color_texture_info_.get(),
-        GL_TEXTURE_2D,
-        0,  // level
-        GL_RGBA,
-        offscreen_size_.width(),
-        offscreen_size_.height(),
-        1,  // depth
-        0,  // border
-        GL_RGBA,
-        GL_UNSIGNED_BYTE,
-        true);
-    parent_texture_manager->SetParameter(
-        "UpdateParentTextureInfo",
-        GetErrorState(),
-        offscreen_saved_color_texture_info_.get(),
-        GL_TEXTURE_MAG_FILTER,
-        GL_NEAREST);
-    parent_texture_manager->SetParameter(
-        "UpdateParentTextureInfo",
-        GetErrorState(),
-        offscreen_saved_color_texture_info_.get(),
-        GL_TEXTURE_MIN_FILTER,
-        GL_NEAREST);
-    parent_texture_manager->SetParameter(
-        "UpdateParentTextureInfo",
-        GetErrorState(),
-        offscreen_saved_color_texture_info_.get(),
-        GL_TEXTURE_WRAP_S,
-        GL_CLAMP_TO_EDGE);
-    parent_texture_manager->SetParameter(
-        "UpdateParentTextureInfo",
-        GetErrorState(),
-        offscreen_saved_color_texture_info_.get(),
-        GL_TEXTURE_WRAP_T,
-        GL_CLAMP_TO_EDGE);
-    TextureRef* texture_ref = GetTextureInfoForTarget(target);
-    glBindTexture(target, texture_ref ? texture_ref->service_id() : 0);
-  } else {
-    offscreen_saved_color_texture_info_ = NULL;
-  }
+  if (!offscreen_saved_color_texture_info_.get())
+    return;
+  GLenum target = offscreen_saved_color_texture_info_->texture()->target();
+  glBindTexture(target, offscreen_saved_color_texture_info_->service_id());
+  texture_manager()->SetLevelInfo(
+      offscreen_saved_color_texture_info_.get(),
+      GL_TEXTURE_2D,
+      0,  // level
+      GL_RGBA,
+      offscreen_size_.width(),
+      offscreen_size_.height(),
+      1,  // depth
+      0,  // border
+      GL_RGBA,
+      GL_UNSIGNED_BYTE,
+      true);
+  texture_manager()->SetParameter(
+      "UpdateParentTextureInfo",
+      GetErrorState(),
+      offscreen_saved_color_texture_info_.get(),
+      GL_TEXTURE_MAG_FILTER,
+      GL_NEAREST);
+  texture_manager()->SetParameter(
+      "UpdateParentTextureInfo",
+      GetErrorState(),
+      offscreen_saved_color_texture_info_.get(),
+      GL_TEXTURE_MIN_FILTER,
+      GL_NEAREST);
+  texture_manager()->SetParameter(
+      "UpdateParentTextureInfo",
+      GetErrorState(),
+      offscreen_saved_color_texture_info_.get(),
+      GL_TEXTURE_WRAP_S,
+      GL_CLAMP_TO_EDGE);
+  texture_manager()->SetParameter(
+      "UpdateParentTextureInfo",
+      GetErrorState(),
+      offscreen_saved_color_texture_info_.get(),
+      GL_TEXTURE_WRAP_T,
+      GL_CLAMP_TO_EDGE);
+  TextureRef* texture_ref = GetTextureInfoForTarget(target);
+  glBindTexture(target, texture_ref ? texture_ref->service_id() : 0);
 }
 
 void GLES2DecoderImpl::SetResizeCallback(
@@ -3106,12 +3093,6 @@ void GLES2DecoderImpl::Destroy(bool have_context) {
 
   DCHECK(!have_context || context_->IsCurrent(NULL));
 
-  ChildList children = children_;
-  for (ChildList::iterator it = children.begin(); it != children.end(); ++it)
-    (*it)->SetParent(NULL, 0);
-  DCHECK(children_.empty());
-  SetParent(NULL, 0);
-
   // Unbind everything.
   state_.vertex_attrib_manager = NULL;
   default_vertex_attrib_manager_ = NULL;
@@ -3123,6 +3104,13 @@ void GLES2DecoderImpl::Destroy(bool have_context) {
   state_.bound_draw_framebuffer = NULL;
   state_.bound_renderbuffer = NULL;
 
+  if (offscreen_saved_color_texture_info_) {
+    DCHECK(offscreen_target_color_texture_);
+    DCHECK_EQ(offscreen_saved_color_texture_info_->service_id(),
+              offscreen_saved_color_texture_->id());
+    offscreen_saved_color_texture_->Invalidate();
+    offscreen_saved_color_texture_info_ = NULL;
+  }
   if (have_context) {
     if (copy_texture_CHROMIUM_.get()) {
       copy_texture_CHROMIUM_->Destroy();
@@ -3233,66 +3221,20 @@ void GLES2DecoderImpl::SetSurface(
   RestoreCurrentFramebufferBindings();
 }
 
-bool GLES2DecoderImpl::SetParent(GLES2Decoder* new_parent,
-                                 uint32 new_parent_texture_id) {
+bool GLES2DecoderImpl::ProduceFrontBuffer(const Mailbox& mailbox) {
   if (!offscreen_saved_color_texture_.get())
     return false;
-
-  // Remove the saved frame buffer mapping from the parent decoder. The
-  // parent pointer is a weak pointer so it will be null if the parent has
-  // already been destroyed.
-  if (parent_) {
-    ChildList::iterator it = std::find(
-        parent_->children_.begin(),
-        parent_->children_.end(),
-        this);
-    DCHECK(it != parent_->children_.end());
-    parent_->children_.erase(it);
-    // First check the texture has been mapped into the parent. This might not
-    // be the case if initialization failed midway through.
-    if (offscreen_saved_color_texture_info_.get() &&
-        offscreen_saved_color_texture_info_->client_id()) {
-      parent_->texture_manager()->RemoveTexture(
-          offscreen_saved_color_texture_info_->client_id());
-    }
-  }
-
-  GLES2DecoderImpl* new_parent_impl = static_cast<GLES2DecoderImpl*>(
-      new_parent);
-  if (new_parent_impl) {
-#ifndef NDEBUG
-    ChildList::iterator it = std::find(
-        new_parent_impl->children_.begin(),
-        new_parent_impl->children_.end(),
-        this);
-    DCHECK(it == new_parent_impl->children_.end());
-#endif
-    new_parent_impl->children_.push_back(this);
-    // Map the ID of the saved offscreen texture into the parent so that
-    // it can reference it.
+  if (!offscreen_saved_color_texture_info_.get()) {
     GLuint service_id = offscreen_saved_color_texture_->id();
-
-    // Replace texture info when ID is already in use by parent.
-    if (new_parent_impl->texture_manager()->GetTexture(
-        new_parent_texture_id))
-      new_parent_impl->texture_manager()->RemoveTexture(
-          new_parent_texture_id);
-
-    offscreen_saved_color_texture_info_ =
-        new_parent_impl->CreateTexture(new_parent_texture_id, service_id);
-    offscreen_saved_color_texture_info_->texture()->SetNotOwned();
-    new_parent_impl->texture_manager()
-        ->SetTarget(offscreen_saved_color_texture_info_.get(), GL_TEXTURE_2D);
-
-    parent_ = base::AsWeakPtr<GLES2DecoderImpl>(new_parent_impl);
-
+    offscreen_saved_color_texture_info_ = CreateTexture(0, service_id);
+    texture_manager()->SetTarget(offscreen_saved_color_texture_info_.get(),
+                                 GL_TEXTURE_2D);
     UpdateParentTextureInfo();
-  } else {
-    parent_.reset();
-    offscreen_saved_color_texture_info_ = NULL;
   }
-
-  return …
(message too large)

[commit-bot: I haz the power, 2013-06-05 00:03:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/15798014/32001

[piman, 2013-06-05 00:55:11]

On 2013/06/04 20:38:24, apatrick_chromium wrote:
> Oh and the "From Dust" game would be a good test if you haven't tried it
> already.

I tried both From Dust and Bastion, as well as Flash.

[commit-bot: I haz the power, 2013-06-05 07:48:52]

Change committed as 204194