Refactor video capturing code in the render process

content/renderer/media/video_capture_impl.cc

Index: content/renderer/media/video_capture_impl.cc
diff --git a/content/renderer/media/video_capture_impl.cc b/content/renderer/media/video_capture_impl.cc
index b9a61f3d05b4db4a0b24afebb01adde58acd056a..bda203a41dc211014b6c1a71c234a4c7cae00d2b 100644
--- a/content/renderer/media/video_capture_impl.cc
+++ b/content/renderer/media/video_capture_impl.cc
@@ -1,6 +1,13 @@
 // Copyright (c) 2012 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.
+//
+// Notes about usage of this object by VideoCaptureImplManager.
+//
+// VideoCaptureImplManager access this object by using a Unretained()
+// binding and tasks on the IO thread. It is then important that
+// VideoCaptureImpl never post task to itself. All operations must be
+// synchronous.
 
 #include "content/renderer/media/video_capture_impl.h"
 
@@ -32,130 +39,81 @@ class VideoCaptureImpl::ClientBuffer
   DISALLOW_COPY_AND_ASSIGN(ClientBuffer);
 };
 
-bool VideoCaptureImpl::CaptureStarted() {
-  return state_ == VIDEO_CAPTURE_STATE_STARTED;
-}
-
-int VideoCaptureImpl::CaptureFrameRate() {
-  return last_frame_format_.frame_rate;
-}
+VideoCaptureImpl::ClientInfo::ClientInfo() {}
+VideoCaptureImpl::ClientInfo::~ClientInfo() {}
 
 VideoCaptureImpl::VideoCaptureImpl(
     const media::VideoCaptureSessionId session_id,
     VideoCaptureMessageFilter* filter)
-    : VideoCapture(),
-      message_filter_(filter),
-      io_message_loop_proxy_(ChildProcess::current()->io_message_loop_proxy()),
+    : message_filter_(filter),
       device_id_(0),
       session_id_(session_id),
       suspended_(false),
       state_(VIDEO_CAPTURE_STATE_STOPPED),
       weak_factory_(this) {
   DCHECK(filter);
+  thread_checker_.DetachFromThread();
 }
 
-VideoCaptureImpl::~VideoCaptureImpl() {}
-
-void VideoCaptureImpl::Init() {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::InitOnIOThread,
-                 base::Unretained(this)));
-}
-
-void VideoCaptureImpl::DeInit(base::Closure done_cb) {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::DeInitOnIOThread,
-                 base::Unretained(this),
-                 done_cb));
-}
-
-void VideoCaptureImpl::SuspendCapture(bool suspend) {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::SuspendCaptureOnIOThread,
-                 base::Unretained(this),
-                 suspend));
-}
-
-void VideoCaptureImpl::StartCapture(
-    media::VideoCapture::EventHandler* handler,
-    const media::VideoCaptureParams& params) {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::StartCaptureOnIOThread,
-                 base::Unretained(this), handler, params));
-}
-
-void VideoCaptureImpl::StopCapture(
-    media::VideoCapture::EventHandler* handler) {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::StopCaptureOnIOThread,
-                 base::Unretained(this), handler));
-}
-
-void VideoCaptureImpl::GetDeviceSupportedFormats(
-    const DeviceFormatsCallback& callback) {
-  DCHECK(!callback.is_null());
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::GetDeviceSupportedFormatsOnIOThread,
-                 base::Unretained(this), media::BindToCurrentLoop(callback)));
-}
-
-void VideoCaptureImpl::GetDeviceFormatsInUse(
-    const DeviceFormatsInUseCallback& callback) {
-  DCHECK(!callback.is_null());
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::GetDeviceFormatsInUseOnIOThread,
-                 base::Unretained(this), media::BindToCurrentLoop(callback)));
+VideoCaptureImpl::~VideoCaptureImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());
 }
 
-void VideoCaptureImpl::InitOnIOThread() {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+void VideoCaptureImpl::Init() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   message_filter_->AddDelegate(this);
 }
 
-void VideoCaptureImpl::DeInitOnIOThread(base::Closure done_cb) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+void VideoCaptureImpl::DeInit() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (state_ == VIDEO_CAPTURE_STATE_STARTED)
     Send(new VideoCaptureHostMsg_Stop(device_id_));
   message_filter_->RemoveDelegate(this);
-  done_cb.Run();
 }
 
-void VideoCaptureImpl::SuspendCaptureOnIOThread(bool suspend) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+void VideoCaptureImpl::SuspendCapture(bool suspend) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   suspended_ = suspend;
 }
 
-void VideoCaptureImpl::StartCaptureOnIOThread(
-    media::VideoCapture::EventHandler* handler,
-    const media::VideoCaptureParams& params) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+void VideoCaptureImpl::StartCapture(
+    int client_id,
+    const media::VideoCaptureParams& params,
+    const VideoCaptureStateUpdateCB& state_update_cb,
+    const VideoCaptureDeliverFrameCB& deliver_frame_cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  ClientInfo client_info;
+  client_info.params = params;
+  client_info.state_update_cb = state_update_cb;
+  client_info.deliver_frame_cb = deliver_frame_cb;
+
   if (state_ == VIDEO_CAPTURE_STATE_ERROR) {
-    handler->OnError(this, 1);
-    handler->OnRemoved(this);
-  } else if ((clients_pending_on_filter_.find(handler) !=
-              clients_pending_on_filter_.end()) ||
-             (clients_pending_on_restart_.find(handler) !=
-              clients_pending_on_restart_.end()) ||
-             clients_.find(handler) != clients_.end() ) {
-    // This client has started.
+    state_update_cb.Run(VIDEO_CAPTURE_STATE_ERROR);
+  } else if (clients_pending_on_filter_.count(client_id) ||
+             clients_pending_on_restart_.count(client_id) ||
+             clients_.count(client_id)) {
+    LOG(FATAL) << "This client has already started.";
   } else if (!device_id_) {
-    clients_pending_on_filter_[handler] = params;
+    clients_pending_on_filter_[client_id] = client_info;
   } else {
-    handler->OnStarted(this);
+    // Note: |state_| might not be started at this point. But we tell
+    // client that we have started.
+    state_update_cb.Run(VIDEO_CAPTURE_STATE_STARTED);
     if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
-      clients_[handler] = params;
+      clients_[client_id] = client_info;
       // TODO(sheu): Allowing resolution change will require that all
       // outstanding clients of a capture session support resolution change.
       DCHECK_EQ(params_.allow_resolution_change,
                 params.allow_resolution_change);
     } else if (state_ == VIDEO_CAPTURE_STATE_STOPPING) {
-      clients_pending_on_restart_[handler] = params;
+      clients_pending_on_restart_[client_id] = client_info;
       DVLOG(1) << "StartCapture: Got new resolution "
                << params.requested_format.frame_size.ToString()
                << " during stopping.";
     } else {
-      clients_[handler] = params;
-      DCHECK_EQ(1ul, clients_.size());
+      clients_[client_id] = client_info;
+      if (state_ == VIDEO_CAPTURE_STATE_STARTED)
+        return;
       params_ = params;
       if (params_.requested_format.frame_rate >
           media::limits::kMaxFramesPerSecond) {
@@ -170,16 +128,18 @@ void VideoCaptureImpl::StartCaptureOnIOThread(
   }
 }
 
-void VideoCaptureImpl::StopCaptureOnIOThread(
-    media::VideoCapture::EventHandler* handler) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+void VideoCaptureImpl::StopCapture(int client_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
 
-  // A handler can be in only one client list.
-  // If this handler is in any client list, we can just remove it from
+  // A client ID can be in only one client list.
+  // If this ID is in any client list, we can just remove it from
   // that client list and don't have to run the other following RemoveClient().
-  RemoveClient(handler, &clients_pending_on_filter_) ||
-  RemoveClient(handler, &clients_pending_on_restart_) ||
-  RemoveClient(handler, &clients_);
+  if (!RemoveClient(client_id, &clients_pending_on_filter_)) {
+    if (!RemoveClient(client_id, &clients_pending_on_restart_)) {
+      bool removed = RemoveClient(client_id, &clients_);
+      DCHECK(removed) << "Removing a non-existent client.";
+    }
+  }
 
   if (clients_.empty()) {
     DVLOG(1) << "StopCapture: No more client, stopping ...";
@@ -189,20 +149,20 @@ void VideoCaptureImpl::StopCaptureOnIOThread(
   }
 }
 
-void VideoCaptureImpl::GetDeviceSupportedFormatsOnIOThread(
-    const DeviceFormatsCallback& callback) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-  device_formats_callback_queue_.push_back(callback);
-  if (device_formats_callback_queue_.size() == 1)
+void VideoCaptureImpl::GetDeviceSupportedFormats(
+    const VideoCaptureDeviceFormatsCB& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  device_formats_cb_queue_.push_back(callback);
+  if (device_formats_cb_queue_.size() == 1)
     Send(new VideoCaptureHostMsg_GetDeviceSupportedFormats(device_id_,
                                                            session_id_));
 }
 
-void VideoCaptureImpl::GetDeviceFormatsInUseOnIOThread(
-    const DeviceFormatsInUseCallback& callback) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-  device_formats_in_use_callback_queue_.push_back(callback);
-  if (device_formats_in_use_callback_queue_.size() == 1)
+void VideoCaptureImpl::GetDeviceFormatsInUse(
+    const VideoCaptureDeviceFormatsCB& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  device_formats_in_use_cb_queue_.push_back(callback);
+  if (device_formats_in_use_cb_queue_.size() == 1)
     Send(
         new VideoCaptureHostMsg_GetDeviceFormatsInUse(device_id_, session_id_));
 }
@@ -210,7 +170,7 @@ void VideoCaptureImpl::GetDeviceFormatsInUseOnIOThread(
 void VideoCaptureImpl::OnBufferCreated(
     base::SharedMemoryHandle handle,
     int length, int buffer_id) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // In case client calls StopCapture before the arrival of created buffer,
   // just close this buffer and return.
@@ -234,7 +194,7 @@ void VideoCaptureImpl::OnBufferCreated(
 }
 
 void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   ClientBufferMap::iterator iter = client_buffers_.find(buffer_id);
   if (iter == client_buffers_.end())
@@ -248,7 +208,7 @@ void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
 void VideoCaptureImpl::OnBufferReceived(int buffer_id,
                                         const media::VideoCaptureFormat& format,
                                         base::TimeTicks timestamp) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // The capture pipeline supports only I420 for now.
   DCHECK_EQ(format.pixel_format, media::PIXEL_FORMAT_I420);
@@ -289,8 +249,10 @@ void VideoCaptureImpl::OnBufferReceived(int buffer_id,
               buffer,
               base::Passed(scoped_ptr<gpu::MailboxHolder>().Pass()))));
 
-  for (ClientInfo::iterator it = clients_.begin(); it != clients_.end(); ++it)
-    it->first->OnFrameReady(this, frame);
+  for (ClientInfoMap::iterator it = clients_.begin(); it != clients_.end();
+       ++it) {
+    it->second.deliver_frame_cb.Run(frame, format);
+  }
 }
 
 static void NullReadPixelsCB(const SkBitmap& bitmap) { NOTIMPLEMENTED(); }
@@ -300,7 +262,7 @@ void VideoCaptureImpl::OnMailboxBufferReceived(
     const gpu::MailboxHolder& mailbox_holder,
     const media::VideoCaptureFormat& format,
     base::TimeTicks timestamp) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (state_ != VIDEO_CAPTURE_STATE_STARTED || suspended_) {
     Send(new VideoCaptureHostMsg_BufferReady(
@@ -325,24 +287,28 @@ void VideoCaptureImpl::OnMailboxBufferReceived(
       timestamp - first_frame_timestamp_,
       base::Bind(&NullReadPixelsCB));
 
-  for (ClientInfo::iterator it = clients_.begin(); it != clients_.end(); ++it)
-    it->first->OnFrameReady(this, frame);
+  for (ClientInfoMap::iterator it = clients_.begin(); it != clients_.end();
+       ++it) {
+    it->second.deliver_frame_cb.Run(frame, format);
+  }
 }
 
 void VideoCaptureImpl::OnClientBufferFinished(
     int buffer_id,
     const scoped_refptr<ClientBuffer>& /* ignored_buffer */,
     scoped_ptr<gpu::MailboxHolder> mailbox_holder) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   const uint32 sync_point = (mailbox_holder ? mailbox_holder->sync_point : 0);
   Send(new VideoCaptureHostMsg_BufferReady(device_id_, buffer_id, sync_point));
 }
 
 void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   switch (state) {
     case VIDEO_CAPTURE_STATE_STARTED:
+      // Camera has started in the browser process. Since we have already
+      // told all clients that we have started there's nothing to do.
       break;
     case VIDEO_CAPTURE_STATE_STOPPED:
       state_ = VIDEO_CAPTURE_STATE_STOPPED;
@@ -353,27 +319,26 @@ void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
         RestartCapture();
       break;
     case VIDEO_CAPTURE_STATE_PAUSED:
-      for (ClientInfo::iterator it = clients_.begin();
+      for (ClientInfoMap::iterator it = clients_.begin();
            it != clients_.end(); ++it) {
-        it->first->OnPaused(this);
+        it->second.state_update_cb.Run(VIDEO_CAPTURE_STATE_PAUSED);
       }
       break;
     case VIDEO_CAPTURE_STATE_ERROR:
       DVLOG(1) << "OnStateChanged: error!, device_id = " << device_id_;
-      for (ClientInfo::iterator it = clients_.begin();
+      for (ClientInfoMap::iterator it = clients_.begin();
            it != clients_.end(); ++it) {
-        // TODO(wjia): browser process would send error code.
-        it->first->OnError(this, 1);
-        it->first->OnRemoved(this);
+        it->second.state_update_cb.Run(VIDEO_CAPTURE_STATE_ERROR);
       }
       clients_.clear();
       state_ = VIDEO_CAPTURE_STATE_ERROR;
       break;
     case VIDEO_CAPTURE_STATE_ENDED:
       DVLOG(1) << "OnStateChanged: ended!, device_id = " << device_id_;
-      for (ClientInfo::iterator it = clients_.begin();
+      for (ClientInfoMap::iterator it = clients_.begin();
           it != clients_.end(); ++it) {
-        it->first->OnRemoved(this);
+        // We'll only notify the client that the stream has stopped.
+        it->second.state_update_cb.Run(VIDEO_CAPTURE_STATE_STOPPED);
       }
       clients_.clear();
       state_ = VIDEO_CAPTURE_STATE_ENDED;
@@ -385,36 +350,41 @@ void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
 
 void VideoCaptureImpl::OnDeviceSupportedFormatsEnumerated(
     const media::VideoCaptureFormats& supported_formats) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-  for (size_t i = 0; i < device_formats_callback_queue_.size(); ++i)
-    device_formats_callback_queue_[i].Run(supported_formats);
-  device_formats_callback_queue_.clear();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  for (size_t i = 0; i < device_formats_cb_queue_.size(); ++i)
+    device_formats_cb_queue_[i].Run(supported_formats);
+  device_formats_cb_queue_.clear();
 }
 
 void VideoCaptureImpl::OnDeviceFormatsInUseReceived(
     const media::VideoCaptureFormats& formats_in_use) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-  for (size_t i = 0; i < device_formats_in_use_callback_queue_.size(); ++i)
-    device_formats_in_use_callback_queue_[i].Run(formats_in_use);
-  device_formats_in_use_callback_queue_.clear();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  for (size_t i = 0; i < device_formats_in_use_cb_queue_.size(); ++i)
+    device_formats_in_use_cb_queue_[i].Run(formats_in_use);
+  device_formats_in_use_cb_queue_.clear();
 }
 
 void VideoCaptureImpl::OnDelegateAdded(int32 device_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "OnDelegateAdded: device_id " << device_id;
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
   device_id_ = device_id;
-  for (ClientInfo::iterator it = clients_pending_on_filter_.begin();
+  for (ClientInfoMap::iterator it = clients_pending_on_filter_.begin();
        it != clients_pending_on_filter_.end(); ) {
-    media::VideoCapture::EventHandler* handler = it->first;
-    const media::VideoCaptureParams params = it->second;
+    int client_id = it->first;
+    VideoCaptureStateUpdateCB state_update_cb =
+        it->second.state_update_cb;
+    VideoCaptureDeliverFrameCB deliver_frame_cb =
+        it->second.deliver_frame_cb;
+    const media::VideoCaptureParams params = it->second.params;
     clients_pending_on_filter_.erase(it++);
-    StartCapture(handler, params);
+    StartCapture(client_id, params, state_update_cb,
+                 deliver_frame_cb);
   }
 }
 
 void VideoCaptureImpl::StopDevice() {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
     state_ = VIDEO_CAPTURE_STATE_STOPPING;
@@ -424,22 +394,20 @@ void VideoCaptureImpl::StopDevice() {
 }
 
 void VideoCaptureImpl::RestartCapture() {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(state_, VIDEO_CAPTURE_STATE_STOPPED);
 
   int width = 0;
   int height = 0;
-  for (ClientInfo::iterator it = clients_.begin();
+  clients_.insert(clients_pending_on_restart_.begin(),
+                  clients_pending_on_restart_.end());
+  clients_pending_on_restart_.clear();
+  for (ClientInfoMap::iterator it = clients_.begin();
        it != clients_.end(); ++it) {
-    width = std::max(width, it->second.requested_format.frame_size.width());
-    height = std::max(height, it->second.requested_format.frame_size.height());
-  }
-  for (ClientInfo::iterator it = clients_pending_on_restart_.begin();
-       it != clients_pending_on_restart_.end(); ) {
-    width = std::max(width, it->second.requested_format.frame_size.width());
-    height = std::max(height, it->second.requested_format.frame_size.height());
-    clients_[it->first] = it->second;
-    clients_pending_on_restart_.erase(it++);
+    width = std::max(width,
+                     it->second.params.requested_format.frame_size.width());
+    height = std::max(height,
+                      it->second.params.requested_format.frame_size.height());
   }
   params_.requested_format.frame_size.SetSize(width, height);
   DVLOG(1) << "RestartCapture, "
@@ -448,7 +416,7 @@ void VideoCaptureImpl::RestartCapture() {
 }
 
 void VideoCaptureImpl::StartCaptureInternal() {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(device_id_);
 
   Send(new VideoCaptureHostMsg_Start(device_id_, session_id_, params_));
@@ -456,21 +424,17 @@ void VideoCaptureImpl::StartCaptureInternal() {
 }
 
 void VideoCaptureImpl::Send(IPC::Message* message) {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(base::IgnoreResult(&VideoCaptureMessageFilter::Send),
-                 message_filter_.get(), message));
+  DCHECK(thread_checker_.CalledOnValidThread());
+  message_filter_->Send(message);
 }
 
-bool VideoCaptureImpl::RemoveClient(
-    media::VideoCapture::EventHandler* handler,
-    ClientInfo* clients) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
+bool VideoCaptureImpl::RemoveClient(int client_id, ClientInfoMap* clients) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   bool found = false;
 
-  ClientInfo::iterator it = clients->find(handler);
+  ClientInfoMap::iterator it = clients->find(client_id);
   if (it != clients->end()) {
-    handler->OnStopped(this);
-    handler->OnRemoved(this);
+    it->second.state_update_cb.Run(VIDEO_CAPTURE_STATE_STOPPED);
     clients->erase(it);
     found = true;
   }