blink: permissions: cleanup and add tests for revoke

blink: permissions: cleanup and add tests for revoke

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

BUG=510405
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199302

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

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

[mlamouri (slow - plz ping), 2015-07-20 13:40:33]

lgtm

[Lalit Maganti, 2015-07-20 15:01:34]

On 2015/07/20 at 13:40:33, mlamouri wrote:
> lgtm

Ping: this patch has changed significantly

[mlamouri (slow - plz ping), 2015-07-21 10:53:29]

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
File LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js
(right):

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:11: tests =
[
var tests

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:17:
assert_unreached('revocing a random permission should fail');
nit: revoking

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:37:
assert_equals(result.status, 'denied');
Could you use .state instead of .status.
Also, could you have:
var DEFAULT_PERMISSION_STATE = 'denied';
at the top of the file and use that instead of 'denied'. Outside of the test
runner, it would be 'prompt' for example.

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:49:
navigator.permissions.revoke({name:'geolocation'}).then(function(result) {
You might want to revoke 'midi-sysex' ;)

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:109:
done();
In other tests done() is run when the test file is loaded. Why is that run at
the end?

[Lalit Maganti, 2015-07-21 16:10:30]

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
File LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js
(right):

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:11: tests =
[
On 2015/07/21 at 10:53:29, Mounir Lamouri wrote:
> var tests

Done.

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:17:
assert_unreached('revocing a random permission should fail');
On 2015/07/21 at 10:53:29, Mounir Lamouri wrote:
> nit: revoking

Done.

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:37:
assert_equals(result.status, 'denied');
On 2015/07/21 at 10:53:29, Mounir Lamouri wrote:
> Could you use .state instead of .status.
> Also, could you have:
> var DEFAULT_PERMISSION_STATE = 'denied';
> at the top of the file and use that instead of 'denied'. Outside of the test
runner, it would be 'prompt' for example.

Done.

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:49:
navigator.permissions.revoke({name:'geolocation'}).then(function(result) {
On 2015/07/21 at 10:53:29, Mounir Lamouri wrote:
> You might want to revoke 'midi-sysex' ;)

Revoke actually expects 'midi' but done.

https://codereview.chromium.org/1241133002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:109:
done();
On 2015/07/21 at 10:53:29, Mounir Lamouri wrote:
> In other tests done() is run when the test file is loaded. Why is that run at
the end?

As discussed offline.

[Lalit Maganti, 2015-07-21 20:03:27]

The CQ bit was checked by lalitm@google.com

[Lalit Maganti, 2015-07-21 20:03:27]

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/1241133002/#ps80001
(title: "Fix comments")

[commit-bot: I haz the power, 2015-07-21 20:03:40]

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

[commit-bot: I haz the power, 2015-07-21 21:21:26]

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

[commit-bot: I haz the power, 2015-07-21 21:21:27]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/71094)

[mlamouri (slow - plz ping), 2015-07-21 23:29:36]

https://codereview.chromium.org/1241133002/diff/80001/LayoutTests/http/tests/...
File LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js
(right):

https://codereview.chromium.org/1241133002/diff/80001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:50:
navigator.permissions.revoke({name:'midi'}).then(function(result) {
{name:'midi',sysex:true}

I wasn't able to check why the tests are failing but that test should actually
fail. Because midi with sysex=false should be 'granted' not 'denied' by default.

https://codereview.chromium.org/1241133002/diff/80001/LayoutTests/http/tests/...
LayoutTests/http/tests/permissions/chromium/resources/test-revoke.js:72: // By
default, the permission query is rejected if "userVisibleOnly" option
s/query/revocation/

[Lalit Maganti, 2015-07-22 09:05:57]

The CQ bit was checked by lalitm@google.com

[Lalit Maganti, 2015-07-22 09:05:57]

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/1241133002/#ps100001
(title: "Fix tests")

[commit-bot: I haz the power, 2015-07-22 09:06:26]

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

[Lalit Maganti, 2015-07-22 09:06:37]

The CQ bit was unchecked by lalitm@google.com

[Lalit Maganti, 2015-07-22 09:08:15]

The CQ bit was checked by lalitm@google.com

[Lalit Maganti, 2015-07-22 09:08:21]

The CQ bit was unchecked by lalitm@google.com

[Lalit Maganti, 2015-07-22 09:08:44]

The CQ bit was checked by lalitm@google.com

[Lalit Maganti, 2015-07-22 09:08:45]

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/1241133002/#ps120001
(title: "Query -> Revoke")

[commit-bot: I haz the power, 2015-07-22 09:09:24]

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

[commit-bot: I haz the power, 2015-07-22 10:30:15]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199302