Use GUID instead of ServicePath in networkingPrivate API

Currently we pass the service path of networks as the "GUID" in the
networkingPrivate implementation on Chrome OS. We should be using the
actual GUID (if one exists) or create a GUID if one does not.

Note: This change should not break any existing usage since the GUID
value should be transparant to the implementation. (This will have the
positive side effect of making the GUID consistent across sessions).

BUG=284827
For fake_wifi_service.cc:

R=asargent@chromium.org, pneubeck@chromium.org
TBR=mef@chromium.org

Review URL: https://codereview.chromium.org/275543005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271106 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/ui_proxy_config_service.cc

@@ -81,8 +81,10 @@ void UIProxyConfigService::SetCurrentNetwork(
 void UIProxyConfigService::UpdateFromPrefs() {
   const FavoriteState* network = NULL;
   if (!current_ui_network_.empty()) {
-    network = NetworkHandler::Get()->network_state_handler()->GetFavoriteState(
-        current_ui_network_);
+    network = NetworkHandler::Get()
+                  ->network_state_handler()
+                  ->GetFavoriteStateFromServicePath(current_ui_network_,
+                                                    true /* configured_only */);
     LOG_IF(ERROR, !network) << "Couldn't find FavoriteState for network "
                             << current_ui_network_;
   }
@@ -109,8 +111,10 @@ void UIProxyConfigService::SetProxyConfig(const UIProxyConfig& config) {
     return;
 
   const FavoriteState* network =
-      NetworkHandler::Get()->network_state_handler()->GetFavoriteState(
-          current_ui_network_);
+      NetworkHandler::Get()
+          ->network_state_handler()
+          ->GetFavoriteStateFromServicePath(current_ui_network_,
+                                            true /* configured_only */);
   if (!network) {
     LOG(ERROR) << "Couldn't find FavoriteState for network "
                << current_ui_network_;

chrome/browser/extensions/api/networking_private/networking_private_api_chromeos.cc

@@ -15,11 +15,13 @@
 #include "chrome/common/extensions/api/networking_private.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_manager_client.h"
+#include "chromeos/network/favorite_state.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_device_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_util.h"
 #include "chromeos/network/onc/onc_signature.h"
 #include "chromeos/network/onc/onc_translator.h"
 #include "chromeos/network/onc/onc_utils.h"
@@ -30,6 +32,7 @@
 namespace api = extensions::api::networking_private;
 
 using chromeos::DBusThreadManager;
+using chromeos::FavoriteState;
 using chromeos::ManagedNetworkConfigurationHandler;
 using chromeos::NetworkHandler;
 using chromeos::NetworkPortalDetector;
@@ -65,6 +68,20 @@ std::string GetUserIdHash(Profile* profile) {
       profile_helper()->GetUserIdHashFromProfile(profile);
 }
 
+bool GetServicePathFromGuid(const std::string& guid,
+                            std::string* service_path,
+                            std::string* error) {
+  const FavoriteState* network =
+      NetworkHandler::Get()->network_state_handler()->GetFavoriteStateFromGuid(
+          guid);
+  if (!network) {
+    *error = "Error.InvalidNetworkGuid";
+    return false;
+  }
+  *service_path = network->path();
+  return true;
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -78,9 +95,12 @@ bool NetworkingPrivateGetPropertiesFunction::RunAsync() {
   scoped_ptr<api::GetProperties::Params> params =
       api::GetProperties::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   NetworkHandler::Get()->managed_network_configuration_handler()->GetProperties(
-      params->network_guid,  // service path
+      service_path,
       base::Bind(&NetworkingPrivateGetPropertiesFunction::GetPropertiesSuccess,
                  this),
       base::Bind(&NetworkingPrivateGetPropertiesFunction::GetPropertiesFailed,
@@ -91,10 +111,7 @@ bool NetworkingPrivateGetPropertiesFunction::RunAsync() {
 void NetworkingPrivateGetPropertiesFunction::GetPropertiesSuccess(
     const std::string& service_path,
     const base::DictionaryValue& dictionary) {
-  base::DictionaryValue* network_properties = dictionary.DeepCopy();
-  network_properties->SetStringWithoutPathExpansion(onc::network_config::kGUID,
-                                                    service_path);
-  SetResult(network_properties);
+  SetResult(dictionary.DeepCopy());
   SendResponse(true);
 }
 
@@ -116,13 +133,16 @@ bool NetworkingPrivateGetManagedPropertiesFunction::RunAsync() {
   scoped_ptr<api::GetManagedProperties::Params> params =
       api::GetManagedProperties::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   std::string user_id_hash;
   GetUserIdHash(GetProfile());
   NetworkHandler::Get()->managed_network_configuration_handler()->
       GetManagedProperties(
           user_id_hash,
-          params->network_guid,  // service path
+          service_path,
           base::Bind(&NetworkingPrivateGetManagedPropertiesFunction::Success,
                      this),
           base::Bind(&NetworkingPrivateGetManagedPropertiesFunction::Failure,
@@ -133,10 +153,7 @@ bool NetworkingPrivateGetManagedPropertiesFunction::RunAsync() {
 void NetworkingPrivateGetManagedPropertiesFunction::Success(
     const std::string& service_path,
     const base::DictionaryValue& dictionary) {
-  base::DictionaryValue* network_properties = dictionary.DeepCopy();
-  network_properties->SetStringWithoutPathExpansion(onc::network_config::kGUID,
-                                                    service_path);
-  SetResult(network_properties);
+  SetResult(dictionary.DeepCopy());
   SendResponse(true);
 }
 
@@ -158,13 +175,14 @@ bool NetworkingPrivateGetStateFunction::RunAsync() {
   scoped_ptr<api::GetState::Params> params =
       api::GetState::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
-  // The |network_guid| parameter is storing the service path.
-  std::string service_path = params->network_guid;
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   const NetworkState* state = NetworkHandler::Get()->network_state_handler()->
       GetNetworkState(service_path);
   if (!state) {
-    error_ = "Error.InvalidParameter";
+    error_ = "Error.NetworkUnavailable";
     return false;
   }
 
@@ -190,12 +208,15 @@ bool NetworkingPrivateSetPropertiesFunction::RunAsync() {
   scoped_ptr<api::SetProperties::Params> params =
       api::SetProperties::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   scoped_ptr<base::DictionaryValue> properties_dict(
       params->properties.ToValue());
 
   NetworkHandler::Get()->managed_network_configuration_handler()->SetProperties(
-      params->network_guid,  // service path
+      service_path,
       *properties_dict,
       base::Bind(&NetworkingPrivateSetPropertiesFunction::ResultCallback,
                  this),
@@ -269,30 +290,12 @@ bool NetworkingPrivateGetVisibleNetworksFunction::RunAsync() {
   scoped_ptr<api::GetVisibleNetworks::Params> params =
       api::GetVisibleNetworks::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
-  NetworkTypePattern type = chromeos::onc::NetworkTypePatternFromOncType(
+  NetworkTypePattern pattern = chromeos::onc::NetworkTypePatternFromOncType(
       api::GetVisibleNetworks::Params::ToString(params->type));
 
-  NetworkStateHandler::NetworkStateList network_states;
-  NetworkHandler::Get()->network_state_handler()->GetNetworkListByType(
-      type, &network_states);
-
-  base::ListValue* network_properties_list = new base::ListValue;
-  for (NetworkStateHandler::NetworkStateList::iterator it =
-           network_states.begin();
-       it != network_states.end(); ++it) {
-    base::DictionaryValue shill_dictionary;
-    (*it)->GetStateProperties(&shill_dictionary);
-
-    scoped_ptr<base::DictionaryValue> onc_network_part =
-        chromeos::onc::TranslateShillServiceToONCPart(
-            shill_dictionary, &chromeos::onc::kNetworkWithStateSignature);
-    // TODO(stevenjb): Fix this to always use GUID: crbug.com/284827
-    onc_network_part->SetStringWithoutPathExpansion(
-        onc::network_config::kGUID, (*it)->path());
-    network_properties_list->Append(onc_network_part.release());
-  }
-
-  SetResult(network_properties_list);
+  scoped_ptr<base::ListValue> network_properties_list =
+      chromeos::network_util::TranslateNetworkListToONC(pattern);
+  SetResult(network_properties_list.release());
   SendResponse(true);
   return true;
 }
@@ -433,10 +436,13 @@ bool NetworkingPrivateStartConnectFunction::RunAsync() {
   scoped_ptr<api::StartConnect::Params> params =
       api::StartConnect::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   const bool check_error_state = false;
   NetworkHandler::Get()->network_connection_handler()->ConnectToNetwork(
-      params->network_guid,  // service path
+      service_path,
       base::Bind(
           &NetworkingPrivateStartConnectFunction::ConnectionStartSuccess,
           this),
@@ -469,9 +475,12 @@ bool NetworkingPrivateStartDisconnectFunction::RunAsync() {
   scoped_ptr<api::StartDisconnect::Params> params =
       api::StartDisconnect::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   NetworkHandler::Get()->network_connection_handler()->DisconnectNetwork(
-      params->network_guid,  // service path
+      service_path,
       base::Bind(
           &NetworkingPrivateStartDisconnectFunction::DisconnectionStartSuccess,
           this),
@@ -530,15 +539,18 @@ bool NetworkingPrivateVerifyAndEncryptCredentialsFunction::RunAsync() {
   scoped_ptr<api::VerifyAndEncryptCredentials::Params> params =
       api::VerifyAndEncryptCredentials::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
-  ShillManagerClient* shill_manager_client =
-      DBusThreadManager::Get()->GetShillManagerClient();
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   ShillManagerClient::VerificationProperties verification_properties =
       ConvertVerificationProperties(params->properties);
 
+  ShillManagerClient* shill_manager_client =
+      DBusThreadManager::Get()->GetShillManagerClient();
   shill_manager_client->VerifyAndEncryptCredentials(
       verification_properties,
-      params->guid,
+      service_path,
       base::Bind(
           &NetworkingPrivateVerifyAndEncryptCredentialsFunction::ResultCallback,
           this),
@@ -688,6 +700,9 @@ bool NetworkingPrivateGetCaptivePortalStatusFunction::RunAsync() {
   scoped_ptr<api::GetCaptivePortalStatus::Params> params =
       api::GetCaptivePortalStatus::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
+  std::string service_path;
+  if (!GetServicePathFromGuid(params->network_guid, &service_path, &error_))
+    return false;
 
   NetworkPortalDetector* detector = NetworkPortalDetector::Get();
   if (!detector) {
@@ -696,7 +711,7 @@ bool NetworkingPrivateGetCaptivePortalStatusFunction::RunAsync() {
   }
 
   NetworkPortalDetector::CaptivePortalState state =
-      detector->GetCaptivePortalState(params->network_path);
+      detector->GetCaptivePortalState(service_path);
 
   SetResult(new base::StringValue(
       NetworkPortalDetector::CaptivePortalStatusString(state.status)));

chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc

@@ -87,10 +87,7 @@ bool NetworkingPrivateGetManagedPropertiesFunction::RunAsync() {
 void NetworkingPrivateGetManagedPropertiesFunction::Success(
     const std::string& network_guid,
     const base::DictionaryValue& dictionary) {
-  scoped_ptr<base::DictionaryValue> network_properties(dictionary.DeepCopy());
-  network_properties->SetStringWithoutPathExpansion(onc::network_config::kGUID,
-                                                    network_guid);
-  SetResult(network_properties.release());
+  SetResult(dictionary.DeepCopy());
   SendResponse(true);
 }
 

chrome/browser/extensions/api/networking_private/networking_private_apitest.cc

@@ -199,6 +199,19 @@ class ExtensionNetworkingPrivateApiTest
     CHECK(!userhash_.empty());
   }
 
+  void AddService(const std::string& service_path,
+                  const std::string& name,
+                  const std::string& type,
+                  const std::string& state) {
+    const bool add_to_watchlist = true;
+    const bool add_to_visible = true;
+    // Tests need a known GUID, so use 'service_path'.
+    service_test_->AddServiceWithIPConfig(
+        service_path, service_path /* guid */, name,
+        type, state, "" /* ipconfig_path */,
+        add_to_visible, add_to_watchlist);
+  }
+
   virtual void SetUpOnMainThread() OVERRIDE {
     detector_ = new NetworkPortalDetectorTestImpl();
     NetworkPortalDetector::InitializeForTesting(detector_);
@@ -246,21 +259,16 @@ class ExtensionNetworkingPrivateApiTest
         kCellularDevicePath, shill::kTypeCellular, "stub_cellular_device1");
 
     // Add Services
-    const bool add_to_watchlist = true;
-    const bool add_to_visible = true;
-    service_test_->AddService("stub_ethernet", "eth0",
-                              shill::kTypeEthernet, shill::kStateOnline,
-                              add_to_visible, add_to_watchlist);
+    AddService("stub_ethernet", "eth0",
+               shill::kTypeEthernet, shill::kStateOnline);
     service_test_->SetServiceProperty(
         "stub_ethernet",
         shill::kProfileProperty,
         base::StringValue(ShillProfileClient::GetSharedProfilePath()));
     profile_test->AddService(ShillProfileClient::GetSharedProfilePath(),
                              "stub_ethernet");
 
-    service_test_->AddService("stub_wifi1", "wifi1",
-                              shill::kTypeWifi, shill::kStateOnline,
-                              add_to_visible, add_to_watchlist);
+    AddService("stub_wifi1", "wifi1", shill::kTypeWifi, shill::kStateOnline);
     service_test_->SetServiceProperty("stub_wifi1",
                                       shill::kSecurityProperty,
                                       base::StringValue(shill::kSecurityWep));
@@ -286,9 +294,7 @@ class ExtensionNetworkingPrivateApiTest
                                       shill::kWifiFrequency,
                                       base::FundamentalValue(2400));
 
-    service_test_->AddService("stub_wifi2", "wifi2_PSK",
-                              shill::kTypeWifi, shill::kStateIdle,
-                              add_to_visible, add_to_watchlist);
+    AddService("stub_wifi2", "wifi2_PSK", shill::kTypeWifi, shill::kStateIdle);
     service_test_->SetServiceProperty("stub_wifi2",
                                       shill::kGuidProperty,
                                       base::StringValue("stub_wifi2"));
@@ -316,10 +322,7 @@ class ExtensionNetworkingPrivateApiTest
                                       base::StringValue(kUser1ProfilePath));
     profile_test->AddService(kUser1ProfilePath, "stub_wifi2");
 
-    service_test_->AddService("stub_vpn1", "vpn1",
-                              shill::kTypeVPN,
-                              shill::kStateOnline,
-                              add_to_visible, add_to_watchlist);
+    AddService("stub_vpn1", "vpn1", shill::kTypeVPN, shill::kStateOnline);
 
     manager_test->SortManagerServices();
 
@@ -537,10 +540,8 @@ IN_PROC_BROWSER_TEST_P(ExtensionNetworkingPrivateApiTest,
 #if defined(OS_CHROMEOS)
 IN_PROC_BROWSER_TEST_P(ExtensionNetworkingPrivateApiTest,
                        GetCaptivePortalStatus) {
-  service_test_->AddService("stub_cellular1", "cellular1",
-                            shill::kTypeCellular, shill::kStateIdle,
-                            true /* add_to_visible */,
-                            true /* add_to_watchlist */);
+  AddService("stub_cellular1", "cellular1",
+             shill::kTypeCellular, shill::kStateIdle);
   service_test_->SetServiceProperty(
       "stub_cellular1",
       shill::kNetworkTechnologyProperty,

chrome/browser/extensions/api/networking_private/networking_private_service_client.cc

@@ -61,7 +61,6 @@ class CryptoVerifyImpl : public NetworkingPrivateServiceClient::CryptoVerify {
     using api::networking_private::VerifyAndEncryptCredentials::Params;
     scoped_ptr<Params> params = Params::Create(*args);
     std::string public_key;
-    std::string network_guid;
 
     if (!VerifyDestination(params->properties)) {
       callback.Run("", "VerifyError");
@@ -76,11 +75,10 @@ class CryptoVerifyImpl : public NetworkingPrivateServiceClient::CryptoVerify {
     scoped_ptr<NetworkingPrivateCredentialsGetter> credentials_getter(
         NetworkingPrivateCredentialsGetter::Create());
 
-    network_guid = params->guid;
     // Start getting credentials. On Windows |callback| will be called
     // asynchronously on a different thread after |credentials_getter|
     // is deleted.
-    credentials_getter->Start(network_guid, public_key, callback);
+    credentials_getter->Start(params->network_guid, public_key, callback);
   }
 
   virtual void VerifyAndEncryptData(scoped_ptr<base::ListValue> args,

chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc

@@ -303,8 +303,10 @@ bool HasPolicyForFavorite(const FavoriteState* favorite,
 bool HasPolicyForNetwork(const NetworkState* network,
                          const PrefService* profile_prefs) {
   const FavoriteState* favorite =
-      NetworkHandler::Get()->network_state_handler()->GetFavoriteState(
-          network->path());
+      NetworkHandler::Get()
+          ->network_state_handler()
+          ->GetFavoriteStateFromServicePath(network->path(),
+                                            true /* configured_only */);
   if (!favorite)
     return false;
   return HasPolicyForFavorite(favorite, profile_prefs);