Move logic to show permission prompts into MediaStreamDevicesController

Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work.

PermissionManager does a combination of checking the permission status and
showing the prompt if needed. So we need to re-arrange the media permission
logic such that these pieces of logic are together. This CL shields consumers
of MediaStreamDevicesController from needing to show the prompt themselves.

TBR=achuith@chromium.org,mgiuca@chromium.org

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

[raymes, 2017-02-21 06:58:23]

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

[raymes, 2017-02-21 06:58:24]

Hey Tim, could you ptal? Note that this is all just shifting logic around so you
don't so much have to understand the logic itself as ensure that it's moved
around consistently.

[raymes, 2017-02-21 06:58:38]

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

[commit-bot: I haz the power, 2017-02-21 06:59:00]

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-02-21 07:20:55]

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

[commit-bot: I haz the power, 2017-02-21 07:20:56]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[raymes, 2017-02-22 23:42:46]

Description was changed from

==========
Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work. This requires moving
the logic for displaying permission prompts to be contained in the same
place as the logic for checking the the permission status and updating
the stored permission state. This CL is a step towards that.

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

to

==========
Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work.

PermissionManager does a combination of checking the permission status and
showing the prompt if needed. So we need to re-arrange the media permission
logic such that these pieces of logic are together. This CL shields consumers
of MediaStreamDevicesController from needing to show the prompt themselves.

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

[Timothy Loh, 2017-02-23 01:03:07]

lgtm

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

https://codereview.chromium.org/2706813004/diff/1/chrome/browser/media/webrtc...
chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc:80:
CreateMediaStreamDevicesController(
Just double checking I understand, this is so we only have to friend this class
right?

[raymes, 2017-02-23 04:52:16]

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

[commit-bot: I haz the power, 2017-02-23 04:52:48]

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

[raymes, 2017-02-23 04:55:43]

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

https://codereview.chromium.org/2706813004/diff/1/chrome/browser/media/webrtc...
chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc:80:
CreateMediaStreamDevicesController(
On 2017/02/23 01:03:07, Timothy Loh wrote:
> Just double checking I understand, this is so we only have to friend this
class
> right?

Yep, exactly.

[raymes, 2017-02-23 04:56:21]

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

[raymes, 2017-02-23 04:56:21]

sergeyu: could you PTAL? Thanks!

[raymes, 2017-02-23 04:58:01]

On 2017/02/23 04:56:21, raymes wrote:
> sergeyu: could you PTAL? Thanks!

Also, if you need more background on the motivations for these changes, I've
written a brief doc:
https://docs.google.com/document/d/1vdWhc6evjiWJ3yZB7FMrtPEYg5qAu0n45U64zda_s...

Happy to chat more if needed :)

[commit-bot: I haz the power, 2017-02-23 06:06:29]

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

[commit-bot: I haz the power, 2017-02-23 06:06:31]

Dry run: This issue passed the CQ dry run.

[Sergey Ulanov, 2017-02-23 22:57:05]

lgtm

[raymes, 2017-02-27 06:01:18]

raymes@chromium.org changed reviewers:
+ achuith@chromium.org, mgiuca@chromium.org

[raymes, 2017-02-27 06:01:18]

TBR consumers of the API:
mgiuca for chrome/browser/ui/app_list/start_page_service.cc
achuith for chrome/browser/chromeos/login/ui/webui_login_view.cc

[raymes, 2017-02-27 06:01:45]

Description was changed from

==========
Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work.

PermissionManager does a combination of checking the permission status and
showing the prompt if needed. So we need to re-arrange the media permission
logic such that these pieces of logic are together. This CL shields consumers
of MediaStreamDevicesController from needing to show the prompt themselves.

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

to

==========
Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work.

PermissionManager does a combination of checking the permission status and
showing the prompt if needed. So we need to re-arrange the media permission
logic such that these pieces of logic are together. This CL shields consumers
of MediaStreamDevicesController from needing to show the prompt themselves.

TBR=achuith@chromium.org,mgiuca@chromium.org

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

[raymes, 2017-02-27 06:02:06]

The CQ bit was checked by raymes@chromium.org

[raymes, 2017-02-27 06:02:06]

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/2706813004/#ps20001
(title: "Move logic to show permission prompts into
MediaStreamDevicesController")

[commit-bot: I haz the power, 2017-02-27 06:02:21]

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-02-27 07:46:04]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488175326315980,
"parent_rev": "5d9fc5b33a8c9b5fdbe53868fac2f3642c7dc024", "commit_rev":
"1417353d43fb4bed6a21dc62edc35c371eba984a"}

[commit-bot: I haz the power, 2017-02-27 07:46:44]

Description was changed from

==========
Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work.

PermissionManager does a combination of checking the permission status and
showing the prompt if needed. So we need to re-arrange the media permission
logic such that these pieces of logic are together. This CL shields consumers
of MediaStreamDevicesController from needing to show the prompt themselves.

TBR=achuith@chromium.org,mgiuca@chromium.org

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

to

==========
Move logic to show permission prompts into MediaStreamDevicesController

This moves the logic for showing media permission prompts into a static
function in MediaStreamDevicesController. This is part of a refactoring
to change media permissions to use the PermissionManager codepaths to
match the way all other permissions in Chrome work.

PermissionManager does a combination of checking the permission status and
showing the prompt if needed. So we need to re-arrange the media permission
logic such that these pieces of logic are together. This CL shields consumers
of MediaStreamDevicesController from needing to show the prompt themselves.

TBR=achuith@chromium.org,mgiuca@chromium.org

BUG=596786

Review-Url: https://codereview.chromium.org/2706813004
Cr-Commit-Position: refs/heads/master@{#453167}
Committed:
https://chromium.googlesource.com/chromium/src/+/1417353d43fb4bed6a21dc62edc3...
==========

[commit-bot: I haz the power, 2017-02-27 07:46:46]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1417353d43fb4bed6a21dc62edc3...