Adding error handling to ONC validation.

chrome/browser/policy/configuration_policy_handler_chromeos.cc

Index: chrome/browser/policy/configuration_policy_handler_chromeos.cc
diff --git a/chrome/browser/policy/configuration_policy_handler_chromeos.cc b/chrome/browser/policy/configuration_policy_handler_chromeos.cc
index 4f72026934de99a877293b61a0a8d546edd6669f..9a2b7a4ecdc3d1cbfb2f15aea69cfdee86d653d8 100644
--- a/chrome/browser/policy/configuration_policy_handler_chromeos.cc
+++ b/chrome/browser/policy/configuration_policy_handler_chromeos.cc
@@ -53,7 +53,7 @@ bool NetworkConfigurationPolicyHandler::CheckPolicySettings(
     scoped_ptr<base::DictionaryValue> root_dict =
         onc::ReadDictionaryFromJson(onc_blob, &json_error);
     if (root_dict.get() == NULL) {
-      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_ERROR,
+      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_FAILED,
                        json_error);
       return false;
     }
@@ -66,12 +66,15 @@ bool NetworkConfigurationPolicyHandler::CheckPolicySettings(
                              true);  // Validate for managed ONC
 
     // ONC policies are always unencrypted.
+    std::string messages;
     root_dict = validator.ValidateAndRepairObject(
-        &onc::kUnencryptedConfigurationSignature,
-        *root_dict);
+        &onc::kToplevelConfigurationSignature, *root_dict, &messages);
+    if (!messages.empty()) {
+      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_ERROR,
+                       messages);

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

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:02]

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]

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.

+    }
 
     if (root_dict.get() == NULL) {
-      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_ERROR);
       // Don't reject the policy, as some networks or certificates could still
       // be applied.
       return true;