Convert the last policy.PolicyTest pyauto test to a browser_test.

Convert the last policy.PolicyTest pyauto test to a browser_test.

This includes test cases to further convert policy_prefs_ui.py in a subsequent CL.

BUG=143637
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=156290

[Joao da Silva, 2012-09-11 22:17:39]

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
chrome/browser/policy/policy_prefs_browsertest.cc:130:
CHECK(PathService::Override(base::FILE_EXE, fixed_chrome_path));
I'd like to avoid this hack if you have a better idea. This hits a CHECK in
base::mac::AmIBundled() through usage of PathService, and happens in this test
only because SetUpTestCaes() runs before InProcessBrowserTest's ctor, which does
the same fix.

The test doesn't work with PolicyTestCase as a parameterized type either due to
the same problem: testing::ValuesIn() would need to use PathService before any
PathService::ProviderFuncs are registered.

This can be avoided if each fixture loads the same data, which is not nice
either :-)

[jam, 2012-09-11 23:18:41]

lgtm

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
chrome/browser/policy/policy_prefs_browsertest.cc:66: void
add_settings_page(const GURL& url) { settings_pages_.push_back(url); }
nit: AddSettingsPage, unix_hacker style is only for getters/setters

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
chrome/browser/policy/policy_prefs_browsertest.cc:68: bool is_os_supported()
const {
ditto, IsOSSupported

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
chrome/browser/policy/policy_prefs_browsertest.cc:84: void
add_supported_os(const std::string& os) { supported_os_.push_back(os); }
AddSupportedOS

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
chrome/browser/policy/policy_prefs_browsertest.cc:92: bool is_supported() const
{
IsSupported

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
chrome/browser/policy/policy_prefs_browsertest.cc:130:
CHECK(PathService::Override(base::FILE_EXE, fixed_chrome_path));
On 2012/09/11 22:17:39, Joao da Silva wrote:
> I'd like to avoid this hack if you have a better idea. This hits a CHECK in
> base::mac::AmIBundled() through usage of PathService, and happens in this test
> only because SetUpTestCaes() runs before InProcessBrowserTest's ctor, which
does
> the same fix.
> 
> The test doesn't work with PolicyTestCase as a parameterized type either due
to
> the same problem: testing::ValuesIn() would need to use PathService before any
> PathService::ProviderFuncs are registered.
> 
> This can be avoided if each fixture loads the same data, which is not nice
> either :-)

I don't know enough about this area, I would check with a Mac person

https://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_...
chrome/browser/policy/policy_prefs_browsertest.cc:156: // Restore |chrome_path|
so that the fix in InProcessBrowserTest() works.
you mean SetUpTestCase?

[Joao da Silva, 2012-09-12 12:13:56]

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
chrome/browser/policy/policy_prefs_browsertest.cc:66: void
add_settings_page(const GURL& url) { settings_pages_.push_back(url); }
On 2012/09/11 23:18:41, John Abd-El-Malek wrote:
> nit: AddSettingsPage, unix_hacker style is only for getters/setters

Done.

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
chrome/browser/policy/policy_prefs_browsertest.cc:68: bool is_os_supported()
const {
On 2012/09/11 23:18:41, John Abd-El-Malek wrote:
> ditto, IsOSSupported

Done.

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
chrome/browser/policy/policy_prefs_browsertest.cc:84: void
add_supported_os(const std::string& os) { supported_os_.push_back(os); }
On 2012/09/11 23:18:41, John Abd-El-Malek wrote:
> AddSupportedOS

Done.

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
chrome/browser/policy/policy_prefs_browsertest.cc:92: bool is_supported() const
{
On 2012/09/11 23:18:41, John Abd-El-Malek wrote:
> IsSupported

Done.

http://codereview.chromium.org/10916244/diff/1/chrome/browser/policy/policy_p...
chrome/browser/policy/policy_prefs_browsertest.cc:156: // Restore |chrome_path|
so that the fix in InProcessBrowserTest() works.
On 2012/09/11 23:18:41, John Abd-El-Malek wrote:
> you mean SetUpTestCase?

InProcessBrowserTest() has code similar to the fix above, which only works if
|chrome_path| has its original value.

[commit-bot: I haz the power, 2012-09-12 12:14:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/10916244/5002

[commit-bot: I haz the power, 2012-09-12 14:00:15]

Change committed as 156290