Add 'setProperties' to InternetOptionsHandler

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/strings/stringprintf.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_type_pattern.h"
@@ skipping 156 matching lines @@
   // SignalStrength > 0.
   if ((type() == shill::kTypeWifi || type() == shill::kTypeWimax) &&
       visible() && signal_strength_ <= 0) {
       signal_strength_ = 1;
   }
 
   return changed;
 }
 
 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,
                                               eap_method());
   }
 
   // Mobile properties
   if (NetworkTypePattern::Mobile().MatchesType(type())) {
     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());
   }
 }
 
 void NetworkState::IPConfigPropertiesChanged(
     const base::DictionaryValue& properties) {
   for (base::DictionaryValue::Iterator iter(properties);
        !iter.IsAtEnd(); iter.Advance()) {
     std::string key = iter.key();
     const base::Value& value = iter.value();
 
@@ skipping 115 matching lines @@
           connection_state == shill::kStateCarrier);
 }
 
 // static
 bool NetworkState::ErrorIsValid(const std::string& error) {
   // Shill uses "Unknown" to indicate an unset or cleared error state.
   return !error.empty() && error != kErrorUnknown;
 }
 
 }  // namespace chromeos