Make RendererGpuVideoDecoderFactories live on arbitrary threads.

content/renderer/media/rtc_video_decoder_factory.cc

Index: content/renderer/media/rtc_video_decoder_factory.cc
diff --git a/content/renderer/media/rtc_video_decoder_factory.cc b/content/renderer/media/rtc_video_decoder_factory.cc
index 3c6b9ad5ac861ae7351c686c7328e157508d3a8d..cfc03b690bb0b34393f7ca8d1c96e301ceecdb99 100644
--- a/content/renderer/media/rtc_video_decoder_factory.cc
+++ b/content/renderer/media/rtc_video_decoder_factory.cc
@@ -11,43 +11,33 @@
 namespace content {
 
 RTCVideoDecoderFactory::RTCVideoDecoderFactory(
+    const scoped_refptr<base::MessageLoopProxy>& vda_loop_proxy,
     const scoped_refptr<media::GpuVideoDecoder::Factories>& gpu_factories)
-    : vda_loop_proxy_(gpu_factories->GetMessageLoop()) {
+    : vda_loop_proxy_(vda_loop_proxy), gpu_factories_(gpu_factories) {
   DVLOG(2) << "RTCVideoDecoderFactory";
-  // The decoder cannot be created in CreateVideoDecoder because VDA has to be
-  // created on |vda_loop_proxy_|, which can be the child thread. The child
-  // thread is blocked when CreateVideoDecoder runs. This supports only one
-  // VDA-powered <video> tag at a time.
-  // TODO(wuchengli): remove this restriction.

[Ami GONE FROM CHROMIUM, 2013/07/22 19:46:21]

Yay!

[wuchengli, 2013/07/23 16:29:28]

:)

-  // |decoder_| can be null if VDA does not support VP8.
-  decoder_ = RTCVideoDecoder::Create(gpu_factories);
 }
 
 RTCVideoDecoderFactory::~RTCVideoDecoderFactory() {
   DVLOG(2) << "~RTCVideoDecoderFactory";
-  if (decoder_) {
-    webrtc::VideoDecoder* decoder = decoder_.release();
-    if (!vda_loop_proxy_->DeleteSoon(FROM_HERE, decoder))
-      delete decoder;
-  }
 }
 
 webrtc::VideoDecoder* RTCVideoDecoderFactory::CreateVideoDecoder(
     webrtc::VideoCodecType type) {
   DVLOG(2) << "CreateVideoDecoder";
-  // Only VP8 is supported.
-  if (type == webrtc::kVideoCodecVP8)
-    return decoder_.release();
-  return NULL;
+  // RendererGpuVideoDecoderFactories is not thread safe. It cannot be shared
+  // by different decoders. This method runs on Chrome_libJingle_WorkerThread
+  // and the child thread is blocked while this runs. We cannot create new gpu
+  // factories here. Clone one instead.
+  scoped_ptr<RTCVideoDecoder> decoder =
+      RTCVideoDecoder::Create(type, vda_loop_proxy_, gpu_factories_->Clone());

[scherkus (not reviewing), 2013/07/22 17:56:40]

Considering this is a problem unique to content/ and RGVDF, I feel like Clone()
should live on RendererGpuVideoDecoderFactories as opposed to on the Factories
interface itself

This class can then hold onto a RGVDF and call its Clone() method.

This way the scope of Clone() is restricted to the places where it needs to be
called (i.e., here) instead of being accessible to the end users (i.e., RTCVD
and GVD)

[wuchengli, 2013/07/23 16:29:28]

Done. That's what I wasn't sure about in
https://codereview.chromium.org/19534002/diff/36001/content/renderer/render_t....
I've moved Clone() out of the interface and made
RenderThreadImpl::GetGpuFactories return RendererGpuVideoDecoderFactories.

+  return decoder.release();
 }
 
 void RTCVideoDecoderFactory::DestroyVideoDecoder(
     webrtc::VideoDecoder* decoder) {
   DVLOG(2) << "DestroyVideoDecoder";
-  // Save back the decoder because it is the only one. VideoDecoder::Release
-  // should have been called.
-  decoder->RegisterDecodeCompleteCallback(NULL);
-  decoder_.reset(decoder);
+  if (!vda_loop_proxy_->DeleteSoon(FROM_HERE, decoder))

[Ami GONE FROM CHROMIUM, 2013/07/22 19:46:21]

You might enjoy
https://plus.sandbox.google.com/104229030418342490750/posts/ezkMjb2k5b8

[wuchengli, 2013/07/23 16:29:28]

Haha. I copied the anti-pattern from other places.

I've removed "delete decoder" because media thread won't stop until this
returns.

+    delete decoder;
 }
 
 }  // namespace content