Adding error handling to ONC validation.

Adding error handling to ONC validation

- Completed the validation of toplevel ONC
- Moved kEmptyConfiguration to onc_utils.* and validated it in the validator unit test.

BUG=162801, 162802
TEST=Check that both policy and user import show correct error messages (on about:policy or about:net-internals).
Check that valid or repairable ONC is still accepted.

[pneubeck (no reviews), 2012-11-28 20:21:39]

I have finally prepared the user visible error handling to the ONC validation.
This should ease debugging a lot.

Please take a look at the part you're owning:
@Mattias
   chrome/browser/policy

@Greg and Steven
   chrome/browser/chromeos

@James
   chrome/browser/resources
   chrome/browser/ui

Thanks!

[Mattias Nissler (ping if slow), 2012-11-29 12:49:24]

https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
File chrome/browser/policy/configuration_policy_handler_chromeos.cc (right):

https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
chrome/browser/policy/configuration_policy_handler_chromeos.cc:74: messages);
It seems weird that instead of adding individual errors here, you instead add
just one error that holds the concatenation of all the errors you encountered.

Have you considered using something like PolicyErrorMap for gathering and
returning errors during parsing/validation?

[pneubeck (no reviews), 2012-11-29 12:58:01]

https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
File chrome/browser/policy/configuration_policy_handler_chromeos.cc (right):

https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
chrome/browser/policy/configuration_policy_handler_chromeos.cc:74: messages);
On 2012/11/29 12:49:24, Mattias Nissler wrote:
> It seems weird that instead of adding individual errors here, you instead add
> just one error that holds the concatenation of all the errors you encountered.
> 
> Have you considered using something like PolicyErrorMap for gathering and
> returning errors during parsing/validation?

What for?
I could imagine, that having a list of errors (sth. like vector<string>) would
help layouting of the message. However, the win seems minimal compared to having
linebreaks between the errors.

[Mattias Nissler (ping if slow), 2012-11-29 13:09:18]

https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
File chrome/browser/policy/configuration_policy_handler_chromeos.cc (right):

https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
chrome/browser/policy/configuration_policy_handler_chromeos.cc:74: messages);
On 2012/11/29 12:58:02, pneubeck wrote:
> On 2012/11/29 12:49:24, Mattias Nissler wrote:
> > It seems weird that instead of adding individual errors here, you instead
add
> > just one error that holds the concatenation of all the errors you
encountered.
> > 
> > Have you considered using something like PolicyErrorMap for gathering and
> > returning errors during parsing/validation?
> 
> What for?
> I could imagine, that having a list of errors (sth. like vector<string>) would
> help layouting of the message. However, the win seems minimal compared to
having
> linebreaks between the errors.

Right, now what if you want to format this in HTML? Linebreaks in HTML don't
cause linebreaks ;)

about:policy just joins errors at the moment (using a blank as the separator, so
this is already inconsistent with your joining), but once we're making it more
user-consumable, we'd have to make it into an itemized list in HTML anyways, and
then we need to have individual items.

[pneubeck (no reviews), 2012-11-29 13:19:46]

On 2012/11/29 13:09:18, Mattias Nissler wrote:
>
https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
> File chrome/browser/policy/configuration_policy_handler_chromeos.cc (right):
> 
>
https://codereview.chromium.org/11415148/diff/6035/chrome/browser/policy/conf...
> chrome/browser/policy/configuration_policy_handler_chromeos.cc:74: messages);
> On 2012/11/29 12:58:02, pneubeck wrote:
> > On 2012/11/29 12:49:24, Mattias Nissler wrote:
> > > It seems weird that instead of adding individual errors here, you instead
> add
> > > just one error that holds the concatenation of all the errors you
> encountered.
> > > 
> > > Have you considered using something like PolicyErrorMap for gathering and
> > > returning errors during parsing/validation?
> > 
> > What for?
> > I could imagine, that having a list of errors (sth. like vector<string>)
would
> > help layouting of the message. However, the win seems minimal compared to
> having
> > linebreaks between the errors.
> 
> Right, now what if you want to format this in HTML? Linebreaks in HTML don't
> cause linebreaks ;)

That's why I had to replace linebreaks with HTML breaks.


> about:policy just joins errors at the moment (using a blank as the separator,
so
> this is already inconsistent with your joining), but once we're making it more
> user-consumable, we'd have to make it into an itemized list in HTML anyways,
and
> then we need to have individual items.

If we already knew that we will do it that way, then using vector<string>*
instead of string* should be sufficient and I'd be okay with that.
I am just worried that I will have to change that all again, once we decided how
we really want to do it.

E.g. someone could come up with hierarchical errors, or that we need
enumerations instead of translated errors (e.g. if we want to do the translation
server side for admins), etc.