Display policy parsing errors in about:policy.

Display policy parsing errors in about:policy.

BUG=99178
TEST=Set policy values with the wrong types (e.g. ShowHomeButton = 2) or with conflicting values. The "status" column in about:policy shows a description of the error.


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

[Joao da Silva, 2011-10-28 15:07:41]

Please review. Thanks!

[Mattias Nissler (ping if slow), 2011-10-28 15:18:50]

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_handler.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_handler.cc:1030:
errors->AddError(kPolicyProxyBypassList, entry.error_message_id);
good catch!

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_reader.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_reader.cc:85: for (handler =
handlers->begin(); handler != handlers->end(); ++handler) {
no curlies necessary, and I can see one instance of no curlies in this file, so
consistency tells me to remove them.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_reader_unittest.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_reader_unittest.cc:30: status_ok_ =
ASCIIToUTF16("OK");
If you think OK is better then ok, then also please fix line 161.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/policy_st...
File chrome/browser/policy/policy_status_info.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/policy_st...
chrome/browser/policy/policy_status_info.cc:49: string16 status_message = status
== ENFORCED ? ASCIIToUTF16("OK")
Can we use IDS_OK here?

http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
File chrome/browser/resources/policy.html (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
chrome/browser/resources/policy.html:143:
jsdisplay="Policy.shouldShowExpand(this)"
maintain 4 spaces of additional indentation after line break.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
chrome/browser/resources/policy.html:146: i18n-content="hideText"></a>
ditto.

[Joao da Silva, 2011-10-28 15:44:59]

Reviewed, please have another look.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_reader.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_reader.cc:85: for (handler =
handlers->begin(); handler != handlers->end(); ++handler) {
On 2011/10/28 15:18:51, Mattias Nissler wrote:
> no curlies necessary, and I can see one instance of no curlies in this file,
so
> consistency tells me to remove them.

Done.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_reader_unittest.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_reader_unittest.cc:30: status_ok_ =
ASCIIToUTF16("OK");
On 2011/10/28 15:18:51, Mattias Nissler wrote:
> If you think OK is better then ok, then also please fix line 161.

Done.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/policy_st...
File chrome/browser/policy/policy_status_info.cc (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/policy_st...
chrome/browser/policy/policy_status_info.cc:49: string16 status_message = status
== ENFORCED ? ASCIIToUTF16("OK")
On 2011/10/28 15:18:51, Mattias Nissler wrote:
> Can we use IDS_OK here?

Done.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
File chrome/browser/resources/policy.html (right):

http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
chrome/browser/resources/policy.html:143:
jsdisplay="Policy.shouldShowExpand(this)"
On 2011/10/28 15:18:51, Mattias Nissler wrote:
> maintain 4 spaces of additional indentation after line break.

Done.

http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
chrome/browser/resources/policy.html:146: i18n-content="hideText"></a>
On 2011/10/28 15:18:51, Mattias Nissler wrote:
> ditto.

Done.

[Mattias Nissler (ping if slow), 2011-10-28 16:07:41]

On 2011/10/28 15:44:59, Joao da Silva wrote:
> Reviewed, please have another look.

Wait, you also reviewed it after I reviewed it? I put a review into your review,
so you can review the review while looking at the review! :)

> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
> File chrome/browser/policy/configuration_policy_reader.cc (right):
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
> chrome/browser/policy/configuration_policy_reader.cc:85: for (handler =
> handlers->begin(); handler != handlers->end(); ++handler) {
> On 2011/10/28 15:18:51, Mattias Nissler wrote:
> > no curlies necessary, and I can see one instance of no curlies in this file,
> so
> > consistency tells me to remove them.
> 
> Done.
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
> File chrome/browser/policy/configuration_policy_reader_unittest.cc (right):
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/configura...
> chrome/browser/policy/configuration_policy_reader_unittest.cc:30: status_ok_ =
> ASCIIToUTF16("OK");
> On 2011/10/28 15:18:51, Mattias Nissler wrote:
> > If you think OK is better then ok, then also please fix line 161.
> 
> Done.
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/policy_st...
> File chrome/browser/policy/policy_status_info.cc (right):
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/policy/policy_st...
> chrome/browser/policy/policy_status_info.cc:49: string16 status_message =
status
> == ENFORCED ? ASCIIToUTF16("OK")
> On 2011/10/28 15:18:51, Mattias Nissler wrote:
> > Can we use IDS_OK here?
> 
> Done.
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
> File chrome/browser/resources/policy.html (right):
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
> chrome/browser/resources/policy.html:143:
> jsdisplay="Policy.shouldShowExpand(this)"
> On 2011/10/28 15:18:51, Mattias Nissler wrote:
> > maintain 4 spaces of additional indentation after line break.
> 
> Done.
> 
>
http://codereview.chromium.org/8413037/diff/1/chrome/browser/resources/policy...
> chrome/browser/resources/policy.html:146: i18n-content="hideText"></a>
> On 2011/10/28 15:18:51, Mattias Nissler wrote:
> > ditto.
> 
> Done.

[Mattias Nissler (ping if slow), 2011-10-28 16:07:45]

http://codereview.chromium.org/8413037/diff/1009/chrome/browser/policy/policy...
File chrome/browser/policy/policy_error_map.h (right):

http://codereview.chromium.org/8413037/diff/1009/chrome/browser/policy/policy...
chrome/browser/policy/policy_error_map.h:44: // Returns all the errors messages
stored for |policy|, separated by a white
errors messages? either s/errors/error/ or drop s/errors messages/errors/

[Joao da Silva, 2011-10-28 16:14:10]

Fixed the Engrish. Prease ave anoder luk.

http://codereview.chromium.org/8413037/diff/1009/chrome/browser/policy/policy...
File chrome/browser/policy/policy_error_map.h (right):

http://codereview.chromium.org/8413037/diff/1009/chrome/browser/policy/policy...
chrome/browser/policy/policy_error_map.h:44: // Returns all the errors messages
stored for |policy|, separated by a white
On 2011/10/28 16:07:45, Mattias Nissler wrote:
> errors messages? either s/errors/error/ or drop s/errors messages/errors/

Done.

[Mattias Nissler (ping if slow), 2011-10-28 16:23:19]

I meant to give the magic 4 bytes on the last round already, but forgot to. Here
you go: LGTM!

[commit-bot: I haz the power, 2011-10-28 16:26:55]

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

[commit-bot: I haz the power, 2011-10-28 17:40:38]

Change committed as 107749