Remove DeviceState dependency for NetworkState.IsRoaming() (Take 2)

I am removing NetworkHandler dependencies from network_icon.cc to
simplify the pending migration to mojo APIs in //ash.

This fixes a use-of-uninitialized-value error found in asan tests.

Original CL: f2cb8dc3

Bug: 923444
Change-Id: Id9405fc9882a84752343fda165871f38e45f224d
Reviewed-on: https://chromium-review.googlesource.com/c/1436188
Auto-Submit: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626139}

ash/system/network/network_icon.cc

@@ -14,7 +14,6 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/vector_icons/vector_icons.h"
-#include "chromeos/network/device_state.h"
 #include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
@@ -28,7 +27,6 @@
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/vector_icon_types.h"
 
-using chromeos::DeviceState;
 using chromeos::NetworkConnectionHandler;
 using chromeos::NetworkHandler;
 using chromeos::NetworkPortalDetector;
@@ -83,7 +81,7 @@ class NetworkIconImpl {
   std::string state_;
 
   // Cached strength index of the network when the icon was last generated.
-  int strength_index_;
+  int strength_index_ = -1;
 
   // Cached technology badge for the network when the icon was last generated.
   Badge technology_badge_ = {};
@@ -92,10 +90,10 @@ class NetworkIconImpl {
   Badge vpn_badge_ = {};
 
   // Cached roaming state of the network when the icon was last generated.
-  std::string roaming_state_;
+  bool is_roaming_ = false;
 
   // Cached portal state of the network when the icon was last generated.
-  bool behind_captive_portal_;
+  bool behind_captive_portal_ = false;
 
   // Generated icon image.
   gfx::ImageSkia image_;
@@ -385,10 +383,7 @@ gfx::ImageSkia GetConnectingVpnImage(IconType icon_type) {
 NetworkIconImpl::NetworkIconImpl(const std::string& path,
                                  IconType icon_type,
                                  const std::string& network_type)
-    : network_path_(path),
-      icon_type_(icon_type),
-      strength_index_(-1),
-      behind_captive_portal_(false) {
+    : network_path_(path), icon_type_(icon_type) {
   // Default image is null.
 }
 
@@ -439,9 +434,9 @@ bool NetworkIconImpl::UpdateCellularState(const NetworkState* network) {
     technology_badge_ = technology_badge;
     dirty = true;
   }
-  std::string roaming_state = network->roaming();
-  if (roaming_state != roaming_state_) {
-    roaming_state_ = roaming_state;
+  bool is_roaming = network->IndicateRoaming();
+  if (is_roaming != is_roaming_) {
+    is_roaming_ = is_roaming;
     dirty = true;
   }
   return dirty;
@@ -492,18 +487,8 @@ void NetworkIconImpl::GetBadges(const NetworkState* network, Badges* badges) {
     technology_badge_ = {&kNetworkBadgeTechnology4gIcon, icon_color};
   } else if (type == shill::kTypeCellular) {
     // technology_badge_ is set in UpdateCellularState.
-    if (network->IsConnectedState() &&
-        network->roaming() == shill::kRoamingStateRoaming) {
-      // For networks that are always in roaming don't show roaming badge.
-      const DeviceState* device =
-          NetworkHandler::Get()->network_state_handler()->GetDeviceState(
-              network->device_path());
-      LOG_IF(WARNING, !device)
-          << "Could not find device state for " << network->device_path();
-      if (!device || !device->provider_requires_roaming()) {
-        badges->bottom_right = {&kNetworkBadgeRoamingIcon, icon_color};
-      }
-    }
+    if (network->IsConnectedState() && network->IndicateRoaming())
+      badges->bottom_right = {&kNetworkBadgeRoamingIcon, icon_color};
   }
   // Only show technology, VPN, and captive portal badges when connected.
   if (network->IsConnectedState()) {

ash/system/network/tray_network_state_observer.cc

@@ -87,11 +87,6 @@ void TrayNetworkStateObserver::NetworkPropertiesUpdated(
   SignalUpdate(false /* notify_a11y */);
 }
 
-void TrayNetworkStateObserver::DevicePropertiesUpdated(
-    const chromeos::DeviceState* device) {
-  SignalUpdate(false /* notify_a11y */);
-}
-
 void TrayNetworkStateObserver::OnPortalDetectionCompleted(
     const chromeos::NetworkState* network,
     const chromeos::NetworkPortalDetector::CaptivePortalState& state) {

ash/system/network/tray_network_state_observer.h

@@ -37,7 +37,6 @@ class TrayNetworkStateObserver
   void NetworkConnectionStateChanged(
       const chromeos::NetworkState* network) override;
   void NetworkPropertiesUpdated(const chromeos::NetworkState* network) override;
-  void DevicePropertiesUpdated(const chromeos::DeviceState* device) override;
 
   // NetworkPortalDetector::Observer
   void OnPortalDetectionCompleted(

chromeos/network/network_state.cc

@@ -320,7 +320,7 @@ void NetworkState::GetStateProperties(base::Value* dictionary) const {
                        base::Value(network_technology()));
     dictionary->SetKey(shill::kActivationStateProperty,
                        base::Value(activation_state()));
-    dictionary->SetKey(shill::kRoamingStateProperty, base::Value(roaming()));
+    dictionary->SetKey(shill::kRoamingStateProperty, base::Value(roaming_));
     dictionary->SetKey(shill::kOutOfCreditsProperty,
                        base::Value(cellular_out_of_credits()));
   }
@@ -405,6 +405,11 @@ bool NetworkState::IsUsingMobileData() const {
          tethering_state() == shill::kTetheringConfirmedState;
 }
 
+bool NetworkState::IndicateRoaming() const {
+  return type() == shill::kTypeCellular &&
+         roaming_ == shill::kRoamingStateRoaming && !provider_requires_roaming_;
+}
+
 bool NetworkState::IsDynamicWep() const {
   return security_class_ == shill::kSecurityWep &&
          eap_key_mgmt_ == shill::kKeyManagementIEEE8021X;

chromeos/network/network_state.h

@@ -116,7 +116,6 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
   const std::string& network_technology() const { return network_technology_; }
   const std::string& activation_type() const { return activation_type_; }
   const std::string& activation_state() const { return activation_state_; }
-  const std::string& roaming() const { return roaming_; }
   const std::string& payment_url() const { return payment_url_; }
   const std::string& payment_post_data() const { return payment_post_data_; }
   bool cellular_out_of_credits() const { return cellular_out_of_credits_; }
@@ -146,7 +145,11 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
   // |onc_source_|).
   bool IsManagedByPolicy() const;
 
-  // Returns true if current connection is using mobile data.
+  // Returns true if the network is romaing and the provider does not require
+  // roaming.
+  bool IndicateRoaming() const;
+
+  // Returns true if the current connection is using mobile data.
   bool IsUsingMobileData() const;
 
   // Returns true if the network securty is WEP_8021x (Dynamic WEP)
@@ -265,6 +268,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
   std::string activation_type_;
   std::string activation_state_;
   std::string roaming_;
+  bool provider_requires_roaming_ = false;
   std::string payment_url_;
   std::string payment_post_data_;
   bool cellular_out_of_credits_ = false;

chromeos/network/network_state_handler.cc

@@ -1257,6 +1257,8 @@ void NetworkStateHandler::UpdateNetworkStateProperties(
 
   UpdateGuid(network);
   UpdateCaptivePortalProvider(network);
+  if (network->Matches(NetworkTypePattern::Cellular()))
+    UpdateCellularStateFromDevice(network);
 
   network_list_sorted_ = false;
 
@@ -1634,6 +1636,13 @@ void NetworkStateHandler::UpdateCaptivePortalProvider(NetworkState* network) {
                                     portal_iter->second.name);
 }
 
+void NetworkStateHandler::UpdateCellularStateFromDevice(NetworkState* network) {
+  const DeviceState* device = GetDeviceState(network->device_path());
+  if (!device)
+    return;
+  network->provider_requires_roaming_ = device->provider_requires_roaming();
+}
+
 std::unique_ptr<NetworkState>
 NetworkStateHandler::MaybeCreateDefaultCellularNetwork() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

chromeos/network/network_state_handler.h

@@ -478,6 +478,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandler
   // |hex_ssid_to_captive_portal_provider_map_|.
   void UpdateCaptivePortalProvider(NetworkState* network);
 
+  // Update networkState properties from the associated DeviceState.
+  void UpdateCellularStateFromDevice(NetworkState* network);
+
   // Cellular networks may not have an associated Shill Service (e.g. when the
   // SIM is locked or a mobile network is not available). This returns a new
   // default cellular network if necessary.