Use Name not SSID in CopyIdentifyingProperties

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 c97c152a6d7c4fd6b7b0ff640caca73419742571..8a8d50b07d6511a99f08fdbaf449270c12510fed 100644
--- a/chromeos/network/shill_property_util.cc
+++ b/chromeos/network/shill_property_util.cc
@@ -180,45 +180,64 @@ void SetUIData(const NetworkUIData& ui_data,
 
 bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
                                base::DictionaryValue* dest) {
-  bool success = true;
-
   // GUID is optional.
   CopyStringFromDictionary(service_properties, flimflam::kGuidProperty, dest);
 
   std::string type;
   service_properties.GetStringWithoutPathExpansion(flimflam::kTypeProperty,
                                                    &type);
-  success &= !type.empty();
+  if (type.empty()) {
+    NET_LOG_ERROR("CopyIdentifyingProperties", "Empty Type");
+    return false;
+  }
   dest->SetStringWithoutPathExpansion(flimflam::kTypeProperty, type);
   if (type == flimflam::kTypeWifi) {
-    success &= CopyStringFromDictionary(
-        service_properties, flimflam::kSecurityProperty, dest);
-    success &= CopyStringFromDictionary(
-        service_properties, flimflam::kSSIDProperty, dest);
-    success &= CopyStringFromDictionary(
-        service_properties, flimflam::kModeProperty, dest);
+    if (!CopyStringFromDictionary(
+            service_properties, flimflam::kNameProperty, dest)) {

[pneubeck (no reviews), 2013/09/13 07:15:10]

I think that this change makes it even worse:
From all previous discussions with Paul, a Wifi service is identified by
Security+SSID+Mode.
This is also what is documented here:
https://cs.corp.google.com/#chromeos_public/src/platform/shill/doc/manager-ap...
and implemented here:
https://cs.corp.google.com/#chromeos_public/src/platform/shill/wifi_provider....

The properties that we set are ok. But, it seems that we ran again into the
issue that Shill doesn't set kSSIDProperty reliably.
This problem should as well be fixed if we change everything to HexSSID.

A temporary fix could be to copy service_properties.kNameProperty to
dest.kSSIDProperty, if service_properties.kSSIDProperty is empty/unset.

+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing Name");
+      return false;
+    }
+    if (!CopyStringFromDictionary(
+            service_properties, flimflam::kSecurityProperty, dest)) {
+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing Security");
+      return false;
+    }
+    if (!CopyStringFromDictionary(
+            service_properties, flimflam::kModeProperty, dest)) {
+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing Mode");
+      return false;
+    }
   } else if (type == flimflam::kTypeVPN) {
-    success &= CopyStringFromDictionary(
-        service_properties, flimflam::kNameProperty, dest);
+    if (!CopyStringFromDictionary(
+            service_properties, flimflam::kNameProperty, dest)) {
+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing Name");
+      return false;
+    }
     // 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(
              flimflam::kProviderProperty, &provider_properties)) {
-      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing VPN provider dict");
+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing VPN Provider dict");
       return false;
     }
     std::string vpn_provider_type;
     provider_properties->GetStringWithoutPathExpansion(flimflam::kTypeProperty,
                                                        &vpn_provider_type);
-    success &= !vpn_provider_type.empty();
+    if (vpn_provider_type.empty()) {
+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing Provider Type");
+      return false;
+    }
     dest->SetStringWithoutPathExpansion(flimflam::kProviderTypeProperty,
                                         vpn_provider_type);
 
     std::string vpn_provider_host;
     provider_properties->GetStringWithoutPathExpansion(flimflam::kHostProperty,
                                                        &vpn_provider_host);
-    success &= !vpn_provider_host.empty();
+    if (vpn_provider_host.empty()) {
+      NET_LOG_ERROR("CopyIdentifyingProperties", "Missing Provider Host");
+      return false;
+    }
     dest->SetStringWithoutPathExpansion(flimflam::kProviderHostProperty,
                                         vpn_provider_host);
   } else if (type == flimflam::kTypeEthernet ||
@@ -227,11 +246,10 @@ bool CopyIdentifyingProperties(const base::DictionaryValue& service_properties,
     // properties.
   } else {
     NOTREACHED() << "Unsupported network type " << type;
-    success = false;
+    NET_LOG_ERROR("CopyIdentifyingProperties", "Bad Type: " + type);
+    return false;
   }
-  if (!success)
-    NET_LOG_ERROR("CopyIdentifyingProperties", "Missing required properties");
-  return success;
+  return true;
 }
 
 }  // namespace shill_property_util