Commit 5daa323d authored by Guido Urdaneta's avatar Guido Urdaneta Committed by Commit Bot

Run constraints processing for video-device capture on the main thread

The constraints processing algorithm operates on data coming directly
from Blink, and it may happen that a future change accesses a
string or some other data that is allowed to be accessed only from
the main thread. So far no such bug has been found for device capture,
but one was recently discovered for screen capture. See
http://crbug.com/791992.

The original motivation to run constraints processing on the worker
thread was to avoid blocking the main thread in case the algorithm
took too long to run, but in practice we have not observed
significant performance issues.

Bug: 792106
Change-Id: I5fa6f9e1b107e680cb4a5b544488fe7f2d8bfb0d
Reviewed-on: https://chromium-review.googlesource.com/809011Reviewed-by: default avatarHenrik Boström <hbos@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521790}
parent 6c40391f
...@@ -117,8 +117,7 @@ UserMediaClientImpl::UserMediaClientImpl( ...@@ -117,8 +117,7 @@ UserMediaClientImpl::UserMediaClientImpl(
UserMediaClientImpl::UserMediaClientImpl( UserMediaClientImpl::UserMediaClientImpl(
RenderFrame* render_frame, RenderFrame* render_frame,
PeerConnectionDependencyFactory* dependency_factory, PeerConnectionDependencyFactory* dependency_factory,
std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer, std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer)
const scoped_refptr<base::TaskRunner>& worker_task_runner)
: UserMediaClientImpl( : UserMediaClientImpl(
render_frame, render_frame,
std::make_unique<UserMediaProcessor>( std::make_unique<UserMediaProcessor>(
...@@ -127,8 +126,7 @@ UserMediaClientImpl::UserMediaClientImpl( ...@@ -127,8 +126,7 @@ UserMediaClientImpl::UserMediaClientImpl(
std::move(media_stream_device_observer), std::move(media_stream_device_observer),
base::BindRepeating( base::BindRepeating(
&UserMediaClientImpl::GetMediaDevicesDispatcher, &UserMediaClientImpl::GetMediaDevicesDispatcher,
base::Unretained(this)), base::Unretained(this)))) {}
worker_task_runner)) {}
UserMediaClientImpl::~UserMediaClientImpl() { UserMediaClientImpl::~UserMediaClientImpl() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
......
...@@ -26,10 +26,6 @@ ...@@ -26,10 +26,6 @@
#include "third_party/WebKit/public/web/WebUserMediaClient.h" #include "third_party/WebKit/public/web/WebUserMediaClient.h"
#include "third_party/WebKit/public/web/WebUserMediaRequest.h" #include "third_party/WebKit/public/web/WebUserMediaRequest.h"
namespace base {
class TaskRunner;
}
namespace content { namespace content {
class ApplyConstraintsProcessor; class ApplyConstraintsProcessor;
...@@ -49,8 +45,7 @@ class CONTENT_EXPORT UserMediaClientImpl : public RenderFrameObserver, ...@@ -49,8 +45,7 @@ class CONTENT_EXPORT UserMediaClientImpl : public RenderFrameObserver,
UserMediaClientImpl( UserMediaClientImpl(
RenderFrame* render_frame, RenderFrame* render_frame,
PeerConnectionDependencyFactory* dependency_factory, PeerConnectionDependencyFactory* dependency_factory,
std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer, std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer);
const scoped_refptr<base::TaskRunner>& worker_task_runner);
UserMediaClientImpl(RenderFrame* render_frame, UserMediaClientImpl(RenderFrame* render_frame,
std::unique_ptr<UserMediaProcessor> user_media_processor); std::unique_ptr<UserMediaProcessor> user_media_processor);
~UserMediaClientImpl() override; ~UserMediaClientImpl() override;
......
...@@ -298,8 +298,7 @@ class UserMediaProcessorUnderTest : public UserMediaProcessor { ...@@ -298,8 +298,7 @@ class UserMediaProcessorUnderTest : public UserMediaProcessor {
std::move(media_stream_device_observer), std::move(media_stream_device_observer),
base::BindRepeating( base::BindRepeating(
&UserMediaProcessorUnderTest::media_devices_dispatcher, &UserMediaProcessorUnderTest::media_devices_dispatcher,
base::Unretained(this)), base::Unretained(this))),
base::ThreadTaskRunnerHandle::Get()),
factory_(dependency_factory), factory_(dependency_factory),
media_devices_dispatcher_(std::move(media_devices_dispatcher)), media_devices_dispatcher_(std::move(media_devices_dispatcher)),
state_(state) {} state_(state) {}
......
...@@ -324,12 +324,10 @@ UserMediaProcessor::UserMediaProcessor( ...@@ -324,12 +324,10 @@ UserMediaProcessor::UserMediaProcessor(
RenderFrame* render_frame, RenderFrame* render_frame,
PeerConnectionDependencyFactory* dependency_factory, PeerConnectionDependencyFactory* dependency_factory,
std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer, std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer,
MediaDevicesDispatcherCallback media_devices_dispatcher_cb, MediaDevicesDispatcherCallback media_devices_dispatcher_cb)
const scoped_refptr<base::TaskRunner>& worker_task_runner)
: dependency_factory_(dependency_factory), : dependency_factory_(dependency_factory),
media_stream_device_observer_(std::move(media_stream_device_observer)), media_stream_device_observer_(std::move(media_stream_device_observer)),
media_devices_dispatcher_cb_(std::move(media_devices_dispatcher_cb)), media_devices_dispatcher_cb_(std::move(media_devices_dispatcher_cb)),
worker_task_runner_(worker_task_runner),
render_frame_id_(render_frame ? render_frame->GetRoutingID() render_frame_id_(render_frame ? render_frame->GetRoutingID()
: MSG_ROUTING_NONE), : MSG_ROUTING_NONE),
weak_factory_(this) { weak_factory_(this) {
...@@ -486,24 +484,11 @@ void UserMediaProcessor::SelectVideoDeviceSettings( ...@@ -486,24 +484,11 @@ void UserMediaProcessor::SelectVideoDeviceSettings(
capabilities.noise_reduction_capabilities = {base::Optional<bool>(), capabilities.noise_reduction_capabilities = {base::Optional<bool>(),
base::Optional<bool>(true), base::Optional<bool>(true),
base::Optional<bool>(false)}; base::Optional<bool>(false)};
base::PostTaskAndReplyWithResult( VideoCaptureSettings settings = SelectSettingsVideoDeviceCapture(
worker_task_runner_.get(), FROM_HERE, std::move(capabilities), web_request.VideoConstraints(),
base::Bind(&SelectSettingsVideoDeviceCapture, std::move(capabilities),
web_request.VideoConstraints(),
MediaStreamVideoSource::kDefaultWidth, MediaStreamVideoSource::kDefaultWidth,
MediaStreamVideoSource::kDefaultHeight, MediaStreamVideoSource::kDefaultHeight,
MediaStreamVideoSource::kDefaultFrameRate), MediaStreamVideoSource::kDefaultFrameRate);
base::Bind(&UserMediaProcessor::FinalizeSelectVideoDeviceSettings,
weak_factory_.GetWeakPtr(), web_request));
}
void UserMediaProcessor::FinalizeSelectVideoDeviceSettings(
const blink::WebUserMediaRequest& web_request,
const VideoCaptureSettings& settings) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsCurrentRequestInfo(web_request))
return;
if (!settings.HasValue()) { if (!settings.HasValue()) {
blink::WebString failed_constraint_name = blink::WebString failed_constraint_name =
blink::WebString::FromASCII(settings.failed_constraint_name()); blink::WebString::FromASCII(settings.failed_constraint_name());
......
...@@ -23,10 +23,6 @@ ...@@ -23,10 +23,6 @@
#include "third_party/WebKit/public/platform/modules/mediastream/media_devices.mojom.h" #include "third_party/WebKit/public/platform/modules/mediastream/media_devices.mojom.h"
#include "third_party/WebKit/public/web/WebUserMediaRequest.h" #include "third_party/WebKit/public/web/WebUserMediaRequest.h"
namespace base {
class TaskRunner;
}
namespace blink { namespace blink {
class WebMediaConstraints; class WebMediaConstraints;
class WebMediaStream; class WebMediaStream;
...@@ -72,8 +68,7 @@ class CONTENT_EXPORT UserMediaProcessor ...@@ -72,8 +68,7 @@ class CONTENT_EXPORT UserMediaProcessor
RenderFrame* render_frame, RenderFrame* render_frame,
PeerConnectionDependencyFactory* dependency_factory, PeerConnectionDependencyFactory* dependency_factory,
std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer, std::unique_ptr<MediaStreamDeviceObserver> media_stream_device_observer,
MediaDevicesDispatcherCallback media_devices_dispatcher_cb, MediaDevicesDispatcherCallback media_devices_dispatcher_cb);
const scoped_refptr<base::TaskRunner>& worker_task_runner);
~UserMediaProcessor() override; ~UserMediaProcessor() override;
// It can be assumed that the output of CurrentRequest() remains the same // It can be assumed that the output of CurrentRequest() remains the same
...@@ -289,8 +284,6 @@ class CONTENT_EXPORT UserMediaProcessor ...@@ -289,8 +284,6 @@ class CONTENT_EXPORT UserMediaProcessor
MediaDevicesDispatcherCallback media_devices_dispatcher_cb_; MediaDevicesDispatcherCallback media_devices_dispatcher_cb_;
base::OnceClosure request_completed_cb_; base::OnceClosure request_completed_cb_;
const scoped_refptr<base::TaskRunner> worker_task_runner_;
const int render_frame_id_; const int render_frame_id_;
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
......
...@@ -6691,8 +6691,7 @@ void RenderFrameImpl::InitializeUserMediaClient() { ...@@ -6691,8 +6691,7 @@ void RenderFrameImpl::InitializeUserMediaClient() {
DCHECK(!web_user_media_client_); DCHECK(!web_user_media_client_);
web_user_media_client_ = new UserMediaClientImpl( web_user_media_client_ = new UserMediaClientImpl(
this, RenderThreadImpl::current()->GetPeerConnectionDependencyFactory(), this, RenderThreadImpl::current()->GetPeerConnectionDependencyFactory(),
std::make_unique<MediaStreamDeviceObserver>(this), std::make_unique<MediaStreamDeviceObserver>(this));
render_thread->GetWorkerTaskRunner());
registry_.AddInterface( registry_.AddInterface(
base::Bind(&MediaDevicesListenerImpl::Create, GetRoutingID())); base::Bind(&MediaDevicesListenerImpl::Create, GetRoutingID()));
#endif #endif
......
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