Eliminate NetworkState dependency from network_icon.cc

NetowrkState usage has been mostly removed from non test consumers of
network_icon.cc.

This removes the single remaining usage in GetSignalStrengthForNetwork,
updates the tests and removes the NetworkState dependency entirely.

This also includes a bit of cleanup to cros_network_config.cc to
separate NetworkStateHandler dependencies from NetworkState/DeviceState
dependencies in the file local helper functions. (These were
temporarily used in the tests but that was reverted, but the cleanup
still seems useful).

Bug: 862420
Change-Id: I566505e5a4f22538d8a862eeb5fcfc9f3c3ffd79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1600306
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659750}

ash/system/network/active_network_icon_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_test_helper.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -20,6 +21,9 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image_unittest_util.h"
 
+using chromeos::network_config::mojom::ConnectionStateType;
+using chromeos::network_config::mojom::NetworkType;
+
 namespace ash {
 
 namespace {
@@ -101,9 +105,17 @@ class ActiveNetworkIconTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
-  gfx::ImageSkia ImageForNetwork(const chromeos::NetworkState* network) {
+  gfx::ImageSkia ImageForNetwork(
+      chromeos::network_config::mojom::NetworkType type,
+      chromeos::network_config::mojom::ConnectionStateType connection_state,
+      int signal_strength = 100) {
+    std::string id = base::StringPrintf("reference_%d", reference_count_++);
+    chromeos::network_config::mojom::NetworkStatePropertiesPtr
+        reference_properties =
+            network_state_helper().CreateStandaloneNetworkProperties(
+                id, type, connection_state, signal_strength);
     return network_icon::GetImageForNonVirtualNetwork(
-        network_icon::NetworkIconState(network), icon_type_,
+        network_icon::NetworkIconState(reference_properties.get()), icon_type_,
         false /* show_vpn_badge */);
   }
 
@@ -122,18 +134,6 @@ class ActiveNetworkIconTest : public testing::Test {
     network_state_helper().SetServiceProperty(service_path, key, value);
   }
 
-  std::unique_ptr<chromeos::NetworkState> CreateStandaloneNetworkState(
-      const std::string& type,
-      const std::string& connection_state,
-      int signal_strength = 100) {
-    std::string id = base::StringPrintf("reference_%d", reference_count_++);
-    VLOG(1) << "CreateStandaloneNetworkState: " << id << " type: " << type
-            << " State: " << connection_state
-            << " Strength: " << signal_strength;
-    return network_state_helper().CreateStandaloneNetworkState(
-        id, type, connection_state, signal_strength);
-  }
-
   chromeos::NetworkStateTestHelper& network_state_helper() {
     return network_config_helper_.network_state_helper();
   }
@@ -190,17 +190,17 @@ TEST_F(ActiveNetworkIconTest, GetSingleImage) {
   bool animating;
   gfx::ImageSkia image =
       active_network_icon()->GetSingleImage(icon_type(), &animating);
-  std::unique_ptr<chromeos::NetworkState> reference_network =
-      CreateStandaloneNetworkState(shill::kTypeCellular, shill::kStateOnline);
-  EXPECT_TRUE(AreImagesEqual(image, ImageForNetwork(reference_network.get())));
+  EXPECT_TRUE(AreImagesEqual(
+      image,
+      ImageForNetwork(NetworkType::kCellular, ConnectionStateType::kOnline)));
   EXPECT_FALSE(animating);
 
   // Cellular + WiFi connected = WiFi connected icon
   SetupWiFi(shill::kStateOnline);
   image = active_network_icon()->GetSingleImage(icon_type(), &animating);
-  reference_network =
-      CreateStandaloneNetworkState(shill::kTypeWifi, shill::kStateOnline);
-  EXPECT_TRUE(AreImagesEqual(image, ImageForNetwork(reference_network.get())));
+  EXPECT_TRUE(AreImagesEqual(
+      image,
+      ImageForNetwork(NetworkType::kWiFi, ConnectionStateType::kOnline)));
   EXPECT_FALSE(animating);
 
   // Cellular + WiFi connecting = WiFi connecting icon
@@ -210,15 +210,17 @@ TEST_F(ActiveNetworkIconTest, GetSingleImage) {
                      base::Value(50));
   base::RunLoop().RunUntilIdle();
   image = active_network_icon()->GetSingleImage(icon_type(), &animating);
-  reference_network = CreateStandaloneNetworkState(
-      shill::kTypeWifi, shill::kStateAssociation, 50);
-  EXPECT_TRUE(AreImagesEqual(image, ImageForNetwork(reference_network.get())));
+  EXPECT_TRUE(AreImagesEqual(
+      image, ImageForNetwork(NetworkType::kWiFi,
+                             ConnectionStateType::kConnecting, 50)));
   EXPECT_TRUE(animating);
 
   // Cellular + WiFi connecting + Ethernet = WiFi connecting icon
   SetupEthernet();
   image = active_network_icon()->GetSingleImage(icon_type(), &animating);
-  EXPECT_TRUE(AreImagesEqual(image, ImageForNetwork(reference_network.get())));
+  EXPECT_TRUE(AreImagesEqual(
+      image, ImageForNetwork(NetworkType::kWiFi,
+                             ConnectionStateType::kConnecting, 50)));
   EXPECT_TRUE(animating);
 
   // Cellular + WiFi connected + Ethernet = No icon
@@ -240,10 +242,9 @@ TEST_F(ActiveNetworkIconTest, CellularUninitialized) {
   bool animating;
   gfx::ImageSkia image =
       active_network_icon()->GetSingleImage(icon_type(), &animating);
-  std::unique_ptr<chromeos::NetworkState> reference_network =
-      CreateStandaloneNetworkState(shill::kTypeCellular,
-                                   shill::kStateAssociation);
-  EXPECT_TRUE(AreImagesEqual(image, ImageForNetwork(reference_network.get())));
+  EXPECT_TRUE(
+      AreImagesEqual(image, ImageForNetwork(NetworkType::kCellular,
+                                            ConnectionStateType::kConnecting)));
   EXPECT_TRUE(animating);
 }
 
@@ -260,10 +261,9 @@ TEST_F(ActiveNetworkIconTest, CellularScanning) {
   bool animating;
   gfx::ImageSkia image =
       active_network_icon()->GetSingleImage(icon_type(), &animating);
-  std::unique_ptr<chromeos::NetworkState> reference_network =
-      CreateStandaloneNetworkState(shill::kTypeCellular,
-                                   shill::kStateAssociation);
-  EXPECT_TRUE(AreImagesEqual(image, ImageForNetwork(reference_network.get())));
+  EXPECT_TRUE(
+      AreImagesEqual(image, ImageForNetwork(NetworkType::kCellular,
+                                            ConnectionStateType::kConnecting)));
   EXPECT_TRUE(animating);
 }
 

ash/system/network/network_feature_pod_button.cc

@@ -90,6 +90,11 @@ const NetworkState* GetCurrentNetwork() {
 base::string16 GetSubLabelForConnectedNetwork(const NetworkState* network) {
   DCHECK(network && network->IsConnectedState());
 
+  if (!network->Matches(NetworkTypePattern::Wireless())) {
+    return l10n_util::GetStringUTF16(
+        IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
+  }
+
   if (NetworkTypePattern::Cellular().MatchesType(network->type())) {
     if (network->network_technology() == shill::kNetworkTechnology1Xrtt) {
       return l10n_util::GetStringUTF16(
@@ -137,7 +142,10 @@ base::string16 GetSubLabelForConnectedNetwork(const NetworkState* network) {
         IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
   }
 
-  switch (network_icon::GetSignalStrengthForNetwork(network)) {
+  switch (network_icon::GetSignalStrength(network->signal_strength())) {
+    case network_icon::SignalStrength::NONE:
+      return l10n_util::GetStringUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
     case network_icon::SignalStrength::WEAK:
       return l10n_util::GetStringUTF16(
           IDS_ASH_STATUS_TRAY_NETWORK_SIGNAL_WEAK_SUBLABEL);
@@ -147,10 +155,10 @@ base::string16 GetSubLabelForConnectedNetwork(const NetworkState* network) {
     case network_icon::SignalStrength::STRONG:
       return l10n_util::GetStringUTF16(
           IDS_ASH_STATUS_TRAY_NETWORK_SIGNAL_STRONG_SUBLABEL);
-    default:
+  }
+  NOTREACHED();
   return l10n_util::GetStringUTF16(
       IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
-  }
 }
 
 }  // namespace

ash/system/network/network_icon.cc

@@ -16,10 +16,6 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chromeos/network/network_state.h"
-#include "chromeos/network/network_type_pattern.h"
-#include "chromeos/network/onc/onc_translation_tables.h"
-#include "chromeos/network/tether_constants.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
 #include "components/onc/onc_constants.h"
 #include "components/vector_icons/vector_icons.h"
@@ -30,7 +26,6 @@
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/vector_icon_types.h"
 
-using chromeos::NetworkTypePattern;
 using chromeos::network_config::mojom::ActivationStateType;
 using chromeos::network_config::mojom::ConnectionStateType;
 using chromeos::network_config::mojom::NetworkType;
@@ -296,6 +291,8 @@ gfx::ImageSkia GetIcon(const NetworkIconState& network,
     return gfx::CreateVectorIcon(kNetworkVpnIcon,
                                  GetDefaultColorForIconType(ICON_TYPE_LIST));
   }
+  DCHECK_GE(strength_index, 0) << "Strength not set for type: " << network.type;
+  DCHECK_LT(strength_index, kNumNetworkImages);
   return GetImageForIndex(ImageTypeForNetworkType(network.type), icon_type,
                           strength_index);
 }
@@ -308,56 +305,13 @@ gfx::ImageSkia GetConnectingVpnImage(IconType icon_type) {
 
 }  // namespace
 
-NetworkIconState::NetworkIconState(const chromeos::NetworkState* network) {
-  guid = network->guid();
-  name = network->name();
-
-  const std::string& network_type = network->type();
-  if (NetworkTypePattern::Cellular().MatchesType(network_type) ||
-      NetworkTypePattern::Wimax().MatchesType(network_type)) {
-    type = NetworkType::kCellular;
-  } else if (NetworkTypePattern::Ethernet().MatchesType(network_type)) {
-    type = NetworkType::kEthernet;
-  } else if (NetworkTypePattern::Tether().MatchesType(network_type)) {
-    type = NetworkType::kTether;
-  } else if (NetworkTypePattern::VPN().MatchesType(network_type)) {
-    type = NetworkType::kVPN;
-  } else {
-    type = NetworkType::kWiFi;
-  }
-
-  if (network->IsCaptivePortal()) {
-    connection_state = ConnectionStateType::kPortal;
-  } else if (network->IsConnectedState()) {
-    connection_state = ConnectionStateType::kConnected;
-  } else if (network->IsConnectingState()) {
-    connection_state = ConnectionStateType::kConnecting;
-  } else {
-    connection_state = ConnectionStateType::kNotConnected;
-  }
-
-  security = network->GetMojoSecurity();
-
-  if (type == NetworkType::kCellular) {
-    if (!network->network_technology().empty()) {
-      chromeos::onc::TranslateStringToONC(
-          chromeos::onc::kNetworkTechnologyTable, network->network_technology(),
-          &network_technology);
-    }
-  }
-
-  activation_state = network->GetMojoActivationState();
-  signal_strength = network->signal_strength();
-  is_roaming = network->IndicateRoaming();
-}
-
 NetworkIconState::NetworkIconState(
     const chromeos::network_config::mojom::NetworkStateProperties* network) {
   guid = network->guid;
   name = network->name;
   type = network->type;
   connection_state = network->connection_state;
-  if (type == NetworkType::kCellular && network->cellular) {
+  if (type == NetworkType::kCellular) {
     activation_state = network->cellular->activation_state;
     network_technology = network->cellular->network_technology;
     is_roaming = network->cellular->roaming;
@@ -365,16 +319,16 @@ NetworkIconState::NetworkIconState(
   } else {
     activation_state = ActivationStateType::kUnknown;
   }
-  if (type == NetworkType::kTether && network->tether) {
+  if (type == NetworkType::kTether) {
     signal_strength = network->tether->signal_strength;
   }
-  if (type == NetworkType::kWiFi && network->wifi) {
+  if (type == NetworkType::kWiFi) {
     security = network->wifi->security;
     signal_strength = network->wifi->signal_strength;
   } else {
     security = chromeos::network_config::mojom::SecurityType::kNone;
   }
-  if (type == NetworkType::kWiMAX && network->wimax) {
+  if (type == NetworkType::kWiMAX) {
     signal_strength = network->wimax->signal_strength;
   }
 }
@@ -409,8 +363,8 @@ void NetworkIconImpl::Update(const NetworkIconState& network,
   }
 
   NetworkType type = network.type;
-  if (type == NetworkType::kCellular || type == NetworkType::kTether ||
-      type == NetworkType::kWiFi) {
+  if (chromeos::network_config::NetworkTypeMatchesType(
+          type, NetworkType::kWireless)) {
     dirty |= UpdateWirelessStrengthIndex(network);
   }
 
@@ -679,16 +633,12 @@ void PurgeNetworkIconCache(const std::set<std::string>& network_guids) {
   PurgeIconMap(ICON_TYPE_MENU_LIST, network_guids);
 }
 
-SignalStrength GetSignalStrengthForNetwork(
-    const chromeos::NetworkState* network) {
-  if (!network->Matches(NetworkTypePattern::Wireless()))
-    return SignalStrength::NOT_WIRELESS;
-
+SignalStrength GetSignalStrength(int strength) {
   // Decide whether the signal is considered weak, medium or strong based on the
   // strength index. Each signal strength corresponds to a bucket which
   // attempted to be split evenly from |kNumNetworkImages| - 1. Remainders go
   // first to the lowest bucket and then the second lowest bucket.
-  const int index = StrengthIndex(network->signal_strength());
+  const int index = StrengthIndex(strength);
   if (index == 0)
     return SignalStrength::NONE;
   const int seperations = kNumNetworkImages - 1;

ash/system/network/network_icon.h

@@ -15,17 +15,11 @@
 #include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image_skia.h"
 
-namespace chromeos {
-class NetworkState;
-}
-
 namespace ash {
 namespace network_icon {
 
 // TODO(stevenjb): Replace with network_config::mojom::NetworkStateProperties.
 struct ASH_EXPORT NetworkIconState {
-  // Constructs a NetworkIconState from a NetworkState.
-  explicit NetworkIconState(const chromeos::NetworkState* network);
   // Constructs a NetworkIconState from mojom::NetworkStateProperties.
   explicit NetworkIconState(
       const chromeos::network_config::mojom::NetworkStateProperties* network);
@@ -54,7 +48,7 @@ enum IconType {
 };
 
 // Strength of a wireless signal.
-enum class SignalStrength { NONE, WEAK, MEDIUM, STRONG, NOT_WIRELESS };
+enum class SignalStrength { NONE, WEAK, MEDIUM, STRONG };
 
 // Returns true if |icon_state| is connected or portal.
 bool IsConnected(const NetworkIconState& icon_state);
@@ -124,10 +118,8 @@ ASH_EXPORT void PurgeNetworkIconCache(
     const std::set<std::string>& network_guids);
 
 // Called by ChromeVox to give a verbal indication of the network icon. Returns
-// the signal strength of |network|, if it is a network type with a signal
-// strength.
-ASH_EXPORT SignalStrength
-GetSignalStrengthForNetwork(const chromeos::NetworkState* network);
+// a signal strength enum for |strength| value 0-100.
+ASH_EXPORT SignalStrength GetSignalStrength(int strength);
 
 }  // namespace network_icon
 }  // namespace ash

ash/system/network/network_tray_icon_strategy.cc

@@ -10,17 +10,8 @@
 #include "ash/system/network/active_network_icon.h"
 #include "ash/system/network/network_icon.h"
 #include "base/logging.h"
-#include "chromeos/network/network_state.h"
-#include "chromeos/network/network_state_handler.h"
-#include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/gfx/image/image_skia.h"
 
-using chromeos::NetworkConnectionHandler;
-using chromeos::NetworkHandler;
-using chromeos::NetworkState;
-using chromeos::NetworkStateHandler;
-using chromeos::NetworkTypePattern;
-
 namespace ash {
 namespace tray {
 

ash/system/network/network_tray_view.cc

@@ -126,16 +126,17 @@ void NetworkTrayView::UpdateConnectionStatus(
         IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED,
         base::UTF8ToUTF16(connected_network->name()));
 
+    if (connected_network->Matches(NetworkTypePattern::Wireless())) {
       // Retrieve the string describing the signal strength, if it is applicable
       // to |connected_network|.
       base::string16 signal_strength_string;
-    switch (network_icon::GetSignalStrengthForNetwork(connected_network)) {
+      switch (network_icon::GetSignalStrength(
+          connected_network->signal_strength())) {
         case SignalStrength::NONE:
-      case SignalStrength::NOT_WIRELESS:
           break;
         case SignalStrength::WEAK:
-        signal_strength_string =
-            l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_SIGNAL_WEAK);
+          signal_strength_string = l10n_util::GetStringUTF16(
+              IDS_ASH_STATUS_TRAY_NETWORK_SIGNAL_WEAK);
           break;
         case SignalStrength::MEDIUM:
           signal_strength_string = l10n_util::GetStringUTF16(
@@ -150,7 +151,9 @@ void NetworkTrayView::UpdateConnectionStatus(
       if (!signal_strength_string.empty()) {
         new_connection_status_string = l10n_util::GetStringFUTF16(
             IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED_ACCESSIBLE,
-          base::UTF8ToUTF16(connected_network->name()), signal_strength_string);
+            base::UTF8ToUTF16(connected_network->name()),
+            signal_strength_string);
+      }
     }
     connection_status_tooltip_ = new_connection_status_string;
   } else {

chromeos/components/tether/active_host_network_state_updater_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/message_loop/message_loop.h"
 #include "chromeos/components/tether/fake_active_host.h"
 #include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"

chromeos/components/tether/tether_network_disconnection_handler_unittest.cc

@@ -17,6 +17,7 @@
 #include "chromeos/components/tether/network_configuration_remover.h"
 #include "chromeos/components/tether/tether_session_completion_logger.h"
 #include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/shill/dbus-constants.h"

chromeos/network/BUILD.gn

@@ -176,6 +176,8 @@ source_set("test_support") {
     "//base",
     "//chromeos:test_utils",
     "//chromeos/dbus:test_support",
+    "//chromeos/services/network_config/public/cpp",
+    "//chromeos/services/network_config/public/mojom",
     "//dbus",
     "//net:test_support",
     "//testing/gmock",

chromeos/network/network_state_test_helper.cc

@@ -10,6 +10,7 @@
 #include "chromeos/dbus/shill/shill_clients.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/onc/onc_utils.h"
+#include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
 #include "dbus/object_path.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ -135,19 +136,52 @@ void NetworkStateTestHelper::SetServiceProperty(const std::string& service_path,
   base::RunLoop().RunUntilIdle();
 }
 
-std::unique_ptr<NetworkState>
-NetworkStateTestHelper::CreateStandaloneNetworkState(
+network_config::mojom::NetworkStatePropertiesPtr
+NetworkStateTestHelper::CreateStandaloneNetworkProperties(
     const std::string& id,
-    const std::string& type,
-    const std::string& connection_state,
+    network_config::mojom::NetworkType type,
+    network_config::mojom::ConnectionStateType connection_state,
     int signal_strength) {
-  auto network = std::make_unique<NetworkState>(id);
-  network->SetGuid(id);
-  network->set_name(id);
-  network->set_type(type);
-  network->set_visible(true);
-  network->SetConnectionState(connection_state);
-  network->set_signal_strength(signal_strength);
+  auto network = network_config::mojom::NetworkStateProperties::New();
+  network->guid = id;
+  network->name = id;
+  network->type = type;
+  network->connection_state = connection_state;
+  switch (type) {
+    case network_config::mojom::NetworkType::kAll:
+    case network_config::mojom::NetworkType::kMobile:
+    case network_config::mojom::NetworkType::kWireless:
+      NOTREACHED();
+      break;
+    case network_config::mojom::NetworkType::kCellular: {
+      auto cellular = network_config::mojom::CellularStateProperties::New();
+      cellular->signal_strength = signal_strength;
+      network->cellular = std::move(cellular);
+      break;
+    }
+    case network_config::mojom::NetworkType::kEthernet:
+      break;
+    case network_config::mojom::NetworkType::kTether: {
+      auto tether = network_config::mojom::TetherStateProperties::New();
+      tether->signal_strength = signal_strength;
+      network->tether = std::move(tether);
+      break;
+    }
+    case network_config::mojom::NetworkType::kVPN:
+      break;
+    case network_config::mojom::NetworkType::kWiFi: {
+      auto wifi = network_config::mojom::WiFiStateProperties::New();
+      wifi->signal_strength = signal_strength;
+      network->wifi = std::move(wifi);
+      break;
+    }
+    case network_config::mojom::NetworkType::kWiMAX: {
+      auto wimax = network_config::mojom::WiMAXStateProperties::New();
+      wimax->signal_strength = signal_strength;
+      network->wimax = std::move(wimax);
+      break;
+    }
+  }
   return network;
 }
 

chromeos/network/network_state_test_helper.h

@@ -13,10 +13,12 @@
 #include "chromeos/dbus/shill/shill_manager_client.h"
 #include "chromeos/dbus/shill/shill_profile_client.h"
 #include "chromeos/dbus/shill/shill_service_client.h"
-#include "chromeos/network/network_state_handler.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
 
 namespace chromeos {
 
+class NetworkStateHandler;
+
 // Helper class for tests that use NetworkStateHandler. Handles initialization,
 // shutdown, and adds default profiles and a wifi device (but no services).
 // NOTE: This is not intended to be used with NetworkHandler::Initialize()
@@ -57,10 +59,11 @@ class NetworkStateTestHelper {
                           const std::string& key,
                           const base::Value& value);
 
-  std::unique_ptr<NetworkState> CreateStandaloneNetworkState(
+  network_config::mojom::NetworkStatePropertiesPtr
+  CreateStandaloneNetworkProperties(
       const std::string& id,
-      const std::string& type,
-      const std::string& connection_state,
+      network_config::mojom::NetworkType type,
+      network_config::mojom::ConnectionStateType connection_state,
       int signal_strength);
 
   // Returns the path used for the shared and user profiles.

chromeos/network/prohibited_technologies_handler_unittest.cc

@@ -17,6 +17,7 @@
 #include "chromeos/network/managed_network_configuration_handler_impl.h"
 #include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_profile_handler.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "chromeos/network/onc/onc_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"

chromeos/services/network_config/cros_network_config.cc

@@ -117,9 +117,8 @@ mojom::DeviceStateType GetMojoDeviceStateType(
   return mojom::DeviceStateType::kUnavailable;
 }
 
-mojom::NetworkStatePropertiesPtr NetworkStateToMojo(
-    NetworkStateHandler* network_state_handler,
-    const NetworkState* network) {
+mojom::NetworkStatePropertiesPtr NetworkStateToMojo(const NetworkState* network,
+                                                    const DeviceState* device) {
   mojom::NetworkType type = ShillTypeToMojo(network->type());
   if (type == mojom::NetworkType::kAll) {
     NET_LOG(ERROR) << "Unexpected network type: " << network->type()
@@ -157,8 +156,6 @@ mojom::NetworkStatePropertiesPtr NetworkStateToMojo(
                                                 onc::kNetworkTechnologyTable);
       cellular->roaming = network->IndicateRoaming();
       cellular->signal_strength = network->signal_strength();
-      const DeviceState* device =
-          network_state_handler->GetDeviceState(network->device_path());
       if (device) {
         cellular->scanning = device->scanning();
         cellular->sim_absent = device->IsSimAbsent();
@@ -223,8 +220,8 @@ mojom::NetworkStatePropertiesPtr NetworkStateToMojo(
 }
 
 mojom::DeviceStatePropertiesPtr DeviceStateToMojo(
-    NetworkStateHandler* network_state_handler,
-    const DeviceState* device) {
+    const DeviceState* device,
+    mojom::DeviceStateType technology_state) {
   mojom::NetworkType type = ShillTypeToMojo(device->type());
   if (type == mojom::NetworkType::kAll) {
     NET_LOG(ERROR) << "Unexpected device type: " << device->type()
@@ -232,17 +229,10 @@ mojom::DeviceStatePropertiesPtr DeviceStateToMojo(
     return nullptr;
   }
 
-  mojom::DeviceStateType state =
-      GetMojoDeviceStateType(network_state_handler->GetTechnologyState(
-          NetworkTypePattern::Primitive(device->type())));
-  if (state == mojom::DeviceStateType::kUnavailable) {
-    NET_LOG(ERROR) << "Device state unavailable";
-    return nullptr;
-  }
   auto result = mojom::DeviceStateProperties::New();
   result->type = type;
   result->scanning = device->scanning();
-  result->state = state;
+  result->state = technology_state;
   result->managed_network_available =
       !device->available_managed_network_path().empty();
 
@@ -290,7 +280,8 @@ void CrosNetworkConfig::GetNetworkState(const std::string& guid,
     std::move(callback).Run(nullptr);
     return;
   }
-  std::move(callback).Run(NetworkStateToMojo(network_state_handler_, network));
+  std::move(callback).Run(NetworkStateToMojo(
+      network, network_state_handler_->GetDeviceState(network->device_path())));
 }
 
 void CrosNetworkConfig::GetNetworkStateList(
@@ -323,8 +314,9 @@ void CrosNetworkConfig::GetNetworkStateList(
   }
   std::vector<mojom::NetworkStatePropertiesPtr> result;
   for (const NetworkState* network : networks) {
-    mojom::NetworkStatePropertiesPtr mojo_network =
-        NetworkStateToMojo(network_state_handler_, network);
+    mojom::NetworkStatePropertiesPtr mojo_network = NetworkStateToMojo(
+        network,
+        network_state_handler_->GetDeviceState(network->device_path()));
     if (mojo_network)
       result.emplace_back(std::move(mojo_network));
   }
@@ -337,8 +329,15 @@ void CrosNetworkConfig::GetDeviceStateList(
   network_state_handler_->GetDeviceList(&devices);
   std::vector<mojom::DeviceStatePropertiesPtr> result;
   for (const DeviceState* device : devices) {
+    mojom::DeviceStateType technology_state =
+        GetMojoDeviceStateType(network_state_handler_->GetTechnologyState(
+            NetworkTypePattern::Primitive(device->type())));
+    if (technology_state == mojom::DeviceStateType::kUnavailable) {
+      NET_LOG(ERROR) << "Device state unavailable: " << device->name();
+      continue;
+    }
     mojom::DeviceStatePropertiesPtr mojo_device =
-        DeviceStateToMojo(network_state_handler_, device);
+        DeviceStateToMojo(device, technology_state);
     if (mojo_device)
       result.emplace_back(std::move(mojo_device));
   }
@@ -362,8 +361,9 @@ void CrosNetworkConfig::ActiveNetworksChanged(
     const std::vector<const NetworkState*>& active_networks) {
   std::vector<mojom::NetworkStatePropertiesPtr> result;
   for (const NetworkState* network : active_networks) {
-    mojom::NetworkStatePropertiesPtr mojo_network =
-        NetworkStateToMojo(network_state_handler_, network);
+    mojom::NetworkStatePropertiesPtr mojo_network = NetworkStateToMojo(
+        network,
+        network_state_handler_->GetDeviceState(network->device_path()));
     if (mojo_network)
       result.emplace_back(std::move(mojo_network));
   }

chromeos/services/network_config/network_config_service_unittest.cc

@@ -5,6 +5,7 @@
 #include "chromeos/services/network_config/network_config_service.h"
 
 #include "base/message_loop/message_loop.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_test_helper.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_test_observer.h"
 #include "testing/gtest/include/gtest/gtest.h"