Add Source property to networkingPrivate.getState

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 841398c3a252d802709815278fa8eca57d3e5f70..fb4b4f55fa154d1d147a995e4b2d65510a7d1298 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -49,33 +49,6 @@ void InvokeErrorCallback(const std::string& service_path,
       error_callback, service_path, error_name, error_msg);
 }
 
-void GetPropertiesCallback(
-    const network_handler::DictionaryResultCallback& callback,
-    const network_handler::ErrorCallback& error_callback,
-    const std::string& service_path,
-    DBusMethodCallStatus call_status,
-    const base::DictionaryValue& properties) {
-  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    // Because network services are added and removed frequently, we will see
-    // failures regularly, so don't log these.
-    network_handler::RunErrorCallback(error_callback,
-                                      service_path,
-                                      network_handler::kDBusFailedError,
-                                      network_handler::kDBusFailedErrorMessage);
-    return;
-  }
-  if (callback.is_null())
-    return;
-
-  // Get the correct name from WifiHex if necessary.
-  scoped_ptr<base::DictionaryValue> properties_copy(properties.DeepCopy());
-  std::string name =
-      shill_property_util::GetNameFromProperties(service_path, properties);
-  if (!name.empty())
-    properties_copy->SetStringWithoutPathExpansion(shill::kNameProperty, name);
-  callback.Run(service_path, *properties_copy.get());
-}
-
 void SetNetworkProfileErrorCallback(
     const std::string& service_path,
     const std::string& profile_path,
@@ -229,12 +202,12 @@ void NetworkConfigurationHandler::RemoveObserver(
 void NetworkConfigurationHandler::GetProperties(
     const std::string& service_path,
     const network_handler::DictionaryResultCallback& callback,
-    const network_handler::ErrorCallback& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) {
   NET_LOG_USER("GetProperties", service_path);
   DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
       dbus::ObjectPath(service_path),
-      base::Bind(&GetPropertiesCallback,
-                 callback, error_callback, service_path));
+      base::Bind(&NetworkConfigurationHandler::GetPropertiesCallback,
+                 AsWeakPtr(), callback, error_callback, service_path));
 }
 
 void NetworkConfigurationHandler::SetProperties(
@@ -432,6 +405,45 @@ void NetworkConfigurationHandler::SetNetworkProfileCompleted(
       OnConfigurationProfileChanged(service_path, profile_path, source));
 }
 
+void NetworkConfigurationHandler::GetPropertiesCallback(
+    const network_handler::DictionaryResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& service_path,
+    DBusMethodCallStatus call_status,
+    const base::DictionaryValue& properties) {
+  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
+    // Because network services are added and removed frequently, we will see
+    // failures regularly, so don't log these.
+    network_handler::RunErrorCallback(error_callback, service_path,
+                                      network_handler::kDBusFailedError,
+                                      network_handler::kDBusFailedErrorMessage);
+    return;
+  }
+  if (callback.is_null())
+    return;
+
+  // Get the correct name from WifiHex if necessary.
+  scoped_ptr<base::DictionaryValue> properties_copy(properties.DeepCopy());
+  std::string name =
+      shill_property_util::GetNameFromProperties(service_path, properties);
+  if (!name.empty())
+    properties_copy->SetStringWithoutPathExpansion(shill::kNameProperty, name);
+
+  // Get the GUID property from NetworkState if it is not set in Shill.
+  std::string guid;
+  properties.GetStringWithoutPathExpansion(::onc::network_config::kGUID, &guid);

[pneubeck (no reviews), 2015/02/17 18:00:49]

can we remove this code eventually, when the guid is always set on first
creation?
if that's the goal, pls add a todo/bug for removal.

[stevenjb, 2015/02/17 18:23:30]

I am unaware of any plans to have Shill set a GUID, so visible networks will
always have a NSH assigned GUID that is not stored in Shill, until / unless they
are configured.

[pneubeck (no reviews), 2015/02/17 18:27:57]

ah, right I forgot that this code is executed also on visible but unconfigured
networks. hm.

but this code will be irrelevant if every body is calling MNCH::GetProperties
instead of this one?
There
(https://code.google.com/p/chromium/codesearch#chromium/src/chromeos/network/m...)
we have exactly the same handling already.

[stevenjb, 2015/02/17 18:34:52]

Yes, in theory if everything uses MNCH then we can deprecate this function
entirely :)

Currently however there is still a fair bit of code that uses Shill properties,
so I don't think that will be the case any time soon.

+  if (guid.empty()) {
+    const NetworkState* network_state =
+        network_state_handler_->GetNetworkState(service_path);
+    if (network_state) {
+      properties_copy->SetStringWithoutPathExpansion(
+          ::onc::network_config::kGUID, network_state->guid());
+    }
+  }
+
+  callback.Run(service_path, *properties_copy.get());
+}
+
 void NetworkConfigurationHandler::SetPropertiesSuccessCallback(
     const std::string& service_path,
     scoped_ptr<base::DictionaryValue> set_properties,