Add ability to retrieve capabilities for audio input devices to MediaDevicesDispatcherHost.

Add ability to retrieve capabilities for audio input devices to MediaDevicesDispatcherHost.

BUG=657733
Review-Url: https://codereview.chromium.org/2868853002
Cr-Commit-Position: refs/heads/master@{#471774}
Committed: https://chromium.googlesource.com/chromium/src/+/5c25608ed9478c41bdcabad1fbea6cd85a6e89e7

[Guido Urdaneta, 2017-05-08 14:50:59]

Patchset #1 (id:1) has been deleted

[Guido Urdaneta, 2017-05-08 14:55:21]

Patchset #1 (id:10009) has been deleted

[Guido Urdaneta, 2017-05-08 14:56:57]

Description was changed from

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

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

to

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

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

[Guido Urdaneta, 2017-05-08 14:56:57]

guidou@chromium.org changed reviewers:
+ mkwst@chromium.org, tommi@chromium.org

[Guido Urdaneta, 2017-05-08 14:59:13]

Hi, PTAL

tommi@: Please review content/browser/* and content/renderer/*

mkwst@: Please review IPC

[Guido Urdaneta, 2017-05-08 14:59:19]

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

[commit-bot: I haz the power, 2017-05-08 14:59:47]

Dry run: CQ is trying da patch.

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

[Guido Urdaneta, 2017-05-08 15:05:27]

Description was changed from

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

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

to

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

Drive-by: remove unused variable in MDDH.

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

[commit-bot: I haz the power, 2017-05-08 15:13:08]

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

[commit-bot: I haz the power, 2017-05-08 15:13:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Guido Urdaneta, 2017-05-08 15:31:35]

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

[commit-bot: I haz the power, 2017-05-08 15:31:55]

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-05-08 16:34:22]

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

[commit-bot: I haz the power, 2017-05-08 16:34:23]

Dry run: This issue passed the CQ dry run.

[tommi (sloooow) - chröme, 2017-05-08 17:52:54]

lgtm

https://codereview.chromium.org/2868853002/diff/50001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_dispatcher_host.cc
(right):

https://codereview.chromium.org/2868853002/diff/50001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_dispatcher_host.cc:497: if
(device_info.device_id == default_device_id)
nit: {}

[Mike West, 2017-05-09 09:30:37]

https://codereview.chromium.org/2868853002/diff/50001/content/common/media/me...
File content/common/media/media_devices.mojom (right):

https://codereview.chromium.org/2868853002/diff/50001/content/common/media/me...
content/common/media/media_devices.mojom:69:
GetAudioInputCapabilities(url.mojom.Origin security_origin)
Do you need to pass in an origin? Shouldn't the browser be able to figure out
what origin triggered the request by looking at the frame tree?

[Guido Urdaneta, 2017-05-09 09:43:33]

https://codereview.chromium.org/2868853002/diff/50001/content/common/media/me...
File content/common/media/media_devices.mojom (right):

https://codereview.chromium.org/2868853002/diff/50001/content/common/media/me...
content/common/media/media_devices.mojom:69:
GetAudioInputCapabilities(url.mojom.Origin security_origin)
On 2017/05/09 09:30:37, Mike West wrote:
> Do you need to pass in an origin? Shouldn't the browser be able to figure out
> what origin triggered the request by looking at the frame tree?

This is how it was done in older IPC-based messaging for similar features (and
the new Mojo versions in this class). I'll try to figure out how to do it
without passing the origin and will get back to you. If I find a way, I will
also patch other methods in this class.

[Mike West, 2017-05-10 06:53:18]

On 2017/05/09 at 09:43:33, guidou wrote:
>
https://codereview.chromium.org/2868853002/diff/50001/content/common/media/me...
> File content/common/media/media_devices.mojom (right):
> 
>
https://codereview.chromium.org/2868853002/diff/50001/content/common/media/me...
> content/common/media/media_devices.mojom:69:
GetAudioInputCapabilities(url.mojom.Origin security_origin)
> On 2017/05/09 09:30:37, Mike West wrote:
> > Do you need to pass in an origin? Shouldn't the browser be able to figure
out
> > what origin triggered the request by looking at the frame tree?
> 
> This is how it was done in older IPC-based messaging for similar features (and
the new Mojo versions in this class). I'll try to figure out how to do it
without passing the origin and will get back to you. If I find a way, I will
also patch other methods in this class.

I'd appreciate it! Ideally, we shouldn't trust the requestor to specify its own
security scope. :)

[Guido Urdaneta, 2017-05-12 09:20:42]

Patchset #3 (id:70001) has been deleted

[Guido Urdaneta, 2017-05-12 09:22:48]

Patchset #3 (id:90001) has been deleted

[Guido Urdaneta, 2017-05-12 09:28:50]

mkwst@: please take another look

[Guido Urdaneta, 2017-05-12 09:33:58]

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

[commit-bot: I haz the power, 2017-05-12 09:34:22]

Dry run: CQ is trying da patch.

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

[Guido Urdaneta, 2017-05-12 09:34:24]

Description was changed from

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

Drive-by: remove unused variable in MDDH.

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

to

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

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

[Mike West, 2017-05-12 09:45:18]

mojo LGTM. Thank you for refactoring this!

[commit-bot: I haz the power, 2017-05-12 10:48:24]

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

[commit-bot: I haz the power, 2017-05-12 10:48:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Guido Urdaneta, 2017-05-15 08:43:33]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2017-05-15 08:43:34]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/2868853002/#ps110001
(title: "Do not send origin from renderer to browser")

[commit-bot: I haz the power, 2017-05-15 08:43:51]

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-05-15 08:47:09]

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

[commit-bot: I haz the power, 2017-05-15 08:47:11]

Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[Guido Urdaneta, 2017-05-15 14:21:50]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2017-05-15 14:21:50]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/2868853002/#ps130001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-15 14:22:06]

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-05-15 15:50:58]

CQ is committing da patch.

Bot data: {"patchset_id": 130001, "attempt_start_ts": 1494858110042190,
"parent_rev": "c4e87be4dac0e0780ebe2f5d54d4e5b02dd16e29", "commit_rev":
"5c25608ed9478c41bdcabad1fbea6cd85a6e89e7"}

[commit-bot: I haz the power, 2017-05-15 15:51:10]

Description was changed from

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

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

to

==========
Add ability to retrieve capabilities for audio input devices to
MediaDevicesDispatcherHost.

BUG=657733

Review-Url: https://codereview.chromium.org/2868853002
Cr-Commit-Position: refs/heads/master@{#471774}
Committed:
https://chromium.googlesource.com/chromium/src/+/5c25608ed9478c41bdcabad1fbea...
==========

[commit-bot: I haz the power, 2017-05-15 15:51:12]

Committed patchset #4 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/5c25608ed9478c41bdcabad1fbea...