Pepper takes ownership of a mailbox before passing it to the texture layer.

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 c9e8470b6cca2e031b61a22bbd12e737b48a5c55..a929aa00cd22875858e0e116632b2f76919ba5fe 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -593,7 +593,10 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
                   const std::vector<int32_t>& attribs) override;
   void Destroy(bool have_context) override;
   void SetSurface(const scoped_refptr<gfx::GLSurface>& surface) override;
-  void ProduceFrontBuffer(const Mailbox& mailbox) override;
+  void ReleaseSurface() override;
+  void TakeFrontBuffer(const Mailbox& mailbox) override;
+  void ReturnFrontBuffer(const Mailbox& mailbox,
+                         bool is_lost) override;
   bool ResizeOffscreenFrameBuffer(const gfx::Size& size) override;
   void UpdateParentTextureInfo();
   bool MakeCurrent() override;
@@ -1994,6 +1997,33 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   scoped_refptr<TextureRef>
       offscreen_saved_color_texture_info_;
 
+  // When a client requests ownership of the swapped front buffer, all
+  // information is saved in this structure, and |in_use| is set to true. When a
+  // client releases ownership, |in_use| is set to false.
+  //
+  // An instance of this struct, with |in_use| = false may be reused instead of
+  // making a new BackTexture.
+  struct SavedBackTexture {
+    std::unique_ptr<BackTexture> back_texture;
+    scoped_refptr<TextureRef> texture_ref;
+    bool in_use;
+  };
+  std::vector<SavedBackTexture> saved_back_textures_;
+
+  // If there's a SavedBackTexture that's not in use, takes that. Otherwise,
+  // generates a new back texture.
+  void CreateBackTexture();
+  size_t create_back_texture_count_for_test_ = 0;
+
+  // Releases all saved BackTextures that are not in use by a client.
+  void ReleaseNotInUseBackTextures();
+
+  // Releases all saved BackTextures.
+  void ReleaseAllBackTextures();
+
+  size_t GetSavedBackTextureCountForTest() override;
+  size_t GetCreatedBackTextureCountForTest() override;
+
   // The copy that is used as the destination for multi-sample resolves.
   std::unique_ptr<BackFramebuffer> offscreen_resolved_frame_buffer_;
   std::unique_ptr<BackTexture> offscreen_resolved_color_texture_;
@@ -4067,6 +4097,7 @@ void GLES2DecoderImpl::Destroy(bool have_context) {
     offscreen_saved_color_texture_->Invalidate();
     offscreen_saved_color_texture_info_ = NULL;
   }
+  ReleaseAllBackTextures();
   if (have_context) {
     if (copy_texture_CHROMIUM_.get()) {
       copy_texture_CHROMIUM_->Destroy();
@@ -4205,11 +4236,23 @@ void GLES2DecoderImpl::SetSurface(
   RestoreCurrentFramebufferBindings();
 }
 
-void GLES2DecoderImpl::ProduceFrontBuffer(const Mailbox& mailbox) {
+void GLES2DecoderImpl::ReleaseSurface() {
+  if (!context_.get())
+    return;
+  if (WasContextLost()) {
+    DLOG(ERROR) << "  GLES2DecoderImpl: Trying to release lost context.";
+    return;
+  }
+  context_->ReleaseCurrent(surface_.get());
+  surface_ = nullptr;
+}
+
+void GLES2DecoderImpl::TakeFrontBuffer(const Mailbox& mailbox) {
   if (!offscreen_saved_color_texture_.get()) {
-    LOG(ERROR) << "Called ProduceFrontBuffer on a non-offscreen context";
+    LOG(ERROR) << "Called TakeFrontBuffer on a non-offscreen context";

[piman, 2016/04/26 01:54:01]

nit: DLOG

[erikchen, 2016/04/27 16:31:23]

Done.

     return;
   }
+
   if (!offscreen_saved_color_texture_info_.get()) {
     GLuint service_id = offscreen_saved_color_texture_->id();
     offscreen_saved_color_texture_info_ = TextureRef::Create(
@@ -4218,8 +4261,93 @@ void GLES2DecoderImpl::ProduceFrontBuffer(const Mailbox& mailbox) {
                                  GL_TEXTURE_2D);
     UpdateParentTextureInfo();
   }
+
   mailbox_manager()->ProduceTexture(
       mailbox, offscreen_saved_color_texture_info_->texture());
+
+  // Save the BackTexture and TextureRef.
+  SavedBackTexture save;
+  save.back_texture.swap(offscreen_saved_color_texture_);
+  save.texture_ref = offscreen_saved_color_texture_info_;
+  offscreen_saved_color_texture_info_ = nullptr;
+  save.in_use = true;
+  saved_back_textures_.push_back(std::move(save));
+
+  CreateBackTexture();
+}
+
+void GLES2DecoderImpl::ReturnFrontBuffer(const Mailbox& mailbox,
+                                         bool is_lost) {
+  Texture* texture = mailbox_manager()->ConsumeTexture(mailbox);
+  for (auto it = saved_back_textures_.begin(); it != saved_back_textures_.end();
+       ++it) {
+    if (texture != it->texture_ref->texture())
+      continue;
+
+    if (is_lost || it->back_texture->size() != offscreen_size_) {
+      it->back_texture->Invalidate();
+      saved_back_textures_.erase(it);
+      return;
+    }
+
+    it->in_use = false;
+    return;
+  }
+
+  NOTREACHED();

[piman, 2016/04/26 01:54:01]

nit: Not NOTREACHED(), this could be reached from a malicious client. It's ok if
it hoses its own state, but we shouldn't make the assumption that this can't
happen.

[erikchen, 2016/04/27 16:31:23]

This seems like the perfect place for NOTREACHED()?

If a Chrome developer misuses ReturnFrontBuffer in a DEBUG build, the assertion
will fire.
In a Release build, a malicious client will hit this logic and nothing will
happen.

If you prefer, I can switch to a DLOG but LOG messages are easily overlooked.

[piman, 2016/04/27 23:00:58]

I insist :) This is consistent in GPU code.
DCHECK/NOTREACHED is there for documentation purpose (as you mention, nothing
will happen with it in production). But it would be documenting the wrong thing
("this can't happen" - it's not true, it can happen, and we need to be aware
that it can). The next person refactoring this code needs to not make the wrong
assumption. E.g. just looking at this function without context one could rewrite
it as:

void GLES2DecoderImpl::ReturnFrontBuffer(const Mailbox& mailbox,
                                         bool is_lost) {	
  Texture* texture = mailbox_manager()->ConsumeTexture(mailbox);
  auto it =
      std::find_if(saved_back_textures_.begin(), saved_back_textures_.end(),
                   [texture](const SavedBackTexture& saved) {
                     return saved.texture_ref->texture() == texture;
                   });

  DCHECK(it != saved_back_textures_.end());  // Surely this is equivalent to the
previous NOTREACHED()
  if (is_lost || it->back_texture->size() != offscreen_size_) {
    it->back_texture->Invalidate();
    saved_back_textures_.erase(it);
  } else {
    it->in_use = false;
  }
}

However this would introduce a security issue (dereferencing out-of-bounds) on
invalid input.

If we want to check the client behavior, we should DCHECK/NOTREACHED it on the
client side, and/or write a test.
Replacing with a DLOG is ok (no LOG please).

[erikchen, 2016/04/27 23:59:24]

Point taken. Thanks for the example. Switched to a DLOG.

+}
+
+void GLES2DecoderImpl::CreateBackTexture() {
+  for (auto it = saved_back_textures_.begin(); it != saved_back_textures_.end();
+       ++it) {
+    if (it->in_use)
+      continue;
+
+    if (it->back_texture->size() != offscreen_size_)
+      continue;
+    offscreen_saved_color_texture_ = std::move(it->back_texture);
+    offscreen_saved_color_texture_info_ = it->texture_ref;
+    saved_back_textures_.erase(it);
+    return;
+  }
+
+  ++create_back_texture_count_for_test_;
+  offscreen_saved_color_texture_.reset(
+      new BackTexture(memory_tracker(), &state_));
+  offscreen_saved_color_texture_->Create();
+  offscreen_saved_color_texture_->AllocateStorage(
+      offscreen_size_, offscreen_saved_color_format_, false);
+  offscreen_saved_frame_buffer_->AttachRenderTexture(
+      offscreen_saved_color_texture_.get());
+}
+
+void GLES2DecoderImpl::ReleaseNotInUseBackTextures() {
+  for (auto& saved_back_texture : saved_back_textures_) {
+    if (!saved_back_texture.in_use)
+      saved_back_texture.back_texture->Invalidate();
+  }
+
+  std::remove_if(saved_back_textures_.begin(), saved_back_textures_.end(),
+                 [](const SavedBackTexture& saved_back_texture) {
+                   return !saved_back_texture.in_use;
+                 });
+}
+
+void GLES2DecoderImpl::ReleaseAllBackTextures() {
+  for (auto& saved_back_texture : saved_back_textures_) {
+    // The texture will be destroyed by texture_ref's destructor.
+    DCHECK(saved_back_texture.texture_ref);
+    saved_back_texture.back_texture->Invalidate();
+  }
+  saved_back_textures_.clear();
+}
+
+size_t GLES2DecoderImpl::GetSavedBackTextureCountForTest() {
+  return saved_back_textures_.size();
+}
+
+size_t GLES2DecoderImpl::GetCreatedBackTextureCountForTest() {
+  return create_back_texture_count_for_test_;
 }
 
 bool GLES2DecoderImpl::ResizeOffscreenFrameBuffer(const gfx::Size& size) {
@@ -12715,6 +12843,10 @@ void GLES2DecoderImpl::DoSwapBuffers() {
         glFinish();
       }
 
+      // The size has changed, so none of the cached BackTextures are useful
+      // anymore.
+      ReleaseNotInUseBackTextures();
+
       // Allocate the offscreen saved color texture.
       DCHECK(offscreen_saved_color_format_);
       offscreen_saved_color_texture_->AllocateStorage(