Cleanup network event logging, add to ManagedNetworkConfigurationHandler

chromeos/network/shill_property_util.cc

Index: chromeos/network/shill_property_util.cc
diff --git a/chromeos/network/shill_property_util.cc b/chromeos/network/shill_property_util.cc
index d0d33d3a6742f4e62509375f7b0ba980b35e6584..7ffa457f5da8a224178325a8d471841002a4430b 100644
--- a/chromeos/network/shill_property_util.cc
+++ b/chromeos/network/shill_property_util.cc
@@ -120,6 +120,22 @@ std::string GetSSIDFromProperties(const base::DictionaryValue& properties,
   return ssid;
 }
 
+std::string GetNetworkIdFromProperties(
+    const base::DictionaryValue& properties) {
+  if (properties.empty())
+    return "EmptyProperties";
+  std::string result;
+  if (properties.GetString(shill::kGuidProperty, &result))

[pneubeck (no reviews), 2014/03/21 09:33:42]

...WithoutPathExpansion...
(I hate that verbosity) :-/

[stevenjb, 2014/03/21 17:08:51]

Yeah, so do I. Strictly speaking it's not necessary here, but I think we did
decide to always use that with Shill since sometime's there's a '.' in there
where we don't expect one. Done.

[pneubeck (no reviews), 2014/03/21 19:52:19]

Yes, and the method names are simply wrong: without path expansion should be the
default, as it's also the function that does less / is more efficient.
The other should be 'WithPathExpansion'...

+    return result;
+  if (properties.GetString(shill::kSSIDProperty, &result))
+    return result;
+  if (properties.GetString(shill::kNameProperty, &result))
+    return result;
+  std::string type = "UnknownType";
+  properties.GetString(shill::kTypeProperty, &type);
+  return "Unidentified " + type;
+}
+
 std::string GetNameFromProperties(const std::string& service_path,
                                   const base::DictionaryValue& properties) {
   std::string name;
@@ -135,6 +151,10 @@ std::string GetNameFromProperties(const std::string& service_path,
 
   std::string type;
   properties.GetStringWithoutPathExpansion(shill::kTypeProperty, &type);
+  if (type.empty()) {
+    NET_LOG_ERROR("GetNameFromProperties: No type", service_path);
+    return validated_name;
+  }
   if (!NetworkTypePattern::WiFi().MatchesType(type))
     return validated_name;
 
@@ -222,8 +242,9 @@ bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
     // with the keys "Provider.Type" and "Provider.Host".
     const base::DictionaryValue* provider_properties = NULL;
     if (!service_properties.GetDictionaryWithoutPathExpansion(
-             shill::kProviderProperty, &provider_properties)) {
-      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing VPN provider dict");
+            shill::kProviderProperty, &provider_properties)) {
+      NET_LOG_ERROR("Missing VPN provider dict",
+                    GetNetworkIdFromProperties(service_properties));
       return false;
     }
     std::string vpn_provider_type;
@@ -246,8 +267,10 @@ bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
     NOTREACHED() << "Unsupported network type " << type;
     success = false;
   }
-  if (!success)
-    NET_LOG_ERROR("CopyIdentifyingProperties", "Missing required properties");
+  if (!success) {
+    NET_LOG_ERROR("Missing required properties",
+                  GetNetworkIdFromProperties(service_properties));
+  }
   return success;
 }