Re-factor network_event_log

chromeos/network/network_handler_callbacks.cc

Index: chromeos/network/network_handler_callbacks.cc
diff --git a/chromeos/network/network_handler_callbacks.cc b/chromeos/network/network_handler_callbacks.cc
index f10a12f1d825b342d1664b6679d4185ca2a4b8a4..597b8bd9f71d4f944f6706260980e7b974eb8ce4 100644
--- a/chromeos/network/network_handler_callbacks.cc
+++ b/chromeos/network/network_handler_callbacks.cc
@@ -11,8 +11,6 @@
 namespace chromeos {
 namespace network_handler {
 
-const char kLogModule[] = "ShillError";
-
 // These are names of fields in the error data dictionary for ErrorCallback.
 const char kErrorName[] = "errorName";
 const char kErrorMessage[] = "errorMessage";
@@ -29,17 +27,16 @@ base::DictionaryValue* CreateErrorData(const std::string& service_path,
   return error_data;
 }
 
-void ShillErrorCallbackFunction(const std::string& module,
-                                const std::string& path,
+void ShillErrorCallbackFunction(const std::string& path,
                                 const ErrorCallback& error_callback,
                                 const std::string& error_name,
                                 const std::string& error_message) {
-  std::string error = "Shill Error in " + module;
+  std::string error = "Shill Error";
   if (!path.empty())
     error += " For " + path;
-  error += ": " + error_name + " : " + error_message;
-  LOG(ERROR) << error;
-  network_event_log::AddEntry(kLogModule, module, error);
+  error += ": " + error_name;
+  NET_LOG_ERROR(error, error_message);

[pneubeck (no reviews), 2013/05/13 13:16:25]

the file/line that is logged by NET_LOG_ERROR will not be helpful in this case
and is not a replacement for |module|.
Instead |module| should be replaced by base::Location and callers should use
FROM_HERE.

[stevenjb, 2013/05/13 19:36:39]

I considered that, but I really think that is overkill. We pretty rarely make
the same shill call from multiple places; I do not think there is likely to be
any ambiguity. We can always add info to the callback and call
network_event_log::AddEntry directly if we need to.

[pneubeck (no reviews), 2013/05/16 08:21:07]

I don't see a overkill on the perfomance side: the copy of a base::Location
should be negligible compared to the string operations.

Code complexity wise: arguable.

Since you seem to use the log messages currently more than I, please feel free
to leave it as is.

+  LOG(ERROR) << error << " : " << error_message;
   if (error_callback.is_null())
     return;
   scoped_ptr<base::DictionaryValue> error_data(