gpu: Reuse transfer buffers more aggresively

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 f913c4883d5ebdb892dd908c2d97842b026ba4fc..eb7987d25371f9427edc0cf51fc3138fb1b2f1ca 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -15,6 +15,7 @@
 
 #include "base/at_exit.h"
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/debug/trace_event_synthetic_delay.h"
@@ -479,6 +480,24 @@ struct FenceCallback {
   scoped_ptr<gfx::GLFence> fence;
 };
 
+class AsyncUploadTokenCompletionObserver
+    : public AsyncPixelTransferCompletionObserver {
+ public:
+  AsyncUploadTokenCompletionObserver(uint32 async_upload_token)

[piman, 2014/02/20 01:52:37]

nit: explicit

+      : async_upload_token_(async_upload_token) {
+  }
+
+  virtual void DidComplete(const AsyncMemoryParams& mem_params) OVERRIDE {
+    void* data = static_cast<int8*>(mem_params.shared_memory->memory()) +
+        mem_params.shm_data_offset;
+    AsyncUploadSync* sync = static_cast<AsyncUploadSync*>(data);
+    sync->SetAsyncUploadToken(async_upload_token_);
+  }
+
+ private:
+  uint32 async_upload_token_;

[piman, 2014/02/20 01:52:37]

nit: DISALLOW_COPY_AND_ASSIGN

+};
+
 // }  // anonymous namespace.
 
 bool GLES2Decoder::GetServiceTextureId(uint32 client_texture_id,
@@ -673,6 +692,13 @@ class GLES2DecoderImpl : public GLES2Decoder,
   bool GenVertexArraysOESHelper(GLsizei n, const GLuint* client_ids);
   void DeleteVertexArraysOESHelper(GLsizei n, const GLuint* client_ids);
 
+  // Helper for async upload token completion notification callback.
+  base::Closure AsyncUploadTokenCompletionClosure(uint32 async_upload_token,
+                                                  uint32 sync_data_shm_id,
+                                                  uint32 sync_data_shm_offset);
+
+
+
   // Workarounds
   void OnFboChanged() const;
   void OnUseFramebuffer() const;
@@ -10290,6 +10316,29 @@ bool GLES2DecoderImpl::ValidateAsyncTransfer(
   return true;
 }
 
+base::Closure GLES2DecoderImpl::AsyncUploadTokenCompletionClosure(
+    uint32 async_upload_token,
+    uint32 sync_data_shm_id,
+    uint32 sync_data_shm_offset) {
+  AsyncMemoryParams mem_params;
+  gpu::Buffer buffer = GetSharedMemoryBuffer(sync_data_shm_id);
+  if (!buffer.shared_memory)
+    return base::Closure();
+  mem_params.shared_memory = buffer.shared_memory;
+  mem_params.shm_size = buffer.size;
+  mem_params.shm_data_offset = sync_data_shm_offset;
+  mem_params.shm_data_size = sizeof(AsyncUploadSync);

[piman, 2014/02/20 01:52:37]

There's a DCHECK in AsyncNotifyCompletion that says it expects
shm_data_offset+shm_data_size <= shm_size

For security we need an overflow-safe check on the size.

[jadahl, 2014/02/20 10:24:22]

This seems to be missing for the mem_params passed to
delegate->AsyncTex(Sub)Image2D as well, or am I missing something?

Considering how we should handle these errors. As it is now, if
GetSharedMemoryBuffer() returns a buffer with shared_memory == NULL, or if we'd
add a this check, we'd not free the async upload buffer on the client side until
(and only if) we either loose context or successfully queue another upload.

I guess that the cases we'd handle here are misbehaving clients, and we'd more
or less make them misbehave some more this way. Any preferences in how to deal
with this?

[epennerAtGoogle, 2014/02/20 19:15:06]

It's checked in in ValidateTexSubImage2D. The 'pixels' should be null if the
memory is invalid in any way for the operation. I'll find where that null is
coming from though. For any of these we should return the same error as
ValidateTexSubImage2D

[epennerAtGoogle, 2014/02/20 19:20:07]

It's in GetSharedMemoryAs:
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/command_buffer...

+
+  scoped_refptr<AsyncUploadTokenCompletionObserver> observer(
+      new AsyncUploadTokenCompletionObserver(async_upload_token));
+
+  return base::Bind(
+      &AsyncPixelTransferManager::AsyncNotifyCompletion,
+      base::Unretained(GetAsyncPixelTransferManager()),
+      mem_params,
+      observer);
+}
+
 error::Error GLES2DecoderImpl::HandleAsyncTexImage2DCHROMIUM(
     uint32 immediate_data_size, const cmds::AsyncTexImage2DCHROMIUM& c) {
   TRACE_EVENT0("gpu", "GLES2DecoderImpl::HandleAsyncTexImage2DCHROMIUM");
@@ -10306,6 +10355,21 @@ error::Error GLES2DecoderImpl::HandleAsyncTexImage2DCHROMIUM(
   uint32 pixels_shm_id = static_cast<uint32>(c.pixels_shm_id);
   uint32 pixels_shm_offset = static_cast<uint32>(c.pixels_shm_offset);
   uint32 pixels_size;
+  uint32 async_upload_token = static_cast<uint32>(c.async_upload_token);
+  uint32 sync_data_shm_id = static_cast<uint32>(c.sync_data_shm_id);
+  uint32 sync_data_shm_offset = static_cast<uint32>(c.sync_data_shm_offset);
+
+  base::ScopedClosureRunner scoped_completion_callback;
+  if (async_upload_token) {
+    base::Closure completion_closure =
+        AsyncUploadTokenCompletionClosure(async_upload_token,
+                                          sync_data_shm_id,
+                                          sync_data_shm_offset);
+    if (completion_closure.is_null())
+      return error::kInvalidArguments;
+
+    scoped_completion_callback.Reset(completion_closure);
+  }
 
   // TODO(epenner): Move this and copies of this memory validation
   // into ValidateTexImage2D step.
@@ -10400,6 +10464,21 @@ error::Error GLES2DecoderImpl::HandleAsyncTexSubImage2DCHROMIUM(
   GLsizei height = static_cast<GLsizei>(c.height);
   GLenum format = static_cast<GLenum>(c.format);
   GLenum type = static_cast<GLenum>(c.type);
+  uint32 async_upload_token = static_cast<uint32>(c.async_upload_token);
+  uint32 sync_data_shm_id = static_cast<uint32>(c.sync_data_shm_id);
+  uint32 sync_data_shm_offset = static_cast<uint32>(c.sync_data_shm_offset);
+
+  base::ScopedClosureRunner scoped_completion_callback;
+  if (async_upload_token) {
+    base::Closure completion_closure =
+        AsyncUploadTokenCompletionClosure(async_upload_token,
+                                          sync_data_shm_id,
+                                          sync_data_shm_offset);
+    if (completion_closure.is_null())
+      return error::kInvalidArguments;
+
+    scoped_completion_callback.Reset(completion_closure);
+  }
 
   // TODO(epenner): Move this and copies of this memory validation
   // into ValidateTexSubImage2D step.
@@ -10510,6 +10589,15 @@ error::Error GLES2DecoderImpl::HandleWaitAsyncTexImage2DCHROMIUM(
   return error::kNoError;
 }
 
+error::Error GLES2DecoderImpl::HandleWaitAllAsyncTexImage2DCHROMIUM(
+    uint32 immediate_data_size, const cmds::WaitAllAsyncTexImage2DCHROMIUM& c) {
+  TRACE_EVENT0("gpu", "GLES2DecoderImpl::HandleWaitAsyncTexImage2DCHROMIUM");
+
+  GetAsyncPixelTransferManager()->WaitAllAsyncTexImage2D();
+  ProcessFinishedAsyncTransfers();
+  return error::kNoError;
+}
+
 void GLES2DecoderImpl::OnTextureRefDetachedFromFramebuffer(
     TextureRef* texture_ref) {
   Texture* texture = texture_ref->texture();