Add tests for the permission context class

Add tests for the permission context class



BUG=392145
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282617

[Miguel Garcia, 2014-07-08 11:27:25]

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
File chrome/browser/content_settings/permission_context_base.cc (right):

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
chrome/browser/content_settings/permission_context_base.cc:134: if (allowed) {
Since the dependent change has not landed yet I'll fix it there as well but just
FYI

[Bernhard Bauer, 2014-07-08 12:16:04]

Could we have a BUG= to group all these CLs together?

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
chrome/browser/content_settings/permission_context_base_unittest.cc:21: 
Nit: remove one empty line

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
chrome/browser/content_settings/permission_context_base_unittest.cc:52: bool
PermissionGranted() {
Nit: very simple accessors are usually in unix_hacker_style (while that is not
required, I find it makes it clearer that they are just simple accessors).

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
chrome/browser/content_settings/permission_context_base_unittest.cc:73:
update_tab_context_ = true;
Nit: Name this |tab_context_updated_|?

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
chrome/browser/content_settings/permission_context_base_unittest.cc:85:
TestContextBase sut(profile(),
sut?

https://codereview.chromium.org/371263003/diff/1/chrome/browser/content_setti...
chrome/browser/content_settings/permission_context_base_unittest.cc:111: 
Nit: Remove one empty line.

[Miguel Garcia, 2014-07-08 15:26:38]

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
chrome/browser/content_settings/permission_context_base_unittest.cc:21: 
On 2014/07/08 12:16:04, Bernhard Bauer wrote:
> Nit: remove one empty line

Done.

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
chrome/browser/content_settings/permission_context_base_unittest.cc:73:
update_tab_context_ = true;
On 2014/07/08 12:16:04, Bernhard Bauer wrote:
> Nit: Name this |tab_context_updated_|?

Done.

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
chrome/browser/content_settings/permission_context_base_unittest.cc:85:
TestContextBase sut(profile(),
On 2014/07/08 12:16:04, Bernhard Bauer wrote:
> sut?

Subject Under Test, it is a common thing in other google projects but apparently
not here :). renamed

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
chrome/browser/content_settings/permission_context_base_unittest.cc:111: 
On 2014/07/08 12:16:04, Bernhard Bauer wrote:
> Nit: Remove one empty line.

Done.

[Miguel Garcia, 2014-07-08 15:27:02]

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://chromiumcodereview.appspot.com/371263003/diff/1/chrome/browser/conten...
chrome/browser/content_settings/permission_context_base_unittest.cc:52: bool
PermissionGranted() {
On 2014/07/08 12:16:04, Bernhard Bauer wrote:
> Nit: very simple accessors are usually in unix_hacker_style (while that is not
> required, I find it makes it clearer that they are just simple accessors).

Done.

[Bernhard Bauer, 2014-07-08 15:31:20]

lgtm

https://codereview.chromium.org/371263003/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context_base.cc (left):

https://codereview.chromium.org/371263003/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base.cc:94: 
Can you revert this change now that this CL doesn't touch this file anymore?

[Michael van Ouwerkerk, 2014-07-08 15:32:30]

lgtm

[Miguel Garcia, 2014-07-11 09:16:25]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-07-11 09:17:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miguelg@chromium.org/371263003/60001

[Miguel Garcia, 2014-07-11 10:03:20]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-07-11 10:04:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miguelg@chromium.org/371263003/80001

[commit-bot: I haz the power, 2014-07-11 12:02:10]

Change committed as 282617