Make PermissionManager use ContentSettingsType internally more

Make PermissionManager use ContentSettingsType internally more

This patch adds the MIDI permission to ContentSettingsType so that the
PermissionManager can use ContentSettingsType internally more. The inner
classes of it, PendingRequest and Subscription, are updated to store a
vector<ContentSettingsType> and a ContentSettingsType respectively.

As the added MIDI permission is always set to allow, we simply maintain
the existing design of having the PermissionManager always resolve it
and not pass it down into the HostContentSettingsMap.

BUG=689799
Review-Url: https://codereview.chromium.org/2714603002
Cr-Commit-Position: refs/heads/master@{#452426}
Committed: https://chromium.googlesource.com/chromium/src/+/592d7328de7543536c9928a5b0d11e3a97379c70

[Timothy Loh, 2017-02-23 00:20:56]

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

[commit-bot: I haz the power, 2017-02-23 00:22:05]

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-23 02:05:46]

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

[commit-bot: I haz the power, 2017-02-23 02:05:47]

Dry run: This issue passed the CQ dry run.

[Timothy Loh, 2017-02-23 04:29:01]

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

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

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-02-23 04:29:51]

Description was changed from

==========
WIP


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

to

==========
Make PermissionManager use ContentSettingsType internally more

This patch adds the MIDI permission to ContentSettingsType so that the
PermissionManager can use ContentSettingsType internally more. The inner
classes of it, PendingRequest and Subscription, are updated to store a
vector<ContentSettingsType> and a ContentSettingsType respectively.

As the added MIDI permission is always set to allow, we simply maintain
the existing design of having the PermissionManager always resolve it
and not pass it down into the HostContentSettingsMap.

BUG=689799
==========

[Timothy Loh, 2017-02-23 04:44:16]

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

[commit-bot: I haz the power, 2017-02-23 05:14:18]

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

[commit-bot: I haz the power, 2017-02-23 05:14:19]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-02-23 05:37:08]

lgtm thanks!

https://codereview.chromium.org/2714603002/diff/40001/chrome/browser/permissi...
File chrome/browser/permissions/permission_manager.cc (right):

https://codereview.chromium.org/2714603002/diff/40001/chrome/browser/permissi...
chrome/browser/permissions/permission_manager.cc:131: }
nit: I wonder if we should just make this:
return type == CONTENT_SETTINGS_TYPE_MIDI;

https://codereview.chromium.org/2714603002/diff/40001/chrome/browser/permissi...
chrome/browser/permissions/permission_manager.cc:367: }
nit: I think the order of these functions is all out of order with the header,
but we should at least try to make new ones fit in place so perhaps move this
under GetPermissionContext below?

[Timothy Loh, 2017-02-23 06:17:10]

https://codereview.chromium.org/2714603002/diff/40001/chrome/browser/permissi...
File chrome/browser/permissions/permission_manager.cc (right):

https://codereview.chromium.org/2714603002/diff/40001/chrome/browser/permissi...
chrome/browser/permissions/permission_manager.cc:131: }
On 2017/02/23 05:37:08, raymes wrote:
> nit: I wonder if we should just make this:
> return type == CONTENT_SETTINGS_TYPE_MIDI;

Done.

https://codereview.chromium.org/2714603002/diff/40001/chrome/browser/permissi...
chrome/browser/permissions/permission_manager.cc:367: }
On 2017/02/23 05:37:08, raymes wrote:
> nit: I think the order of these functions is all out of order with the header,
> but we should at least try to make new ones fit in place so perhaps move this
> under GetPermissionContext below?

As discussed, I've made this new function public and moved it up next to the
RequestPermission(ContentSettingsType, ...) function.

[Timothy Loh, 2017-02-23 06:17:31]

The CQ bit was checked by timloh@chromium.org

[Timothy Loh, 2017-02-23 06:17:31]

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/2714603002/#ps60001
(title: "address comments")

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

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-23 07:23:58]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487830651471440,
"parent_rev": "fe32f771bfe35eddb7700df0827109c0a409ca7a", "commit_rev":
"592d7328de7543536c9928a5b0d11e3a97379c70"}

[commit-bot: I haz the power, 2017-02-23 07:24:37]

Description was changed from

==========
Make PermissionManager use ContentSettingsType internally more

This patch adds the MIDI permission to ContentSettingsType so that the
PermissionManager can use ContentSettingsType internally more. The inner
classes of it, PendingRequest and Subscription, are updated to store a
vector<ContentSettingsType> and a ContentSettingsType respectively.

As the added MIDI permission is always set to allow, we simply maintain
the existing design of having the PermissionManager always resolve it
and not pass it down into the HostContentSettingsMap.

BUG=689799
==========

to

==========
Make PermissionManager use ContentSettingsType internally more

This patch adds the MIDI permission to ContentSettingsType so that the
PermissionManager can use ContentSettingsType internally more. The inner
classes of it, PendingRequest and Subscription, are updated to store a
vector<ContentSettingsType> and a ContentSettingsType respectively.

As the added MIDI permission is always set to allow, we simply maintain
the existing design of having the PermissionManager always resolve it
and not pass it down into the HostContentSettingsMap.

BUG=689799

Review-Url: https://codereview.chromium.org/2714603002
Cr-Commit-Position: refs/heads/master@{#452426}
Committed:
https://chromium.googlesource.com/chromium/src/+/592d7328de7543536c9928a5b0d1...
==========

[commit-bot: I haz the power, 2017-02-23 07:24:39]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/592d7328de7543536c9928a5b0d1...