[HBD] Add Plugins permission type and PermissionContext

[HBD] Add Plugins permission type and PermissionContext

This is in preparation for the new prompt that is planned for plugins. This
adds all the boilerplate needed for the new permission.

BUG=641615
Committed: https://crrev.com/0314d8d374677bc03ffb708fc86bec191142d249
Cr-Commit-Position: refs/heads/master@{#419411}

[raymes, 2016-09-14 06:47:04]

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

[dominickn, 2016-09-14 07:18:44]

First pass. I think you need to add "Plugins" to histograms.xml -
<histogram_suffixes name="PermissionTypes"> as well to ensure the metrics are
captured correctly.

https://codereview.chromium.org/2339093002/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2339093002/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:7900: +        Run Adobe Flash Player
The design doc says this should be "Run Flash". Do we need to check with UI
review?

https://codereview.chromium.org/2339093002/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_manager.cc (right):

https://codereview.chromium.org/2339093002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_manager.cc:7: #include <stddef.h>
Nit: this should probably #include <memory> for std::unique_ptr

https://codereview.chromium.org/2339093002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_manager.cc:225:
permission_contexts_[PermissionType::PUSH_MESSAGING] =
base::WrapUnique is dispreferred in favour of
base::MakeUnique<PluginsPermissionContext>(profile). While you're here, could
you change all of the WrapUniques to MakeUniques?

[raymes, 2016-09-15 03:40:31]

https://codereview.chromium.org/2339093002/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2339093002/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:7900: +        Run Adobe Flash Player
On 2016/09/14 07:18:42, dominickn wrote:
> The design doc says this should be "Run Flash". Do we need to check with UI
> review?

I've changed it to Run Flash for now, as per the design doc. We can loop back
with UI review before release (which should happen anyway).

https://codereview.chromium.org/2339093002/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_manager.cc (right):

https://codereview.chromium.org/2339093002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_manager.cc:7: #include <stddef.h>
On 2016/09/14 07:18:42, dominickn wrote:
> Nit: this should probably #include <memory> for std::unique_ptr

Done.

https://codereview.chromium.org/2339093002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_manager.cc:225:
permission_contexts_[PermissionType::PUSH_MESSAGING] =
On 2016/09/14 07:18:43, dominickn wrote:
> base::WrapUnique is dispreferred in favour of
> base::MakeUnique<PluginsPermissionContext>(profile). While you're here, could
> you change all of the WrapUniques to MakeUniques?

Done.

[raymes, 2016-09-15 03:43:44]

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

[commit-bot: I haz the power, 2016-09-15 03:43:56]

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, 2016-09-15 03:46:23]

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

[commit-bot: I haz the power, 2016-09-15 03:46:23]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[raymes, 2016-09-15 04:00:16]

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

[commit-bot: I haz the power, 2016-09-15 04:00:37]

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, 2016-09-15 04:19:46]

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

[commit-bot: I haz the power, 2016-09-15 04:19:47]

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

[dominickn, 2016-09-15 04:43:07]

Looks like there's a switch on PermissionType in
android_webview/browser/aw_permission_manager.cc

[raymes, 2016-09-15 05:07:42]

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

[commit-bot: I haz the power, 2016-09-15 05:07:55]

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

[raymes, 2016-09-15 05:09:39]

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

[commit-bot: I haz the power, 2016-09-15 05:09:51]

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

[raymes, 2016-09-15 05:09:57]

On 2016/09/15 04:43:07, dominickn wrote:
> Looks like there's a switch on PermissionType in
> android_webview/browser/aw_permission_manager.cc

Done!

[commit-bot: I haz the power, 2016-09-15 05:30:06]

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

[commit-bot: I haz the power, 2016-09-15 05:30:07]

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_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[raymes, 2016-09-15 05:41:29]

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

[commit-bot: I haz the power, 2016-09-15 05:41:47]

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

[dominickn, 2016-09-15 06:11:50]

lgtm

[raymes, 2016-09-15 06:19:14]

raymes@chromium.org changed reviewers:
+ bauerb@chromium.org, jochen@chromium.org, mpearson@chromium.org,
nparker@chromium.org, tobiasjs@chromium.org

[raymes, 2016-09-15 06:19:15]

+OWNERS

jochen for content
mpearson for t/m/histograms
nparker for c/b/safe_browsing
bauerb for c/b/plugins
tobiasjs for android_webview

[commit-bot: I haz the power, 2016-09-15 07:02:31]

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

[commit-bot: I haz the power, 2016-09-15 07:02:32]

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

[Bernhard Bauer, 2016-09-15 08:39:34]

plugins/ LGTM, but would it make sense to make the new permission type Flash
straight away?

[Tobias Sargeant, 2016-09-15 08:52:46]

android_webview/ LGTM

[jochen (gone - plz use gerrit), 2016-09-15 09:28:36]

content lgtm

[Nathan Parker, 2016-09-15 21:31:45]

safe_browsing LGTM

[Mark P, 2016-09-15 21:54:04]

histograms.xml lgtm

[raymes, 2016-09-16 06:36:13]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2016-09-16 06:36:27]

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

[raymes, 2016-09-16 06:37:22]

The CQ bit was unchecked by raymes@chromium.org

[raymes, 2016-09-16 06:38:28]

On 2016/09/15 08:39:34, Bernhard Bauer wrote:
> plugins/ LGTM, but would it make sense to make the new permission type Flash
> straight away?

Good thought - originally I was thinking that flash isn't exposed in content/
but it turns out there is already a bunch of flash stuff in content/ so maybe
that's better? Do you think I should rename it?

[raymes, 2016-09-19 01:27:09]

On 2016/09/16 06:38:28, raymes wrote:
> On 2016/09/15 08:39:34, Bernhard Bauer wrote:
> > plugins/ LGTM, but would it make sense to make the new permission type Flash
> > straight away?
> 
> Good thought - originally I was thinking that flash isn't exposed in content/
> but it turns out there is already a bunch of flash stuff in content/ so maybe
> that's better? Do you think I should rename it?

Since this is ready to go, I'm going to land it and we can rename in a separate
CL.

[raymes, 2016-09-19 01:27:13]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2016-09-19 01:27:29]

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, 2016-09-19 02:34:09]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-19 02:36:06]

Description was changed from

==========
[HBD] Add Plugins permission type and PermissionContext

This is in preparation for the new prompt that is planned for plugins. This
adds all the boilerplate needed for the new permission.

BUG=641615
==========

to

==========
[HBD] Add Plugins permission type and PermissionContext

This is in preparation for the new prompt that is planned for plugins. This
adds all the boilerplate needed for the new permission.

BUG=641615

Committed: https://crrev.com/0314d8d374677bc03ffb708fc86bec191142d249
Cr-Commit-Position: refs/heads/master@{#419411}
==========

[commit-bot: I haz the power, 2016-09-19 02:36:07]

Patchset 6 (id:??) landed as
https://crrev.com/0314d8d374677bc03ffb708fc86bec191142d249
Cr-Commit-Position: refs/heads/master@{#419411}

[Bernhard Bauer, 2016-09-19 08:43:04]

On 2016/09/19 01:27:09, raymes wrote:
> On 2016/09/16 06:38:28, raymes wrote:
> > On 2016/09/15 08:39:34, Bernhard Bauer wrote:
> > > plugins/ LGTM, but would it make sense to make the new permission type
Flash
> > > straight away?
> > 
> > Good thought - originally I was thinking that flash isn't exposed in
content/
> > but it turns out there is already a bunch of flash stuff in content/ so
maybe
> > that's better? Do you think I should rename it?
> 
> Since this is ready to go, I'm going to land it and we can rename in a
separate
> CL.

Eep, sorry. Yeah, I think renaming would be nice.