MediaStreamDevicesControllerBrowserTest

Update MediaStreamDevicesControllerTest to test with kUsePermissionManagerForMediaRequests feature enabled

This updates MediaStreamDevicesControllerTest to run tests with the
kUsePermissionManagerForMediaRequests flag enabled (as well as without it) by
using a paramaterized test.

A few other changes that were needed to make the test work:
1) |prompt_delegate_| is not used for testing when the feature is enabled.
Instead, |prompt_factory_| is used to check the assumptions about what prompts
are shown. The functions on these 2 objects are slightly different, so
functions in the test harness have been introduced to encapsulate them
(SetPromptResponseType, TotalPromptRequestCount, WasPermissionShown,
ResetPromptCounters).
2) MockPermissionPromptFactory was modified to make it possible to test whether
a specific permission type had been displayed in a prompt.
3) RequestPermissions runs asynchronously. So a RunLoop is used to make
execution of it synchronous for the test.
4) A small bug in PermissionRequestManager::DoAutoResponseForTesting was fixed
where an assumption was made that the function would only be called for single
permission requests (not grouped ones).


BUG=596786
TBR=tommycli@chromium.org

Review-Url: https://codereview.chromium.org/2835863003
Cr-Commit-Position: refs/heads/master@{#468891}
Committed: https://chromium.googlesource.com/chromium/src/+/db97ef8df226e314878defd22582184a6b0f7ec9

[raymes, 2017-04-26 00:23:53]

Description was changed from

==========
MediaStreamDevicesControllerBrowserTest

BUG=
==========

to

==========
Update MediaStreamDevicesControllerTest to test with
kUsePermissionManagerForMediaRequests feature enabled

This updates MediaStreamDevicesControllerTest to run tests with the
kUsePermissionManagerForMediaRequests flag enabled (as well as without it) by
using a paramaterized test.

A few other changes that were needed to make the test work:
1) |prompt_delegate_| is not used for testing when the feature is enabled.
Instead, |prompt_factory_| is used to check the assumptions about what prompts
are shown. The functions on these 2 objects are slightly different, so
functions in the test harness have been introduced to encapsulate them
(SetPromptResponseType, TotalPromptRequestCount, WasPermissionShown,
ResetPromptCounters).
2) MockPermissionPromptFactory was modified to make it possible to test whether
a specific permission type had been displayed in a prompt.
3) RequestPermissions runs asynchronously. So a RunLoop is used to make
execution of it synchronous for the test.
4) A small bug in PermissionRequestManager::DoAutoResponseForTesting was fixed
where an assumption was made that the function would only be called for single
permission requests (not grouped ones).


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

[raymes, 2017-04-26 00:27:17]

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

[raymes, 2017-04-26 00:27:18]

(Hopefully the last CL :)

[Timothy Loh, 2017-04-28 03:42:07]

lgtm

https://codereview.chromium.org/2835863003/diff/20001/chrome/browser/media/we...
File chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc
(right):

https://codereview.chromium.org/2835863003/diff/20001/chrome/browser/media/we...
chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc:897:
::testing::Range<int>(static_cast<int>(TestType::TEST_TYPE_START),
Can you WithParamInterface<TestType> and use Values here instead of Range?

[raymes, 2017-05-03 03:11:15]

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

[commit-bot: I haz the power, 2017-05-03 03:11:39]

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-03 04:02:01]

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

[commit-bot: I haz the power, 2017-05-03 04:02:02]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-05-03 04:12:45]

The CQ bit was checked by raymes@chromium.org

[raymes, 2017-05-03 04:12:45]

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/2835863003/#ps40001
(title: "MediaStreamDevicesControllerBrowserTest")

[raymes, 2017-05-03 04:12:54]

https://codereview.chromium.org/2835863003/diff/20001/chrome/browser/media/we...
File chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc
(right):

https://codereview.chromium.org/2835863003/diff/20001/chrome/browser/media/we...
chrome/browser/media/webrtc/media_stream_devices_controller_browsertest.cc:897:
::testing::Range<int>(static_cast<int>(TestType::TEST_TYPE_START),
On 2017/04/28 03:42:07, Timothy Loh wrote:
> Can you WithParamInterface<TestType> and use Values here instead of Range?

Done.

[commit-bot: I haz the power, 2017-05-03 04:12:58]

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

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

[commit-bot: I haz the power, 2017-05-03 04:22:20]

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

[raymes, 2017-05-03 04:23:54]

Description was changed from

==========
Update MediaStreamDevicesControllerTest to test with
kUsePermissionManagerForMediaRequests feature enabled

This updates MediaStreamDevicesControllerTest to run tests with the
kUsePermissionManagerForMediaRequests flag enabled (as well as without it) by
using a paramaterized test.

A few other changes that were needed to make the test work:
1) |prompt_delegate_| is not used for testing when the feature is enabled.
Instead, |prompt_factory_| is used to check the assumptions about what prompts
are shown. The functions on these 2 objects are slightly different, so
functions in the test harness have been introduced to encapsulate them
(SetPromptResponseType, TotalPromptRequestCount, WasPermissionShown,
ResetPromptCounters).
2) MockPermissionPromptFactory was modified to make it possible to test whether
a specific permission type had been displayed in a prompt.
3) RequestPermissions runs asynchronously. So a RunLoop is used to make
execution of it synchronous for the test.
4) A small bug in PermissionRequestManager::DoAutoResponseForTesting was fixed
where an assumption was made that the function would only be called for single
permission requests (not grouped ones).


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

to

==========
Update MediaStreamDevicesControllerTest to test with
kUsePermissionManagerForMediaRequests feature enabled

This updates MediaStreamDevicesControllerTest to run tests with the
kUsePermissionManagerForMediaRequests flag enabled (as well as without it) by
using a paramaterized test.

A few other changes that were needed to make the test work:
1) |prompt_delegate_| is not used for testing when the feature is enabled.
Instead, |prompt_factory_| is used to check the assumptions about what prompts
are shown. The functions on these 2 objects are slightly different, so
functions in the test harness have been introduced to encapsulate them
(SetPromptResponseType, TotalPromptRequestCount, WasPermissionShown,
ResetPromptCounters).
2) MockPermissionPromptFactory was modified to make it possible to test whether
a specific permission type had been displayed in a prompt.
3) RequestPermissions runs asynchronously. So a RunLoop is used to make
execution of it synchronous for the test.
4) A small bug in PermissionRequestManager::DoAutoResponseForTesting was fixed
where an assumption was made that the function would only be called for single
permission requests (not grouped ones).


BUG=596786
TBR=tommycli@chromium.org
==========

[raymes, 2017-05-03 04:23:55]

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

[raymes, 2017-05-03 04:24:18]

TBR=tommycli@chromium.org for trivial changes in flash_permission_context.cc\h

[raymes, 2017-05-03 04:24:27]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-05-03 04:24:47]

CQ is trying da patch.

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

[raymes, 2017-05-03 04:24:51]

On 2017/05/03 04:24:18, raymes wrote:
> mailto:TBR=tommycli@chromium.org for trivial changes in
flash_permission_context.cc\h

*flash_permission_browsertest.cc rather

[commit-bot: I haz the power, 2017-05-03 04:29:20]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493785467557470,
"parent_rev": "cbe7b7a3a4f5839a11461031904555c4c9eac92a", "commit_rev":
"db97ef8df226e314878defd22582184a6b0f7ec9"}

[commit-bot: I haz the power, 2017-05-03 04:30:02]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493785467557470,
"parent_rev": "cbe7b7a3a4f5839a11461031904555c4c9eac92a", "commit_rev":
"db97ef8df226e314878defd22582184a6b0f7ec9"}

[commit-bot: I haz the power, 2017-05-03 04:30:32]

Description was changed from

==========
Update MediaStreamDevicesControllerTest to test with
kUsePermissionManagerForMediaRequests feature enabled

This updates MediaStreamDevicesControllerTest to run tests with the
kUsePermissionManagerForMediaRequests flag enabled (as well as without it) by
using a paramaterized test.

A few other changes that were needed to make the test work:
1) |prompt_delegate_| is not used for testing when the feature is enabled.
Instead, |prompt_factory_| is used to check the assumptions about what prompts
are shown. The functions on these 2 objects are slightly different, so
functions in the test harness have been introduced to encapsulate them
(SetPromptResponseType, TotalPromptRequestCount, WasPermissionShown,
ResetPromptCounters).
2) MockPermissionPromptFactory was modified to make it possible to test whether
a specific permission type had been displayed in a prompt.
3) RequestPermissions runs asynchronously. So a RunLoop is used to make
execution of it synchronous for the test.
4) A small bug in PermissionRequestManager::DoAutoResponseForTesting was fixed
where an assumption was made that the function would only be called for single
permission requests (not grouped ones).


BUG=596786
TBR=tommycli@chromium.org
==========

to

==========
Update MediaStreamDevicesControllerTest to test with
kUsePermissionManagerForMediaRequests feature enabled

This updates MediaStreamDevicesControllerTest to run tests with the
kUsePermissionManagerForMediaRequests flag enabled (as well as without it) by
using a paramaterized test.

A few other changes that were needed to make the test work:
1) |prompt_delegate_| is not used for testing when the feature is enabled.
Instead, |prompt_factory_| is used to check the assumptions about what prompts
are shown. The functions on these 2 objects are slightly different, so
functions in the test harness have been introduced to encapsulate them
(SetPromptResponseType, TotalPromptRequestCount, WasPermissionShown,
ResetPromptCounters).
2) MockPermissionPromptFactory was modified to make it possible to test whether
a specific permission type had been displayed in a prompt.
3) RequestPermissions runs asynchronously. So a RunLoop is used to make
execution of it synchronous for the test.
4) A small bug in PermissionRequestManager::DoAutoResponseForTesting was fixed
where an assumption was made that the function would only be called for single
permission requests (not grouped ones).


BUG=596786
TBR=tommycli@chromium.org

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

[commit-bot: I haz the power, 2017-05-03 04:30:33]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/db97ef8df226e314878defd22582...