Add MacAddress to ONC and networkingPrivate

BUG=373965 For fake_wifi_service.cc R=armansito@chromium.org, pneubeck@chromium.org TBR=mef@chromium.org Review URL: https://codereview.chromium.org/285233008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271679 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · May 20, 2014 · fad51d5 · fad51d5
1 parent dc27479
commit fad51d5
Show file tree

Hide file tree

Showing 18 changed files with 90 additions and 40 deletions.
diff --git a/chrome/browser/extensions/api/networking_private/networking_private_apitest.cc b/chrome/browser/extensions/api/networking_private/networking_private_apitest.cc
@@ -244,8 +244,8 @@ class ExtensionNetworkingPrivateApiTest
     ipconfig.SetStringWithoutPathExpansion(shill::kAddressProperty, "0.0.0.0");
     ipconfig.SetStringWithoutPathExpansion(shill::kGatewayProperty, "0.0.0.1");
     ipconfig.SetIntegerWithoutPathExpansion(shill::kPrefixlenProperty, 0);
-    ipconfig.SetStringWithoutPathExpansion(
-        shill::kMethodProperty, shill::kTypeIPv4);
+    ipconfig.SetStringWithoutPathExpansion(shill::kMethodProperty,
+                                           shill::kTypeIPv4);
     ip_config_test->AddIPConfig(kIPConfigPath, ipconfig);
 
     // Add Devices
@@ -255,6 +255,9 @@ class ExtensionNetworkingPrivateApiTest
     wifi_ip_configs.AppendString(kIPConfigPath);
     device_test->SetDeviceProperty(
         kWifiDevicePath, shill::kIPConfigsProperty, wifi_ip_configs);
+    device_test->SetDeviceProperty(kWifiDevicePath,
+                                   shill::kAddressProperty,
+                                   base::StringValue("001122aabbcc"));
     device_test->AddDevice(
         kCellularDevicePath, shill::kTypeCellular, "stub_cellular_device1");
 

diff --git a/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
@@ -1524,8 +1524,11 @@ void InternetOptionsHandler::PopulateDictionaryDetailsCallback(
   // Device hardware address
   const DeviceState* device = NetworkHandler::Get()->network_state_handler()->
       GetDeviceState(network->device_path());
-  if (device)
-    dictionary.SetString(kTagHardwareAddress, device->GetFormattedMacAddress());
+  if (device) {
+    dictionary.SetString(
+        kTagHardwareAddress,
+        network_util::FormattedMacAddress(device->mac_address()));
+  }
 
   // IP config
   scoped_ptr<base::DictionaryValue> ipconfig_dhcp(new base::DictionaryValue);

diff --git a/chrome/test/data/extensions/api_test/networking/test.js b/chrome/test/data/extensions/api_test/networking/test.js
@@ -269,6 +269,7 @@ var availableTests = [
         assertEq({ "Connectable": true,
                    "ConnectionState": "Connected",
                    "GUID": "stub_wifi1",
+                   "MacAddress": "00:11:22:AA:BB:CC",
                    "IPConfigs": [{
                      "Gateway": "0.0.0.1",
                      "IPAddress": "0.0.0.0",

diff --git a/chromeos/dbus/fake_shill_manager_client.cc b/chromeos/dbus/fake_shill_manager_client.cc
@@ -611,6 +611,9 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     AddTechnology(shill::kTypeEthernet, enabled);
     devices->AddDevice(
         "/device/eth1", shill::kTypeEthernet, "stub_eth_device1");
+    devices->SetDeviceProperty("/device/eth1",
+                               shill::kAddressProperty,
+                               base::StringValue("0123456789ab"));
     base::ListValue eth_ip_configs;
     eth_ip_configs.AppendString("ipconfig_v4_path");
     eth_ip_configs.AppendString("ipconfig_v6_path");
@@ -634,6 +637,9 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     }
     AddTechnology(shill::kTypeWifi, enabled);
     devices->AddDevice("/device/wifi1", shill::kTypeWifi, "stub_wifi_device1");
+    devices->SetDeviceProperty("/device/wifi1",
+                               shill::kAddressProperty,
+                               base::StringValue("23456789abc"));
     base::ListValue wifi_ip_configs;
     wifi_ip_configs.AppendString("ipconfig_v4_path");
     wifi_ip_configs.AppendString("ipconfig_v6_path");

diff --git a/chromeos/network/device_state.cc b/chromeos/network/device_state.cc
@@ -158,18 +158,6 @@ void DeviceState::IPConfigPropertiesChanged(
   ip_config->MergeDictionary(&properties);
 }
 
-std::string DeviceState::GetFormattedMacAddress() const {
-  if (mac_address_.size() % 2 != 0)
-    return mac_address_;
-  std::string result;
-  for (size_t i = 0; i < mac_address_.size(); ++i) {
-    if ((i != 0) && (i % 2 == 0))
-      result.push_back(':');
-    result.push_back(mac_address_[i]);
-  }
-  return result;
-}
-
 bool DeviceState::IsSimAbsent() const {
   return technology_family_ == shill::kTechnologyFamilyGsm && !sim_present_;
 }

diff --git a/chromeos/network/device_state.h b/chromeos/network/device_state.h
@@ -32,9 +32,6 @@ class CHROMEOS_EXPORT DeviceState : public ManagedState {
   // Accessors
   const std::string& mac_address() const { return mac_address_; }
 
-  // Returns |mac_address_| in aa:bb format.
-  std::string GetFormattedMacAddress() const;
-
   // Cellular specific accessors
   const std::string& home_provider_id() const { return home_provider_id_; }
   bool allow_roaming() const { return allow_roaming_; }

diff --git a/chromeos/network/managed_network_configuration_handler_impl.cc b/chromeos/network/managed_network_configuration_handler_impl.cc
@@ -162,8 +162,8 @@ void ManagedNetworkConfigurationHandlerImpl::GetManagedPropertiesCallback(
 
   scoped_ptr<base::DictionaryValue> properties_copy(
       shill_properties.DeepCopy());
-  // Add the IPConfigs to the dictionary before the ONC translation.
-  GetIPConfigs(service_path, properties_copy.get());
+  // Add associated Device properties before the ONC translation.
+  GetDeviceProperties(service_path, properties_copy.get());
 
   scoped_ptr<base::DictionaryValue> active_settings(
       onc::TranslateShillServiceToONCPart(
@@ -223,8 +223,8 @@ void ManagedNetworkConfigurationHandlerImpl::GetPropertiesCallback(
     const base::DictionaryValue& shill_properties) {
   scoped_ptr<base::DictionaryValue> properties_copy(
       shill_properties.DeepCopy());
-  // Add the IPConfigs to the dictionary before the ONC translation.
-  GetIPConfigs(service_path, properties_copy.get());
+  // Add associated Device properties before the ONC translation.
+  GetDeviceProperties(service_path, properties_copy.get());
 
   scoped_ptr<base::DictionaryValue> onc_network(
       onc::TranslateShillServiceToONCPart(
@@ -613,7 +613,7 @@ void ManagedNetworkConfigurationHandlerImpl::OnPolicyAppliedToNetwork(
       NetworkPolicyObserver, observers_, PolicyApplied(service_path));
 }
 
-void ManagedNetworkConfigurationHandlerImpl::GetIPConfigs(
+void ManagedNetworkConfigurationHandlerImpl::GetDeviceProperties(
     const std::string& service_path,
     base::DictionaryValue* properties) {
   std::string connection_state;
@@ -631,9 +631,16 @@ void ManagedNetworkConfigurationHandlerImpl::GetIPConfigs(
   const DeviceState* device_state =
       network_state_handler_->GetDeviceState(device);
   if (!device_state) {
-    NET_LOG_ERROR("GetIPConfigs: no device: " + device, service_path);
+    NET_LOG_ERROR("GetDeviceProperties: no device: " + device, service_path);
     return;
   }
+
+  // Get the hardware MAC address from the DeviceState.
+  if (!device_state->mac_address().empty()) {
+    properties->SetStringWithoutPathExpansion(
+        shill::kAddressProperty, device_state->mac_address());
+  }
+
   // Convert IPConfig dictionary to a ListValue.
   base::ListValue* ip_configs = new base::ListValue;
   for (base::DictionaryValue::Iterator iter(device_state->ip_configs());

diff --git a/chromeos/network/managed_network_configuration_handler_impl.h b/chromeos/network/managed_network_configuration_handler_impl.h
@@ -131,10 +131,9 @@ class CHROMEOS_EXPORT ManagedNetworkConfigurationHandlerImpl
 
   void OnPolicyAppliedToNetwork(const std::string& service_path);
 
-  // Helper method to append "IPConfigs" property to |properties| by extracting
-  // them from the associated DeviceState.
-  void GetIPConfigs(const std::string& service_path,
-                    base::DictionaryValue* properties);
+  // Helper method to append associated Device properties to |properties|.
+  void GetDeviceProperties(const std::string& service_path,
+                           base::DictionaryValue* properties);
 
   // If present, the empty string maps to the device policy.
   UserToPoliciesMap policies_by_user_;

diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
@@ -260,7 +260,7 @@ std::string NetworkStateHandler::FormattedHardwareAddressForType(
     device = GetDeviceStateByType(type);
   if (!device)
     return std::string();
-  return device->GetFormattedMacAddress();
+  return network_util::FormattedMacAddress(device->mac_address());
 }
 
 void NetworkStateHandler::GetNetworkList(NetworkStateList* list) const {

diff --git a/chromeos/network/network_util.cc b/chromeos/network/network_util.cc
@@ -5,6 +5,7 @@
 #include "chromeos/network/network_util.h"
 
 #include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chromeos/network/favorite_state.h"
 #include "chromeos/network/network_state.h"
@@ -99,6 +100,18 @@ int32 NetmaskToPrefixLength(const std::string& netmask) {
   return prefix_length;
 }
 
+std::string FormattedMacAddress(const std::string& shill_mac_address) {
+  if (shill_mac_address.size() % 2 != 0)
+    return shill_mac_address;
+  std::string result;
+  for (size_t i = 0; i < shill_mac_address.size(); ++i) {
+    if ((i != 0) && (i % 2 == 0))
+      result.push_back(':');
+    result.push_back(base::ToUpperASCII(shill_mac_address[i]));
+  }
+  return result;
+}
+
 bool ParseCellularScanResults(const base::ListValue& list,
                               std::vector<CellularScanResult>* scan_results) {
   scan_results->clear();

diff --git a/chromeos/network/network_util.h b/chromeos/network/network_util.h
@@ -81,6 +81,10 @@ CHROMEOS_EXPORT std::string PrefixLengthToNetmask(int32 prefix_length);
 // e.g. a |netmask| of 255.255.255.0 is converted to a prefixlen of 24
 CHROMEOS_EXPORT int32 NetmaskToPrefixLength(const std::string& netmask);
 
+// Returns |shill_mac_address| in aa:bb format.
+CHROMEOS_EXPORT std::string FormattedMacAddress(
+    const std::string& shill_mac_address);
+
 // Parses |list|, which contains DictionaryValues and returns a vector of
 // CellularScanResult in |scan_results|. Returns false if parsing fails,
 // in which case the contents of |scan_results| will be undefined.

diff --git a/chromeos/network/onc/onc_signature.cc b/chromeos/network/onc/onc_signature.cc
@@ -282,6 +282,7 @@ const OncFieldSignature network_with_state_fields[] = {
     { ::onc::network_config::kConnectionState, &kStringSignature},
     { ::onc::network_config::kConnectable, &kBoolSignature},
     { ::onc::network_config::kErrorState, &kStringSignature},
+    { ::onc::network_config::kMacAddress, &kStringSignature},
     { ::onc::network_config::kWiFi, &kWiFiWithStateSignature},
     {NULL}};
 

diff --git a/chromeos/network/onc/onc_translation_tables.cc b/chromeos/network/onc/onc_translation_tables.cc
@@ -162,20 +162,22 @@ const FieldTranslationEntry cellular_fields[] = {
     {NULL}};
 
 const FieldTranslationEntry network_fields[] = {
+    { ::onc::network_config::kGUID, shill::kGuidProperty},
+    { ::onc::network_config::kConnectable, shill::kConnectableProperty },
+    { ::onc::network_config::kErrorState, shill::kErrorProperty },
+
     // Shill doesn't allow setting the name for non-VPN networks.
-    // This field is conditionally translated, see onc_translator_*.
+    // Name is conditionally translated, see onc_translator_*.
     // { ::onc::network_config::kName, shill::kNameProperty },
-    { ::onc::network_config::kGUID, shill::kGuidProperty},
-    // This field is converted during translation, see onc_translator_*.
+
+    // Type is converted during translation, see onc_translator_*.
     // { ::onc::network_config::kType, shill::kTypeProperty },
 
-    // This field is converted during translation, see
-    // onc_translator_shill_to_onc.cc. It is only converted when going from
+    // These fields are converted during translation, see
+    // onc_translator_shill_to_onc.cc. They are only converted when going from
     // Shill->ONC, and ignored otherwise.
     // { ::onc::network_config::kConnectionState, shill::kStateProperty },
-
-    { ::onc::network_config::kConnectable, shill::kConnectableProperty },
-    { ::onc::network_config::kErrorState, shill::kErrorProperty },
+    // { ::onc::network_config::kMacAddress, shill::kAddressProperty },
     {NULL}};
 
 const FieldTranslationEntry ipconfig_fields[] = {

diff --git a/chromeos/network/onc/onc_translator_shill_to_onc.cc b/chromeos/network/onc/onc_translator_shill_to_onc.cc
@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/values.h"
 #include "chromeos/network/network_state.h"
+#include "chromeos/network/network_util.h"
 #include "chromeos/network/onc/onc_signature.h"
 #include "chromeos/network/onc/onc_translation_tables.h"
 #include "chromeos/network/shill_property_util.h"
@@ -290,6 +291,7 @@ void ShillToONCTranslator::TranslateNetworkWithState() {
   onc_object_->SetStringWithoutPathExpansion(::onc::network_config::kName,
                                              name);
 
+  // Limit ONC state to "NotConnected", "Connected", or "Connecting".
   std::string state;
   if (shill_dictionary_->GetStringWithoutPathExpansion(shill::kStateProperty,
                                                        &state)) {
@@ -303,8 +305,19 @@ void ShillToONCTranslator::TranslateNetworkWithState() {
         ::onc::network_config::kConnectionState, onc_state);
   }
 
-  // Shill's Service has an IPConfig property (note the singular, and not a
-  // IPConfigs property). However, we require the caller of the translation to
+  // Use a human-readable aa:bb format for any hardware MAC address. Note:
+  // this property is provided by the caller but is not part of the Shill
+  // Service properties (it is copied from the Device properties).
+  std::string address;
+  if (shill_dictionary_->GetStringWithoutPathExpansion(shill::kAddressProperty,
+                                                       &address)) {
+    onc_object_->SetStringWithoutPathExpansion(
+        ::onc::network_config::kMacAddress,
+        network_util::FormattedMacAddress(address));
+  }
+
+  // Shill's Service has an IPConfig property (note the singular), not an
+  // IPConfigs property. However, we require the caller of the translation to
   // patch the Shill dictionary before passing it to the translator.
   const base::ListValue* shill_ipconfigs = NULL;
   if (shill_dictionary_->GetListWithoutPathExpansion(shill::kIPConfigsProperty,

diff --git a/components/onc/docs/onc_spec.html b/components/onc/docs/onc_spec.html
@@ -390,6 +390,16 @@ <h1>Network Configuration</h1>
       in an error state.
     </dd>
 
+    <dt class="field">MacAddress</dt>
+    <dd>
+      <span class="field_meta">
+        (optional, read-only)
+        <span class="type">string</span>
+      </span>
+      The MAC address for the network. Only applies to connected non-virtual
+      networks. The format is 00:11:22:AA:BB:CC.
+    </dd>
+
   </dl>
 
 <section>

diff --git a/components/onc/onc_constants.cc b/components/onc/onc_constants.cc
@@ -37,6 +37,7 @@ const char kCellular[] = "Cellular";
 const char kEthernet[] = "Ethernet";
 const char kGUID[] = "GUID";
 const char kIPConfigs[] = "IPConfigs";
+const char kMacAddress[] = "MacAddress";
 const char kName[] = "Name";
 const char kNameServers[] = "NameServers";
 const char kProxySettings[] = "ProxySettings";

diff --git a/components/onc/onc_constants.h b/components/onc/onc_constants.h
@@ -61,6 +61,7 @@ ONC_EXPORT extern const char kCellular[];
 ONC_EXPORT extern const char kEthernet[];
 ONC_EXPORT extern const char kGUID[];
 ONC_EXPORT extern const char kIPConfigs[];
+ONC_EXPORT extern const char kMacAddress[];
 ONC_EXPORT extern const char kName[];
 ONC_EXPORT extern const char kNameServers[];
 ONC_EXPORT extern const char kProxySettings[];

diff --git a/components/wifi/fake_wifi_service.cc b/components/wifi/fake_wifi_service.cc
@@ -25,6 +25,7 @@ FakeWiFiService::FakeWiFiService() {
     network_properties.signal_strength = 40;
     network_properties.json_extra =
       "{"
+      "  \"MacAddress\": \"00:11:22:AA:BB:CC\","
       "  \"IPConfigs\": [{"
       "     \"Gateway\": \"0.0.0.1\","
       "     \"IPAddress\": \"0.0.0.0\","