Remove threading from RendererGpuVideoAcceleratorFactories

content/renderer/media/renderer_gpu_video_accelerator_factories.cc

Index: content/renderer/media/renderer_gpu_video_accelerator_factories.cc
diff --git a/content/renderer/media/renderer_gpu_video_accelerator_factories.cc b/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
index 86d40cd960b67deb86d3dc954cd3c8d96bb6a687..3e8e8280158bfd4407dfc933d75871bd71d2bb34 100644
--- a/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
+++ b/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
@@ -14,6 +14,7 @@
 #include "content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h"
 #include "content/renderer/render_thread_impl.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkPixelRef.h"
 
 namespace content {
@@ -26,33 +27,22 @@ RendererGpuVideoAcceleratorFactories::RendererGpuVideoAcceleratorFactories(
           RenderThreadImpl::current()->GetMediaThreadMessageLoopProxy()),
       gpu_channel_host_(gpu_channel_host),
       context_provider_(context_provider),
-      thread_safe_sender_(ChildThread::current()->thread_safe_sender()),
-      aborted_waiter_(true, false),
-      message_loop_async_waiter_(false, false) {
+      thread_safe_sender_(ChildThread::current()->thread_safe_sender()) {
   // |context_provider_| is only required to support HW-accelerated decode.
   if (!context_provider_)
     return;
 
   if (message_loop_->BelongsToCurrentThread()) {

[Ami GONE FROM CHROMIUM, 2013/10/22 17:21:19]

This is never the case, right?
But I guess you want to maintain the flexibility of the ctor being callable on
any thread to allow either creation on the compositing thread (for GVD) or the
libjingle worker thread (for RVD)?

[sheu, 2013/10/24 01:16:39]

Superseded (see below)

     AsyncBindContext();
-    message_loop_async_waiter_.Reset();
     return;
   }
   // Wait for the context to be acquired.

[wuchengli, 2013/10/21 05:59:01]

Update or remove the comment. This doesn't wait anymore.

[sheu, 2013/10/24 01:16:39]

Superseded (see below)

   message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&RendererGpuVideoAcceleratorFactories::AsyncBindContext,

[Ami GONE FROM CHROMIUM, 2013/10/22 17:21:19]

Would it make sense to have RenderThreadImpl::GetGpuFactories() PostTask to the
media thread to BindToCurrentThread instead of monkeying around with conditional
trampolining here?

[sheu, 2013/10/24 01:16:39]

I'll do that actually.  And along the way undo a previous change suggested by
scherkus@ :-P

-                 // Unretained to avoid ref/deref'ing |*this|, which is not yet
-                 // stored in a scoped_refptr.  Safe because the Wait() below
-                 // keeps us alive until this task completes.
-                 base::Unretained(this)));
-  message_loop_async_waiter_.Wait();
+                 this));
 }
 
-RendererGpuVideoAcceleratorFactories::RendererGpuVideoAcceleratorFactories()
-    : aborted_waiter_(true, false),
-      message_loop_async_waiter_(false, false) {}
-
 WebGraphicsContext3DCommandBufferImpl*
 RendererGpuVideoAcceleratorFactories::GetContext3d() {
   DCHECK(message_loop_->BelongsToCurrentThread());
@@ -72,40 +62,21 @@ void RendererGpuVideoAcceleratorFactories::AsyncBindContext() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   if (!context_provider_->BindToCurrentThread())
     context_provider_ = NULL;
-  message_loop_async_waiter_.Signal();
 }
 
 scoped_ptr<media::VideoDecodeAccelerator>
 RendererGpuVideoAcceleratorFactories::CreateVideoDecodeAccelerator(
     media::VideoCodecProfile profile,
     media::VideoDecodeAccelerator::Client* client) {
-  if (message_loop_->BelongsToCurrentThread()) {
-    AsyncCreateVideoDecodeAccelerator(profile, client);
-    message_loop_async_waiter_.Reset();
-    return vda_.Pass();
-  }
-  // The VDA is returned in the vda_ member variable by the
-  // AsyncCreateVideoDecodeAccelerator() function.
-  message_loop_->PostTask(FROM_HERE,
-                          base::Bind(&RendererGpuVideoAcceleratorFactories::
-                                         AsyncCreateVideoDecodeAccelerator,
-                                     this,
-                                     profile,
-                                     client));
-
-  base::WaitableEvent* objects[] = {&aborted_waiter_,
-                                    &message_loop_async_waiter_};
-  if (base::WaitableEvent::WaitMany(objects, arraysize(objects)) == 0) {
-    // If we are aborting and the VDA is created by the
-    // AsyncCreateVideoDecodeAccelerator() function later we need to ensure
-    // that it is destroyed on the same thread.
-    message_loop_->PostTask(FROM_HERE,
-                            base::Bind(&RendererGpuVideoAcceleratorFactories::
-                                           AsyncDestroyVideoDecodeAccelerator,
-                                       this));
-    return scoped_ptr<media::VideoDecodeAccelerator>();
+  DCHECK(message_loop_->BelongsToCurrentThread());
+
+  WebGraphicsContext3DCommandBufferImpl* context = GetContext3d();
+  if (context && context->GetCommandBufferProxy()) {
+    return gpu_channel_host_->CreateVideoDecoder(
+        context->GetCommandBufferProxy()->GetRouteID(), profile, client);
   }
-  return vda_.Pass();
+
+  return scoped_ptr<media::VideoDecodeAccelerator>();
 }
 
 scoped_ptr<media::VideoEncodeAccelerator>
@@ -116,19 +87,6 @@ RendererGpuVideoAcceleratorFactories::CreateVideoEncodeAccelerator(
   return gpu_channel_host_->CreateVideoEncoder(client);
 }
 
-void RendererGpuVideoAcceleratorFactories::AsyncCreateVideoDecodeAccelerator(
-    media::VideoCodecProfile profile,
-    media::VideoDecodeAccelerator::Client* client) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-
-  WebGraphicsContext3DCommandBufferImpl* context = GetContext3d();
-  if (context && context->GetCommandBufferProxy()) {
-    vda_ = gpu_channel_host_->CreateVideoDecoder(
-        context->GetCommandBufferProxy()->GetRouteID(), profile, client);
-  }
-  message_loop_async_waiter_.Signal();
-}
-
 uint32 RendererGpuVideoAcceleratorFactories::CreateTextures(
     int32 count,
     const gfx::Size& size,
@@ -205,39 +163,11 @@ void RendererGpuVideoAcceleratorFactories::WaitSyncPoint(uint32 sync_point) {
 void RendererGpuVideoAcceleratorFactories::ReadPixels(uint32 texture_id,
                                                       const gfx::Size& size,
                                                       const SkBitmap& pixels) {
-  // SkBitmaps use the SkPixelRef object to refcount the underlying pixels.
-  // Multiple SkBitmaps can share a SkPixelRef instance. We use this to
-  // ensure that the underlying pixels in the SkBitmap passed in remain valid
-  // until the AsyncReadPixels() call completes.
-  read_pixels_bitmap_.setPixelRef(pixels.pixelRef());
-
-  if (!message_loop_->BelongsToCurrentThread()) {
-    message_loop_->PostTask(
-        FROM_HERE,
-        base::Bind(&RendererGpuVideoAcceleratorFactories::AsyncReadPixels,
-                   this,
-                   texture_id,
-                   size));
-    base::WaitableEvent* objects[] = {&aborted_waiter_,
-                                      &message_loop_async_waiter_};
-    if (base::WaitableEvent::WaitMany(objects, arraysize(objects)) == 0)
-      return;
-  } else {
-    AsyncReadPixels(texture_id, size);
-    message_loop_async_waiter_.Reset();
-  }
-  read_pixels_bitmap_.setPixelRef(NULL);
-}
-
-void RendererGpuVideoAcceleratorFactories::AsyncReadPixels(
-    uint32 texture_id,
-    const gfx::Size& size) {
   DCHECK(message_loop_->BelongsToCurrentThread());
+
   WebGraphicsContext3DCommandBufferImpl* context = GetContext3d();
-  if (!context) {
-    message_loop_async_waiter_.Signal();
+  if (!context)
     return;
-  }
 
   gpu::gles2::GLES2Implementation* gles2 = context->GetImplementation();
 
@@ -263,11 +193,10 @@ void RendererGpuVideoAcceleratorFactories::AsyncReadPixels(
                     size.height(),
                     GL_BGRA_EXT,
                     GL_UNSIGNED_BYTE,
-                    read_pixels_bitmap_.pixelRef()->pixels());
+                    pixels.pixelRef()->pixels());
   gles2->DeleteFramebuffers(1, &fb);
   gles2->DeleteTextures(1, &tmp_texture);
   DCHECK_EQ(gles2->GetError(), static_cast<GLenum>(GL_NO_ERROR));
-  message_loop_async_waiter_.Signal();
 }
 
 base::SharedMemory* RendererGpuVideoAcceleratorFactories::CreateSharedMemory(
@@ -281,28 +210,4 @@ RendererGpuVideoAcceleratorFactories::GetMessageLoop() {
   return message_loop_;
 }
 
-void RendererGpuVideoAcceleratorFactories::Abort() { aborted_waiter_.Signal(); }
-
-bool RendererGpuVideoAcceleratorFactories::IsAborted() {
-  return aborted_waiter_.IsSignaled();
-}
-
-scoped_refptr<RendererGpuVideoAcceleratorFactories>
-RendererGpuVideoAcceleratorFactories::Clone() {
-  scoped_refptr<RendererGpuVideoAcceleratorFactories> factories =
-      new RendererGpuVideoAcceleratorFactories();
-  factories->message_loop_ = message_loop_;
-  factories->gpu_channel_host_ = gpu_channel_host_;
-  factories->context_provider_ = context_provider_;
-  factories->thread_safe_sender_ = thread_safe_sender_;
-  return factories;
-}
-
-void
-RendererGpuVideoAcceleratorFactories::AsyncDestroyVideoDecodeAccelerator() {
-  // OK to release because Destroy() will delete the VDA instance.
-  if (vda_)
-    vda_.release()->Destroy();
-}
-
 }  // namespace content