Improve permission persistence tests.

Improve permission persistence tests.

This CL adds a browser test for desktop geolocation where persistence is
toggled off, ensuring that the underlying content setting isn't changed.
Additional checks of the content setting are added to the existing grant
and block tests.

This CL also simplifies the feature list setting used in the
PermissionContextBase test.

BUG=632269
Committed: https://crrev.com/c2726ec8f91d0c790aa924daf017675d3ebbe164
Cr-Commit-Position: refs/heads/master@{#418841}

[dominickn, 2016-09-14 05:56:48]

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

[commit-bot: I haz the power, 2016-09-14 05:56:58]

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-14 06:05:37]

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

[dominickn, 2016-09-14 06:05:37]

PTAL, thanks!

[commit-bot: I haz the power, 2016-09-14 06:54:33]

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

[commit-bot: I haz the power, 2016-09-14 06:54:34]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-09-15 03:55:47]

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

[dominickn, 2016-09-15 03:55:47]

+mvanouwekerk - PTAL at geolocation. This patch is depended on by the other
patch I sent you (to ensure that if the re-enable all tests patch is reverted,
this one doesn't have to be reverted too).

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

Thanks Dom!

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
File chrome/browser/geolocation/geolocation_browsertest.cc (right):

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:457: std::string()));
Maybe we could do WatchPositionAndObservePermissionRequest(false) instead of
checking the setting? Actually I think this case is already covered so it might
be ok not to test this

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:480: std::string()));
Same with this (see below)

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:572: std::string()));
Can we do WatchPositionAndObservePermissionRequest(true)? I think that should
cover the codepath of actually re-requesting position and making sure the prompt
pops up again.

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:584: std::string()));
Same here.

[dominickn, 2016-09-15 05:40:19]

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

[dominickn, 2016-09-15 05:40:29]

Thanks!

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
File chrome/browser/geolocation/geolocation_browsertest.cc (right):

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:457: std::string()));
On 2016/09/15 05:16:08, raymes wrote:
> Maybe we could do WatchPositionAndObservePermissionRequest(false) instead of
> checking the setting? Actually I think this case is already covered so it
might
> be ok not to test this

Done.

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:480: std::string()));
On 2016/09/15 05:16:08, raymes wrote:
> Same with this (see below)

Done.

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:572: std::string()));
On 2016/09/15 05:16:08, raymes wrote:
> Can we do WatchPositionAndObservePermissionRequest(true)? I think that should
> cover the codepath of actually re-requesting position and making sure the
prompt
> pops up again.

Done.

https://codereview.chromium.org/2338173002/diff/1/chrome/browser/geolocation/...
chrome/browser/geolocation/geolocation_browsertest.cc:584: std::string()));
On 2016/09/15 05:16:08, raymes wrote:
> Same here.

Done.

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

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:44:19]

lgtm thanks!

[commit-bot: I haz the power, 2016-09-15 06:12:48]

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

[commit-bot: I haz the power, 2016-09-15 06:12:49]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2016-09-15 09:23:27]

lgtm

[dominickn, 2016-09-15 12:11:23]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-09-15 12:11:37]

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 12:16:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-15 12:17:03]

Description was changed from

==========
Improve permission persistence tests.

This CL adds a browser test for desktop geolocation where persistence is
toggled off, ensuring that the underlying content setting isn't changed.
Additional checks of the content setting are added to the existing grant
and block tests.

This CL also simplifies the feature list setting used in the
PermissionContextBase test.

BUG=632269
==========

to

==========
Improve permission persistence tests.

This CL adds a browser test for desktop geolocation where persistence is
toggled off, ensuring that the underlying content setting isn't changed.
Additional checks of the content setting are added to the existing grant
and block tests.

This CL also simplifies the feature list setting used in the
PermissionContextBase test.

BUG=632269

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

[commit-bot: I haz the power, 2016-09-15 12:17:04]

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