content: permissions: add plumbing to revoke permissions

content: permissions: add plumbing to revoke permissions

Add plumbing code in content layer to allow revocation of permissions.

NOTE: this is a part of a three sided patch:
https://codereview.chromium.org/1234023002
https://codereview.chromium.org/1238673002 (this)
https://codereview.chromium.org/1241133002

BUG=510405
Committed: https://crrev.com/0fddf11f6ad757bf55191be6af3643df2d66478f
Cr-Commit-Position: refs/heads/master@{#339715}

[Lalit Maganti, 2015-07-15 16:11:08]

lalitm@google.com changed reviewers:
+ mlamouri@chromium.org

[Lalit Maganti, 2015-07-15 16:19:55]

Mounir: obviously this can't land without the Blink change but this CL is much
more straightforward than the Blink one - that needs a bit more work.

[mlamouri (slow - plz ping), 2015-07-17 13:21:27]

https://codereview.chromium.org/1238673002/diff/40001/content/child/permissio...
File content/child/permissions/permission_dispatcher.h (right):

https://codereview.chromium.org/1238673002/diff/40001/content/child/permissio...
content/child/permissions/permission_dispatcher.h:90: void
OnQueryOrRevokePermission(int request_id, PermissionStatus status);
Could you keep that in the same block as OnPermissionChanged (they are used for
callbacks). Also, could you name it OnPermissionResponse() and explain in the
comment that it is being used for Query and Revoke?

[Lalit Maganti, 2015-07-20 15:02:02]

On 2015/07/17 at 13:21:27, mlamouri wrote:
>
https://codereview.chromium.org/1238673002/diff/40001/content/child/permissio...
> File content/child/permissions/permission_dispatcher.h (right):
> 
>
https://codereview.chromium.org/1238673002/diff/40001/content/child/permissio...
> content/child/permissions/permission_dispatcher.h:90: void
OnQueryOrRevokePermission(int request_id, PermissionStatus status);
> Could you keep that in the same block as OnPermissionChanged (they are used
for callbacks). Also, could you name it OnPermissionResponse() and explain in
the comment that it is being used for Query and Revoke?

Have done so.

[mlamouri (slow - plz ping), 2015-07-20 17:02:10]

lgtm with the comments applied.

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
File content/child/permissions/permission_dispatcher.cc (right):

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.cc:255: PermissionStatus result)
{
nit: indentation

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
File content/child/permissions/permission_dispatcher.h (right):

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.h:45:
blink::WebPermissionCallback*);
Please keep the argument names. Blink and Chromium have different coding style.

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.h:59: int worker_thread_id);
ditto

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.h:91: // This is the callback
function used for request and revoke
s/request and revoke/query and revoke/

https://codereview.chromium.org/1238673002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1238673002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:57831: +  <int value="897"
label="V8Permissions_Revoke_Method"/>
Make sure that this number matches the number that lands ;)

[Lalit Maganti, 2015-07-21 09:13:36]

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
File content/child/permissions/permission_dispatcher.cc (right):

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.cc:255: PermissionStatus result)
{
On 2015/07/20 at 17:02:10, Mounir Lamouri wrote:
> nit: indentation

Done.

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
File content/child/permissions/permission_dispatcher.h (right):

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.h:45:
blink::WebPermissionCallback*);
On 2015/07/20 at 17:02:10, Mounir Lamouri wrote:
> Please keep the argument names. Blink and Chromium have different coding
style.

Done.

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.h:59: int worker_thread_id);
On 2015/07/20 at 17:02:10, Mounir Lamouri wrote:
> ditto

Done.

https://codereview.chromium.org/1238673002/diff/100001/content/child/permissi...
content/child/permissions/permission_dispatcher.h:91: // This is the callback
function used for request and revoke
On 2015/07/20 at 17:02:10, Mounir Lamouri wrote:
> s/request and revoke/query and revoke/

Done.

https://codereview.chromium.org/1238673002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1238673002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:57831: +  <int value="897"
label="V8Permissions_Revoke_Method"/>
On 2015/07/20 at 17:02:10, Mounir Lamouri wrote:
> Make sure that this number matches the number that lands ;)

Will do :)

https://codereview.chromium.org/1238673002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:57831: +  <int value="897"
label="V8Permissions_Revoke_Method"/>
On 2015/07/20 at 17:02:10, Mounir Lamouri wrote:
> Make sure that this number matches the number that lands ;)

Will do :)

[Lalit Maganti, 2015-07-21 09:38:12]

lalitm@google.com changed reviewers:
+ asvitkine@chromium.org

[Lalit Maganti, 2015-07-21 09:38:12]

asvitkine@: could you please review the histogram change?

[Alexei Svitkine (slow), 2015-07-21 15:38:57]

https://codereview.chromium.org/1238673002/diff/140001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1238673002/diff/140001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:57903: +  <int value="897"
label="V8Permissions_Revoke_Method"/>
It seems this is missing a bunch of values between 876 and 897. Could you update
those as well while you're changing this? Thanks!

[Lalit Maganti, 2015-07-21 15:45:59]

https://codereview.chromium.org/1238673002/diff/140001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1238673002/diff/140001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:57903: +  <int value="897"
label="V8Permissions_Revoke_Method"/>
On 2015/07/21 at 15:38:57, Alexei Svitkine wrote:
> It seems this is missing a bunch of values between 876 and 897. Could you
update those as well while you're changing this? Thanks!

Have done so :)

[Alexei Svitkine (slow), 2015-07-21 15:50:30]

lgtm

[Lalit Maganti, 2015-07-21 15:50:38]

The CQ bit was checked by lalitm@google.com

[Lalit Maganti, 2015-07-21 15:50:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1238673002/#ps160001
(title: "Update entire histograms file")

[commit-bot: I haz the power, 2015-07-21 15:50:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1238673002/160001

[Lalit Maganti, 2015-07-21 15:56:31]

The CQ bit was unchecked by lalitm@google.com

[Lalit Maganti, 2015-07-21 18:03:01]

The CQ bit was checked by lalitm@google.com

[commit-bot: I haz the power, 2015-07-21 18:04:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1238673002/160001

[commit-bot: I haz the power, 2015-07-21 19:31:12]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-07-21 19:49:09]

Patchset 9 (id:??) landed as
https://crrev.com/0fddf11f6ad757bf55191be6af3643df2d66478f
Cr-Commit-Position: refs/heads/master@{#339715}