Cleanup NetworkHealth and NetworkDiagnostics classes

The NetworkHealthService class owns both a NetworkHealth instance
and a NetworkDiagnostics{Impl} instance.

This CL cleans these classes up a bit to make them more consistant
and to make the relationships a bit more clear.

No functionatlity is changed.

TBR=tbegin@, khegde@

Bug: None
Change-Id: I6d07f0e6f7fc0b24ba51e4c999672ff0865c36ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2356268Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798219}

chrome/browser/chromeos/BUILD.gn

@@ -1856,8 +1856,8 @@ source_set("chromeos") {
     "net/network_diagnostics/has_secure_wifi_connection_routine.h",
     "net/network_diagnostics/lan_connectivity_routine.cc",
     "net/network_diagnostics/lan_connectivity_routine.h",
-    "net/network_diagnostics/network_diagnostics_impl.cc",
-    "net/network_diagnostics/network_diagnostics_impl.h",
+    "net/network_diagnostics/network_diagnostics.cc",
+    "net/network_diagnostics/network_diagnostics.h",
     "net/network_diagnostics/network_diagnostics_routine.cc",
     "net/network_diagnostics/network_diagnostics_routine.h",
     "net/network_diagnostics/signal_strength_routine.cc",
@@ -3304,8 +3304,8 @@ source_set("unit_tests") {
     "net/network_diagnostics/gateway_can_be_pinged_routine_unittest.cc",
     "net/network_diagnostics/has_secure_wifi_connection_routine_unittest.cc",
     "net/network_diagnostics/lan_connectivity_routine_unittest.cc",
-    "net/network_diagnostics/network_diagnostics_impl_unittest.cc",
     "net/network_diagnostics/network_diagnostics_routine_unittest.cc",
+    "net/network_diagnostics/network_diagnostics_unittest.cc",
     "net/network_diagnostics/signal_strength_routine_unittest.cc",
     "net/network_health/network_health_unittest.cc",
     "net/network_portal_detector_impl_unittest.cc",

chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl.cc → chrome/browser/chromeos/net/network_diagnostics/network_diagnostics.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl.h"
+#include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics.h"
 
 #include <memory>
 #include <utility>
@@ -22,7 +22,7 @@
 namespace chromeos {
 namespace network_diagnostics {
 
-NetworkDiagnosticsImpl::NetworkDiagnosticsImpl(
+NetworkDiagnostics::NetworkDiagnostics(
     chromeos::DebugDaemonClient* debug_daemon_client) {
   DCHECK(debug_daemon_client);
   if (debug_daemon_client) {
@@ -30,15 +30,15 @@ NetworkDiagnosticsImpl::NetworkDiagnosticsImpl(
   }
 }
 
-NetworkDiagnosticsImpl::~NetworkDiagnosticsImpl() {}
+NetworkDiagnostics::~NetworkDiagnostics() {}
 
-void NetworkDiagnosticsImpl::BindReceiver(
+void NetworkDiagnostics::BindReceiver(
     mojo::PendingReceiver<mojom::NetworkDiagnosticsRoutines> receiver) {
-  NET_LOG(EVENT) << "NetworkDiagnosticsImpl::BindReceiver()";
+  NET_LOG(EVENT) << "NetworkDiagnostics::BindReceiver()";
   receivers_.Add(this, std::move(receiver));
 }
 
-void NetworkDiagnosticsImpl::LanConnectivity(LanConnectivityCallback callback) {
+void NetworkDiagnostics::LanConnectivity(LanConnectivityCallback callback) {
   auto routine = std::make_unique<LanConnectivityRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
   // This ensures that the routine stays alive when it makes asynchronous mojo
@@ -50,7 +50,7 @@ void NetworkDiagnosticsImpl::LanConnectivity(LanConnectivityCallback callback) {
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::SignalStrength(SignalStrengthCallback callback) {
+void NetworkDiagnostics::SignalStrength(SignalStrengthCallback callback) {
   auto routine = std::make_unique<SignalStrengthRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
   // This ensures that the routine stays alive when it makes asynchronous mojo
@@ -64,7 +64,7 @@ void NetworkDiagnosticsImpl::SignalStrength(SignalStrengthCallback callback) {
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::GatewayCanBePinged(
+void NetworkDiagnostics::GatewayCanBePinged(
     GatewayCanBePingedCallback callback) {
   auto routine =
       std::make_unique<GatewayCanBePingedRoutine>(debug_daemon_client_);
@@ -80,7 +80,7 @@ void NetworkDiagnosticsImpl::GatewayCanBePinged(
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::HasSecureWiFiConnection(
+void NetworkDiagnostics::HasSecureWiFiConnection(
     HasSecureWiFiConnectionCallback callback) {
   auto routine = std::make_unique<HasSecureWiFiConnectionRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
@@ -96,7 +96,7 @@ void NetworkDiagnosticsImpl::HasSecureWiFiConnection(
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::DnsResolverPresent(
+void NetworkDiagnostics::DnsResolverPresent(
     DnsResolverPresentCallback callback) {
   auto routine = std::make_unique<DnsResolverPresentRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
@@ -111,7 +111,7 @@ void NetworkDiagnosticsImpl::DnsResolverPresent(
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::DnsLatency(DnsLatencyCallback callback) {
+void NetworkDiagnostics::DnsLatency(DnsLatencyCallback callback) {
   auto routine = std::make_unique<DnsLatencyRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
   // This ensures that the routine stays alive when it makes asynchronous mojo
@@ -125,7 +125,7 @@ void NetworkDiagnosticsImpl::DnsLatency(DnsLatencyCallback callback) {
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::DnsResolution(DnsResolutionCallback callback) {
+void NetworkDiagnostics::DnsResolution(DnsResolutionCallback callback) {
   auto routine = std::make_unique<DnsResolutionRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
   // This ensures that the routine stays alive when it makes asynchronous mojo
@@ -139,7 +139,7 @@ void NetworkDiagnosticsImpl::DnsResolution(DnsResolutionCallback callback) {
       std::move(routine), std::move(callback)));
 }
 
-void NetworkDiagnosticsImpl::CaptivePortal(CaptivePortalCallback callback) {
+void NetworkDiagnostics::CaptivePortal(CaptivePortalCallback callback) {
   auto routine = std::make_unique<CaptivePortalRoutine>();
   // RunRoutine() takes a lambda callback that takes ownership of the routine.
   // This ensures that the routine stays alive when it makes asynchronous mojo

chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl.h → chrome/browser/chromeos/net/network_diagnostics/network_diagnostics.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROMEOS_NET_NETWORK_DIAGNOSTICS_NETWORK_DIAGNOSTICS_IMPL_H_
-#define CHROME_BROWSER_CHROMEOS_NET_NETWORK_DIAGNOSTICS_NETWORK_DIAGNOSTICS_IMPL_H_
+#ifndef CHROME_BROWSER_CHROMEOS_NET_NETWORK_DIAGNOSTICS_NETWORK_DIAGNOSTICS_H_
+#define CHROME_BROWSER_CHROMEOS_NET_NETWORK_DIAGNOSTICS_NETWORK_DIAGNOSTICS_H_
 
 #include "base/memory/weak_ptr.h"
 #include "chromeos/services/network_health/public/mojom/network_diagnostics.mojom.h"
@@ -15,17 +15,14 @@ class DebugDaemonClient;
 
 namespace network_diagnostics {
 
-class NetworkDiagnosticsImpl : public mojom::NetworkDiagnosticsRoutines {
+class NetworkDiagnostics : public mojom::NetworkDiagnosticsRoutines {
  public:
-  explicit NetworkDiagnosticsImpl(
-      chromeos::DebugDaemonClient* debug_daemon_client);
-  NetworkDiagnosticsImpl(const NetworkDiagnosticsImpl&) = delete;
-  NetworkDiagnosticsImpl& operator=(const NetworkDiagnosticsImpl&) = delete;
-  ~NetworkDiagnosticsImpl() override;
-
-  // Binds this instance, an implementation of
-  // chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines, to
-  // multiple mojom::NetworkDiagnosticsRoutines receivers.
+  explicit NetworkDiagnostics(chromeos::DebugDaemonClient* debug_daemon_client);
+  NetworkDiagnostics(const NetworkDiagnostics&) = delete;
+  NetworkDiagnostics& operator=(const NetworkDiagnostics&) = delete;
+  ~NetworkDiagnostics() override;
+
+  // Binds this instance to |receiver|.
   void BindReceiver(
       mojo::PendingReceiver<mojom::NetworkDiagnosticsRoutines> receiver);
 
@@ -41,13 +38,13 @@ class NetworkDiagnosticsImpl : public mojom::NetworkDiagnosticsRoutines {
   void CaptivePortal(CaptivePortalCallback callback) override;
 
  private:
-  mojo::ReceiverSet<mojom::NetworkDiagnosticsRoutines> receivers_;
   // An unowned pointer to the DebugDaemonClient instance.
   chromeos::DebugDaemonClient* debug_daemon_client_;
-  base::WeakPtrFactory<NetworkDiagnosticsImpl> weak_factory_{this};
+  // Receivers for external requests (WebUI, Feedback, CrosHealthdClient).
+  mojo::ReceiverSet<mojom::NetworkDiagnosticsRoutines> receivers_;
 };
 
 }  // namespace network_diagnostics
 }  // namespace chromeos
 
-#endif  // CHROME_BROWSER_CHROMEOS_NET_NETWORK_DIAGNOSTICS_NETWORK_DIAGNOSTICS_IMPL_H_
+#endif  // CHROME_BROWSER_CHROMEOS_NET_NETWORK_DIAGNOSTICS_NETWORK_DIAGNOSTICS_H_

chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl_unittest.cc → chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl.h"
+#include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics.h"
 
 #include <memory>
 #include <utility>
@@ -83,15 +83,15 @@ const char kFakeValidICMPOutput[] = R"(
 
 }  // namespace
 
-class NetworkDiagnosticsImplTest : public ::testing::Test {
+class NetworkDiagnosticsTest : public ::testing::Test {
  public:
-  NetworkDiagnosticsImplTest() {
+  NetworkDiagnosticsTest() {
     // Set TestDebugDaemonClient
     test_debug_daemon_client_ = std::make_unique<TestDebugDaemonClient>();
-    network_diagnostics_impl_ = std::make_unique<NetworkDiagnosticsImpl>(
-        test_debug_daemon_client_.get());
-    network_diagnostics_impl_->BindReceiver(
-        network_diagnostics_.BindNewPipeAndPassReceiver());
+    network_diagnostics_ =
+        std::make_unique<NetworkDiagnostics>(test_debug_daemon_client_.get());
+    network_diagnostics_->BindReceiver(
+        network_diagnostics_remote_.BindNewPipeAndPassReceiver());
 
     // Initialize the ManagedNetworkConfigurationHandler and any associated
     // properties.
@@ -100,7 +100,7 @@ class NetworkDiagnosticsImplTest : public ::testing::Test {
     InitializeManagedNetworkConfigurationHandler();
     // Note that |cros_network_config_test_helper_| must be initialized before
     // any routine is initialized (routine initialization is done in
-    // NetworkDiagnosticsImpl). This is because |g_network_config_override| in
+    // NetworkDiagnostics). This is because |g_network_config_override| in
     // OverrideInProcessInstanceForTesting() must be set up before the routines
     // invoke BindToInProcessInstance(). See
     // chromeos/services/network_config/in_process_instance.cc for further
@@ -114,7 +114,7 @@ class NetworkDiagnosticsImplTest : public ::testing::Test {
     SetUpWiFi();
   }
 
-  ~NetworkDiagnosticsImplTest() override {
+  ~NetworkDiagnosticsTest() override {
     NetworkCertLoader::Shutdown();
     LoginState::Shutdown();
     managed_network_configuration_handler_.reset();
@@ -227,12 +227,12 @@ class NetworkDiagnosticsImplTest : public ::testing::Test {
     return cros_network_config_test_helper_.network_state_helper();
   }
 
-  base::WeakPtr<NetworkDiagnosticsImplTest> weak_ptr() {
+  base::WeakPtr<NetworkDiagnosticsTest> weak_ptr() {
     return weak_factory_.GetWeakPtr();
   }
 
-  NetworkDiagnosticsImpl* network_diagnostics_impl() {
-    return network_diagnostics_impl_.get();
+  NetworkDiagnostics* network_diagnostics() {
+    return network_diagnostics_.get();
   }
 
   TestDebugDaemonClient* test_debug_daemon_client() {
@@ -256,17 +256,17 @@ class NetworkDiagnosticsImplTest : public ::testing::Test {
   // InitializeManagedNetworkConfigurationHandler().
   network_config::CrosNetworkConfigTestHelper cros_network_config_test_helper_{
       false};
-  mojo::Remote<mojom::NetworkDiagnosticsRoutines> network_diagnostics_;
-  std::unique_ptr<NetworkDiagnosticsImpl> network_diagnostics_impl_;
-  base::WeakPtrFactory<NetworkDiagnosticsImplTest> weak_factory_{this};
+  mojo::Remote<mojom::NetworkDiagnosticsRoutines> network_diagnostics_remote_;
+  std::unique_ptr<NetworkDiagnostics> network_diagnostics_;
+  base::WeakPtrFactory<NetworkDiagnosticsTest> weak_factory_{this};
 };
 
-// Test whether NetworkDiagnosticsImpl can successfully invoke the
+// Test whether NetworkDiagnostics can successfully invoke the
 // LanConnectivity routine.
-TEST_F(NetworkDiagnosticsImplTest, LanConnectivityReachability) {
+TEST_F(NetworkDiagnosticsTest, LanConnectivityReachability) {
   mojom::RoutineVerdict received_verdict;
   base::RunLoop run_loop;
-  network_diagnostics_impl()->LanConnectivity(base::BindOnce(
+  network_diagnostics()->LanConnectivity(base::BindOnce(
       [](mojom::RoutineVerdict* received_verdict,
          base::OnceClosure quit_closure, mojom::RoutineVerdict actual_verdict) {
         *received_verdict = actual_verdict;
@@ -277,13 +277,13 @@ TEST_F(NetworkDiagnosticsImplTest, LanConnectivityReachability) {
   EXPECT_EQ(received_verdict, mojom::RoutineVerdict::kNoProblem);
 }
 
-// Test whether NetworkDiagnosticsImpl can successfully invoke the
+// Test whether NetworkDiagnostics can successfully invoke the
 // SignalStrength routine.
-TEST_F(NetworkDiagnosticsImplTest, SignalStrengthReachability) {
+TEST_F(NetworkDiagnosticsTest, SignalStrengthReachability) {
   mojom::RoutineVerdict received_verdict;
   std::vector<mojom::SignalStrengthProblem> received_problems;
   base::RunLoop run_loop;
-  network_diagnostics_impl()->SignalStrength(base::BindOnce(
+  network_diagnostics()->SignalStrength(base::BindOnce(
       [](mojom::RoutineVerdict* received_verdict,
          std::vector<mojom::SignalStrengthProblem>* received_problems,
          base::OnceClosure quit_closure, mojom::RoutineVerdict actual_verdict,
@@ -299,14 +299,14 @@ TEST_F(NetworkDiagnosticsImplTest, SignalStrengthReachability) {
   EXPECT_EQ(received_problems, no_problems);
 }
 
-// Test whether NetworkDiagnosticsImpl can successfully invoke the
+// Test whether NetworkDiagnostics can successfully invoke the
 // GatewayCanBePinged routine.
-TEST_F(NetworkDiagnosticsImplTest, GatewayCanBePingedReachability) {
+TEST_F(NetworkDiagnosticsTest, GatewayCanBePingedReachability) {
   test_debug_daemon_client()->set_icmp_output(kFakeValidICMPOutput);
   mojom::RoutineVerdict received_verdict;
   std::vector<mojom::GatewayCanBePingedProblem> received_problems;
   base::RunLoop run_loop;
-  network_diagnostics_impl()->GatewayCanBePinged(base::BindOnce(
+  network_diagnostics()->GatewayCanBePinged(base::BindOnce(
       [](mojom::RoutineVerdict* received_verdict,
          std::vector<mojom::GatewayCanBePingedProblem>* received_problems,
          base::OnceClosure quit_closure, mojom::RoutineVerdict actual_verdict,
@@ -322,13 +322,13 @@ TEST_F(NetworkDiagnosticsImplTest, GatewayCanBePingedReachability) {
   EXPECT_EQ(received_problems, no_problems);
 }
 
-// Test whether NetworkDiagnosticsImpl can successfully invoke the
+// Test whether NetworkDiagnostics can successfully invoke the
 // HasSecureWiFiConnection routine.
-TEST_F(NetworkDiagnosticsImplTest, HasSecureWiFiConnectionReachability) {
+TEST_F(NetworkDiagnosticsTest, HasSecureWiFiConnectionReachability) {
   mojom::RoutineVerdict received_verdict;
   std::vector<mojom::HasSecureWiFiConnectionProblem> received_problems;
   base::RunLoop run_loop;
-  network_diagnostics_impl()->HasSecureWiFiConnection(base::BindOnce(
+  network_diagnostics()->HasSecureWiFiConnection(base::BindOnce(
       [](mojom::RoutineVerdict* received_verdict,
          std::vector<mojom::HasSecureWiFiConnectionProblem>* received_problems,
          base::OnceClosure quit_closure, mojom::RoutineVerdict actual_verdict,
@@ -345,16 +345,16 @@ TEST_F(NetworkDiagnosticsImplTest, HasSecureWiFiConnectionReachability) {
   EXPECT_EQ(received_problems, no_problems);
 }
 
-// Test whether NetworkDiagnosticsImpl can successfully invoke the
+// Test whether NetworkDiagnostics can successfully invoke the
 // DnsResolverPresent routine.
-TEST_F(NetworkDiagnosticsImplTest, DnsResolverPresentReachability) {
+TEST_F(NetworkDiagnosticsTest, DnsResolverPresentReachability) {
   // Attach nameservers to the IPConfigs.
   SetUpNameServers(kWellFormedDnsServers);
 
   mojom::RoutineVerdict received_verdict;
   std::vector<mojom::DnsResolverPresentProblem> received_problems;
   base::RunLoop run_loop;
-  network_diagnostics_impl()->DnsResolverPresent(base::BindOnce(
+  network_diagnostics()->DnsResolverPresent(base::BindOnce(
       [](mojom::RoutineVerdict* received_verdict,
          std::vector<mojom::DnsResolverPresentProblem>* received_problems,
          base::OnceClosure quit_closure, mojom::RoutineVerdict actual_verdict,
@@ -370,17 +370,17 @@ TEST_F(NetworkDiagnosticsImplTest, DnsResolverPresentReachability) {
   EXPECT_EQ(received_problems, no_problems);
 }
 
-// TODO(khegde): Test whether NetworkDiagnosticsImpl can successfully invoke the
+// TODO(khegde): Test whether NetworkDiagnostics can successfully invoke the
 // DnsLatency routine. This would require a way to fake and inject the following
 // into the DnsLatency routine: base::TickClock, network::mojom::HostResolver,
 // and network::TestNetworkContext.
-// TEST_F(NetworkDiagnosticsImplTest, DnsLatencyReachability) {}
+// TEST_F(NetworkDiagnosticsTest, DnsLatencyReachability) {}
 
-// TODO(khegde): Test whether NetworkDiagnosticsImpl can successfully invoke the
+// TODO(khegde): Test whether NetworkDiagnostics can successfully invoke the
 // DnsResolution routine. This would require a way to fake and inject the
 // following into the DnsResolution routine: network::mojom::HostResolver and
 // network::TestNetworkContext.
-// TEST_F(NetworkDiagnosticsImplTest, DnsResolutionReachability) {}
+// TEST_F(NetworkDiagnosticsTest, DnsResolutionReachability) {}
 
 }  // namespace network_diagnostics
 }  // namespace chromeos

chrome/browser/chromeos/net/network_health/network_health.cc

@@ -88,6 +88,54 @@ NetworkHealth::NetworkHealth() {
 
 NetworkHealth::~NetworkHealth() = default;
 
+void NetworkHealth::BindReceiver(
+    mojo::PendingReceiver<mojom::NetworkHealthService> receiver) {
+  receivers_.Add(this, std::move(receiver));
+}
+
+const mojom::NetworkHealthStatePtr NetworkHealth::GetNetworkHealthState() {
+  NET_LOG(EVENT) << "Network Health State Requested";
+  return network_health_state_.Clone();
+}
+
+void NetworkHealth::GetNetworkList(GetNetworkListCallback callback) {
+  std::move(callback).Run(mojo::Clone(network_health_state_.networks));
+}
+
+void NetworkHealth::GetHealthSnapshot(GetHealthSnapshotCallback callback) {
+  std::move(callback).Run(network_health_state_.Clone());
+}
+
+void NetworkHealth::OnNetworkStateListChanged() {
+  RequestNetworkStateList();
+}
+
+void NetworkHealth::OnDeviceStateListChanged() {
+  RequestDeviceStateList();
+}
+
+void NetworkHealth::OnActiveNetworksChanged(
+    std::vector<network_config::mojom::NetworkStatePropertiesPtr>) {}
+
+void NetworkHealth::OnNetworkStateChanged(
+    network_config::mojom::NetworkStatePropertiesPtr) {}
+
+void NetworkHealth::OnVpnProvidersChanged() {}
+
+void NetworkHealth::OnNetworkCertificatesChanged() {}
+
+void NetworkHealth::OnNetworkStateListReceived(
+    std::vector<network_config::mojom::NetworkStatePropertiesPtr> props) {
+  network_properties_.swap(props);
+  CreateNetworkHealthState();
+}
+
+void NetworkHealth::OnDeviceStateListReceived(
+    std::vector<network_config::mojom::DeviceStatePropertiesPtr> props) {
+  device_properties_.swap(props);
+  CreateNetworkHealthState();
+}
+
 void NetworkHealth::CreateNetworkHealthState() {
   // If the device information has not been collected, the NetworkHealthState
   // cannot be created.
@@ -135,49 +183,11 @@ void NetworkHealth::CreateNetworkHealthState() {
   }
 }
 
-void NetworkHealth::BindReceiver(
-    mojo::PendingReceiver<mojom::NetworkHealthService> receiver) {
-  receivers_.Add(this, std::move(receiver));
-}
-
-const mojom::NetworkHealthStatePtr NetworkHealth::GetNetworkHealthState() {
-  NET_LOG(EVENT) << "Network Health State Requested";
-  return network_health_state_.Clone();
-}
-
 void NetworkHealth::RefreshNetworkHealthState() {
   RequestNetworkStateList();
   RequestDeviceStateList();
 }
 
-void NetworkHealth::GetNetworkList(GetNetworkListCallback callback) {
-  std::move(callback).Run(mojo::Clone(network_health_state_.networks));
-}
-
-void NetworkHealth::GetHealthSnapshot(GetHealthSnapshotCallback callback) {
-  std::move(callback).Run(network_health_state_.Clone());
-}
-
-void NetworkHealth::OnNetworkStateListReceived(
-    std::vector<network_config::mojom::NetworkStatePropertiesPtr> props) {
-  network_properties_.swap(props);
-  CreateNetworkHealthState();
-}
-
-void NetworkHealth::OnDeviceStateListReceived(
-    std::vector<network_config::mojom::DeviceStatePropertiesPtr> props) {
-  device_properties_.swap(props);
-  CreateNetworkHealthState();
-}
-
-void NetworkHealth::OnNetworkStateListChanged() {
-  RequestNetworkStateList();
-}
-
-void NetworkHealth::OnDeviceStateListChanged() {
-  RequestDeviceStateList();
-}
-
 void NetworkHealth::RequestNetworkStateList() {
   remote_cros_network_config_->GetNetworkStateList(
       network_config::mojom::NetworkFilter::New(

chrome/browser/chromeos/net/network_health/network_health.h

@@ -24,38 +24,36 @@ class NetworkHealth : public mojom::NetworkHealthService,
 
   ~NetworkHealth() override;
 
-  // Function to bind a NetworkHealthService |receiver|.
+  // Binds this instance to |receiver|.
   void BindReceiver(
       mojo::PendingReceiver<mojom::NetworkHealthService> receiver);
 
   // Returns the current NetworkHealthState.
   const mojom::NetworkHealthStatePtr GetNetworkHealthState();
 
-  // Handler for receiving the network state list.
-  void OnNetworkStateListReceived(
-      std::vector<network_config::mojom::NetworkStatePropertiesPtr>);
-
-  // Handler for receiving networking devices.
-  void OnDeviceStateListReceived(
-      std::vector<network_config::mojom::DeviceStatePropertiesPtr>);
-
-  // NetworkHealthService implementation
+  // NetworkHealthService
   void GetNetworkList(GetNetworkListCallback) override;
   void GetHealthSnapshot(GetHealthSnapshotCallback) override;
 
-  // CrosNetworkConfigObserver implementation
+  // CrosNetworkConfigObserver
   void OnNetworkStateListChanged() override;
   void OnDeviceStateListChanged() override;
-
-  // CrosNetworkConfigObserver unimplemented callbacks
   void OnActiveNetworksChanged(
-      std::vector<network_config::mojom::NetworkStatePropertiesPtr>) override {}
+      std::vector<network_config::mojom::NetworkStatePropertiesPtr>) override;
   void OnNetworkStateChanged(
-      network_config::mojom::NetworkStatePropertiesPtr) override {}
-  void OnVpnProvidersChanged() override {}
-  void OnNetworkCertificatesChanged() override {}
+      network_config::mojom::NetworkStatePropertiesPtr) override;
+  void OnVpnProvidersChanged() override;
+  void OnNetworkCertificatesChanged() override;
 
  private:
+  // Handler for receiving the network state list.
+  void OnNetworkStateListReceived(
+      std::vector<network_config::mojom::NetworkStatePropertiesPtr>);
+
+  // Handler for receiving networking devices.
+  void OnDeviceStateListReceived(
+      std::vector<network_config::mojom::DeviceStatePropertiesPtr>);
+
   // Creates the NetworkHealthState structure from cached network information.
   void CreateNetworkHealthState();
 
@@ -64,17 +62,16 @@ class NetworkHealth : public mojom::NetworkHealthService,
   void RequestNetworkStateList();
   void RequestDeviceStateList();
 
+  // Remote for sending requests to the CrosNetworkConfig service.
   mojo::Remote<network_config::mojom::CrosNetworkConfig>
       remote_cros_network_config_;
+  // Receiver for the CrosNetworkConfigObserver events.
   mojo::Receiver<network_config::mojom::CrosNetworkConfigObserver>
       cros_network_config_observer_receiver_{this};
-  mojo::Receiver<network_health::mojom::NetworkHealthService>
-      network_health_receiver_{this};
-
+  // Receivers for external requests (WebUI, Feedback, CrosHealthdClient).
   mojo::ReceiverSet<mojom::NetworkHealthService> receivers_;
 
   mojom::NetworkHealthState network_health_state_;
-
   std::vector<network_config::mojom::DeviceStatePropertiesPtr>
       device_properties_;
   std::vector<network_config::mojom::NetworkStatePropertiesPtr>

chrome/browser/chromeos/net/network_health/network_health_service.cc

@@ -5,14 +5,17 @@
 #include "chrome/browser/chromeos/net/network_health/network_health_service.h"
 
 #include "base/no_destructor.h"
+#include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics.h"
+#include "chrome/browser/chromeos/net/network_health/network_health.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 
 namespace chromeos {
 namespace network_health {
 
 NetworkHealthService::NetworkHealthService() {
+  network_health_ = std::make_unique<NetworkHealth>();
   network_diagnostics_ =
-      std::make_unique<network_diagnostics::NetworkDiagnosticsImpl>(
+      std::make_unique<network_diagnostics::NetworkDiagnostics>(
           chromeos::DBusThreadManager::Get()->GetDebugDaemonClient());
 }
 
@@ -33,7 +36,7 @@ NetworkHealthService::GetDiagnosticsRemoteAndBindReceiver() {
 
 void NetworkHealthService::BindHealthReceiver(
     mojo::PendingReceiver<mojom::NetworkHealthService> receiver) {
-  network_health_.BindReceiver(std::move(receiver));
+  network_health_->BindReceiver(std::move(receiver));
 }
 
 void NetworkHealthService::BindDiagnosticsReceiver(

chrome/browser/chromeos/net/network_health/network_health_service.h

@@ -5,13 +5,20 @@
 #ifndef CHROME_BROWSER_CHROMEOS_NET_NETWORK_HEALTH_NETWORK_HEALTH_SERVICE_H_
 #define CHROME_BROWSER_CHROMEOS_NET_NETWORK_HEALTH_NETWORK_HEALTH_SERVICE_H_
 
-#include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl.h"
-#include "chrome/browser/chromeos/net/network_health/network_health.h"
+#include "chromeos/services/network_health/public/mojom/network_diagnostics.mojom.h"
+#include "chromeos/services/network_health/public/mojom/network_health.mojom.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 
 namespace chromeos {
+
+namespace network_diagnostics {
+class NetworkDiagnostics;
+}
+
 namespace network_health {
 
+class NetworkHealth;
+
 class NetworkHealthService {
  public:
   static NetworkHealthService* GetInstance();
@@ -31,9 +38,8 @@ class NetworkHealthService {
           network_diagnostics::mojom::NetworkDiagnosticsRoutines> receiver);
 
  private:
-  NetworkHealth network_health_;
-  std::unique_ptr<network_diagnostics::NetworkDiagnosticsImpl>
-      network_diagnostics_;
+  std::unique_ptr<NetworkHealth> network_health_;
+  std::unique_ptr<network_diagnostics::NetworkDiagnostics> network_diagnostics_;
 };
 
 }  // namespace network_health