Add 'setProperties' to InternetOptionsHandler

chromeos/network/network_state.cc

Index: chromeos/network/network_state.cc
diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc
index 915a688de3cdaa9c51afe49888bfdb6e5243b8db..b6a42d2d128f99eb9a7d714ad59abda5f3af3f6b 100644
--- a/chromeos/network/network_state.cc
+++ b/chromeos/network/network_state.cc
@@ -174,31 +174,43 @@ bool NetworkState::InitialPropertiesReceived(
 }
 
 void NetworkState::GetStateProperties(base::DictionaryValue* dictionary) const {
-  ManagedState::GetStateProperties(dictionary);
+  GetConfigurationStateProperties(dictionary);
 

[pneubeck (no reviews), 2014/11/11 16:09:31]

maybe add a comment that the remainder of this function adds all
non-configuration (i.e. state) properties.

-  // Properties shared by all types.
-  dictionary->SetStringWithoutPathExpansion(shill::kGuidProperty, guid());
-  dictionary->SetStringWithoutPathExpansion(shill::kSecurityProperty,
-                                            security());
+  if (!visible())
+    return;
 
-  if (visible()) {
-    if (!error().empty())
-      dictionary->SetStringWithoutPathExpansion(shill::kErrorProperty, error());
-    dictionary->SetStringWithoutPathExpansion(shill::kStateProperty,
-                                              connection_state());
-  }
+  dictionary->SetStringWithoutPathExpansion(shill::kStateProperty,
+                                            connection_state());
+  if (!error().empty())
+    dictionary->SetStringWithoutPathExpansion(shill::kErrorProperty, error());
 
   // Wireless properties
-  if (!NetworkTypePattern::Wireless().MatchesType(type()))
-    return;
-
-  if (visible()) {
+  if (NetworkTypePattern::Wireless().MatchesType(type())) {
     dictionary->SetBooleanWithoutPathExpansion(shill::kConnectableProperty,
                                                connectable());
     dictionary->SetIntegerWithoutPathExpansion(shill::kSignalStrengthProperty,
                                                signal_strength());
   }
 
+  // Mobile properties
+  if (NetworkTypePattern::Mobile().MatchesType(type())) {
+    dictionary->SetStringWithoutPathExpansion(shill::kActivationStateProperty,
+                                              activation_state());
+    dictionary->SetStringWithoutPathExpansion(shill::kRoamingStateProperty,
+                                              roaming());
+    dictionary->SetBooleanWithoutPathExpansion(shill::kOutOfCreditsProperty,
+                                               cellular_out_of_credits());
+  }
+}
+
+void NetworkState::GetConfigurationStateProperties(
+    base::DictionaryValue* dictionary) const {
+  // Properties shared by all types.
+  ManagedState::GetStateProperties(dictionary);

[pneubeck (no reviews), 2014/11/11 16:09:31]

this seems not intuitive.
The function description explicitly says that state properties are not included.
I think you have to rename or split the ManagedState::GetStateProperties
function as well.

[stevenjb, 2014/11/12 01:23:29]

Separated out GetNetworkStateProperties and GetVisibleStateProperties helpers to
make this a bit more clear.

+  dictionary->SetStringWithoutPathExpansion(shill::kGuidProperty, guid());
+  dictionary->SetStringWithoutPathExpansion(shill::kSecurityProperty,
+                                            security());
+
   // Wifi properties
   if (NetworkTypePattern::WiFi().MatchesType(type())) {
     dictionary->SetStringWithoutPathExpansion(shill::kEapMethodProperty,
@@ -210,12 +222,6 @@ void NetworkState::GetStateProperties(base::DictionaryValue* dictionary) const {
     dictionary->SetStringWithoutPathExpansion(
         shill::kNetworkTechnologyProperty,
         network_technology());
-    dictionary->SetStringWithoutPathExpansion(shill::kActivationStateProperty,
-                                              activation_state());
-    dictionary->SetStringWithoutPathExpansion(shill::kRoamingStateProperty,
-                                              roaming());
-    dictionary->SetBooleanWithoutPathExpansion(shill::kOutOfCreditsProperty,
-                                               cellular_out_of_credits());
   }
 }