Make UI with PermissionRequestManager enabled match PermissionQueueController

Make UI with PermissionRequestManager enabled match PermissionQueueController

This patch updates the permission prompt infobars shown when the
PermissionRequestManager is enabled on Android match what they would be
when it's disabled (the current default). Since we no longer support
grouping permission requests, except for mic+camera, those requests are
special-cased to display the same message as they currently display
(this logic is in PermissionPromptAndroid as we'll still show a grouped
UI on desktop and there are two separate PermissionRequest objects).

BUG=606138
Review-Url: https://codereview.chromium.org/2923393002
Cr-Commit-Position: refs/heads/master@{#478899}
Committed: https://chromium.googlesource.com/chromium/src/+/47c391ce0d0d65703c290d7fbc6e3739ed14bc21

[Timothy Loh, 2017-06-07 04:55:39]

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

[commit-bot: I haz the power, 2017-06-07 04:56:01]

Dry run: CQ is trying da patch.

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

[Timothy Loh, 2017-06-07 05:02:08]

Description was changed from

==========
wip

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

to

==========
Make UI with PermissionRequestManager enabled match PermissionQueueController

This patch updates the permission prompt infobars shown when the
PermissionRequestManager is enabled on Android match what they would be
when it's disabled (the current default). Since we no longer support
grouping permission requests, except for mic+camera, we just special
case that.

BUG=606138
==========

[Timothy Loh, 2017-06-07 05:02:08]

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

[Timothy Loh, 2017-06-07 05:03:39]

Description was changed from

==========
Make UI with PermissionRequestManager enabled match PermissionQueueController

This patch updates the permission prompt infobars shown when the
PermissionRequestManager is enabled on Android match what they would be
when it's disabled (the current default). Since we no longer support
grouping permission requests, except for mic+camera, we just special
case that.

BUG=606138
==========

to

==========
Make UI with PermissionRequestManager enabled match PermissionQueueController

This patch updates the permission prompt infobars shown when the
PermissionRequestManager is enabled on Android match what they would be
when it's disabled (the current default). Since we no longer support
grouping permission requests, except for mic+camera, those requests are
special-cased to display the same message as they currently display
(this logic is in PermissionPromptAndroid as we'll still show a grouped
UI on desktop and there are two separate PermissionRequest objects).

BUG=606138
==========

[commit-bot: I haz the power, 2017-06-07 06:15:45]

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

[commit-bot: I haz the power, 2017-06-07 06:15:47]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-06-08 00:45:31]

lgtm

I think we should clarify the UI with UX once we have a decision on whether to
move forward with modals or not. Specifically we should find out whether we
should have a list-style prompt for media like on desktop, or stick with the
current string. Personally I think the list is better and consistency is good,
but I'm not sure if it's worth changing anything until we decide on modals.

https://codereview.chromium.org/2923393002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_prompt_android.cc (right):

https://codereview.chromium.org/2923393002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:119:
DCHECK_EQ(requests[0]->GetOrigin(), requests[1]->GetOrigin());
nit: perhaps add a helper function which checks the validity of the 2 requests.
It would be good to check that the types are mic/camera too.

https://codereview.chromium.org/2923393002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:126: return
requests[0]->GetMessageText();
We previously added a note in PermissionRequestImpl::GetMessageText that it was
only for modals. We should probably remove that now.

[Timothy Loh, 2017-06-08 03:49:33]

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

[commit-bot: I haz the power, 2017-06-08 03:49:44]

Dry run: CQ is trying da patch.

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

[Timothy Loh, 2017-06-08 03:52:21]

https://codereview.chromium.org/2923393002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_prompt_android.cc (right):

https://codereview.chromium.org/2923393002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:119:
DCHECK_EQ(requests[0]->GetOrigin(), requests[1]->GetOrigin());
On 2017/06/08 00:45:30, raymes wrote:
> nit: perhaps add a helper function which checks the validity of the 2
requests.
> It would be good to check that the types are mic/camera too.

Done.

https://codereview.chromium.org/2923393002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:126: return
requests[0]->GetMessageText();
On 2017/06/08 00:45:30, raymes wrote:
> We previously added a note in PermissionRequestImpl::GetMessageText that it
was
> only for modals. We should probably remove that now.

Good catch, removed.

[Timothy Loh, 2017-06-08 03:53:04]

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

[Timothy Loh, 2017-06-08 03:53:04]

+dfalcantara for chrome/android and chrome/browser/ui/android OWNERS

[commit-bot: I haz the power, 2017-06-08 05:19:51]

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

[commit-bot: I haz the power, 2017-06-08 05:19:51]

Dry run: This issue passed the CQ dry run.

[gone, 2017-06-12 16:52:36]

lgtm

[Timothy Loh, 2017-06-13 02:51:27]

The CQ bit was checked by timloh@chromium.org

[Timothy Loh, 2017-06-13 02:51:27]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org
Link to the patchset: https://codereview.chromium.org/2923393002/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2017-06-13 02:51: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-06-13 04:26:20]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497322287192350,
"parent_rev": "178801f31a4a630c65e0078b5dcb4af07b580fb7", "commit_rev":
"47c391ce0d0d65703c290d7fbc6e3739ed14bc21"}

[commit-bot: I haz the power, 2017-06-13 04:26:37]

Description was changed from

==========
Make UI with PermissionRequestManager enabled match PermissionQueueController

This patch updates the permission prompt infobars shown when the
PermissionRequestManager is enabled on Android match what they would be
when it's disabled (the current default). Since we no longer support
grouping permission requests, except for mic+camera, those requests are
special-cased to display the same message as they currently display
(this logic is in PermissionPromptAndroid as we'll still show a grouped
UI on desktop and there are two separate PermissionRequest objects).

BUG=606138
==========

to

==========
Make UI with PermissionRequestManager enabled match PermissionQueueController

This patch updates the permission prompt infobars shown when the
PermissionRequestManager is enabled on Android match what they would be
when it's disabled (the current default). Since we no longer support
grouping permission requests, except for mic+camera, those requests are
special-cased to display the same message as they currently display
(this logic is in PermissionPromptAndroid as we'll still show a grouped
UI on desktop and there are two separate PermissionRequest objects).

BUG=606138

Review-Url: https://codereview.chromium.org/2923393002
Cr-Commit-Position: refs/heads/master@{#478899}
Committed:
https://chromium.googlesource.com/chromium/src/+/47c391ce0d0d65703c290d7fbc6e...
==========

[commit-bot: I haz the power, 2017-06-13 04:26:38]

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