media: GpuMemoryBufferVideoFramePool uses BindToCurrentLoop()

media/video/gpu_memory_buffer_video_frame_pool.cc

Index: media/video/gpu_memory_buffer_video_frame_pool.cc
diff --git a/media/video/gpu_memory_buffer_video_frame_pool.cc b/media/video/gpu_memory_buffer_video_frame_pool.cc
index bdc2b39a0040d57129d00acab5b1d79b2c245dcb..0a26be5c0bdda947392c87d744ff569e7b49fb31 100644
--- a/media/video/gpu_memory_buffer_video_frame_pool.cc
+++ b/media/video/gpu_memory_buffer_video_frame_pool.cc
@@ -24,6 +24,7 @@
 #include "base/trace_event/trace_event.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/renderers/gpu_video_accelerator_factories.h"
 #include "third_party/libyuv/include/libyuv.h"
 #include "ui/gfx/buffer_format_util.h"
@@ -139,14 +140,10 @@ class GpuMemoryBufferVideoFramePool::PoolImpl
 
   // Callback called when a VideoFrame generated with GetFrameResources is no
   // longer referenced.
-  // This could be called by any thread.
+  // This could be called on the thread where |media_task_runner_| is current.

[Daniele Castagna, 2016/03/21 18:45:19]

nit: s/could/must.

[dshwang, 2016/03/21 19:04:38]

Done.

   void MailboxHoldersReleased(FrameResources* frame_resources,
                               const gpu::SyncToken& sync_token);
 
-  // Return frame resources to the pool. This has to be called on the thread
-  // where |media_task_runner_| is current.
-  void ReturnFrameResources(FrameResources* frame_resources);
-
   // Delete resources. This has to be called on the thread where |task_runner|
   // is current.
   static void DeleteFrameResources(GpuVideoAcceleratorFactories* gpu_factories,
@@ -589,8 +586,8 @@ void GpuMemoryBufferVideoFramePool::PoolImpl::
 
   scoped_refptr<VideoFrame> frame;
 
-  auto release_mailbox_callback =
-      base::Bind(&PoolImpl::MailboxHoldersReleased, this, frame_resources);
+  auto release_mailbox_callback = BindToCurrentLoop(
+      base::Bind(&PoolImpl::MailboxHoldersReleased, this, frame_resources));

[dshwang, 2016/03/21 18:31:23]

No, this patch doesn't have a race, because I use BindToCurrentLoop().
BindToCurrentLoop() is helper function to avoid two redundant callback. :)

[Daniele Castagna, 2016/03/21 18:45:19]

Ooh, sorry, I missed the BindToCurrentLoop. This is definitely better than the
helper function.
Do you mind mentioning it in the CL description?

[dshwang, 2016/03/21 19:04:38]

Done.

 
   // Create the VideoFrame backed by native textures.
   gfx::Size visible_size = video_frame->visible_rect().size();
@@ -732,18 +729,11 @@ void GpuMemoryBufferVideoFramePool::PoolImpl::DeleteFrameResources(
 }
 
 // Called when a VideoFrame is no longer references.

[Daniele Castagna, 2016/03/21 18:45:19]

nit: s/references/referenced.

[dshwang, 2016/03/21 19:04:38]

Done.

+// Put back the resources in the pool.
 void GpuMemoryBufferVideoFramePool::PoolImpl::MailboxHoldersReleased(
     FrameResources* frame_resources,
-    const gpu::SyncToken& sync_token) {
-  // Return the resource on the media thread.
-  media_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&PoolImpl::ReturnFrameResources, this, frame_resources));
-}
-
-// Put back the resources in the pool.
-void GpuMemoryBufferVideoFramePool::PoolImpl::ReturnFrameResources(
-    FrameResources* frame_resources) {
+    const gpu::SyncToken& release_sync_token) {
+  DCHECK(media_task_runner_->BelongsToCurrentThread());
   auto it = std::find(resources_pool_.begin(), resources_pool_.end(),
                       frame_resources);
   DCHECK(it != resources_pool_.end());
@@ -751,6 +741,8 @@ void GpuMemoryBufferVideoFramePool::PoolImpl::ReturnFrameResources(
   // This minimizes the chances of locking the buffer that might be
   // still needed for drawing.
   std::swap(*it, resources_pool_.back());
+
+  gpu_factories_->WaitSyncToken(release_sync_token);
   frame_resources->SetIsInUse(false);
 }