Don't treat Shill ClearProperty errors as failures.

chromeos/network/network_configuration_handler.cc

Index: chromeos/network/network_configuration_handler.cc
diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index ea81a75cc398d43fbc3113a2003d8226291c6d1c..6477193851d7a5f8f62a2a3778608d9f45eb40b8 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -400,31 +400,17 @@ void NetworkConfigurationHandler::ClearPropertiesSuccessCallback(
   DCHECK(names.size() == result.GetSize())
       << "Incorrect result size from ClearProperties.";
 
-  bool some_failed = false;
   for (size_t i = 0; i < result.GetSize(); ++i) {
     bool success = false;
     result.GetBoolean(i, &success);
     if (!success) {
+      // If we try to clear a property that hasn't been set the clear will

[pneubeck (no reviews), 2014/04/22 07:16:41]

can this error be distinguished from other errors?
if so, you might want to set some_failed in the other cases.

[stevenjb, 2014/04/22 19:06:23]

I'm not sure what you mean by "this error". We do not keep track of which
properties have been explicitly set (and I don't think we should try to). 
This is really a "warning", but I intentionally didn't distinguish between
"error" and "warning" in NET_LOG since they are effectively the same.

[pneubeck (no reviews), 2014/04/22 19:12:11]

Oh, I meant whether the error 'tried to clear a not-existent property' reported
by Shill can be distinguished from other Shill errors like 'service unknown'.
If so, you might want to log the error only in the former case and set
'some_failed = true' in the latter cases.

+      // fail, so just log the error.
       NET_LOG_ERROR("ClearProperties Failed: " + names[i], service_path);
-      some_failed = true;
     }
   }
 
-  if (some_failed) {
-    if (!error_callback.is_null()) {
-      scoped_ptr<base::DictionaryValue> error_data(
-          network_handler::CreateErrorData(
-              service_path, kClearPropertiesFailedError,
-              base::StringPrintf("Errors: %" PRIuS, result.GetSize())));
-      error_data->Set("errors", result.DeepCopy());
-      scoped_ptr<base::ListValue> name_list(new base::ListValue);
-      name_list->AppendStrings(names);
-      error_data->Set("names", name_list.release());
-      error_callback.Run(kClearPropertiesFailedError, error_data.Pass());
-    }
-  } else if (!callback.is_null()) {
-    callback.Run();
-  }
+  callback.Run();

[pneubeck (no reviews), 2014/04/22 07:16:41]

seems unintentional to remove 
 if (!callback.is_null())


depending on your answer to the above question, you should remove error_callback
if its never being used.

[stevenjb, 2014/04/22 19:06:23]

Good catch, and you are correct, will do.

   network_state_handler_->RequestUpdateForNetwork(service_path);
 }