Simplify the code for updating Android permissions in MediaStreamDevicesController

Simplify the code for updating Android permissions in MediaStreamDevicesController

Currently the checks for whether to update Android permissions happen
twice: once in the constructor of MediaStreamDevicesController and once
in the static RequestPermissionsWithDelegate function, where the
permissions will be requested. This is currently needed because if the
check fails, RunCallback can be run from inside the constructor.

This CL refactors that code to eliminate the duplication. The call to
RunCallback no longer occurs in the constructor, instead a function is
exposed: RequestFinishedNoPrompt() which calls RunCallback if no
permission prompt needs to be shown.

Since RunCallback is no longer called from the constructor, we need to
also track the value of the denial reason so that it is known at the
time RequestFinishedNoPrompt is called. This is stored in a member
variable. The values of the content setting are also tracked in member
variables and updated through the lifetime of the request.

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

[raymes, 2017-04-21 01:48:31]

Description was changed from

==========
Simplify the code for updating Android permissions in
MediaStreamDevicesController

Currently the checks for whether to update Android permissions happen
twice: once in the constructor of MediaStreamDevicesController and once
in the static RequestPermissionsWithDelegate function, where the
permissions will be requested. This is currently needed because if the
check fails, RunCallback can be run from inside the constructor.

This CL refactors that code to eliminate the duplication. The call to
RunCallback no longer occurs in the constructor, instead a function is
exposed: RequestFinishedNoPrompt() which calls RunCallback if no
permission prompt needs to be shown.

Since RunCallback is no longer called from the constructor, we need to
also track the value of the denial reason so that it is known at the
time RequestFinishedNoPrompt is called. This is stored in a member
variable. The values of the content setting are also tracked in member
variables and updated through the lifetime of the request.

Finally, the callback that gets called when the Android permission
prompt has been answered has been simplified. It now calls directly into
the member function AndroidOSPromptAnswered.

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

to

==========
Simplify the code for updating Android permissions in
MediaStreamDevicesController

Currently the checks for whether to update Android permissions happen
twice: once in the constructor of MediaStreamDevicesController and once
in the static RequestPermissionsWithDelegate function, where the
permissions will be requested. This is currently needed because if the
check fails, RunCallback can be run from inside the constructor.

This CL refactors that code to eliminate the duplication. The call to
RunCallback no longer occurs in the constructor, instead a function is
exposed: RequestFinishedNoPrompt() which calls RunCallback if no
permission prompt needs to be shown.

Since RunCallback is no longer called from the constructor, we need to
also track the value of the denial reason so that it is known at the
time RequestFinishedNoPrompt is called. This is stored in a member
variable. The values of the content setting are also tracked in member
variables and updated through the lifetime of the request.

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

[raymes, 2017-04-21 03:03:10]

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

[raymes, 2017-04-21 03:03:20]

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

[commit-bot: I haz the power, 2017-04-21 03:03:27]

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-04-21 03:27:19]

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

[commit-bot: I haz the power, 2017-04-21 03:27:20]

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_...)

[Timothy Loh, 2017-04-21 04:50:05]

lgtm

https://codereview.chromium.org/2814993003/diff/100001/chrome/browser/media/w...
File chrome/browser/media/webrtc/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/2814993003/diff/100001/chrome/browser/media/w...
chrome/browser/media/webrtc/media_stream_devices_controller.cc:392:
DCHECK(old_audio_setting_ != CONTENT_SETTING_ASK &&
Trybots will probably catch this, but these need to be updated

[raymes, 2017-04-21 05:05:34]

The CQ bit was checked by raymes@chromium.org

[raymes, 2017-04-21 05:05:34]

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/2814993003/#ps110003
(title: "Simplify the code for updating Android permissions in
MediaStreamDevicesController")

[commit-bot: I haz the power, 2017-04-21 05:05:49]

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

[raymes, 2017-04-21 05:05:57]

The CQ bit was unchecked by raymes@chromium.org

[raymes, 2017-04-21 05:07:50]

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

[commit-bot: I haz the power, 2017-04-21 05:08:14]

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-04-21 05:50:40]

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

[commit-bot: I haz the power, 2017-04-21 05:50:41]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-04-23 22:15:24]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-04-23 22:15:28]

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-04-23 22:49:17]

CQ is committing da patch.

Bot data: {"patchset_id": 110003, "attempt_start_ts": 1492985724403530,
"parent_rev": "47d7aae76735286b86b85a3c582870677fce05d2", "commit_rev":
"583e49bd24f395a014cc4ac9c51cc8b359fe97a3"}

[commit-bot: I haz the power, 2017-04-23 22:49:31]

Description was changed from

==========
Simplify the code for updating Android permissions in
MediaStreamDevicesController

Currently the checks for whether to update Android permissions happen
twice: once in the constructor of MediaStreamDevicesController and once
in the static RequestPermissionsWithDelegate function, where the
permissions will be requested. This is currently needed because if the
check fails, RunCallback can be run from inside the constructor.

This CL refactors that code to eliminate the duplication. The call to
RunCallback no longer occurs in the constructor, instead a function is
exposed: RequestFinishedNoPrompt() which calls RunCallback if no
permission prompt needs to be shown.

Since RunCallback is no longer called from the constructor, we need to
also track the value of the denial reason so that it is known at the
time RequestFinishedNoPrompt is called. This is stored in a member
variable. The values of the content setting are also tracked in member
variables and updated through the lifetime of the request.

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

to

==========
Simplify the code for updating Android permissions in
MediaStreamDevicesController

Currently the checks for whether to update Android permissions happen
twice: once in the constructor of MediaStreamDevicesController and once
in the static RequestPermissionsWithDelegate function, where the
permissions will be requested. This is currently needed because if the
check fails, RunCallback can be run from inside the constructor.

This CL refactors that code to eliminate the duplication. The call to
RunCallback no longer occurs in the constructor, instead a function is
exposed: RequestFinishedNoPrompt() which calls RunCallback if no
permission prompt needs to be shown.

Since RunCallback is no longer called from the constructor, we need to
also track the value of the denial reason so that it is known at the
time RequestFinishedNoPrompt is called. This is stored in a member
variable. The values of the content setting are also tracked in member
variables and updated through the lifetime of the request.

BUG=596786

Review-Url: https://codereview.chromium.org/2814993003
Cr-Commit-Position: refs/heads/master@{#466569}
Committed:
https://chromium.googlesource.com/chromium/src/+/583e49bd24f395a014cc4ac9c51c...
==========

[commit-bot: I haz the power, 2017-04-23 22:49:32]

Committed patchset #7 (id:110003) as
https://chromium.googlesource.com/chromium/src/+/583e49bd24f395a014cc4ac9c51c...