Refactor video capturing code in the render process

content/renderer/media/video_capture_impl.cc

 // 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.
 
 #include "content/renderer/media/video_capture_impl.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "content/child/child_process.h"
 #include "content/common/media/video_capture_messages.h"
@@ skipping 14 matching lines @@
   const size_t buffer_size;
 
  private:
   friend class base::RefCountedThreadSafe<ClientBuffer>;
 
   virtual ~ClientBuffer() {}
 
   DISALLOW_COPY_AND_ASSIGN(ClientBuffer);
 };
 
-bool VideoCaptureImpl::CaptureStarted() {
-  return state_ == VIDEO_CAPTURE_STATE_STARTED;
-}
+VideoCaptureImpl::ClientInfo::ClientInfo() {}
+VideoCaptureImpl::ClientInfo::~ClientInfo() {}
 
 int VideoCaptureImpl::CaptureFrameRate() {
   return last_frame_format_.frame_rate;
 }
 
 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() {}
+VideoCaptureImpl::~VideoCaptureImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
 
 void VideoCaptureImpl::Init() {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::InitOnIOThread,
-                 base::Unretained(this)));
+  DCHECK(thread_checker_.CalledOnValidThread());
+  message_filter_->AddDelegate(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::DeInit() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (state_ == VIDEO_CAPTURE_STATE_STARTED)
+    Send(new VideoCaptureHostMsg_Stop(device_id_));
+  message_filter_->RemoveDelegate(this);
 }
 
 void VideoCaptureImpl::SuspendCapture(bool suspend) {
-  io_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::SuspendCaptureOnIOThread,
-                 base::Unretained(this),
-                 suspend));
+  DCHECK(thread_checker_.CalledOnValidThread());
+  suspended_ = 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));
-}
+    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;
 
-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)));
-}
-
-void VideoCaptureImpl::InitOnIOThread() {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-  message_filter_->AddDelegate(this);
-}
-
-void VideoCaptureImpl::DeInitOnIOThread(base::Closure done_cb) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-  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());
-  suspended_ = suspend;
-}
-
-void VideoCaptureImpl::StartCaptureOnIOThread(
-    media::VideoCapture::EventHandler* handler,
-    const media::VideoCaptureParams& params) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
   if (state_ == VIDEO_CAPTURE_STATE_ERROR) {
-    handler->OnError(this, 1);
-    handler->OnRemoved(this);
-  } else if ((clients_pending_on_filter_.find(handler) !=
+    state_update_cb.Run(VIDEO_CAPTURE_STATE_ERROR);
+  } else if ((clients_pending_on_filter_.find(client_id) !=

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

map_.find(foo) != map_.end()
is shorter to write as
map_.count(foo)
(and avoids the pitfall of accidentally specifying a different map_ the second
time)

[Alpha Left Google, 2014/04/23 18:48:33]

Done.

               clients_pending_on_filter_.end()) ||
-             (clients_pending_on_restart_.find(handler) !=
+             (clients_pending_on_restart_.find(client_id) !=
               clients_pending_on_restart_.end()) ||
-             clients_.find(handler) != clients_.end() ) {
+             clients_.find(client_id) != clients_.end() ) {
     // This client has started.

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

what about the callbacks?  This seems like it should be a DFATAL.

[Alpha Left Google, 2014/04/23 18:48:33]

Done.

   } 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;
+      clients_[client_id] = client_info;
       DCHECK_EQ(1ul, clients_.size());

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

'l' should not be necessary here.

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

what prevents two clients from registering almost-simultaneously?

[Alpha Left Google, 2014/04/23 18:48:33]

Done.

[Alpha Left Google, 2014/04/23 18:48:33]

I *think* the intention in this code is to verify that it doesn't start twice.
Added a check for state. Also a test for this.

       params_ = params;
       if (params_.requested_format.frame_rate >
           media::limits::kMaxFramesPerSecond) {
         params_.requested_format.frame_rate =
             media::limits::kMaxFramesPerSecond;
       }
       DVLOG(1) << "StartCapture: starting with first resolution "
                << params_.requested_format.frame_size.ToString();
       first_frame_timestamp_ = base::TimeTicks();
       StartCaptureInternal();
     }
   }
 }
 
-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_);
+  RemoveClient(client_id, &clients_pending_on_filter_) ||
+  RemoveClient(client_id, &clients_pending_on_restart_) ||
+  RemoveClient(client_id, &clients_);

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

ick to using boolean short-cutting like this (in a context where the
expression's value is dropped)

[Alpha Left Google, 2014/04/23 18:48:33]

RemoveClient actually calls the client and give it a stopped state. I have made
this cleaner a bit.

 
   if (clients_.empty()) {
     DVLOG(1) << "StopCapture: No more client, stopping ...";
     StopDevice();
     client_buffers_.clear();
     weak_factory_.InvalidateWeakPtrs();
   }
 }
 
-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_));
 }
 
 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.
   if (state_ != VIDEO_CAPTURE_STATE_STARTED) {
     base::SharedMemory::CloseHandle(handle);
     return;
   }
 
   scoped_ptr<base::SharedMemory> shm(new base::SharedMemory(handle, false));
   if (!shm->Map(length)) {
     DLOG(ERROR) << "OnBufferCreated: Map failed.";
     return;
   }
 
   bool inserted =
       client_buffers_.insert(std::make_pair(
                                  buffer_id,
                                  new ClientBuffer(shm.Pass(),
                                                   length))).second;
   DCHECK(inserted);
 }
 
 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())
     return;
 
   DCHECK(!iter->second || iter->second->HasOneRef())
       << "Instructed to delete buffer we are still using.";
   client_buffers_.erase(iter);
 }
 
 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);
 
   if (state_ != VIDEO_CAPTURE_STATE_STARTED || suspended_) {
     Send(new VideoCaptureHostMsg_BufferReady(device_id_, buffer_id, 0));
     return;
   }
 
   last_frame_format_ = format;
@@ skipping 20 matching lines @@
           buffer->buffer_size,
           buffer->buffer->handle(),
           timestamp - first_frame_timestamp_,
           media::BindToCurrentLoop(base::Bind(
               &VideoCaptureImpl::OnClientBufferFinished,
               weak_factory_.GetWeakPtr(),
               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, timestamp);
+  }
 }
 
 static void NullReadPixelsCB(const SkBitmap& bitmap) { NOTIMPLEMENTED(); }
 
 void VideoCaptureImpl::OnMailboxBufferReceived(
     int buffer_id,
     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(
         device_id_, buffer_id, mailbox_holder.sync_point));
     return;
   }
 
   last_frame_format_ = format;
   if (first_frame_timestamp_.is_null())
     first_frame_timestamp_ = timestamp;
 
   scoped_refptr<media::VideoFrame> frame = media::VideoFrame::WrapNativeTexture(
       make_scoped_ptr(new gpu::MailboxHolder(mailbox_holder)),
       media::BindToCurrentLoop(
           base::Bind(&VideoCaptureImpl::OnClientBufferFinished,
                      weak_factory_.GetWeakPtr(),
                      buffer_id,
                      scoped_refptr<ClientBuffer>())),
       last_frame_format_.frame_size,
       gfx::Rect(last_frame_format_.frame_size),
       last_frame_format_.frame_size,
       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, timestamp);
+  }
 }
 
 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:

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

seems strange that this is a no-op.

[Alpha Left Google, 2014/04/23 18:48:33]

Added a comment.

       break;
     case VIDEO_CAPTURE_STATE_STOPPED:
       state_ = VIDEO_CAPTURE_STATE_STOPPED;
       DVLOG(1) << "OnStateChanged: stopped!, device_id = " << device_id_;
       client_buffers_.clear();
       weak_factory_.InvalidateWeakPtrs();
       if (!clients_.empty() || !clients_pending_on_restart_.empty())
         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;
       break;
     default:
       break;
   }
 }
 
 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;
     Send(new VideoCaptureHostMsg_Stop(device_id_));
     params_.requested_format.frame_size.SetSize(0, 0);
   }
 }
 
 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();
+  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());
+    width = std::max(width,
+                     it->second.params.requested_format.frame_size.width());
+    height = std::max(height,
+                      it->second.params.requested_format.frame_size.height());
   }
-  for (ClientInfo::iterator it = clients_pending_on_restart_.begin();
+  for (ClientInfoMap::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());
+    width = std::max(width,
+                     it->second.params.requested_format.frame_size.width());
+    height = std::max(height,
+                      it->second.params.requested_format.frame_size.height());
     clients_[it->first] = it->second;
     clients_pending_on_restart_.erase(it++);
   }

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

if you did 
clients_.insert(clients_pending_on_restart_.begin(),
clients_pending_on_restart_.end());
at l.395 then you could delete this loop.

[Alpha Left Google, 2014/04/23 18:48:33]

Done.

   params_.requested_format.frame_size.SetSize(width, height);
   DVLOG(1) << "RestartCapture, "
            << params_.requested_format.frame_size.ToString();
   StartCaptureInternal();
 }
 
 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_));
   state_ = VIDEO_CAPTURE_STATE_STARTED;
 }
 
 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;
   }
   return found;

[Ami GONE FROM CHROMIUM, 2014/04/21 23:42:53]

l.432-440 can be replaced with
return clients->erase(client_id) != 0;

(and, given the only callsite's ick comment above, if it were me I'd drop this
helper function altogether in favor of
size_t erased = clients_->erase(client_id) +
    clients_at_blah_->erase(client_id) +
    clients_other_foo->erase(client_id);
DCHECK_EQ(erased, 1);
)

[Alpha Left Google, 2014/04/23 18:48:33]

I still need it to send the client the stopped signal.

 }
 
 }  // namespace content