Use unique_ptr for NetworkState IP and Proxy configs.

IP config only exists for connected networks.
Proxy config only exists for networks with a proxy configured.

Storing these as a DictionaryValue takes 32 bytes, whereas storing these
as a unique_ptr<> only takes 8 bytes. It also makes the symantics of
whether or not the property as set a bit more clear.

Bug: 651157
Change-Id: I4ab04ada996406bcace85c9038ac35f32b77054d
Reviewed-on: https://chromium-review.googlesource.com/1152148
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578787}

chrome/browser/chromeos/net/network_portal_detector_impl.cc

@@ -346,22 +346,26 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
   default_network_name_ = default_network->name();
 
   bool network_changed = (default_network_id_ != default_network->guid());
-  if (network_changed) {
+  if (network_changed)
     default_network_id_ = default_network->guid();
-    default_proxy_config_ =
-        std::make_unique<base::Value>(default_network->proxy_config().Clone());
-  }
 
   bool connection_state_changed =
       (default_connection_state_ != default_network->connection_state());
   default_connection_state_ = default_network->connection_state();
 
-  bool proxy_config_changed =
-      default_proxy_config_.get() &&
-      (*default_proxy_config_ != default_network->proxy_config());
-  if (proxy_config_changed) {
-    default_proxy_config_ =
-        std::make_unique<base::Value>(default_network->proxy_config().Clone());
+  bool proxy_config_changed = false;
+  if (!default_network->proxy_config()) {
+    if (default_proxy_config_) {
+      proxy_config_changed = true;
+      default_proxy_config_.reset();
+    }
+  } else {
+    if (!default_proxy_config_ || network_changed ||
+        (*default_proxy_config_ != *default_network->proxy_config())) {
+      proxy_config_changed = true;
+      default_proxy_config_ = std::make_unique<base::Value>(
+          default_network->proxy_config()->Clone());
+    }
   }
 
   if (default_network->is_captive_portal())

chrome/browser/ui/webui/chromeos/login/network_state_informer.cc

@@ -193,13 +193,13 @@ bool NetworkStateInformer::UpdateState() {
 bool NetworkStateInformer::UpdateProxyConfig() {
   const NetworkState* default_network =
       NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
-  if (!default_network)
+  if (!default_network || !default_network->proxy_config())
     return false;
 
-  if (proxy_config_ && *proxy_config_ == default_network->proxy_config())
+  if (proxy_config_ && *proxy_config_ == *default_network->proxy_config())
     return false;
   proxy_config_ =
-      std::make_unique<base::Value>(default_network->proxy_config().Clone());
+      std::make_unique<base::Value>(default_network->proxy_config()->Clone());
   return true;
 }
 

chromeos/network/managed_network_configuration_handler_impl.cc

@@ -935,8 +935,8 @@ void ManagedNetworkConfigurationHandlerImpl::GetDeviceStateProperties(
     NET_LOG(DEBUG)
         << "GetDeviceStateProperties: Setting IPv4 properties from network: "
         << service_path;
-    if (!network->ipv4_config().empty())
-      ip_configs->GetList().push_back(network->ipv4_config().Clone());
+    if (network->ipv4_config())
+      ip_configs->GetList().push_back(network->ipv4_config()->Clone());
   } else {
     // Convert the DeviceState IPConfigs dictionary to a ListValue.
     for (const auto iter : device_state->ip_configs().DictItems())

chromeos/network/network_change_notifier_chromeos_unittest.cc

@@ -151,15 +151,17 @@ class NetworkChangeNotifierChromeosUpdateTest : public testing::Test {
     default_network_.network_technology_ =
         default_network_state.network_technology;
     default_network_.path_ = default_network_state.service_path;
-    default_network_.ipv4_config_.SetKey(
-        shill::kAddressProperty, base::Value(default_network_state.ip_address));
+    base::DictionaryValue ipv4_properties;
+    ipv4_properties.SetKey(shill::kAddressProperty,
+                           base::Value(default_network_state.ip_address));
     std::vector<std::string> dns_servers =
         base::SplitString(default_network_state.dns_servers, ",",
                           base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
     base::ListValue dns_servers_value;
     dns_servers_value.AppendStrings(dns_servers);
-    default_network_.ipv4_config_.SetKey(shill::kNameServersProperty,
-                                         std::move(dns_servers_value));
+    ipv4_properties.SetKey(shill::kNameServersProperty,
+                           std::move(dns_servers_value));
+    default_network_.IPConfigPropertiesChanged(ipv4_properties);
   }
 
   // Process an default network update based on the state of |default_network_|.

chromeos/network/network_state.cc

@@ -68,9 +68,9 @@ bool IsCaptivePortalState(const base::DictionaryValue& properties, bool log) {
   return is_captive_portal;
 }
 
-std::string GetStringFromDictionary(const base::DictionaryValue& dict,
+std::string GetStringFromDictionary(const std::unique_ptr<base::Value>& dict,
                                     const char* key) {
-  const base::Value* v = dict.FindKey(key);
+  const base::Value* v = dict ? dict->FindKey(key) : nullptr;
   return v ? v->GetString() : std::string();
 }
 
@@ -158,15 +158,14 @@ bool NetworkState::PropertyChanged(const std::string& key,
       return false;
     }
 
-    proxy_config_.Clear();
+    proxy_config_.reset();
     if (proxy_config_str.empty())
       return true;
 
     std::unique_ptr<base::Value> proxy_config_dict(
         onc::ReadDictionaryFromJson(proxy_config_str));
     if (proxy_config_dict) {
-      proxy_config_.MergeDictionary(
-          base::DictionaryValue::From(std::move(proxy_config_dict)).get());
+      proxy_config_ = std::move(proxy_config_dict);
     } else {
       NET_LOG(ERROR) << "Failed to parse " << path() << "." << key;
     }
@@ -304,8 +303,11 @@ void NetworkState::GetStateProperties(base::DictionaryValue* dictionary) const {
 
 void NetworkState::IPConfigPropertiesChanged(
     const base::DictionaryValue& properties) {
-  ipv4_config_.Clear();
-  ipv4_config_.MergeDictionary(&properties);
+  if (properties.empty()) {
+    ipv4_config_.reset();
+    return;
+  }
+  ipv4_config_ = std::make_unique<base::Value>(properties.Clone());
 }
 
 std::string NetworkState::GetIpAddress() const {
@@ -423,7 +425,9 @@ std::string NetworkState::GetHexSsid() const {
 }
 
 std::string NetworkState::GetDnsServersAsString() const {
-  const base::Value* listv = ipv4_config_.FindKey(shill::kNameServersProperty);
+  const base::Value* listv =
+      ipv4_config_ ? ipv4_config_->FindKey(shill::kNameServersProperty)
+                   : nullptr;
   if (!listv)
     return std::string();
   std::string result;
@@ -436,7 +440,8 @@ std::string NetworkState::GetDnsServersAsString() const {
 }
 
 std::string NetworkState::GetNetmask() const {
-  const base::Value* v = ipv4_config_.FindKey(shill::kPrefixlenProperty);
+  const base::Value* v =
+      ipv4_config_ ? ipv4_config_->FindKey(shill::kPrefixlenProperty) : nullptr;
   int prefixlen = v ? v->GetInt() : -1;
   return network_util::PrefixLengthToNetmask(prefixlen);
 }

chromeos/network/network_state.h

@@ -84,9 +84,8 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   std::string connection_state() const;
   void set_connection_state(const std::string connection_state);
 
-  const base::DictionaryValue& proxy_config() const { return proxy_config_; }
-
-  const base::DictionaryValue& ipv4_config() const { return ipv4_config_; }
+  const base::Value* proxy_config() const { return proxy_config_.get(); }
+  const base::Value* ipv4_config() const { return ipv4_config_.get(); }
   std::string GetIpAddress() const;
   std::string GetGateway() const;
   GURL GetWebProxyAutoDiscoveryUrl() const;
@@ -235,7 +234,7 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
 
   // Cached copy of the Shill Service IPConfig object. For ipv6 properties use
   // the ip_configs_ property in the corresponding DeviceState.
-  base::DictionaryValue ipv4_config_;
+  std::unique_ptr<base::Value> ipv4_config_;
 
   // Wireless properties, used for icons and Connect logic.
   bool connectable_ = false;
@@ -272,7 +271,7 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
 
   // TODO(pneubeck): Remove this once (Managed)NetworkConfigurationHandler
   // provides proxy configuration. crbug.com/241775
-  base::DictionaryValue proxy_config_;
+  std::unique_ptr<base::Value> proxy_config_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkState);
 };

chromeos/network/proxy/proxy_config_handler.cc

@@ -88,10 +88,10 @@ std::unique_ptr<ProxyConfigDictionary> GetProxyConfigForNetwork(
   // unshared) configuration.
   // The user's proxy setting is not stored in the Chrome preference yet. We
   // still rely on Shill storing it.
-  const base::DictionaryValue& value = network.proxy_config();
-  if (value.empty())
+  const base::Value* value = network.proxy_config();
+  if (!value)
     return nullptr;
-  return std::make_unique<ProxyConfigDictionary>(value.Clone());
+  return std::make_unique<ProxyConfigDictionary>(value->Clone());
 }
 
 void SetProxyConfigForNetwork(const ProxyConfigDictionary& proxy_config,