Hoist device logic out of permissions check in MediaStreamDevicesController

Hoist device logic out of permissions check in MediaStreamDevicesController

Currently the logic to check whether devices are available for a WebRTC request
is nested between various permissions checks. This CL hoists that logic out
such that it occurs before any other permission checks occur.

BUG=596786
Review-Url: https://codereview.chromium.org/2728573002
Cr-Commit-Position: refs/heads/master@{#455627}
Committed: https://chromium.googlesource.com/chromium/src/+/c5ea96944724669ccfd65035a89a434f1fbe27cf

[raymes, 2017-03-01 04:17:33]

raymes@chromium.org changed reviewers:
+ timloh@chromium.org

[Timothy Loh, 2017-03-02 00:55:58]

https://codereview.chromium.org/2728573002/diff/1/chrome/browser/media/webrtc...
File chrome/browser/media/webrtc/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/2728573002/diff/1/chrome/browser/media/webrtc...
chrome/browser/media/webrtc/media_stream_devices_controller.cc:422:
!ContentTypeIsRequested(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
Is this supposed to be HasAvailableDevices?

[raymes, 2017-03-02 01:48:53]

https://codereview.chromium.org/2728573002/diff/1/chrome/browser/media/webrtc...
File chrome/browser/media/webrtc/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/2728573002/diff/1/chrome/browser/media/webrtc...
chrome/browser/media/webrtc/media_stream_devices_controller.cc:422:
!ContentTypeIsRequested(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
On 2017/03/02 00:55:58, Timothy Loh wrote:
> Is this supposed to be HasAvailableDevices?

Yes, good catch!

[Timothy Loh, 2017-03-03 03:29:02]

lgtm

https://codereview.chromium.org/2728573002/diff/40001/chrome/browser/media/we...
File chrome/browser/media/webrtc/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/2728573002/diff/40001/chrome/browser/media/we...
chrome/browser/media/webrtc/media_stream_devices_controller.cc:225: bool
audio_blocked() const { return audio_blocked_; }
Could a name like audio_device_unavailable() make more sense?

[raymes, 2017-03-07 02:09:25]

https://codereview.chromium.org/2728573002/diff/40001/chrome/browser/media/we...
File chrome/browser/media/webrtc/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/2728573002/diff/40001/chrome/browser/media/we...
chrome/browser/media/webrtc/media_stream_devices_controller.cc:225: bool
audio_blocked() const { return audio_blocked_; }
On 2017/03/03 03:29:02, Timothy Loh wrote:
> Could a name like audio_device_unavailable() make more sense?

Hmm, it's a good suggestion. I think this might become more generic though. If
it doesn't I'll change it in a future CL to what you suggest.

[raymes, 2017-03-07 02:10:24]

The CQ bit was checked by raymes@chromium.org

[raymes, 2017-03-07 02:10:25]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/2728573002/#ps60001
(title: "Hoist device logic out of permissions check in
MediaStreamDevicesController")

[commit-bot: I haz the power, 2017-03-07 02:11:29]

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-03-07 02:55:30]

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

[commit-bot: I haz the power, 2017-03-07 02:55:31]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[raymes, 2017-03-07 04:30:30]

The CQ bit was checked by raymes@chromium.org

[raymes, 2017-03-07 04:30:31]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/2728573002/#ps80001
(title: "Hoist device logic out of permissions check in
MediaStreamDevicesController")

[commit-bot: I haz the power, 2017-03-07 04:31:14]

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-03-07 06:59:07]

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

[commit-bot: I haz the power, 2017-03-07 06:59:09]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[raymes, 2017-03-08 04:29:56]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-03-08 04:30:10]

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-03-08 04:34:15]

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

[commit-bot: I haz the power, 2017-03-08 04:34:17]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[raymes, 2017-03-08 23:52:10]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-03-08 23:52:36]

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-03-09 01:38:58]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489017130389380,
"parent_rev": "334f3cfa4383af5f17c35e0f23ee7485f37136ff", "commit_rev":
"c5ea96944724669ccfd65035a89a434f1fbe27cf"}

[commit-bot: I haz the power, 2017-03-09 01:39:39]

Description was changed from

==========
Hoist device logic out of permissions check in MediaStreamDevicesController

Currently the logic to check whether devices are available for a WebRTC request
is nested between various permissions checks. This CL hoists that logic out
such that it occurs before any other permission checks occur.

BUG=596786
==========

to

==========
Hoist device logic out of permissions check in MediaStreamDevicesController

Currently the logic to check whether devices are available for a WebRTC request
is nested between various permissions checks. This CL hoists that logic out
such that it occurs before any other permission checks occur.

BUG=596786

Review-Url: https://codereview.chromium.org/2728573002
Cr-Commit-Position: refs/heads/master@{#455627}
Committed:
https://chromium.googlesource.com/chromium/src/+/c5ea96944724669ccfd65035a89a...
==========

[commit-bot: I haz the power, 2017-03-09 01:39:40]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/c5ea96944724669ccfd65035a89a...