ppapi: PPB_VpnProvider: Implement permission check

ppapi: PPB_VpnProvider: Implement permission check

Infrastructure needed for checking for the "vpnProvider" permission.

BUG=506490
Committed: https://crrev.com/d54465ba8954149b9bdc0b978694a837ff2bd166
Cr-Commit-Position: refs/heads/master@{#394375}

[adrian.belgun, 2016-05-17 16:57:00]

adrian.belgun@intel.com changed reviewers:
+ bbudge@chromium.org, boliu@chromium.org, piman@chromium.org, sky@chromium.org

[adrian.belgun, 2016-05-17 16:57:00]

adrian.belgun@intel.com changed required reviewers:
+ boliu@chromium.org, piman@chromium.org, sky@chromium.org

[adrian.belgun, 2016-05-17 16:57:00]

sky@chromium.org: Please review changes in chrome/browser/

boliu@chromium.org: Please review changes in android_webview/

piman@chromium.org: Please review changes in content/public/browser/

This patch adds the necessary infrastructure needed for checking for the
"vpnProvider" permission in the VpnProvider Resource Host. 

Patches in this series:

Permission: https://codereview.chromium.org/1985093002/
Service Helper: https://codereview.chromium.org/1988613005/
Resource Stub: https://codereview.chromium.org/1931513002/
Resource Host: https://codereview.chromium.org/1735473002/
Vpn Service: https://codereview.chromium.org/1922873005/

Thank you!

[piman, 2016-05-17 17:05:33]

lgtm

[boliu, 2016-05-17 17:14:33]

rs lgtm

[sky, 2016-05-17 19:08:37]

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_client.h (right):

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_client.h:231: bool
AllowPepperVpnProviderAPI(content::BrowserContext* browser_context,
Similar comment on naming here.

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc
(right):

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:120:
Profile* profile = Profile::FromBrowserContext(browser_context);
early return if !profile.

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:121: const
extensions::ExtensionSet* extension_set = NULL;
nullptr

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.h
(right):

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.h:35: static
bool AllowPepperVpnProviderAPI(
This sounds like it's allowing something, vs querying if something is enabled.
How about IsPepperVpnProviderAPIAllowed() ?

[adrian.belgun, 2016-05-17 20:13:32]

Thanks for the reviews! Uploaded a new patch set.

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_client.h (right):

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_client.h:231: bool
AllowPepperVpnProviderAPI(content::BrowserContext* browser_context,
On 2016/05/17 19:08:37, sky wrote:
> Similar comment on naming here.

Done.

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc
(right):

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:120:
Profile* profile = Profile::FromBrowserContext(browser_context);
On 2016/05/17 19:08:37, sky wrote:
> early return if !profile.

Done.

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:121: const
extensions::ExtensionSet* extension_set = NULL;
On 2016/05/17 19:08:37, sky wrote:
> nullptr

Refactored.

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.h
(right):

https://codereview.chromium.org/1985093002/diff/1/chrome/browser/plugins/chro...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.h:35: static
bool AllowPepperVpnProviderAPI(
On 2016/05/17 19:08:37, sky wrote:
> This sounds like it's allowing something, vs querying if something is enabled.
> How about IsPepperVpnProviderAPIAllowed() ?

In this patch I was closely following the naming scheme and behavior of the
above AllowPepperSocketAPI(). Anyway, the new name works for me.

Changed as per request.

[sky, 2016-05-17 21:12:40]

LGTM

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc
(right):

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:121: if
(!profile) {
nit: no {}

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:126: if
(!extension_set) {
nit: no {}

[bbudge, 2016-05-18 07:28:35]

LGTM w/nit

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc
(right):

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:136: if
(permissions_data->HasAPIPermission(
nit: Instead of local permissions_data, just:

if (extension->permissions_data()->HasAPIPermission(

[adrian.belgun, 2016-05-18 09:09:54]

Thank you for reviewing this.

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
File chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc
(right):

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:121: if
(!profile) {
On 2016/05/17 21:12:40, sky wrote:
> nit: no {}

Done.

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:126: if
(!extension_set) {
On 2016/05/17 21:12:39, sky wrote:
> nit: no {}

Done.

https://codereview.chromium.org/1985093002/diff/20001/chrome/browser/plugins/...
chrome/browser/plugins/chrome_content_browser_client_plugins_part.cc:136: if
(permissions_data->HasAPIPermission(
On 2016/05/18 07:28:35, bbudge wrote:
> nit: Instead of local permissions_data, just:
> 
> if (extension->permissions_data()->HasAPIPermission(

Done.

[adrian.belgun, 2016-05-18 09:10:05]

The CQ bit was checked by adrian.belgun@intel.com

[adrian.belgun, 2016-05-18 09:10:06]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org,
piman@chromium.org, bbudge@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1985093002/#ps40001
(title: "Nits")

[commit-bot: I haz the power, 2016-05-18 09:10:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1985093002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1985093002/40001

[commit-bot: I haz the power, 2016-05-18 10:04:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-18 10:05:33]

Description was changed from

==========
ppapi: PPB_VpnProvider: Implement permission check

Infrastructure needed for checking for the "vpnProvider" permission.

BUG=506490
==========

to

==========
ppapi: PPB_VpnProvider: Implement permission check

Infrastructure needed for checking for the "vpnProvider" permission.

BUG=506490

Committed: https://crrev.com/d54465ba8954149b9bdc0b978694a837ff2bd166
Cr-Commit-Position: refs/heads/master@{#394375}
==========

[commit-bot: I haz the power, 2016-05-18 10:05:34]

Patchset 3 (id:??) landed as
https://crrev.com/d54465ba8954149b9bdc0b978694a837ff2bd166
Cr-Commit-Position: refs/heads/master@{#394375}