gpu: Clean up pending async transfers.

Async transfers are currently tracked in the texture
manager. This gives less flexibility to the async
implementation, and has much higher coupling between
the decoder/async-delegate. This patch moves everything
into the delegate.

BUG=161337

Review URL: https://chromiumcodereview.appspot.com/12330118

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187790 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 89 insertions, 133 deletions

diff --git a/gpu/command_buffer/service/async_pixel_transfer_delegate_mock.h b/gpu/command_buffer/service/async_pixel_transfer_delegate_mock.h
index 7432737..bc72269 100644
--- a/gpu/command_buffer/service/async_pixel_transfer_delegate_mock.h
+++ b/gpu/command_buffer/service/async_pixel_transfer_delegate_mock.h
@@ -30,15 +30,18 @@ class MockAsyncPixelTransferDelegate : public gfx::AsyncPixelTransferDelegate {
   virtual ~MockAsyncPixelTransferDelegate();
 
   // Implement AsyncPixelTransferDelegate.
-  MOCK_METHOD1(CreateRawPixelTransferState,
-      gfx::AsyncPixelTransferState*(GLuint service_id));
+  MOCK_METHOD2(CreateRawPixelTransferState,
+      gfx::AsyncPixelTransferState*(
+          GLuint service_id, const AsyncTexImage2DParams& define_params));
+  MOCK_METHOD0(BindCompletedAsyncTransfers, bool());
   MOCK_METHOD2(AsyncNotifyCompletion,
       void(const AsyncMemoryParams& mem_params,
            const CompletionCallback& callback));
-  MOCK_METHOD3(AsyncTexImage2D,
+  MOCK_METHOD4(AsyncTexImage2D,
       void(gfx::AsyncPixelTransferState*,
           const AsyncTexImage2DParams& tex_params,
-          const AsyncMemoryParams& mem_params));
+          const AsyncMemoryParams& mem_params,
+          const base::Closure& bind_callback));
   MOCK_METHOD3(AsyncTexSubImage2D,
       void(gfx::AsyncPixelTransferState*,
           const AsyncTexSubImage2DParams& tex_params,
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 94de0f5..1d642fd 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -2857,23 +2857,13 @@ void GLES2DecoderImpl::ProcessFinishedAsyncTransfers() {
   if (engine() && query_manager_.get())
     query_manager_->ProcessPendingTransferQueries();
 
-  // TODO(epenner): Is there a better place to do this? Transfers
-  // can complete any time we yield the main thread. So we *must*
-  // process transfers after any such yield, before resuming.
-  bool frame_buffer_dirty = false;
-  bool texture_dirty = false;
-  texture_manager()->BindFinishedAsyncPixelTransfers(
-      &texture_dirty, &frame_buffer_dirty);
-  // Texture unit zero might be stomped.
-  if (texture_dirty)
+  // TODO(epenner): Is there a better place to do this?
+  // This needs to occur before we execute any batch of commands
+  // from the client, as the client may have recieved an async
+  // completion while issuing those commands.
+  // "DidFlushStart" would be ideal if we had such a callback.
+  if (async_pixel_transfer_delegate_->BindCompletedAsyncTransfers())
     RestoreCurrentTexture2DBindings();
-  // A texture attached to frame-buffer might have changed size.
-  if (frame_buffer_dirty) {
-    clear_state_dirty_ = true;
-    // TODO(gman): If textures tracked which framebuffers they were attached to
-    // we could just mark those framebuffers as not complete.
-    framebuffer_manager()->IncFramebufferStateChangeCount();
-  }
 }
 
 void GLES2DecoderImpl::ReleaseCurrent() {
@@ -10197,6 +10187,15 @@ error::Error GLES2DecoderImpl::HandleAsyncTexImage2DCHROMIUM(
     return error::kNoError;
   }
 
+  // Since we don't allow async redefinition, this is the only
+  // time the size of this texture can change while bound
+  // as a frame-buffer.
+  if (texture->IsAttachedToFramebuffer()) {
+    // TODO(gman): If textures tracked which framebuffers they were attached to
+    // we could just mark those framebuffers as not complete.
+    framebuffer_manager()->IncFramebufferStateChangeCount();
+  }
+
   if (!EnsureGPUMemoryAvailable(pixels_size)) {
     LOCAL_SET_GL_ERROR(
         GL_OUT_OF_MEMORY, "glAsyncTexImage2DCHROMIUM", "out of memory");
@@ -10211,16 +10210,7 @@ error::Error GLES2DecoderImpl::HandleAsyncTexImage2DCHROMIUM(
   uint32 shm_data_offset = c.pixels_shm_offset;
   uint32 shm_data_size = pixels_size;
 
-  // Set up the async state if needed, and make the texture
-  // immutable so the async state stays valid. The level texture
-  // is set up lazily when the transfer completes.
-  DCHECK(!texture->GetAsyncTransferState());
-  texture->SetAsyncTransferState(
-      async_pixel_transfer_delegate_->
-          CreatePixelTransferState(texture->service_id()));
-  texture->SetImmutable(true);
-
-  // Issue the async call and set up the texture.
+  // Setup the parameters.
   GLenum gl_internal_format =
       GetTexInternalFormat(internal_format, format, type);
   gfx::AsyncTexImage2DParams tex_params = {target, level, gl_internal_format,
@@ -10228,14 +10218,27 @@ error::Error GLES2DecoderImpl::HandleAsyncTexImage2DCHROMIUM(
   gfx::AsyncMemoryParams mem_params = {shared_memory, shm_size,
                                        shm_data_offset, shm_data_size};
 
-  // Add a pending transfer to the texture manager, which will bind the
-  // transfer data to the texture and set the level texture at the same time,
-  // after the the transfer is complete.
-  texture_manager()->AddPendingAsyncPixelTransfer(
-      texture->GetAsyncTransferState()->AsWeakPtr(), texture);
+  // Set up the async state if needed, and make the texture
+  // immutable so the async state stays valid. The level info
+  // is set up lazily when the transfer completes.
+  DCHECK(!texture->GetAsyncTransferState());
+  texture->SetAsyncTransferState(
+      async_pixel_transfer_delegate_->
+          CreatePixelTransferState(texture->service_id(),
+                                   tex_params));
+  texture->SetImmutable(true);
 
   async_pixel_transfer_delegate_->AsyncTexImage2D(
-      texture->GetAsyncTransferState(), tex_params, mem_params);
+      texture->GetAsyncTransferState(),
+      tex_params,
+      mem_params,
+      base::Bind(&TextureManager::SetLevelInfoFromParams,
+                 // The callback is only invoked if the transfer state
+                 // still exists, which implies through manager->info->state
+                 // ownership that both of these pointers are valid.
+                 base::Unretained(texture_manager()),
+                 base::Unretained(texture),
+                 tex_params));
   return error::kNoError;
 }
 
@@ -10298,19 +10301,30 @@ error::Error GLES2DecoderImpl::HandleAsyncTexSubImage2DCHROMIUM(
   uint32 shm_data_offset = c.data_shm_offset;
   uint32 shm_data_size = data_size;
 
+  // Setup the parameters.
+  gfx::AsyncTexSubImage2DParams tex_params = {target, level, xoffset, yoffset,
+                                              width, height, format, type};
+  gfx::AsyncMemoryParams mem_params = {shared_memory, shm_size,
+                                       shm_data_offset, shm_data_size};
   if (!texture->GetAsyncTransferState()) {
+    // TODO(epenner): We may want to enforce exclusive use
+    // of async APIs in which case this should become an error,
+    // (the texture should have been async defined).
+    gfx::AsyncTexImage2DParams define_params = {target, level,
+                                                0, 0, 0, 0, 0, 0};
+    texture->GetLevelSize(target, level, &define_params.width,
+                                         &define_params.height);
+    texture->GetLevelType(target, level, &define_params.type,
+                                         &define_params.internal_format);
     // Set up the async state if needed, and make the texture
     // immutable so the async state stays valid.
     texture->SetAsyncTransferState(
         async_pixel_transfer_delegate_->
-            CreatePixelTransferState(texture->service_id()));
+            CreatePixelTransferState(texture->service_id(),
+                                     define_params));
     texture->SetImmutable(true);
   }
 
-  gfx::AsyncTexSubImage2DParams tex_params = {target, level, xoffset, yoffset,
-                                              width, height, format, type};
-  gfx::AsyncMemoryParams mem_params = {shared_memory, shm_size,
-                                       shm_data_offset, shm_data_size};
   async_pixel_transfer_delegate_->AsyncTexSubImage2D(
       texture->GetAsyncTransferState(), tex_params, mem_params);
   return error::kNoError;
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index e4dd9cb..a385f81 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -38,6 +38,7 @@ using ::testing::Invoke;
 using ::testing::MatcherCast;
 using ::testing::Pointee;
 using ::testing::Return;
+using ::testing::SaveArg;
 using ::testing::SetArrayArgument;
 using ::testing::SetArgumentPointee;
 using ::testing::SetArgPointee;
@@ -8000,20 +8001,21 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
                       GL_UNSIGNED_BYTE, kSharedMemoryId, kSharedMemoryOffset);
   WaitAsyncTexImage2DCHROMIUM wait_cmd;
   wait_cmd.Init(GL_TEXTURE_2D);
-  gfx::AsyncTexImage2DParams teximage_params =
-      {GL_TEXTURE_2D, 0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE};
 
   // No transfer state exists initially.
   EXPECT_FALSE(texture->GetAsyncTransferState());
 
+  base::Closure bind_callback;
+
   // AsyncTexImage2D
   {
     // Create transfer state since it doesn't exist.
-    EXPECT_CALL(*delegate, CreateRawPixelTransferState(kServiceTextureId))
+    EXPECT_CALL(*delegate, CreateRawPixelTransferState(kServiceTextureId, _))
         .WillOnce(Return(
             state = new StrictMock<gfx::MockAsyncPixelTransferState>))
         .RetiresOnSaturation();
-    EXPECT_CALL(*delegate, AsyncTexImage2D(state, _, _))
+    EXPECT_CALL(*delegate, AsyncTexImage2D(state, _, _, _))
+        .WillOnce(SaveArg<3>(&bind_callback))
         .RetiresOnSaturation();
     // Command succeeds.
     EXPECT_EQ(error::kNoError, ExecuteCmd(teximage_cmd));
@@ -8035,24 +8037,17 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
     EXPECT_TRUE(texture->SafeToRenderFrom());
   }
 
-  // Lazy binding/defining of the async transfer
+  // Binding/defining of the async transfer
   {
-    // We the code should check that the transfer is done,
-    // call bind transfer on it, and update the texture texture.
-    InSequence scoped_in_sequence;
-    EXPECT_CALL(*state, TransferIsInProgress())
-        .WillOnce(Return(false))
-        .RetiresOnSaturation();
-    EXPECT_CALL(*state, BindTransfer(_))
-        .WillOnce(SetArgPointee<0>(teximage_params))
-        .RetiresOnSaturation();
-    TextureManager* manager = decoder_->GetContextGroup()->texture_manager();
-    bool texture_dirty, framebuffer_dirty;
-    manager->BindFinishedAsyncPixelTransfers(&texture_dirty,
-                                             &framebuffer_dirty);
-    EXPECT_TRUE(texture_dirty);
-    EXPECT_FALSE(framebuffer_dirty);
-    // Texture is safe, and has the right size etc.
+    // TODO(epenner): We should check that the delegate gets the
+    // BindCompletedAsyncTransfers() call, which is required to
+    // guarantee the delegate calls the bind callback.
+
+    // Simulate the bind callback from the delegate.
+    bind_callback.Run();
+
+    // After the bind callback is run, the texture is safe,
+    // and has the right size etc.
     EXPECT_TRUE(texture->SafeToRenderFrom());
     GLsizei width, height;
     EXPECT_TRUE(texture->GetLevelSize(GL_TEXTURE_2D, 0, &width, &height));
@@ -8065,7 +8060,7 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
   texture->SetImmutable(false);
   {
     // Create transfer state since it doesn't exist.
-    EXPECT_CALL(*delegate, CreateRawPixelTransferState(kServiceTextureId))
+    EXPECT_CALL(*delegate, CreateRawPixelTransferState(kServiceTextureId, _))
         .WillOnce(Return(
             state = new StrictMock<gfx::MockAsyncPixelTransferState>))
         .RetiresOnSaturation();
@@ -8119,11 +8114,11 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
     texture->SetAsyncTransferState(scoped_ptr<gfx::AsyncPixelTransferState>());
     texture->SetImmutable(false);
     // Create transfer state since it doesn't exist.
-    EXPECT_CALL(*delegate, CreateRawPixelTransferState(kServiceTextureId))
+    EXPECT_CALL(*delegate, CreateRawPixelTransferState(kServiceTextureId, _))
         .WillOnce(Return(
             state = new StrictMock<gfx::MockAsyncPixelTransferState>))
         .RetiresOnSaturation();
-    EXPECT_CALL(*delegate, AsyncTexImage2D(state, _, _))
+    EXPECT_CALL(*delegate, AsyncTexImage2D(state, _, _, _))
         .RetiresOnSaturation();
     // Start async transfer.
     EXPECT_EQ(error::kNoError, ExecuteCmd(teximage_cmd));
@@ -8132,11 +8127,8 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
     EXPECT_TRUE(texture->IsImmutable());
     // Wait for completion.
     EXPECT_CALL(*delegate, WaitForTransferCompletion(state));
-    EXPECT_CALL(*state, TransferIsInProgress())
-        .WillOnce(Return(false))
-        .RetiresOnSaturation();
-    EXPECT_CALL(*state, BindTransfer(_))
-        .WillOnce(SetArgPointee<0>(teximage_params))
+    EXPECT_CALL(*delegate, BindCompletedAsyncTransfers())
+        .WillOnce(Return(true))
         .RetiresOnSaturation();
     // State restoration after binding.
     EXPECT_CALL(*gl_, ActiveTexture(_));
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 4b6fe58..8f72cf4 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -1230,53 +1230,5 @@ void TextureManager::AddToSignature(
   texture->AddToSignature(feature_info_.get(), target, level, signature);
 }
 
-void TextureManager::AddPendingAsyncPixelTransfer(
-    base::WeakPtr<gfx::AsyncPixelTransferState> state, Texture* texture) {
-  pending_async_transfers_.push_back(PendingAsyncTransfer(state, texture));
-}
-
-void TextureManager::BindFinishedAsyncPixelTransfers(
-    bool* texture_dirty, bool* framebuffer_dirty) {
-  DCHECK(texture_dirty);
-  DCHECK(framebuffer_dirty);
-  *texture_dirty = false;
-  *framebuffer_dirty = false;
-
-  // Remove finished transfers from the list, while
-  // marking whether texture unit 0 or frame_buffer status is dirty.
-  while(!pending_async_transfers_.empty()) {
-    PendingAsyncTransfer state_info = pending_async_transfers_.front();
-    if (!state_info.first.get()) {
-      // The AsyncState is owned by the Texture. So if the
-      // async state is deleted, so is the Texture.
-      pending_async_transfers_.pop_front();
-      continue;
-    }
-    // Terminate early, as all transfers finish in order.
-    if (state_info.first->TransferIsInProgress())
-      break;
-    // If the transfer is finished, bind it to the texture,
-    // update the Texture, and remove it from pending list.
-    *texture_dirty = true;
-    *framebuffer_dirty |= state_info.second->IsAttachedToFramebuffer();
-    gfx::AsyncTexImage2DParams tex_define_params;
-    state_info.second->
-        GetAsyncTransferState()->BindTransfer(&tex_define_params);
-    SetLevelInfo(
-        state_info.second,
-        tex_define_params.target,
-        tex_define_params.level,
-        tex_define_params.internal_format,
-        tex_define_params.width,
-        tex_define_params.height,
-        1, // depth
-        tex_define_params.border,
-        tex_define_params.format,
-        tex_define_params.type,
-        true); // cleared
-    pending_async_transfers_.pop_front();
-  }
-}
-
 }  // namespace gles2
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h
index 18dda7a..f1d1fea 100644
--- a/gpu/command_buffer/service/texture_manager.h
+++ b/gpu/command_buffer/service/texture_manager.h
@@ -448,6 +448,16 @@ class GPU_EXPORT TextureManager {
       GLenum type,
       bool cleared);
 
+  // Adapter to call above function.
+  void SetLevelInfoFromParams(Texture* texture,
+                              const gfx::AsyncTexImage2DParams& params) {
+    SetLevelInfo(
+        texture, params.target, params.level, params.internal_format,
+        params.width, params.height, 1 /* depth */,
+        params.border, params.format,
+        params.type, true /* cleared */ );
+  }
+
   // Save the texture definition and leave it undefined.
   TextureDefinition* Save(Texture* texture);
 
@@ -554,13 +564,6 @@ class GPU_EXPORT TextureManager {
       GLint level,
       std::string* signature) const;
 
-  // Transfers added will get their Texture updated at the same time
-  // the async transfer is bound to the real texture.
-  void AddPendingAsyncPixelTransfer(
-      base::WeakPtr<gfx::AsyncPixelTransferState> state, Texture* texture);
-  void BindFinishedAsyncPixelTransfers(bool* texture_dirty,
-                                       bool* framebuffer_dirty);
-
  private:
   friend class Texture;
 
@@ -605,14 +608,6 @@ class GPU_EXPORT TextureManager {
   // The default textures for each target (texture name = 0)
   scoped_refptr<Texture> default_textures_[kNumDefaultTextures];
 
-  // Async texture allocations which haven't been bound to their textures
-  // yet. This facilitates updating the Texture at the same time the
-  // real texture data is bound.
-  typedef std::pair<base::WeakPtr<gfx::AsyncPixelTransferState>,
-                    Texture*> PendingAsyncTransfer;
-  typedef std::list<PendingAsyncTransfer> PendingAsyncTransferList;
-  PendingAsyncTransferList pending_async_transfers_;
-
   DISALLOW_COPY_AND_ASSIGN(TextureManager);
 };