Skip to content

GitLab

T
tangled
Commit 76a4b2c9 authored by emircan's avatar emircan Committed by Commit bot
Browse files
Options

Changed thread_checker usage on VideoCaptureImpl and VideoCaptureImplManager... 

Changed thread_checker usage on VideoCaptureImpl and VideoCaptureImplManager to make it more explicit that the calls are run on the IO thread.

BUG=463009
TEST=Succesfully ran VideoCaptureImplTest.* and VideoCaptureImplManagerTest.* in content_unittests.

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

Cr-Commit-Position: refs/heads/master@{#322051}
parent edb762e8
Hide whitespace changes
Inline Side-by-side
Showing with 76 additions and 77 deletions
content/renderer/media/video_capture_impl.cc
View file @ 76a4b2c9
......@@ -52,27 +52,31 @@ VideoCaptureImpl::VideoCaptureImpl(
state_(VIDEO_CAPTURE_STATE_STOPPED),
weak_factory_(this) {
DCHECK(filter);
render_io_thread_checker_.DetachFromThread();
}
VideoCaptureImpl::~VideoCaptureImpl() {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
}
void VideoCaptureImpl::Init() {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
// For creating callbacks in unittest, this class may be constructed from a
// different thread than the IO thread, e.g. wherever unittest runs on.
// Therefore, this function should define the thread ownership.
#if DCHECK_IS_ON()
io_message_loop_ = base::MessageLoopProxy::current();
#endif
message_filter_->AddDelegate(this);
}
void VideoCaptureImpl::DeInit() {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
if (state_ == VIDEO_CAPTURE_STATE_STARTED)
Send(new VideoCaptureHostMsg_Stop(device_id_));
message_filter_->RemoveDelegate(this);
}
void VideoCaptureImpl::SuspendCapture(bool suspend) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
Send(suspend ?
static_cast<IPC::Message*>(new VideoCaptureHostMsg_Pause(device_id_)) :
static_cast<IPC::Message*>(
......@@ -84,7 +88,7 @@ void VideoCaptureImpl::StartCapture(
const media::VideoCaptureParams& params,
const VideoCaptureStateUpdateCB& state_update_cb,
const VideoCaptureDeliverFrameCB& deliver_frame_cb) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
ClientInfo client_info;
client_info.params = params;
client_info.state_update_cb = state_update_cb;
......@@ -132,7 +136,7 @@ void VideoCaptureImpl::StartCapture(
}
void VideoCaptureImpl::StopCapture(int client_id) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
// A client ID can be in only one client list.
// If this ID is in any client list, we can just remove it from
......@@ -153,7 +157,7 @@ void VideoCaptureImpl::StopCapture(int client_id) {
void VideoCaptureImpl::GetDeviceSupportedFormats(
const VideoCaptureDeviceFormatsCB& callback) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
device_formats_cb_queue_.push_back(callback);
if (device_formats_cb_queue_.size() == 1)
Send(new VideoCaptureHostMsg_GetDeviceSupportedFormats(device_id_,
......@@ -162,7 +166,7 @@ void VideoCaptureImpl::GetDeviceSupportedFormats(
void VideoCaptureImpl::GetDeviceFormatsInUse(
const VideoCaptureDeviceFormatsCB& callback) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
device_formats_in_use_cb_queue_.push_back(callback);
if (device_formats_in_use_cb_queue_.size() == 1)
Send(
......@@ -172,7 +176,7 @@ void VideoCaptureImpl::GetDeviceFormatsInUse(
void VideoCaptureImpl::OnBufferCreated(
base::SharedMemoryHandle handle,
int length, int buffer_id) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
// In case client calls StopCapture before the arrival of created buffer,
// just close this buffer and return.
......@@ -196,7 +200,7 @@ void VideoCaptureImpl::OnBufferCreated(
}
void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
const ClientBufferMap::iterator iter = client_buffers_.find(buffer_id);
if (iter == client_buffers_.end())
......@@ -212,7 +216,7 @@ void VideoCaptureImpl::OnBufferReceived(int buffer_id,
const gfx::Rect& visible_rect,
base::TimeTicks timestamp,
const base::DictionaryValue& metadata) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
if (state_ != VIDEO_CAPTURE_STATE_STARTED || suspended_) {
Send(new VideoCaptureHostMsg_BufferReady(device_id_, buffer_id, 0));
......@@ -261,7 +265,7 @@ void VideoCaptureImpl::OnMailboxBufferReceived(
const gfx::Size& packed_frame_size,
base::TimeTicks timestamp,
const base::DictionaryValue& metadata) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
if (state_ != VIDEO_CAPTURE_STATE_STARTED || suspended_) {
Send(new VideoCaptureHostMsg_BufferReady(device_id_, buffer_id, 0));
......@@ -288,13 +292,13 @@ void VideoCaptureImpl::OnClientBufferFinished(
int buffer_id,
const scoped_refptr<ClientBuffer>& /* ignored_buffer */,
uint32 release_sync_point) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
Send(new VideoCaptureHostMsg_BufferReady(
device_id_, buffer_id, release_sync_point));
}
void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
switch (state) {
case VIDEO_CAPTURE_STATE_STARTED:
......@@ -336,7 +340,7 @@ void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
void VideoCaptureImpl::OnDeviceSupportedFormatsEnumerated(
const media::VideoCaptureFormats& supported_formats) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
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();
......@@ -344,14 +348,14 @@ void VideoCaptureImpl::OnDeviceSupportedFormatsEnumerated(
void VideoCaptureImpl::OnDeviceFormatsInUseReceived(
const media::VideoCaptureFormats& formats_in_use) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
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(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
DVLOG(1) << "OnDelegateAdded: device_id " << device_id;
device_id_ = device_id;
......@@ -366,7 +370,7 @@ void VideoCaptureImpl::OnDelegateAdded(int32 device_id) {
}
void VideoCaptureImpl::StopDevice() {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
state_ = VIDEO_CAPTURE_STATE_STOPPING;
......@@ -376,7 +380,7 @@ void VideoCaptureImpl::StopDevice() {
}
void VideoCaptureImpl::RestartCapture() {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
DCHECK_EQ(state_, VIDEO_CAPTURE_STATE_STOPPED);
int width = 0;
......@@ -397,7 +401,7 @@ void VideoCaptureImpl::RestartCapture() {
}
void VideoCaptureImpl::StartCaptureInternal() {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
DCHECK(device_id_);
Send(new VideoCaptureHostMsg_Start(device_id_, session_id_, params_));
......@@ -405,12 +409,12 @@ void VideoCaptureImpl::StartCaptureInternal() {
}
void VideoCaptureImpl::Send(IPC::Message* message) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
message_filter_->Send(message);
}
bool VideoCaptureImpl::RemoveClient(int client_id, ClientInfoMap* clients) {
DCHECK(render_io_thread_checker_.CalledOnValidThread());
DCHECK(io_message_loop_->BelongsToCurrentThread());
bool found = false;
const ClientInfoMap::iterator it = clients->find(client_id);
......
content/renderer/media/video_capture_impl.h
View file @ 76a4b2c9
......@@ -2,21 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// VideoCaptureImpl represents a capture device in renderer process. It provides
// interfaces for clients to Start/Stop capture. It also communicates to clients
// when buffer is ready, state of capture device is changed.
// VideoCaptureImpl is also a delegate of VideoCaptureMessageFilter which relays
// operation of a capture device to the browser process and receives responses
// from browser process.
//
// VideoCaptureImpl is an IO thread only object. See the comments in
// video_capture_impl_manager.cc for the lifetime of this object.
// All methods must be called on the IO thread.
//
// This is an internal class used by VideoCaptureImplManager only. Do not access
// this directly.
#ifndef CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_
#define CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_
......@@ -45,6 +30,20 @@ class VideoFrame;
namespace content {
// VideoCaptureImpl represents a capture device in renderer process. It provides
// interfaces for clients to Start/Stop capture. It also communicates to clients
// when buffer is ready, state of capture device is changed.
// VideoCaptureImpl is also a delegate of VideoCaptureMessageFilter which relays
// operation of a capture device to the browser process and receives responses
// from browser process.
//
// VideoCaptureImpl is an IO thread only object. See the comments in
// video_capture_impl_manager.cc for the lifetime of this object.
// All methods must be called on the IO thread.
//
// This is an internal class used by VideoCaptureImplManager only. Do not access
// this directly.
class CONTENT_EXPORT VideoCaptureImpl
: public VideoCaptureMessageFilter::Delegate {
public:
......@@ -170,8 +169,8 @@ class CONTENT_EXPORT VideoCaptureImpl
bool suspended_;
VideoCaptureState state_;
// |weak_factory_| and |thread_checker_| are bound to the IO thread.
base::ThreadChecker render_io_thread_checker_;
// IO message loop reference for checking correct class operation.
scoped_refptr<base::MessageLoopProxy> io_message_loop_;
// WeakPtrFactory pointing back to |this| object, for use with
// media::VideoFrames constructed in OnBufferReceived() from buffers cached
......
content/renderer/media/video_capture_impl_manager.cc
View file @ 76a4b2c9
......@@ -36,11 +36,12 @@ namespace content {
VideoCaptureImplManager::VideoCaptureImplManager()
: next_client_id_(0),
filter_(new VideoCaptureMessageFilter()),
render_main_message_loop_(base::MessageLoopProxy::current()),
weak_factory_(this) {
}
VideoCaptureImplManager::~VideoCaptureImplManager() {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
if (devices_.empty())
return;
// Forcibly release all video capture resources.
......@@ -58,7 +59,7 @@ VideoCaptureImplManager::~VideoCaptureImplManager() {
base::Closure VideoCaptureImplManager::UseDevice(
media::VideoCaptureSessionId id) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
VideoCaptureImpl* impl = NULL;
const VideoCaptureDeviceMap::iterator it = devices_.find(id);
......@@ -83,7 +84,7 @@ base::Closure VideoCaptureImplManager::StartCapture(
const media::VideoCaptureParams& params,
const VideoCaptureStateUpdateCB& state_update_cb,
const VideoCaptureDeliverFrameCB& deliver_frame_cb) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
const VideoCaptureDeviceMap::const_iterator it = devices_.find(id);
DCHECK(it != devices_.end());
VideoCaptureImpl* const impl = it->second.second;
......@@ -107,7 +108,7 @@ base::Closure VideoCaptureImplManager::StartCapture(
void VideoCaptureImplManager::GetDeviceSupportedFormats(
media::VideoCaptureSessionId id,
const VideoCaptureDeviceFormatsCB& callback) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
const VideoCaptureDeviceMap::const_iterator it = devices_.find(id);
DCHECK(it != devices_.end());
VideoCaptureImpl* const impl = it->second.second;
......@@ -120,7 +121,7 @@ void VideoCaptureImplManager::GetDeviceSupportedFormats(
void VideoCaptureImplManager::GetDeviceFormatsInUse(
media::VideoCaptureSessionId id,
const VideoCaptureDeviceFormatsCB& callback) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
const VideoCaptureDeviceMap::const_iterator it = devices_.find(id);
DCHECK(it != devices_.end());
VideoCaptureImpl* const impl = it->second.second;
......@@ -137,9 +138,9 @@ VideoCaptureImplManager::CreateVideoCaptureImplForTesting(
return NULL;
}
void VideoCaptureImplManager::StopCapture(
int client_id, media::VideoCaptureSessionId id) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
void VideoCaptureImplManager::StopCapture(int client_id,
media::VideoCaptureSessionId id) {
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
const VideoCaptureDeviceMap::const_iterator it = devices_.find(id);
DCHECK(it != devices_.end());
VideoCaptureImpl* const impl = it->second.second;
......@@ -151,7 +152,7 @@ void VideoCaptureImplManager::StopCapture(
void VideoCaptureImplManager::UnrefDevice(
media::VideoCaptureSessionId id) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
const VideoCaptureDeviceMap::iterator it = devices_.find(id);
DCHECK(it != devices_.end());
VideoCaptureImpl* const impl = it->second.second;
......@@ -171,13 +172,12 @@ void VideoCaptureImplManager::UnrefDevice(
}
void VideoCaptureImplManager::SuspendDevices(bool suspend) {
DCHECK(render_main_thread_checker_.CalledOnValidThread());
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
for (const auto& device : devices_) {
VideoCaptureImpl* const impl = device.second.second;
ChildProcess::current()->io_message_loop_proxy()->PostTask(
FROM_HERE,
base::Bind(&VideoCaptureImpl::SuspendCapture,
base::Unretained(impl), suspend));
FROM_HERE, base::Bind(&VideoCaptureImpl::SuspendCapture,
base::Unretained(impl), suspend));
}
}
......
content/renderer/media/video_capture_impl_manager.h
View file @ 76a4b2c9
......@@ -2,20 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// TODO(hclam): This class should be renamed to VideoCaptureService.
// This class provides access to a video capture device in the browser
// process through IPC. The main function is to deliver video frames
// to a client.
//
// THREADING
//
// VideoCaptureImplManager lives only on the render thread. All methods
// must be called on this thread.
//
// VideoFrames are delivered on the IO thread. Callbacks provided by
// a client are also called on the IO thread.
#ifndef CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_MANAGER_H_
#define CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_MANAGER_H_
......@@ -39,6 +25,19 @@ namespace content {
class VideoCaptureImpl;
class VideoCaptureMessageFilter;
// TODO(hclam): This class should be renamed to VideoCaptureService.
// This class provides access to a video capture device in the browser
// process through IPC. The main function is to deliver video frames
// to a client.
//
// THREADING
//
// VideoCaptureImplManager lives only on the Render Main thread. All methods
// must be called on this thread.
//
// VideoFrames are delivered on the IO thread. Callbacks provided by
// a client are also called on the IO thread.
class CONTENT_EXPORT VideoCaptureImplManager {
public:
VideoCaptureImplManager();
......@@ -116,8 +115,9 @@ class CONTENT_EXPORT VideoCaptureImplManager {
const scoped_refptr<VideoCaptureMessageFilter> filter_;
// Bound to the render thread.
base::ThreadChecker render_main_thread_checker_;
// Hold a pointer to the Render Main message loop to check we operate on the
// right thread.
const scoped_refptr<base::MessageLoopProxy> render_main_message_loop_;
// Bound to the render thread.
// NOTE: Weak pointers must be invalidated before all other member variables.
......
content/renderer/media/video_capture_impl_manager_unittest.cc
View file @ 76a4b2c9
......@@ -137,10 +137,8 @@ TEST_F(VideoCaptureImplManagerTest, MultipleClients) {
base::Closure stop_cb1, stop_cb2;
{
base::RunLoop run_loop;
base::Closure quit_closure = BindToCurrentLoop(
run_loop.QuitClosure());
EXPECT_CALL(*this, OnStarted()).WillOnce(
RunClosure(quit_closure));
base::Closure quit_closure = BindToCurrentLoop(run_loop.QuitClosure());
EXPECT_CALL(*this, OnStarted()).WillOnce(RunClosure(quit_closure));
EXPECT_CALL(*this, OnStarted()).RetiresOnSaturation();
stop_cb1 = StartCapture(params_);
stop_cb2 = StartCapture(params_);
......@@ -150,10 +148,8 @@ TEST_F(VideoCaptureImplManagerTest, MultipleClients) {
{
base::RunLoop run_loop;
base::Closure quit_closure = BindToCurrentLoop(
run_loop.QuitClosure());
EXPECT_CALL(*this, OnStopped()).WillOnce(
RunClosure(quit_closure));
base::Closure quit_closure = BindToCurrentLoop(run_loop.QuitClosure());
EXPECT_CALL(*this, OnStopped()).WillOnce(RunClosure(quit_closure));
EXPECT_CALL(*this, OnStopped()).RetiresOnSaturation();
stop_cb1.Run();
stop_cb2.Run();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment