Add IPC to query capabilities of video input devices.

Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for getUserMedia
such as device and settings selection based on constraints, and the applyConstraints
function.

BUG=657733
Review-Url: https://codereview.chromium.org/2646833002
Cr-Commit-Position: refs/heads/master@{#446977}
Committed: https://chromium.googlesource.com/chromium/src/+/8a440b84693ff261a07858d64655f90c285359fb

[Guido Urdaneta, 2017-01-19 19:22:58]

Description was changed from

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
==========

to

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Guido Urdaneta, 2017-01-19 19:44:00]

guidou@chromium.org changed reviewers:
+ clamy@chromium.org, dcheng@chromium.org, tommi@chromium.org

[Guido Urdaneta, 2017-01-19 19:44:00]

tommi@chromium.org: Please review changes in content/browser/renderer_host/media
and content/renderer/media

clamy@chromium.org: Please review changes in content/browser/

dcheng@chromium.org: Please review IPC

[Guido Urdaneta, 2017-01-19 19:46:28]

Description was changed from

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[dcheng, 2017-01-20 07:56:50]

https://codereview.chromium.org/2646833002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_delegate.h (right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_delegate.h:158: virtual bool
CheckMediaAccessPermission(const GURL& security_origin,
It's not introduced by this CL, but please add a TODO and file a bug to use
url::Origin here rather than GURL.

https://codereview.chromium.org/2646833002/diff/1/content/browser/media/media...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/media/media...
content/browser/media/media_devices_util.cc:71: <<
switches::kUseFakeDeviceForMediaStream << ".";
DLOG. This doesn't seem like a switch real users are going to use.

https://codereview.chromium.org/2646833002/diff/1/content/browser/media/media...
content/browser/media/media_devices_util.cc:76:
base::EqualsCaseInsensitiveASCII(param.front(),
Similarly, since this isn't really exposed to real users, I think a simple
comparison should suffice. *shrug*

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:158:
bad_message::ReceivedBadMessage(render_process_id_,
Hmm. +rockot... do you forsee replacing bad_message::ReceivedBadMessage with
mojo::ReportBadMessage?

[Guido Urdaneta, 2017-01-20 12:40:58]

https://codereview.chromium.org/2646833002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_delegate.h (right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_delegate.h:158: virtual bool
CheckMediaAccessPermission(const GURL& security_origin,
On 2017/01/20 07:56:50, dcheng wrote:
> It's not introduced by this CL, but please add a TODO and file a bug to use
> url::Origin here rather than GURL.

Done.

https://codereview.chromium.org/2646833002/diff/1/content/browser/media/media...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/media/media...
content/browser/media/media_devices_util.cc:71: <<
switches::kUseFakeDeviceForMediaStream << ".";
On 2017/01/20 07:56:50, dcheng wrote:
> DLOG. This doesn't seem like a switch real users are going to use.

Done.

https://codereview.chromium.org/2646833002/diff/1/content/browser/media/media...
content/browser/media/media_devices_util.cc:76:
base::EqualsCaseInsensitiveASCII(param.front(),
On 2017/01/20 07:56:50, dcheng wrote:
> Similarly, since this isn't really exposed to real users, I think a simple
> comparison should suffice. *shrug*

Done.

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:158:
bad_message::ReceivedBadMessage(render_process_id_,
On 2017/01/20 07:56:50, dcheng wrote:
> Hmm. +rockot... do you forsee replacing bad_message::ReceivedBadMessage with
> mojo::ReportBadMessage?

Should I make this change or was this a question for rockot?

[dcheng, 2017-01-20 18:28:29]

mojo lgtm

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:158:
bad_message::ReceivedBadMessage(render_process_id_,
On 2017/01/20 12:40:58, Guido Urdaneta wrote:
> On 2017/01/20 07:56:50, dcheng wrote:
> > Hmm. +rockot... do you forsee replacing bad_message::ReceivedBadMessage with
> > mojo::ReportBadMessage?
> 
> Should I make this change or was this a question for rockot?

This is a question for +rockot, I'm trying to understand what the long-term
plans are here. =)

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:347:
capabilities->device_id = std::move(translated_info.device_id);
FWIW, this should normally be typemapped... that being said, this particular
instance looks rather tricky given Mojo's semantics around serialization. Since
Mojo serialization calls the getters multiple times, we'd have to build all this
information in a "context" object. I guess this is fine for now, but I hope we
can do something better in here after https://crbug.com/681080 is resolved.

[Ken Rockot(use gerrit already), 2017-01-20 18:34:35]

rockot@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-01-20 18:34:36]

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:158:
bad_message::ReceivedBadMessage(render_process_id_,
On 2017/01/20 at 18:28:29, dcheng wrote:
> On 2017/01/20 12:40:58, Guido Urdaneta wrote:
> > On 2017/01/20 07:56:50, dcheng wrote:
> > > Hmm. +rockot... do you forsee replacing bad_message::ReceivedBadMessage
with
> > > mojo::ReportBadMessage?
> > 
> > Should I make this change or was this a question for rockot?
> 
> This is a question for +rockot, I'm trying to understand what the long-term
plans are here. =)

Yes, I think we should replace ReceivedBadMessage with ReportBadMessage. I'm not
sure there are any good reasons not to do so. Can you think of any?

ReportBadMessage doesn't have content dependencies or funky layering, its usage
is more flexible in general, it collects more useful data, and it's equally
effective at helping us shoot renderers.

[Ken Rockot(use gerrit already), 2017-01-20 18:37:47]

On 2017/01/20 at 18:34:36, Ken Rockot wrote:
>
https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
> File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):
> 
>
https://codereview.chromium.org/2646833002/diff/1/content/browser/renderer_ho...
> content/browser/renderer_host/media/media_devices_dispatcher_host.cc:158:
bad_message::ReceivedBadMessage(render_process_id_,
> On 2017/01/20 at 18:28:29, dcheng wrote:
> > On 2017/01/20 12:40:58, Guido Urdaneta wrote:
> > > On 2017/01/20 07:56:50, dcheng wrote:
> > > > Hmm. +rockot... do you forsee replacing bad_message::ReceivedBadMessage
with
> > > > mojo::ReportBadMessage?
> > > 
> > > Should I make this change or was this a question for rockot?
> > 
> > This is a question for +rockot, I'm trying to understand what the long-term
plans are here. =)
> 
> Yes, I think we should replace ReceivedBadMessage with ReportBadMessage. I'm
not sure there are any good reasons not to do so. Can you think of any?
> 
> ReportBadMessage doesn't have content dependencies or funky layering, its
usage is more flexible in general, it collects more useful data, and it's
equally effective at helping us shoot renderers.

I guess one difference is right now we have a fixed set of bad message "reasons"
and we aggregate a histogram of them, but I have no idea how useful that is in
practice. Killed renderers get crash dumps, and crash dumps have a useful stack
and a crash key with detailed error information.

[Guido Urdaneta, 2017-01-24 16:13:21]

guidou@chromium.org changed reviewers:
- tommi@chromium.org

[Guido Urdaneta, 2017-01-24 16:13:35]

guidou@chromium.org changed reviewers:
+ mcasas@chromium.org

[Guido Urdaneta, 2017-01-24 16:14:47]

moving tommi@ to cc.

mcasas@chromium.org: please review content/browser/renderer_host/media and
content/renderer/media

[Guido Urdaneta, 2017-01-24 16:24:30]

Patchset #4 (id:60001) has been deleted

[Guido Urdaneta, 2017-01-24 17:10:34]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 17:10:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 17:15:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 17:15:05]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Guido Urdaneta, 2017-01-24 17:52:26]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 17:52:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 20:36:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 20:36:59]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2017-01-27 09:18:14]

Friendly ping to mcasas@ and clamy@

[hta - Chromium, 2017-01-27 10:40:46]

hta@chromium.org changed reviewers:
+ hta@chromium.org

[hta - Chromium, 2017-01-27 10:40:47]

lgtm

except for a small comment on naming

https://codereview.chromium.org/2646833002/diff/100001/content/common/media/m...
File content/common/media/media_devices.mojom (right):

https://codereview.chromium.org/2646833002/diff/100001/content/common/media/m...
content/common/media/media_devices.mojom:25: UNKNOWN
Is there a particular reason for not calling it NONE and putting it first? This
just seems like a departure from existing patterns.

[Guido Urdaneta, 2017-01-27 14:10:47]

https://codereview.chromium.org/2646833002/diff/100001/content/common/media/m...
File content/common/media/media_devices.mojom (right):

https://codereview.chromium.org/2646833002/diff/100001/content/common/media/m...
content/common/media/media_devices.mojom:25: UNKNOWN
On 2017/01/27 10:40:47, hta - Chromium wrote:
> Is there a particular reason for not calling it NONE and putting it first?
This
> just seems like a departure from existing patterns.

Changed to NONE. Wasn't aware of the pattern.

[tommi (sloooow) - chröme, 2017-01-27 16:36:59]

tommi@chromium.org changed reviewers:
+ tommi@chromium.org

[tommi (sloooow) - chröme, 2017-01-27 16:37:00]

lgtm

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:373: if
(descriptor.device_id == default_device_id) {
supernit: use of {} here and none above is inconsistent

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
File
content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc:119:
std::vector<::mojom::VideoInputDeviceCapabilitiesPtr> capabilities) {
I'm assuming this would be passed via std::move

[clamy, 2017-01-27 16:44:34]

Thanks! One question below.

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
When is this supposed to be used?

[Guido Urdaneta, 2017-01-27 17:02:14]

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
On 2017/01/27 16:44:34, clamy wrote:
> When is this supposed to be used?

It is used when the kUseFakeDeviceForMediaStream command-line switch is passed.
See line 96.
Unit tests often use this switch. In this CL, MediaDevicesDispatcherHostTest
uses this switch and exercises this code.

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:373: if
(descriptor.device_id == default_device_id) {
On 2017/01/27 16:37:00, tommi (krómíum) wrote:
> supernit: use of {} here and none above is inconsistent

My interpretation of the style guide is that braces are required when the
statement inside the if has more than one line. Is that incorrect?

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
File
content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc:119:
std::vector<::mojom::VideoInputDeviceCapabilitiesPtr> capabilities) {
On 2017/01/27 16:37:00, tommi (krómíum) wrote:
> I'm assuming this would be passed via std::move

Correct. ::mojom::VideoInputDeviceCapabilitesPtr is move only, so in this case
the argument cannot even be passed without std::move.
This function is called by mojo to pass the result of the IPC call.

[clamy, 2017-01-27 17:05:48]

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
On 2017/01/27 17:02:14, Guido Urdaneta wrote:
> On 2017/01/27 16:44:34, clamy wrote:
> > When is this supposed to be used?
> 
> It is used when the kUseFakeDeviceForMediaStream command-line switch is
passed.
> See line 96.
> Unit tests often use this switch. In this CL, MediaDevicesDispatcherHostTest
> uses this switch and exercises this code.

If this is meant to be used in test, then I'd suggest making it explicit in the
function name that this is meant to be used for testing.

[Guido Urdaneta, 2017-01-27 17:14:44]

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
On 2017/01/27 17:05:47, clamy wrote:
> On 2017/01/27 17:02:14, Guido Urdaneta wrote:
> > On 2017/01/27 16:44:34, clamy wrote:
> > > When is this supposed to be used?
> > 
> > It is used when the kUseFakeDeviceForMediaStream command-line switch is
> passed.
> > See line 96.
> > Unit tests often use this switch. In this CL, MediaDevicesDispatcherHostTest
> > uses this switch and exercises this code.
> 
> If this is meant to be used in test, then I'd suggest making it explicit in
the
> function name that this is meant to be used for testing.

If I use a "ForTesting" suffix, I get the following warning from the presubmit
script:
  You might be calling functions intended only for testing from
  production code.  It is OK to ignore this warning if you know what
  you are doing, as the heuristics used to detect the situation are
  not perfect.  The commit queue will not block on this warning.
    content/browser/media/media_devices_util.cc:95
      callback.Run(GetDefaultMediaDeviceIDForTesting(device_type));

It won't actually be called unless one passes the command-line switch.
I guess it's OK to ignore the warning. Are you OK with that too?

[tommi (sloooow) - chröme, 2017-01-27 17:14:51]

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/render...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:373: if
(descriptor.device_id == default_device_id) {
On 2017/01/27 17:02:14, Guido Urdaneta wrote:
> On 2017/01/27 16:37:00, tommi (krómíum) wrote:
> > supernit: use of {} here and none above is inconsistent
> 
> My interpretation of the style guide is that braces are required when the
> statement inside the if has more than one line. Is that incorrect?

you are absolutely correct, I withdraw my supernit

[clamy, 2017-01-27 17:16:14]

Thanks! Lgtm.

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
On 2017/01/27 17:14:44, Guido Urdaneta wrote:
> On 2017/01/27 17:05:47, clamy wrote:
> > On 2017/01/27 17:02:14, Guido Urdaneta wrote:
> > > On 2017/01/27 16:44:34, clamy wrote:
> > > > When is this supposed to be used?
> > > 
> > > It is used when the kUseFakeDeviceForMediaStream command-line switch is
> > passed.
> > > See line 96.
> > > Unit tests often use this switch. In this CL,
MediaDevicesDispatcherHostTest
> > > uses this switch and exercises this code.
> > 
> > If this is meant to be used in test, then I'd suggest making it explicit in
> the
> > function name that this is meant to be used for testing.
> 
> If I use a "ForTesting" suffix, I get the following warning from the presubmit
> script:
>   You might be calling functions intended only for testing from
>   production code.  It is OK to ignore this warning if you know what
>   you are doing, as the heuristics used to detect the situation are
>   not perfect.  The commit queue will not block on this warning.
>     content/browser/media/media_devices_util.cc:95
>       callback.Run(GetDefaultMediaDeviceIDForTesting(device_type));
> 
> It won't actually be called unless one passes the command-line switch.
> I guess it's OK to ignore the warning. Are you OK with that too?

Acknowledged.

[Guido Urdaneta, 2017-01-27 17:17:51]

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
On 2017/01/27 17:16:14, clamy wrote:
> On 2017/01/27 17:14:44, Guido Urdaneta wrote:
> > On 2017/01/27 17:05:47, clamy wrote:
> > > On 2017/01/27 17:02:14, Guido Urdaneta wrote:
> > > > On 2017/01/27 16:44:34, clamy wrote:
> > > > > When is this supposed to be used?
> > > > 
> > > > It is used when the kUseFakeDeviceForMediaStream command-line switch is
> > > passed.
> > > > See line 96.
> > > > Unit tests often use this switch. In this CL,
> MediaDevicesDispatcherHostTest
> > > > uses this switch and exercises this code.
> > > 
> > > If this is meant to be used in test, then I'd suggest making it explicit
in
> > the
> > > function name that this is meant to be used for testing.
> > 
> > If I use a "ForTesting" suffix, I get the following warning from the
presubmit
> > script:
> >   You might be calling functions intended only for testing from
> >   production code.  It is OK to ignore this warning if you know what
> >   you are doing, as the heuristics used to detect the situation are
> >   not perfect.  The commit queue will not block on this warning.
> >     content/browser/media/media_devices_util.cc:95
> >       callback.Run(GetDefaultMediaDeviceIDForTesting(device_type));
> > 
> > It won't actually be called unless one passes the command-line switch.
> > I guess it's OK to ignore the warning. Are you OK with that too?
> 
> Acknowledged.

So, should I proceed with the rename?

[clamy, 2017-01-27 17:19:15]

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
File content/browser/media/media_devices_util.cc (right):

https://codereview.chromium.org/2646833002/diff/120001/content/browser/media/...
content/browser/media/media_devices_util.cc:52: std::string
GetDefaultMediaDeviceIDFromCommandLine(
On 2017/01/27 17:17:51, Guido Urdaneta wrote:
> On 2017/01/27 17:16:14, clamy wrote:
> > On 2017/01/27 17:14:44, Guido Urdaneta wrote:
> > > On 2017/01/27 17:05:47, clamy wrote:
> > > > On 2017/01/27 17:02:14, Guido Urdaneta wrote:
> > > > > On 2017/01/27 16:44:34, clamy wrote:
> > > > > > When is this supposed to be used?
> > > > > 
> > > > > It is used when the kUseFakeDeviceForMediaStream command-line switch
is
> > > > passed.
> > > > > See line 96.
> > > > > Unit tests often use this switch. In this CL,
> > MediaDevicesDispatcherHostTest
> > > > > uses this switch and exercises this code.
> > > > 
> > > > If this is meant to be used in test, then I'd suggest making it explicit
> in
> > > the
> > > > function name that this is meant to be used for testing.
> > > 
> > > If I use a "ForTesting" suffix, I get the following warning from the
> presubmit
> > > script:
> > >   You might be calling functions intended only for testing from
> > >   production code.  It is OK to ignore this warning if you know what
> > >   you are doing, as the heuristics used to detect the situation are
> > >   not perfect.  The commit queue will not block on this warning.
> > >     content/browser/media/media_devices_util.cc:95
> > >       callback.Run(GetDefaultMediaDeviceIDForTesting(device_type));
> > > 
> > > It won't actually be called unless one passes the command-line switch.
> > > I guess it's OK to ignore the warning. Are you OK with that too?
> > 
> > Acknowledged.
> 
> So, should I proceed with the rename?

No it's ok without. You could add a comment that explains it's used in tests
though :).

[Guido Urdaneta, 2017-01-30 09:35:56]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2017-01-30 09:35:57]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
hta@chromium.org, tommi@chromium.org, clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/2646833002/#ps140001
(title: "address comment from clamy")

[commit-bot: I haz the power, 2017-01-30 09:36:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 11:37:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 11:37:12]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Guido Urdaneta, 2017-01-30 11:37:48]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2017-01-30 11:37:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 13:30:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 13:30:20]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Guido Urdaneta, 2017-01-30 13:54:16]

Description was changed from

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
==========

[Guido Urdaneta, 2017-01-30 13:54:40]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2017-01-30 13:54:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-30 13:58:46]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1485784480590620,
"parent_rev": "8a47ea33cfcecc2cc0a9631d3b7baa80d420f06f", "commit_rev":
"8a440b84693ff261a07858d64655f90c285359fb"}

[commit-bot: I haz the power, 2017-01-30 13:59:19]

Description was changed from

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733
==========

to

==========
Add IPC to query capabilities of video input devices.

This IPC is intended to support new spec-compliant functionality for
getUserMedia
such as device and settings selection based on constraints, and the
applyConstraints
function.

BUG=657733

Review-Url: https://codereview.chromium.org/2646833002
Cr-Commit-Position: refs/heads/master@{#446977}
Committed:
https://chromium.googlesource.com/chromium/src/+/8a440b84693ff261a07858d64655...
==========

[commit-bot: I haz the power, 2017-01-30 13:59:22]

Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/8a440b84693ff261a07858d64655...