Eliminate video capture thread in renderer

The main motivation of this change is to remove the video capture thread
in the renderer. All users of a video capture device already handles the
video frame on their thread. There is no need to call the clients with
an additional thread.

Summary of this change:
* Video capture thread eliminated
  VideoCaptureImpl now runs on the IO thread. Clients are called on the
  IO thread.
* Simplified VideoCaptureImplManager
  We still need to keep this object for the purpose of sharing a
  VideoCaptureImpl object with multiple clients. It should own these
  objects and maintain the usage count. A couple clean up items are done
  on this class:
  * It doesn't own the video capture thread now.
  * It is now a render thread only object.
  * It maintains refcount of a VideoCaptureImpl explicitly.
  * It is no longer refcounted.
  * Clients access it through RenderThreadImpl. Which ensures usage is
    on the render thread.
* New VideoCaptureHandle class
  Object of this class is returned by VideoCaptureImplManager to give
  access to a media::VideoCapture object. It is purely a wrapper and
  helps to do refcounting on the render thread.

Testing:
Added unit tests for VideoCaptureImplManager to test refcounting.
Also updated unit test for VideoCaptureImpl due to the threading
changes.

Review URL: https://codereview.chromium.org/120893002

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

content/content_tests.gypi

@@ -541,6 +541,7 @@
         'renderer/media/media_stream_video_source_unittest.cc',
         'renderer/media/test_response_generator.cc',
         'renderer/media/test_response_generator.h',
+        'renderer/media/video_capture_impl_manager_unittest.cc',
         'renderer/media/video_capture_impl_unittest.cc',
         'renderer/media/video_capture_message_filter_unittest.cc',
         'renderer/media/video_destination_handler_unittest.cc',

content/renderer/media/media_stream_dependency_factory.cc

@@ -20,7 +20,6 @@
 #include "content/renderer/media/rtc_video_capturer.h"
 #include "content/renderer/media/rtc_video_decoder_factory.h"
 #include "content/renderer/media/rtc_video_encoder_factory.h"
-#include "content/renderer/media/video_capture_impl_manager.h"
 #include "content/renderer/media/webaudio_capturer_source.h"
 #include "content/renderer/media/webrtc_audio_device_impl.h"
 #include "content/renderer/media/webrtc_local_audio_track.h"
@@ -241,13 +240,11 @@ class SourceStateObserver : public webrtc::ObserverInterface,
 };
 
 MediaStreamDependencyFactory::MediaStreamDependencyFactory(
-    VideoCaptureImplManager* vc_manager,
     P2PSocketDispatcher* p2p_socket_dispatcher)
     : network_manager_(NULL),
 #if defined(GOOGLE_TV)
       decoder_factory_tv_(NULL),
 #endif
-      vc_manager_(vc_manager),
       p2p_socket_dispatcher_(p2p_socket_dispatcher),
       signaling_thread_(NULL),
       worker_thread_(NULL),
@@ -730,7 +727,7 @@ MediaStreamDependencyFactory::CreateLocalVideoSource(
     bool is_screencast,
     const webrtc::MediaConstraintsInterface* constraints) {
   RtcVideoCapturer* capturer = new RtcVideoCapturer(
-      video_session_id, vc_manager_.get(), is_screencast);
+      video_session_id, is_screencast);
 
   // The video source takes ownership of |capturer|.
   scoped_refptr<webrtc::VideoSourceInterface> source =

content/renderer/media/media_stream_dependency_factory.h

@@ -46,7 +46,6 @@ namespace content {
 class IpcNetworkManager;
 class IpcPacketSocketFactory;
 class RTCMediaConstraints;
-class VideoCaptureImplManager;
 class WebAudioCapturerSource;
 class WebRtcAudioCapturer;
 class WebRtcAudioDeviceImpl;
@@ -67,7 +66,6 @@ class CONTENT_EXPORT MediaStreamDependencyFactory
   typedef base::Callback<void(blink::WebMediaStream* web_stream,
                               bool live)> MediaSourcesCreatedCallback;
   MediaStreamDependencyFactory(
-      VideoCaptureImplManager* vc_manager,
       P2PSocketDispatcher* p2p_socket_dispatcher);
   virtual ~MediaStreamDependencyFactory();
 
@@ -255,7 +253,6 @@ class CONTENT_EXPORT MediaStreamDependencyFactory
   RTCVideoDecoderFactoryTv* decoder_factory_tv_;
 #endif
 
-  scoped_refptr<VideoCaptureImplManager> vc_manager_;
   scoped_refptr<P2PSocketDispatcher> p2p_socket_dispatcher_;
   scoped_refptr<WebRtcAudioDeviceImpl> audio_device_;
 

content/renderer/media/mock_media_stream_dependency_factory.cc

@@ -367,7 +367,7 @@ class MockIceCandidate : public IceCandidateInterface {
 };
 
 MockMediaStreamDependencyFactory::MockMediaStreamDependencyFactory()
-    : MediaStreamDependencyFactory(NULL, NULL),
+    : MediaStreamDependencyFactory(NULL),
       mock_pc_factory_created_(false) {
 }
 

content/renderer/media/rtc_video_capture_delegate.cc

@@ -5,25 +5,27 @@
 #include "content/renderer/media/rtc_video_capture_delegate.h"
 
 #include "base/bind.h"
+#include "base/location.h"
+#include "content/renderer/media/video_capture_impl_manager.h"
+#include "content/renderer/render_thread_impl.h"
 #include "media/base/video_frame.h"
 
 namespace content {
 
 RtcVideoCaptureDelegate::RtcVideoCaptureDelegate(
-    const media::VideoCaptureSessionId id,
-    VideoCaptureImplManager* vc_manager)
+    const media::VideoCaptureSessionId id)
     : session_id_(id),
-      vc_manager_(vc_manager),
-      capture_engine_(NULL),
       got_first_frame_(false),
       error_occured_(false) {
   DVLOG(3) << " RtcVideoCaptureDelegate::ctor";
-  capture_engine_ = vc_manager_->AddDevice(session_id_, this);
+  capture_engine_ =
+      RenderThreadImpl::current()->video_capture_impl_manager()
+         ->UseDevice(session_id_);
 }
 
 RtcVideoCaptureDelegate::~RtcVideoCaptureDelegate() {
   DVLOG(3) << " RtcVideoCaptureDelegate::dtor";
-  vc_manager_->RemoveDevice(session_id_, this);
+  StopCapture();
 }
 
 void RtcVideoCaptureDelegate::StartCapture(

content/renderer/media/rtc_video_capture_delegate.h

@@ -8,11 +8,12 @@
 #include "base/callback.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "content/common/media/video_capture.h"
-#include "content/renderer/media/video_capture_impl_manager.h"
 #include "media/video/capture/video_capture.h"
 
 namespace content {
 
+class VideoCaptureHandle;
+
 // Implements a simple reference counted video capturer that guarantees that
 // methods in RtcVideoCaptureDelegateEventHandler is only called from when
 // StartCapture have been called until after StopCapture have been called.
@@ -33,16 +34,17 @@ class RtcVideoCaptureDelegate
       FrameCapturedCallback;
   typedef base::Callback<void(CaptureState)> StateChangeCallback;
 
-  RtcVideoCaptureDelegate(const media::VideoCaptureSessionId id,
-                          VideoCaptureImplManager* vc_manager);
+  RtcVideoCaptureDelegate(const media::VideoCaptureSessionId id);
 
   void StartCapture(const media::VideoCaptureParams& params,
                     const FrameCapturedCallback& captured_callback,
                     const StateChangeCallback& state_callback);
   void StopCapture();
 
+ protected:
   // media::VideoCapture::EventHandler implementation.
-  // These functions are called from a thread owned by |vc_manager_|.
+  // These functions are called on the IO thread (same as where
+  // |capture_engine_| runs).
   virtual void OnStarted(media::VideoCapture* capture) OVERRIDE;
   virtual void OnStopped(media::VideoCapture* capture) OVERRIDE;
   virtual void OnPaused(media::VideoCapture* capture) OVERRIDE;
@@ -66,9 +68,7 @@ class RtcVideoCaptureDelegate
   // The id identifies which video capture device is used for this video
   // capture session.
   media::VideoCaptureSessionId session_id_;
-  // The video capture manager handles open/close of video capture devices.
-  scoped_refptr<VideoCaptureImplManager> vc_manager_;
-  media::VideoCapture* capture_engine_;
+  scoped_ptr<VideoCaptureHandle> capture_engine_;
 
   // Accessed on the thread where StartCapture is called.
   bool got_first_frame_;

content/renderer/media/rtc_video_capturer.cc

@@ -11,10 +11,9 @@
 namespace content {
 
 RtcVideoCapturer::RtcVideoCapturer(const media::VideoCaptureSessionId id,
-                                   VideoCaptureImplManager* vc_manager,
                                    bool is_screencast)
     : is_screencast_(is_screencast),
-      delegate_(new RtcVideoCaptureDelegate(id, vc_manager)),
+      delegate_(new RtcVideoCaptureDelegate(id)),
       state_(VIDEO_CAPTURE_STATE_STOPPED) {}
 
 RtcVideoCapturer::~RtcVideoCapturer() {

content/renderer/media/rtc_video_capturer.h

@@ -12,7 +12,6 @@
 #include "third_party/libjingle/source/talk/media/base/videocapturer.h"
 
 namespace content {
-class VideoCaptureImplManager;
 
 // RtcVideoCapturer implements a simple cricket::VideoCapturer that is used for
 // VideoCapturing in libJingle and especially in PeerConnections.
@@ -24,7 +23,6 @@ class RtcVideoCapturer
     : public cricket::VideoCapturer {
  public:
   RtcVideoCapturer(const media::VideoCaptureSessionId id,
-                   VideoCaptureImplManager* vc_manager,
                    bool is_screencast);
   virtual ~RtcVideoCapturer();
 

content/renderer/media/video_capture_impl.h

@@ -9,23 +9,21 @@
 // VideoCaptureImpl is also a delegate of VideoCaptureMessageFilter which relays
 // operation of a capture device to the browser process and receives responses
 // from browser process.
-
-// The media::VideoCapture and VideoCaptureMessageFilter::Delegate are
-// asynchronous interfaces, which means callers can call those interfaces
-// from any threads without worrying about thread safety.
-// The |capture_message_loop_proxy_| is the working thread of VideoCaptureImpl.
-// All non-const members are accessed only on that working thread.
 //
-// Implementation note: tasks are posted bound to Unretained(this) to both the
-// I/O and Capture threads and this is safe (even though the I/O thread is
-// scoped to the renderer process and the capture_message_loop_proxy_ thread is
-// scoped to the VideoCaptureImplManager) because VideoCaptureImplManager only
-// triggers deletion of its VideoCaptureImpl's by calling DeInit which detours
-// through the capture & I/O threads, so as long as nobody posts tasks after the
-// DeInit() call is made, it is guaranteed none of these Unretained posted tasks
-// will dangle after the delete goes through.  The "as long as" is guaranteed by
-// clients of VideoCaptureImplManager not using devices after they've
-// RemoveDevice'd them.
+// All public methods of VideoCaptureImpl can be called on any thread.
+// Internally it runs on the IO thread. Clients of this class implement
+// interface media::VideoCapture::EventHandler which is called only on the IO
+// thread.
+//
+// Implementation note: tasks are posted bound to Unretained(this) to the I/O
+// thread and this is safe (even though the I/O thread is scoped to the renderer
+// process) because VideoCaptureImplManager only triggers deletion of its
+// VideoCaptureImpl's by calling DeInit which detours through the I/O thread, so
+// as long as nobody posts tasks after the DeInit() call is made, it is
+// guaranteed none of these Unretained posted tasks will dangle after the delete
+// goes through. The "as long as" is guaranteed by clients of
+// VideoCaptureImplManager not using devices after they've released
+// VideoCaptureHandle, which is a wrapper of this object.
 
 #ifndef CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_
 #define CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_
@@ -49,6 +47,19 @@ namespace content {
 class CONTENT_EXPORT VideoCaptureImpl
     : public media::VideoCapture, public VideoCaptureMessageFilter::Delegate {
  public:
+  VideoCaptureImpl(media::VideoCaptureSessionId session_id,
+                   VideoCaptureMessageFilter* filter);
+  virtual ~VideoCaptureImpl();
+
+  // Start listening to IPC messages.
+  void Init();
+
+  // Stop listening to IPC messages. Call |done_cb| when done.
+  void DeInit(base::Closure done_cb);
+
+  // Stop/resume delivering video frames to clients, based on flag |suspend|.
+  void SuspendCapture(bool suspend);
+
   // media::VideoCapture interface.
   virtual void StartCapture(
       media::VideoCapture::EventHandler* handler,
@@ -57,23 +68,9 @@ class CONTENT_EXPORT VideoCaptureImpl
   virtual bool CaptureStarted() OVERRIDE;
   virtual int CaptureFrameRate() OVERRIDE;
 
-  // VideoCaptureMessageFilter::Delegate interface.
-  virtual void OnBufferCreated(base::SharedMemoryHandle handle,
-                               int length,
-                               int buffer_id) OVERRIDE;
-  virtual void OnBufferDestroyed(int buffer_id) OVERRIDE;
-  virtual void OnBufferReceived(int buffer_id,
-                                base::TimeTicks timestamp,
-                                const media::VideoCaptureFormat& format)
-      OVERRIDE;
-  virtual void OnStateChanged(VideoCaptureState state) OVERRIDE;
-  virtual void OnDelegateAdded(int32 device_id) OVERRIDE;
-
-  // Stop/resume delivering video frames to clients, based on flag |suspend|.
-  virtual void SuspendCapture(bool suspend);
+  media::VideoCaptureSessionId session_id() const { return session_id_; }
 
  private:
-  friend class VideoCaptureImplManager;
   friend class VideoCaptureImplTest;
   friend class MockVideoCaptureImpl;
 
@@ -81,38 +78,35 @@ class CONTENT_EXPORT VideoCaptureImpl
   typedef std::map<media::VideoCapture::EventHandler*,
                    media::VideoCaptureParams> ClientInfo;
 
-  VideoCaptureImpl(media::VideoCaptureSessionId session_id,
-                   base::MessageLoopProxy* capture_message_loop_proxy,
-                   VideoCaptureMessageFilter* filter);
-  virtual ~VideoCaptureImpl();
-
-  void DoStartCaptureOnCaptureThread(
+  void InitOnIOThread();
+  void DeInitOnIOThread(base::Closure done_cb);
+  void SuspendCaptureOnIOThread(bool suspend);
+  void StartCaptureOnIOThread(
       media::VideoCapture::EventHandler* handler,
       const media::VideoCaptureParams& params);
-  void DoStopCaptureOnCaptureThread(media::VideoCapture::EventHandler* handler);
-  void DoBufferCreatedOnCaptureThread(base::SharedMemoryHandle handle,
+  void StopCaptureOnIOThread(media::VideoCapture::EventHandler* handler);
+
+  // VideoCaptureMessageFilter::Delegate interface.
+  virtual void OnBufferCreated(base::SharedMemoryHandle handle,
                                int length,
-                                      int buffer_id);
-  void DoBufferDestroyedOnCaptureThread(int buffer_id);
-  void DoBufferReceivedOnCaptureThread(int buffer_id,
+                               int buffer_id) OVERRIDE;
+  virtual void OnBufferDestroyed(int buffer_id) OVERRIDE;
+  virtual void OnBufferReceived(
+      int buffer_id,
       base::TimeTicks timestamp,
-                                       const media::VideoCaptureFormat& format);
-  void DoClientBufferFinishedOnCaptureThread(
+      const media::VideoCaptureFormat& format) OVERRIDE;
+  virtual void OnStateChanged(VideoCaptureState state) OVERRIDE;
+  virtual void OnDelegateAdded(int32 device_id) OVERRIDE;
+
+  // Sends an IPC message to browser process when all clients are done with the
+  // buffer.
+  void OnClientBufferFinished(
       int buffer_id,
       const scoped_refptr<ClientBuffer>& buffer);
-  void DoStateChangedOnCaptureThread(VideoCaptureState state);
-  void DoDelegateAddedOnCaptureThread(int32 device_id);
 
-  void DoSuspendCaptureOnCaptureThread(bool suspend);
-
-  void Init();
-  void DeInit(base::Closure task);
-  void DoDeInitOnCaptureThread(base::Closure task);
   void StopDevice();
   void RestartCapture();
   void StartCaptureInternal();
-  void AddDelegateOnIOThread();
-  void RemoveDelegateOnIOThread(base::Closure task);
   virtual void Send(IPC::Message* message);
 
   // Helpers.
@@ -120,7 +114,6 @@ class CONTENT_EXPORT VideoCaptureImpl
                     ClientInfo* clients);
 
   const scoped_refptr<VideoCaptureMessageFilter> message_filter_;
-  const scoped_refptr<base::MessageLoopProxy> capture_message_loop_proxy_;
   const scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy_;
   int device_id_;
   const int session_id_;
@@ -134,7 +127,7 @@ class CONTENT_EXPORT VideoCaptureImpl
   ClientInfo clients_pending_on_restart_;
 
   // Member params_ represents the video format requested by the
-  // client to this class via DoStartCaptureOnCaptureThread.
+  // client to this class via StartCapture().
   media::VideoCaptureParams params_;
 
   // The device's video capture format sent from browser process side.

content/renderer/media/video_capture_impl_manager.cc

@@ -5,84 +5,103 @@
 #include "content/renderer/media/video_capture_impl_manager.h"
 
 #include "base/bind.h"
-#include "base/stl_util.h"
+#include "base/bind_helpers.h"
+#include "content/public/renderer/render_thread.h"
 #include "content/renderer/media/video_capture_impl.h"
 #include "content/renderer/media/video_capture_message_filter.h"
+#include "media/base/bind_to_loop.h"
 
 namespace content {
 
-VideoCaptureImplManager::VideoCaptureImplManager()
-    : thread_("VC manager") {
-  thread_.Start();
-  message_loop_proxy_ = thread_.message_loop_proxy();
-  filter_ = new VideoCaptureMessageFilter();
+VideoCaptureHandle::VideoCaptureHandle(
+    media::VideoCapture* impl, base::Closure destruction_cb)
+    : impl_(impl), destruction_cb_(destruction_cb) {
 }
 
-media::VideoCapture* VideoCaptureImplManager::AddDevice(
-    media::VideoCaptureSessionId id,
-    media::VideoCapture::EventHandler* handler) {
-  DCHECK(handler);
+VideoCaptureHandle::~VideoCaptureHandle() {
+  destruction_cb_.Run();
+}
 
-  base::AutoLock auto_lock(lock_);
-  Devices::iterator it = devices_.find(id);
-  if (it == devices_.end()) {
-    VideoCaptureImpl* vc =
-        new VideoCaptureImpl(id, message_loop_proxy_.get(), filter_.get());
-    devices_[id] = new Device(vc, handler);
-    vc->Init();
-    return vc;
-  }
+void VideoCaptureHandle::StartCapture(
+    EventHandler* handler,
+    const media::VideoCaptureParams& params) {
+  impl_->StartCapture(handler, params);
+}
 
-  devices_[id]->clients.push_front(handler);
-  return it->second->vc;
+void VideoCaptureHandle::StopCapture(EventHandler* handler) {
+  impl_->StopCapture(handler);
 }
 
-void VideoCaptureImplManager::SuspendDevices(bool suspend) {
-  base::AutoLock auto_lock(lock_);
-  for (Devices::iterator it = devices_.begin(); it != devices_.end(); ++it)
-    it->second->vc->SuspendCapture(suspend);
+bool VideoCaptureHandle::CaptureStarted() {
+  return impl_->CaptureStarted();
 }
 
-void VideoCaptureImplManager::RemoveDevice(
-    media::VideoCaptureSessionId id,
-    media::VideoCapture::EventHandler* handler) {
-  DCHECK(handler);
+int VideoCaptureHandle::CaptureFrameRate() {
+  return impl_->CaptureFrameRate();
+}
+
+VideoCaptureImplManager::VideoCaptureImplManager()
+    : filter_(new VideoCaptureMessageFilter()),
+      weak_factory_(this) {
+}
 
-  base::AutoLock auto_lock(lock_);
-  Devices::iterator it = devices_.find(id);
-  if (it == devices_.end())
-    return;
+VideoCaptureImplManager::~VideoCaptureImplManager() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
 
-  size_t size = it->second->clients.size();
-  it->second->clients.remove(handler);
+scoped_ptr<VideoCaptureHandle> VideoCaptureImplManager::UseDevice(
+    media::VideoCaptureSessionId id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (size == it->second->clients.size() || size > 1)
-    return;
+  VideoCaptureImpl* video_capture_device = NULL;
+  VideoCaptureDeviceMap::iterator it = devices_.find(id);
+  if (it == devices_.end()) {
+    video_capture_device = CreateVideoCaptureImpl(id, filter_.get());
+    devices_[id] =
+        std::make_pair(1, linked_ptr<VideoCaptureImpl>(video_capture_device));
+    video_capture_device->Init();
+  } else {
+    ++it->second.first;
+    video_capture_device = it->second.second.get();
+  }
 
-  devices_[id]->vc->DeInit(base::Bind(&VideoCaptureImplManager::FreeDevice,
-                                      this, devices_[id]->vc));
-  delete devices_[id];
-  devices_.erase(id);
+  // This callback ensures UnrefDevice() happens on the render thread.
+  return scoped_ptr<VideoCaptureHandle>(
+      new VideoCaptureHandle(
+          video_capture_device,
+          media::BindToCurrentLoop(
+              base::Bind(
+                  &VideoCaptureImplManager::UnrefDevice,
+                  weak_factory_.GetWeakPtr(),
+                  id))));
 }
 
-void VideoCaptureImplManager::FreeDevice(VideoCaptureImpl* vc) {
-  delete vc;
+VideoCaptureImpl* VideoCaptureImplManager::CreateVideoCaptureImpl(
+    media::VideoCaptureSessionId id,
+    VideoCaptureMessageFilter* filter) const {
+  return new VideoCaptureImpl(id, filter);
 }
 
-VideoCaptureImplManager::~VideoCaptureImplManager() {
-  thread_.Stop();
-  // TODO(wjia): uncomment the line below after collecting enough info for
-  // crbug.com/152418.
-  // STLDeleteContainerPairSecondPointers(devices_.begin(), devices_.end());
-}
+void VideoCaptureImplManager::UnrefDevice(
+    media::VideoCaptureSessionId id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  VideoCaptureDeviceMap::iterator it = devices_.find(id);
+  DCHECK(it != devices_.end());
 
-VideoCaptureImplManager::Device::Device(
-    VideoCaptureImpl* device,
-    media::VideoCapture::EventHandler* handler)
-    : vc(device) {
-  clients.push_front(handler);
+  DCHECK(it->second.first);
+  --it->second.first;
+  if (!it->second.first) {
+    VideoCaptureImpl* impl = it->second.second.release();
+    devices_.erase(id);
+    impl->DeInit(base::Bind(&base::DeletePointer<VideoCaptureImpl>, impl));
+  }
 }
 
-VideoCaptureImplManager::Device::~Device() {}
+void VideoCaptureImplManager::SuspendDevices(bool suspend) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  for (VideoCaptureDeviceMap::iterator it = devices_.begin();
+       it != devices_.end(); ++it)
+    it->second.second->SuspendCapture(suspend);
+}
 
 }  // namespace content

content/renderer/media/video_capture_impl_manager.h

@@ -2,77 +2,104 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// VideoCaptureImplManager manages video capture devices in renderer process.
-// The video capture clients use AddDevice() to get a pointer to
-// video capture device. VideoCaputreImplManager supports multiple clients
-// accessing same device.
+// VideoCaptureImplManager owns VideoCaptureImpl objects. Clients who
+// want access to a video capture device call UseDevice() to get a handle
+// to VideoCaptureImpl.
+//
+// THREADING
+//
+// VideoCaptureImplManager lives only on the render thread. All methods
+// must be called on this thread.
+//
+// The handle returned by UseDevice() is thread-safe. It ensures
+// destruction is handled on the render thread.
 
 #ifndef CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_MANAGER_H_
 #define CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_MANAGER_H_
 
-#include <list>
 #include <map>
 
+#include "base/callback.h"
+#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop_proxy.h"
-#include "base/threading/thread.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
 #include "content/common/content_export.h"
 #include "media/video/capture/video_capture.h"
 
 namespace content {
 
 class VideoCaptureImpl;
+class VideoCaptureImplManager;
 class VideoCaptureMessageFilter;
 
-class CONTENT_EXPORT VideoCaptureImplManager
-    : public base::RefCountedThreadSafe<VideoCaptureImplManager> {
+// Thread-safe wrapper for a media::VideoCapture object. During
+// destruction |destruction_cb| is called. This mechanism is used
+// by VideoCaptureImplManager to ensure de-initialization and
+// destruction of the media::VideoCapture object happens on the render
+// thread.
+class CONTENT_EXPORT VideoCaptureHandle : media::VideoCapture {
  public:
-  VideoCaptureImplManager();
+  virtual ~VideoCaptureHandle();
 
-  // Called by video capture client |handler| to add device referenced
-  // by |id| to VideoCaptureImplManager's list of opened device list.
-  // A pointer to VideoCapture is returned to client so that client can
-  // operate on that pointer, such as StartCaptrue, StopCapture.
-  virtual media::VideoCapture* AddDevice(
-      media::VideoCaptureSessionId id,
-      media::VideoCapture::EventHandler* handler);
+  // media::VideoCapture implementations.
+  virtual void StartCapture(
+      EventHandler* handler,
+      const media::VideoCaptureParams& params) OVERRIDE;
+  virtual void StopCapture(EventHandler* handler) OVERRIDE;
+  virtual bool CaptureStarted() OVERRIDE;
+  virtual int CaptureFrameRate() OVERRIDE;
+
+ private:
+  friend class VideoCaptureImplManager;
+
+  VideoCaptureHandle(media::VideoCapture* impl,
+                     base::Closure destruction_cb);
 
-  // Called by video capture client |handler| to remove device referenced
-  // by |id| from VideoCaptureImplManager's list of opened device list.
-  virtual void RemoveDevice(media::VideoCaptureSessionId id,
-                            media::VideoCapture::EventHandler* handler);
+  media::VideoCapture* impl_;
+  base::Closure destruction_cb_;
+
+  DISALLOW_COPY_AND_ASSIGN(VideoCaptureHandle);
+};
+
+class CONTENT_EXPORT VideoCaptureImplManager {
+ public:
+  VideoCaptureImplManager();
+  virtual ~VideoCaptureImplManager();
+
+  // Returns a video capture device referenced by |id|.
+  scoped_ptr<VideoCaptureHandle> UseDevice(media::VideoCaptureSessionId id);
 
   // Make all existing VideoCaptureImpl instances stop/resume delivering
   // video frames to their clients, depends on flag |suspend|.
-  virtual void SuspendDevices(bool suspend);
+  void SuspendDevices(bool suspend);
 
   VideoCaptureMessageFilter* video_capture_message_filter() const {
     return filter_.get();
   }
 
  protected:
-  virtual ~VideoCaptureImplManager();
+  // Used in tests to inject a mock VideoCaptureImpl.
+  virtual VideoCaptureImpl* CreateVideoCaptureImpl(
+      media::VideoCaptureSessionId id,
+      VideoCaptureMessageFilter* filter) const;
 
  private:
-  friend class base::RefCountedThreadSafe<VideoCaptureImplManager>;
-
-  struct Device {
-    Device(VideoCaptureImpl* device,
-           media::VideoCapture::EventHandler* handler);
-    ~Device();
+  void UnrefDevice(media::VideoCaptureSessionId id);
 
-    VideoCaptureImpl* vc;
-    std::list<media::VideoCapture::EventHandler*> clients;
-  };
+  // The int is used to count clients of the corresponding VideoCaptureImpl.
+  typedef std::map<media::VideoCaptureSessionId,
+                   std::pair<int, linked_ptr<VideoCaptureImpl> > >
+      VideoCaptureDeviceMap;
+  VideoCaptureDeviceMap devices_;
 
-  void FreeDevice(VideoCaptureImpl* vc);
-
-  typedef std::map<media::VideoCaptureSessionId, Device*> Devices;
-  Devices devices_;
-  base::Lock lock_;
   scoped_refptr<VideoCaptureMessageFilter> filter_;
-  base::Thread thread_;
-  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
+
+  // Following two members are bound to the render thread.
+  base::WeakPtrFactory<VideoCaptureImplManager> weak_factory_;
+  base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureImplManager);
 };

content/renderer/media/video_capture_impl_manager_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/bind.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "content/child/child_process.h"
+#include "content/renderer/media/video_capture_impl.h"
+#include "content/renderer/media/video_capture_impl_manager.h"
+#include "content/renderer/media/video_capture_message_filter.h"
+#include "media/base/bind_to_loop.h"
+#include "media/video/capture/mock_video_capture_event_handler.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::DoAll;
+using ::testing::SaveArg;
+using media::BindToCurrentLoop;
+using media::MockVideoCaptureEventHandler;
+
+namespace content {
+
+ACTION_P(RunClosure, closure) {
+  closure.Run();
+}
+
+class MockVideoCaptureImpl : public VideoCaptureImpl {
+ public:
+  MockVideoCaptureImpl(media::VideoCaptureSessionId session_id,
+                       VideoCaptureMessageFilter* filter)
+      : VideoCaptureImpl(session_id, filter) {
+  }
+
+  virtual ~MockVideoCaptureImpl() {
+    Destruct();
+  }
+
+  MOCK_METHOD0(Destruct, void());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockVideoCaptureImpl);
+};
+
+class MockVideoCaptureImplManager : public VideoCaptureImplManager {
+ public:
+  MockVideoCaptureImplManager() {}
+
+ protected:
+  virtual VideoCaptureImpl* CreateVideoCaptureImpl(
+      media::VideoCaptureSessionId id,
+      VideoCaptureMessageFilter* filter) const OVERRIDE {
+    return new MockVideoCaptureImpl(id, filter);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockVideoCaptureImplManager);
+};
+
+class VideoCaptureImplManagerTest : public ::testing::Test {
+ public:
+  VideoCaptureImplManagerTest() {
+    params_.requested_format = media::VideoCaptureFormat(
+        gfx::Size(176, 144), 30, media::PIXEL_FORMAT_I420);
+    child_process_.reset(new ChildProcess());
+  }
+
+  void FakeChannelSetup() {
+    scoped_refptr<base::MessageLoopProxy> loop =
+        child_process_->io_message_loop_proxy();
+    if (!loop->BelongsToCurrentThread()) {
+      loop->PostTask(
+          FROM_HERE,
+          base::Bind(
+              &VideoCaptureImplManagerTest::FakeChannelSetup,
+              base::Unretained(this)));
+      return;
+    }
+    manager_.video_capture_message_filter()->OnFilterAdded(NULL);
+  }
+
+  void Quit(base::RunLoop* run_loop) {
+    message_loop_.PostTask(FROM_HERE, run_loop->QuitClosure());
+  }
+
+ protected:
+  base::MessageLoop message_loop_;
+  scoped_ptr<ChildProcess> child_process_;
+  media::VideoCaptureParams params_;
+  MockVideoCaptureImplManager manager_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(VideoCaptureImplManagerTest);
+};
+
+// Multiple clients with the same session id. There is only one
+// media::VideoCapture object.
+TEST_F(VideoCaptureImplManagerTest, MultipleClients) {
+  scoped_ptr<MockVideoCaptureEventHandler> client1(
+      new MockVideoCaptureEventHandler);
+  scoped_ptr<MockVideoCaptureEventHandler> client2(
+      new MockVideoCaptureEventHandler);
+
+  media::VideoCapture* device1 = NULL;
+  media::VideoCapture* device2 = NULL;
+
+  scoped_ptr<VideoCaptureHandle> handle1;
+  scoped_ptr<VideoCaptureHandle> handle2;
+  {
+    base::RunLoop run_loop;
+    base::Closure quit_closure = BindToCurrentLoop(
+        run_loop.QuitClosure());
+
+    EXPECT_CALL(*client1, OnStarted(_)).WillOnce(SaveArg<0>(&device1));
+    EXPECT_CALL(*client2, OnStarted(_)).WillOnce(
+        DoAll(
+            SaveArg<0>(&device2),
+            RunClosure(quit_closure)));
+    handle1 = manager_.UseDevice(1);
+    handle2 = manager_.UseDevice(1);
+    handle1->StartCapture(client1.get(), params_);
+    handle2->StartCapture(client2.get(), params_);
+    FakeChannelSetup();
+    run_loop.Run();
+  }
+
+  {
+    base::RunLoop run_loop;
+    base::Closure quit_closure = BindToCurrentLoop(
+        run_loop.QuitClosure());
+
+    EXPECT_CALL(*client1, OnStopped(_));
+    EXPECT_CALL(*client1, OnRemoved(_));
+    EXPECT_CALL(*client2, OnStopped(_));
+    EXPECT_CALL(*client2, OnRemoved(_)).WillOnce(
+        RunClosure(quit_closure));
+    handle1->StopCapture(client1.get());
+    handle2->StopCapture(client2.get());
+    run_loop.Run();
+  }
+
+  EXPECT_TRUE(device1 == device2);
+  EXPECT_CALL(*static_cast<MockVideoCaptureImpl*>(device1), Destruct());
+}
+
+}  // namespace content

content/renderer/pepper/pepper_platform_video_capture.cc

@@ -28,7 +28,6 @@ PepperPlatformVideoCapture::PepperPlatformVideoCapture(
       handler_proxy_(new media::VideoCaptureHandlerProxy(
           this, base::MessageLoopProxy::current())),
       handler_(handler),
-      video_capture_(NULL),
       unbalanced_start_(false),
       pending_open_device_(false),
       pending_open_device_id_(-1) {
@@ -83,12 +82,7 @@ void PepperPlatformVideoCapture::DetachEventHandler() {
   handler_ = NULL;
   StopCapture(NULL);
 
-  if (video_capture_) {
-    VideoCaptureImplManager* manager =
-        RenderThreadImpl::current()->video_capture_impl_manager();
-    manager->RemoveDevice(session_id_, handler_proxy_.get());
-    video_capture_ = NULL;
-  }
+  video_capture_.reset();
 
   if (render_view_.get()) {
     if (!label_.empty()) {
@@ -147,7 +141,7 @@ PepperPlatformVideoCapture::~PepperPlatformVideoCapture() {
 void PepperPlatformVideoCapture::Initialize() {
   VideoCaptureImplManager* manager =
       RenderThreadImpl::current()->video_capture_impl_manager();
-  video_capture_ = manager->AddDevice(session_id_, handler_proxy_.get());
+  video_capture_ = manager->UseDevice(session_id_);
 }
 
 void PepperPlatformVideoCapture::OnDeviceOpened(int request_id,

content/renderer/pepper/pepper_platform_video_capture.h

@@ -24,6 +24,7 @@ namespace content {
 class PepperMediaDeviceManager;
 class PepperVideoCaptureHost;
 class RenderViewImpl;
+class VideoCaptureHandle;
 
 class PepperPlatformVideoCapture
     : public media::VideoCapture,
@@ -80,7 +81,7 @@ class PepperPlatformVideoCapture
 
   PepperVideoCaptureHost* handler_;
 
-  media::VideoCapture* video_capture_;
+  scoped_ptr<VideoCaptureHandle> video_capture_;
 
   // StartCapture() must be balanced by StopCapture(), otherwise this object
   // will leak.

content/renderer/render_thread_impl.cc

@@ -347,7 +347,7 @@ void RenderThreadImpl::Init() {
   db_message_filter_ = new DBMessageFilter();
   AddFilter(db_message_filter_.get());
 
-  vc_manager_ = new VideoCaptureImplManager();
+  vc_manager_.reset(new VideoCaptureImplManager());
   AddFilter(vc_manager_->video_capture_message_filter());
 
 #if defined(ENABLE_WEBRTC)
@@ -361,7 +361,7 @@ void RenderThreadImpl::Init() {
   webrtc_identity_service_.reset(new WebRTCIdentityService());
 
   media_stream_factory_.reset(new MediaStreamDependencyFactory(
-      vc_manager_.get(), p2p_socket_dispatcher_.get()));
+      p2p_socket_dispatcher_.get()));
   AddObserver(media_stream_factory_.get());
 #endif  // defined(ENABLE_WEBRTC)
 
@@ -464,7 +464,13 @@ void RenderThreadImpl::Shutdown() {
   RemoveFilter(audio_message_filter_.get());
   audio_message_filter_ = NULL;
 
+  // |media_stream_factory_| produces users of |vc_manager_| so it must be
+  // destroyed first.
+#if defined(ENABLE_WEBRTC)
+  media_stream_factory_.reset();
+#endif
   RemoveFilter(vc_manager_->video_capture_message_filter());
+  vc_manager_.reset();
 
   RemoveFilter(db_message_filter_.get());
   db_message_filter_ = NULL;

content/renderer/render_thread_impl.h

@@ -426,8 +426,8 @@ class CONTENT_EXPORT RenderThreadImpl : public RenderThread,
   // Dispatches all P2P sockets.
   scoped_refptr<P2PSocketDispatcher> p2p_socket_dispatcher_;
 
-  // Used on multiple threads.
-  scoped_refptr<VideoCaptureImplManager> vc_manager_;
+  // Used on the render thread.
+  scoped_ptr<VideoCaptureImplManager> vc_manager_;
 
   // The count of RenderWidgets running through this thread.
   int widget_count_;

media/media.gyp

@@ -1219,6 +1219,8 @@
         'base/test_helpers.h',
         'filters/mock_gpu_video_accelerator_factories.cc',
         'filters/mock_gpu_video_accelerator_factories.h',
+        'video/capture/mock_video_capture_event_handler.cc',
+        'video/capture/mock_video_capture_event_handler.h',
         'video/mock_video_decode_accelerator.cc',
         'video/mock_video_decode_accelerator.h',
       ],

media/video/capture/mock_video_capture_event_handler.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/video/capture/mock_video_capture_event_handler.h"
+
+namespace media {
+
+MockVideoCaptureEventHandler::MockVideoCaptureEventHandler() {
+}
+
+MockVideoCaptureEventHandler::~MockVideoCaptureEventHandler() {
+}
+
+}  // namespace media

media/video/capture/mock_video_capture_event_handler.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_EVENT_HANDLER_H_
+#define MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_EVENT_HANDLER_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "media/video/capture/video_capture.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace media {
+
+class MockVideoCaptureEventHandler : public VideoCapture::EventHandler {
+ public:
+  MockVideoCaptureEventHandler();
+  virtual ~MockVideoCaptureEventHandler();
+
+  // EventHandler implementation.
+  MOCK_METHOD1(OnStarted, void(VideoCapture* capture));
+  MOCK_METHOD1(OnStopped, void(VideoCapture* capture));
+  MOCK_METHOD1(OnPaused, void(VideoCapture* capture));
+  MOCK_METHOD2(OnError, void(VideoCapture* capture, int error_code));
+  MOCK_METHOD1(OnRemoved, void(VideoCapture* capture));
+  MOCK_METHOD2(OnFrameReady,
+               void(VideoCapture* capture,
+                    const scoped_refptr<VideoFrame>& frame));
+  MOCK_METHOD2(OnDeviceInfoReceived,
+               void(VideoCapture* capture,
+                    const VideoCaptureFormat& device_info));
+};
+
+}  // namespace media
+
+#endif  // MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_EVENT_HANDLER_H_