Cleanup network event logging, add to ManagedNetworkConfigurationHandler

chromeos/network/shill_property_util.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/shill_property_util.h"
 
 #include "base/i18n/icu_encoding_detection.h"
 #include "base/i18n/icu_string_conversions.h"
 #include "base/json/json_writer.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 102 matching lines @@
   }
 
   if (unknown_encoding)
     *unknown_encoding = true;
   NET_LOG_DEBUG(
       "GetSSIDFromProperties",
       base::StringPrintf("Unrecognized Encoding=%s", encoding.c_str()));
   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;
   properties.GetStringWithoutPathExpansion(shill::kNameProperty, &name);
 
   std::string validated_name = ValidateUTF8(name);
   if (validated_name != name) {
     NET_LOG_DEBUG("GetNameFromProperties",
                   base::StringPrintf("Validated name %s: UTF8: %s",
                                      service_path.c_str(),
                                      validated_name.c_str()));
   }
 
   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;
 
   bool unknown_ssid_encoding = false;
   std::string ssid = GetSSIDFromProperties(properties, &unknown_ssid_encoding);
   if (ssid.empty())
     NET_LOG_ERROR("GetNameFromProperties", "No SSID set: " + service_path);
 
   // Use |validated_name| if |ssid| is empty.
   // And if the encoding of the SSID is unknown, use |ssid|, which contains raw
@@ skipping 67 matching lines @@
         CopyStringFromDictionary(service_properties, shill::kWifiHexSsid, dest);
     success &= CopyStringFromDictionary(
         service_properties, shill::kModeProperty, dest);
   } else if (type == shill::kTypeVPN) {
     success &= CopyStringFromDictionary(
         service_properties, shill::kNameProperty, dest);
     // VPN Provider values are read from the "Provider" dictionary, but written
     // 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;
     provider_properties->GetStringWithoutPathExpansion(shill::kTypeProperty,
                                                        &vpn_provider_type);
     success &= !vpn_provider_type.empty();
     dest->SetStringWithoutPathExpansion(shill::kProviderTypeProperty,
                                         vpn_provider_type);
 
     std::string vpn_provider_host;
     provider_properties->GetStringWithoutPathExpansion(shill::kHostProperty,
                                                        &vpn_provider_host);
     success &= !vpn_provider_host.empty();
     dest->SetStringWithoutPathExpansion(shill::kProviderHostProperty,
                                         vpn_provider_host);
   } else if (type == shill::kTypeEthernet || type == shill::kTypeEthernetEap) {
     // Ethernet and EthernetEAP don't have any additional identifying
     // properties.
   } else {
     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;
 }
 
 bool DoIdentifyingPropertiesMatch(const base::DictionaryValue& properties_a,
                                   const base::DictionaryValue& properties_b) {
   base::DictionaryValue identifying_a;
   if (!CopyIdentifyingProperties(properties_a, &identifying_a))
     return false;
   base::DictionaryValue identifying_b;
   if (!CopyIdentifyingProperties(properties_b, &identifying_b))
@@ skipping 135 matching lines @@
     if (!str.empty())
       str += "|";
     str += shill_type_to_flag[i].shill_network_type;
   }
   return str;
 }
 
 NetworkTypePattern::NetworkTypePattern(int pattern) : pattern_(pattern) {}
 
 }  // namespace chromeos