NetworkState cleanup, pass properties to InitialPropertiesReceived

chromeos/network/network_state.cc

 // Copyright (c) 2012 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/network_state.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 33 matching lines @@
     } else {
       const uint32 kReplacementChar = 0xFFFD;
       // Puts kReplacementChar if character is a control character [0,0x20)
       // or is not readable UTF8.
       base::WriteUnicodeCharacter(kReplacementChar, &result);
     }
   }
   return result;
 }
 
+// If |ui_data_value| is empty, returns true and does not set |ui_data|.
+// Otherwise returns true and sets |ui_data| if |ui_data_value| is a valid

[pneubeck (no reviews), 2013/07/29 19:13:25]

You can simplify this by setting ui_data to new NetworkUIData(), if
|ui_data_value| is empty.
And setting ui_data to NULL (or not setting it) can replace the result false.

[stevenjb, 2013/07/29 20:25:47]

Done

+// NetworkUIData dictionary string.
+bool GetUIDataFromValue(const base::Value& ui_data_value,
+                        scoped_ptr<chromeos::NetworkUIData>* ui_data) {
+  std::string ui_data_str;
+  if (!ui_data_value.GetAsString(&ui_data_str))
+    return false;
+  if (ui_data_str.empty())
+    return true;  // Empty string is valid; |ui_data| is NULL.
+
+  scoped_ptr<base::DictionaryValue> ui_data_dict(
+      chromeos::onc::ReadDictionaryFromJson(ui_data_str));
+  if (!ui_data_dict)
+    return false;
+  ui_data->reset(new chromeos::NetworkUIData(*ui_data_dict));
+  return true;
+}
+
 }  // namespace
 
 namespace chromeos {
 
 NetworkState::NetworkState(const std::string& path)
     : ManagedState(MANAGED_TYPE_NETWORK, path),
       auto_connect_(false),
       favorite_(false),
       priority_(0),
+      user_profile_required_(false),
       onc_source_(onc::ONC_SOURCE_NONE),
       prefix_length_(0),
       signal_strength_(0),
       connectable_(false),
       passphrase_required_(false),
       activate_over_non_cellular_networks_(false),
       cellular_out_of_credits_(false) {
 }
 
 NetworkState::~NetworkState() {
@@ skipping 83 matching lines @@
       NET_LOG_ERROR("Failed to parse " + key, path());
     }
     return true;
   } else if (key == flimflam::kUIDataProperty) {
     if (!GetOncSource(value, &onc_source_)) {
       NET_LOG_ERROR("Failed to parse " + key, path());
       return false;
     }
     return true;
   } else if (key == flimflam::kNetworkTechnologyProperty) {
-    return GetStringValue(key, value, &technology_);
+    return GetStringValue(key, value, &network_technology_);
   } else if (key == flimflam::kDeviceProperty) {
     return GetStringValue(key, value, &device_path_);
   } else if (key == flimflam::kGuidProperty) {
     return GetStringValue(key, value, &guid_);
   } else if (key == flimflam::kProfileProperty) {
     return GetStringValue(key, value, &profile_path_);
   } else if (key == shill::kActivateOverNonCellularNetworkProperty) {
     return GetBooleanValue(key, value, &activate_over_non_cellular_networks_);
   } else if (key == shill::kOutOfCreditsProperty) {
     return GetBooleanValue(key, value, &cellular_out_of_credits_);
   } else if (key == flimflam::kUsageURLProperty) {
     return GetStringValue(key, value, &usage_url_);
   } else if (key == flimflam::kPaymentPortalProperty) {
     const DictionaryValue* dict;
     if (!value.GetAsDictionary(&dict))
       return false;
     if (!dict->GetStringWithoutPathExpansion(
             flimflam::kPaymentPortalURL, &payment_url_) ||
         !dict->GetStringWithoutPathExpansion(
             flimflam::kPaymentPortalMethod, &post_method_) ||
         !dict->GetStringWithoutPathExpansion(
             flimflam::kPaymentPortalPostData, &post_data_)) {
       return false;
     }
     return true;
-  } else if (key == flimflam::kWifiHexSsid) {
-    return GetStringValue(key, value, &hex_ssid_);
-  } else if (key == flimflam::kCountryProperty) {
-    // TODO(stevenjb): This is currently experimental. If we find a case where
-    // base::DetectEncoding() fails in UpdateName(), where country_code_ is
-    // set, figure out whether we can use country_code_ with ConvertToUtf8().
-    // crbug.com/233267.
-    return GetStringValue(key, value, &country_code_);
   }
   return false;
 }
 
-void NetworkState::InitialPropertiesReceived() {
-  UpdateName();
+void NetworkState::InitialPropertiesReceived(

[pneubeck (no reviews), 2013/07/29 19:13:25]

I'm missing a return value that indicates if a property was changed.
Only if something changed, a notification should be triggered.

Currently, if properties are changed _only_ in this function (and not in
PropertyChanged), then no notification is emitted.

[stevenjb, 2013/07/29 20:25:47]

Hmm, I guess that's theoretically possible. Fixed.

+    const base::DictionaryValue& properties) {
+  UpdateName(properties);
+  UpdateUserProfileRequired(properties);
 }
 
 void NetworkState::GetProperties(base::DictionaryValue* dictionary) const {
   // Keep care that these properties are the same as in |PropertyChanged|.
   dictionary->SetStringWithoutPathExpansion(flimflam::kNameProperty, name());
   dictionary->SetStringWithoutPathExpansion(flimflam::kTypeProperty, type());
   dictionary->SetIntegerWithoutPathExpansion(flimflam::kSignalStrengthProperty,
                                              signal_strength_);
   dictionary->SetStringWithoutPathExpansion(flimflam::kStateProperty,
                                             connection_state_);
@@ skipping 43 matching lines @@
                                              favorite_);
   dictionary->SetIntegerWithoutPathExpansion(flimflam::kPriorityProperty,
                                              priority_);
   // Proxy config and ONC source are intentionally omitted: These properties are
   // placed in NetworkState to transition ProxyConfigServiceImpl from
   // NetworkLibrary to the new network stack. The networking extension API
   // shouldn't depend on this member. Once ManagedNetworkConfigurationHandler
   // is used instead of NetworkLibrary, we can remove them again.
   dictionary->SetStringWithoutPathExpansion(
       flimflam::kNetworkTechnologyProperty,
-      technology_);
+      network_technology_);
   dictionary->SetStringWithoutPathExpansion(flimflam::kDeviceProperty,
                                             device_path_);
   dictionary->SetStringWithoutPathExpansion(flimflam::kGuidProperty, guid_);
   dictionary->SetStringWithoutPathExpansion(flimflam::kProfileProperty,
                                             profile_path_);
   dictionary->SetBooleanWithoutPathExpansion(
       shill::kActivateOverNonCellularNetworkProperty,
       activate_over_non_cellular_networks_);
   dictionary->SetBooleanWithoutPathExpansion(shill::kOutOfCreditsProperty,
                                              cellular_out_of_credits_);
@@ skipping 26 matching lines @@
 
 bool NetworkState::IsConnectingState() const {
   return StateIsConnecting(connection_state_);
 }
 
 bool NetworkState::IsManaged() const {
   return onc_source_ == onc::ONC_SOURCE_DEVICE_POLICY ||
          onc_source_ == onc::ONC_SOURCE_USER_POLICY;
 }
 
-bool NetworkState::IsShared() const {
-  return profile_path_ == NetworkProfileHandler::kSharedProfilePath;
+bool NetworkState::IsPrivate() const {
+  return !profile_path_.empty() &&
+      profile_path_ != NetworkProfileHandler::kSharedProfilePath;
 }
 
 std::string NetworkState::GetDnsServersAsString() const {
   std::string result;
   for (size_t i = 0; i < dns_servers_.size(); ++i) {
     if (i != 0)
       result += ",";
     result += dns_servers_[i];
   }
   return result;
 }
 
 std::string NetworkState::GetNetmask() const {
   return network_util::PrefixLengthToNetmask(prefix_length_);
 }
 
 bool NetworkState::HasAuthenticationError() const {
   return (error_ == flimflam::kErrorBadPassphrase ||
           error_ == flimflam::kErrorBadWEPKey ||
           error_ == flimflam::kErrorPppAuthFailed ||
           error_ == shill::kErrorEapLocalTlsFailed ||
           error_ == shill::kErrorEapRemoteTlsFailed ||
           error_ == shill::kErrorEapAuthenticationFailed);
 }
 
-void NetworkState::UpdateName() {
-  if (hex_ssid_.empty()) {
+void NetworkState::UpdateName(const base::DictionaryValue& properties) {
+  std::string hex_ssid;
+  properties.GetStringWithoutPathExpansion(flimflam::kWifiHexSsid, &hex_ssid);
+  if (hex_ssid.empty()) {
     // Validate name for UTF8.
     std::string valid_ssid = ValidateUTF8(name());
     if (valid_ssid != name()) {
       set_name(valid_ssid);
       NET_LOG_DEBUG("UpdateName", base::StringPrintf(
           "%s: UTF8: %s", path().c_str(), name().c_str()));
     }
     return;
   }
 
   std::string ssid;
   std::vector<uint8> raw_ssid_bytes;
-  if (base::HexStringToBytes(hex_ssid_, &raw_ssid_bytes)) {
+  if (base::HexStringToBytes(hex_ssid, &raw_ssid_bytes)) {
     ssid = std::string(raw_ssid_bytes.begin(), raw_ssid_bytes.end());
   } else {
     std::string desc = base::StringPrintf("%s: Error processing: %s",
-                                          path().c_str(), hex_ssid_.c_str());
+                                          path().c_str(), hex_ssid.c_str());
     NET_LOG_DEBUG("UpdateName", desc);
     LOG(ERROR) << desc;
     ssid = name();
   }
 
   if (IsStringUTF8(ssid)) {
     if (ssid != name()) {
       set_name(ssid);
       NET_LOG_DEBUG("UpdateName", base::StringPrintf(
           "%s: UTF8: %s", path().c_str(), name().c_str()));
     }
     return;
   }
 
   // Detect encoding and convert to UTF-8.
+  std::string country_code;
+  properties.GetStringWithoutPathExpansion(
+      flimflam::kCountryProperty, &country_code);
   std::string encoding;
   if (!base::DetectEncoding(ssid, &encoding)) {
-    // TODO(stevenjb): Test this. See comment in PropertyChanged() under
-    // flimflam::kCountryProperty.
-    encoding = country_code_;
+    // TODO(stevenjb): This is currently experimental. If we find a case where
+    // base::DetectEncoding() fails, we need to figure out whether we can use
+    // country_code with ConvertToUtf8(). crbug.com/233267.
+    encoding = country_code;
   }
   if (!encoding.empty()) {
     std::string utf8_ssid;
     if (base::ConvertToUtf8AndNormalize(ssid, encoding, &utf8_ssid)) {
       set_name(utf8_ssid);
       NET_LOG_DEBUG("UpdateName", base::StringPrintf(
           "%s: Encoding=%s: %s", path().c_str(),
           encoding.c_str(), name().c_str()));
       return;
     }
   }
 
   // Unrecognized encoding. Only use raw bytes if name_ is empty.
   if (name().empty())
     set_name(ssid);
   NET_LOG_DEBUG("UpdateName", base::StringPrintf(
       "%s: Unrecognized Encoding=%s: %s", path().c_str(),
       encoding.c_str(), name().c_str()));
 }
 
+void NetworkState::UpdateUserProfileRequired(
+    const base::DictionaryValue& properties) {
+  // VPN always requires a user profile.
+  if (type() == flimflam::kTypeVPN) {
+    user_profile_required_ = true;
+    return;
+  }
+
+  // 8021x EAP-TLS wifi networks with a certificate or pattern set require a
+  // user profile.
+  if (type() == flimflam::kTypeWifi &&
+      security() == flimflam::kSecurity8021x) {
+    std::string eap_method;
+    properties.GetStringWithoutPathExpansion(
+        flimflam::kEapMethodProperty, &eap_method);
+    if (eap_method != flimflam::kEapMethodTLS)
+      return;
+
+    std::string eap_cert_id;
+    properties.GetStringWithoutPathExpansion(
+        flimflam::kEapCertIdProperty, &eap_cert_id);
+    if (!eap_cert_id.empty()) {
+      user_profile_required_ = true;
+      return;
+    }
+
+    const base::Value* ui_data_value;
+    if (properties.GetWithoutPathExpansion(
+            flimflam::kUIDataProperty, &ui_data_value)) {
+      scoped_ptr<NetworkUIData> ui_data;
+      if (!GetUIDataFromValue(*ui_data_value, &ui_data) || !ui_data)
+        return;
+      if (ui_data->certificate_type() == CLIENT_CERT_TYPE_PATTERN)

[pneubeck (no reviews), 2013/07/29 19:13:25]

ClientCertPatterns can currently only be set by user policy.
If a user policy is already set, then IsPrivate is already true. Why would you
have to check certificate_type here additionally?

[stevenjb, 2013/07/29 20:25:47]

A PSK network that is Private can be shared.
A network requiring a certificate may be Private but can not be shared.
This property is used to make that distinction.

+        user_profile_required_ = true;
+    }
+  }
+}
+
 // static
 bool NetworkState::StateIsConnected(const std::string& connection_state) {
   return (connection_state == flimflam::kStateReady ||
           connection_state == flimflam::kStateOnline ||
           connection_state == flimflam::kStatePortal);
 }
 
 // static
 bool NetworkState::StateIsConnecting(const std::string& connection_state) {
   return (connection_state == flimflam::kStateAssociation ||
           connection_state == flimflam::kStateConfiguration ||
           connection_state == flimflam::kStateCarrier);
 }
 
 // static
 std::string NetworkState::IPConfigProperty(const char* key) {
   return base::StringPrintf("%s.%s", shill::kIPConfigProperty, key);
 }
 
 // static
-bool NetworkState::GetOncSource(const base::Value& value,
+bool NetworkState::GetOncSource(const base::Value& ui_data_value,
                                 onc::ONCSource* out) {
-  std::string ui_data_str;
-  if (!value.GetAsString(&ui_data_str))
+  scoped_ptr<NetworkUIData> ui_data;
+  if (!GetUIDataFromValue(ui_data_value, &ui_data))
     return false;
-  if (ui_data_str.empty()) {
+  if (!ui_data) {
     *out = onc::ONC_SOURCE_NONE;
     return true;
   }
-  scoped_ptr<base::DictionaryValue> ui_data_dict(
-      onc::ReadDictionaryFromJson(ui_data_str));
-  if (!ui_data_dict)
-    return false;
-  *out = NetworkUIData(*ui_data_dict).onc_source();
+  *out = ui_data->onc_source();
   return true;
 }
 
 }  // namespace chromeos