NetworkState: Use empty Value instead of unique_ptr

It is preferred to embed base::Value and use is_none() over unique_ptr.
This simplifies some existing code and will further simplify some code
for tracking captive portal state in NetworkState.

Bug: 1133376
Change-Id: I7bce0e322bb5f34a280e6be8db168c78d096afb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2450489
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814238}

chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc

@@ -236,7 +236,7 @@ class ArcSettingsServiceImpl
   // network has changed.
   std::string default_network_name_;
   // Proxy configuration of the default network.
-  base::Optional<base::Value> default_proxy_config_;
+  base::Value default_proxy_config_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcSettingsServiceImpl);
 };
@@ -319,18 +319,15 @@ void ArcSettingsServiceImpl::DefaultNetworkChanged(
   // Trigger a proxy settings sync to ARC if the default network changes.
   if (default_network_name_ != network->name()) {
     default_network_name_ = network->name();
-    default_proxy_config_.reset();
+    default_proxy_config_ = base::Value();
     sync_proxy = true;
   }
   // Trigger a proxy settings sync to ARC if the proxy configuration of the
   // default network changes. Note: this code is only called if kProxy pref is
   // not set.
-  if (network->proxy_config()) {
-    if (!default_proxy_config_ ||
-        (*default_proxy_config_ != *network->proxy_config())) {
-      default_proxy_config_ = network->proxy_config()->Clone();
-      sync_proxy = true;
-    }
+  if (default_proxy_config_ != network->proxy_config()) {
+    default_proxy_config_ = network->proxy_config().Clone();
+    sync_proxy = true;
   }
   if (!sync_proxy)
     return;

chrome/browser/chromeos/net/network_portal_detector_impl.cc

@@ -198,7 +198,7 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
   if (!default_network) {
     NET_LOG(EVENT) << "Default network changed: None";
 
-    default_proxy_config_.reset();
+    default_proxy_config_ = base::Value();
 
     StopDetection();
 
@@ -217,24 +217,24 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
   default_connection_state_ = default_network->connection_state();
 
   bool proxy_config_changed = false;
-  if (!default_network->proxy_config()) {
-    if (default_proxy_config_) {
+  const base::Value& default_network_proxy_config =
+      default_network->proxy_config();
+  if (default_network_proxy_config.is_none()) {
+    if (!default_proxy_config_.is_none()) {
       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());
+      default_proxy_config_ = base::Value();
     }
+  } else if (network_changed ||
+             default_proxy_config_ != default_network_proxy_config) {
+    proxy_config_changed = true;
+    default_proxy_config_ = default_network_proxy_config.Clone();
   }
 
   NET_LOG(EVENT) << "Default network changed:"
                  << " id=" << NetworkGuidId(default_network_id_)
                  << " state=" << default_connection_state_
                  << " changed=" << network_changed
+                 << " proxy_config_changed=" << proxy_config_changed
                  << " state_changed=" << connection_state_changed;
 
   if (network_changed || connection_state_changed || proxy_config_changed)

chrome/browser/chromeos/net/network_portal_detector_impl.h

@@ -182,7 +182,7 @@ class NetworkPortalDetectorImpl : public NetworkPortalDetector,
   std::string default_connection_state_;
 
   // Proxy configuration of the default network.
-  std::unique_ptr<base::Value> default_proxy_config_;
+  base::Value default_proxy_config_;
 
   State state_ = STATE_IDLE;
   CaptivePortalStateMap portal_state_map_;

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

@@ -208,7 +208,7 @@ bool NetworkStateInformer::UpdateState() {
 
   state_ = new_state;
   network_path_ = new_network_path;
-  proxy_config_.reset();
+  proxy_config_ = base::Value();
 
   if (state_ == ONLINE) {
     for (NetworkStateInformerObserver& observer : observers_)
@@ -221,13 +221,12 @@ bool NetworkStateInformer::UpdateState() {
 bool NetworkStateInformer::UpdateProxyConfig() {
   const NetworkState* default_network =
       NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
-  if (!default_network || !default_network->proxy_config())
+  if (!default_network)
     return false;
 
-  if (proxy_config_ && *proxy_config_ == *default_network->proxy_config())
+  if (proxy_config_ == default_network->proxy_config())
     return false;
-  proxy_config_ =
-      std::make_unique<base::Value>(default_network->proxy_config()->Clone());
+  proxy_config_ = default_network->proxy_config().Clone();
   return true;
 }
 

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

@@ -95,7 +95,7 @@ class NetworkStateInformer
 
   State state_;
   std::string network_path_;
-  std::unique_ptr<base::Value> proxy_config_;
+  base::Value proxy_config_;
 
   base::ObserverList<NetworkStateInformerObserver>::Unchecked observers_;
 

chromeos/network/managed_network_configuration_handler_impl.cc

@@ -958,8 +958,8 @@ void ManagedNetworkConfigurationHandlerImpl::GetDeviceStateProperties(
     NET_LOG(DEBUG)
         << "GetDeviceStateProperties: Setting IPv4 properties from network: "
         << NetworkId(network);
-    if (network->ipv4_config())
-      ip_configs.Append(network->ipv4_config()->Clone());
+    if (!network->ipv4_config().is_none())
+      ip_configs.Append(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_state.cc

@@ -30,15 +30,17 @@ const char kDefaultCellularNetworkPath[] = "/cellular";
 // TODO(tbarzic): Add payment portal method values to shill/dbus-constants.
 constexpr char kPaymentPortalMethodPost[] = "POST";
 
-std::string GetStringFromDictionary(const base::Value* dict, const char* key) {
-  const base::Value* v = dict ? dict->FindKey(key) : nullptr;
-  return v ? v->GetString() : std::string();
+// |dict| may be an empty value, in which case return an empty string.
+std::string GetStringFromDictionary(const base::Value& dict, const char* key) {
+  const std::string* stringp =
+      dict.is_none() ? nullptr : dict.FindStringKey(key);
+  return stringp ? *stringp : std::string();
 }
 
 bool IsCaptivePortalState(const base::Value& properties,
                           const std::string& log_id) {
   std::string state =
-      GetStringFromDictionary(&properties, shill::kStateProperty);
+      GetStringFromDictionary(properties, shill::kStateProperty);
   if (!chromeos::NetworkState::StateIsPortalled(state))
     return false;
   if (!properties.FindKey(shill::kPortalDetectionFailedPhaseProperty) ||
@@ -49,9 +51,9 @@ bool IsCaptivePortalState(const base::Value& properties,
   }
 
   std::string portal_detection_phase = GetStringFromDictionary(
-      &properties, shill::kPortalDetectionFailedPhaseProperty);
+      properties, shill::kPortalDetectionFailedPhaseProperty);
   std::string portal_detection_status = GetStringFromDictionary(
-      &properties, shill::kPortalDetectionFailedStatusProperty);
+      properties, shill::kPortalDetectionFailedStatusProperty);
 
   // Shill reports the phase in which it determined that the device is behind a
   // captive portal. We only want to rely only on incorrect content being
@@ -176,15 +178,16 @@ bool NetworkState::PropertyChanged(const std::string& key,
       return false;
     }
 
-    proxy_config_.reset();
-    if (proxy_config_str.empty())
+    if (proxy_config_str.empty()) {
+      proxy_config_ = base::Value();
       return true;
-
+    }
     std::unique_ptr<base::Value> proxy_config_dict(
         onc::ReadDictionaryFromJson(proxy_config_str));
     if (proxy_config_dict) {
-      proxy_config_ = std::move(proxy_config_dict);
+      proxy_config_ = std::move(*proxy_config_dict);
     } else {
+      proxy_config_ = base::Value();
       NET_LOG(ERROR) << "Failed to parse " << path() << "." << key;
     }
     return true;
@@ -339,23 +342,23 @@ bool NetworkState::IsActive() const {
 
 void NetworkState::IPConfigPropertiesChanged(const base::Value& properties) {
   if (properties.DictEmpty()) {
-    ipv4_config_.reset();
+    ipv4_config_ = base::Value();
     return;
   }
-  ipv4_config_ = std::make_unique<base::Value>(properties.Clone());
+  ipv4_config_ = properties.Clone();
 }
 
 std::string NetworkState::GetIpAddress() const {
-  return GetStringFromDictionary(ipv4_config_.get(), shill::kAddressProperty);
+  return GetStringFromDictionary(ipv4_config_, shill::kAddressProperty);
 }
 
 std::string NetworkState::GetGateway() const {
-  return GetStringFromDictionary(ipv4_config_.get(), shill::kGatewayProperty);
+  return GetStringFromDictionary(ipv4_config_, shill::kGatewayProperty);
 }
 
 GURL NetworkState::GetWebProxyAutoDiscoveryUrl() const {
   std::string url = GetStringFromDictionary(
-      ipv4_config_.get(), shill::kWebProxyAutoDiscoveryUrlProperty);
+      ipv4_config_, shill::kWebProxyAutoDiscoveryUrlProperty);
   if (url.empty())
     return GURL();
   GURL gurl(url);
@@ -517,8 +520,9 @@ std::string NetworkState::GetHexSsid() const {
 
 std::string NetworkState::GetDnsServersAsString() const {
   const base::Value* listv =
-      ipv4_config_ ? ipv4_config_->FindKey(shill::kNameServersProperty)
-                   : nullptr;
+      ipv4_config_.is_none()
+          ? nullptr
+          : ipv4_config_.FindListKey(shill::kNameServersProperty);
   if (!listv)
     return std::string();
   std::string result;
@@ -531,9 +535,8 @@ std::string NetworkState::GetDnsServersAsString() const {
 }
 
 std::string NetworkState::GetNetmask() const {
-  const base::Value* v =
-      ipv4_config_ ? ipv4_config_->FindKey(shill::kPrefixlenProperty) : nullptr;
-  int prefixlen = v ? v->GetInt() : -1;
+  int prefixlen =
+      ipv4_config_.FindIntKey(shill::kPrefixlenProperty).value_or(-1);
   return network_util::PrefixLengthToNetmask(prefixlen);
 }
 

chromeos/network/network_state.h

@@ -100,8 +100,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
 
   int priority() const { return priority_; }
 
-  const base::Value* proxy_config() const { return proxy_config_.get(); }
-  const base::Value* ipv4_config() const { return ipv4_config_.get(); }
+  const base::Value& proxy_config() const { return proxy_config_; }
+  const base::Value& ipv4_config() const { return ipv4_config_; }
   std::string GetIpAddress() const;
   std::string GetGateway() const;
   GURL GetWebProxyAutoDiscoveryUrl() const;
@@ -301,7 +301,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
 
   // Cached copy of the Shill Service IPConfig object. For ipv6 properties use
   // the ip_configs_ property in the corresponding DeviceState.
-  std::unique_ptr<base::Value> ipv4_config_;
+  base::Value ipv4_config_;
 
   // Wireless properties, used for icons and Connect logic.
   bool connectable_ = false;
@@ -341,7 +341,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
 
   // TODO(pneubeck): Remove this once (Managed)NetworkConfigurationHandler
   // provides proxy configuration. crbug.com/241775
-  std::unique_ptr<base::Value> proxy_config_;
+  base::Value proxy_config_;
 
   // Set while a network connect request is queued. Cleared on connect or
   // if the request is aborted.

chromeos/network/proxy/proxy_config_handler.cc

@@ -87,10 +87,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::Value* value = network.proxy_config();
-  if (!value)
+  const base::Value& value = network.proxy_config();
+  if (value.is_none())
     return nullptr;
-  return std::make_unique<ProxyConfigDictionary>(value->Clone());
+  return std::make_unique<ProxyConfigDictionary>(value.Clone());
 }
 
 void SetProxyConfigForNetwork(const ProxyConfigDictionary& proxy_config,