network-health: send mojo remote to cros_healthd on startup

When the Chrome OS browser starts up, pass a bound remote to the
platform service cros_healthd. This allows the platform daemon to access
the network telemetry information.

Bug: chromium:1057739
Change-Id: I6084107e8a5364ed0698f521c5e47dbd26265b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2323921
Commit-Queue: Trent Begin <tbegin@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Paul Moy <pmoy@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795935}

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -900,14 +900,17 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
   network_pref_state_observer_ = std::make_unique<NetworkPrefStateObserver>();
 
   // Initialize the NetworkHealth aggregator.
-  network_health::NetworkHealthService* network_health_service =
-      network_health::NetworkHealthService::GetInstance();
-  DCHECK(network_health_service);
-
-  // Create the service connection to cros_healthd.
-  cros_healthd::ServiceConnection* cros_healthd =
-      cros_healthd::ServiceConnection::GetInstance();
-  DCHECK(cros_healthd);
+  network_health::NetworkHealthService::GetInstance();
+
+  // Create the service connection to CrosHealthd platform service instance.
+  auto* cros_healthd = cros_healthd::ServiceConnection::GetInstance();
+
+  // Pass a callback to the CrosHealthd service connection that binds a pending
+  // remote to service.
+  cros_healthd->SetBindNetworkHealthServiceCallback(base::BindRepeating([] {
+    return network_health::NetworkHealthService::GetInstance()
+        ->GetHealthRemoteAndBindReceiver();
+  }));
 
   // Initialize input methods.
   input_method::InputMethodManager* manager =

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

@@ -16,6 +16,13 @@ NetworkHealthService::NetworkHealthService() {
           chromeos::DBusThreadManager::Get()->GetDebugDaemonClient());
 }
 
+mojo::PendingRemote<mojom::NetworkHealthService>
+NetworkHealthService::GetHealthRemoteAndBindReceiver() {
+  mojo::PendingRemote<mojom::NetworkHealthService> remote;
+  BindHealthReceiver(remote.InitWithNewPipeAndPassReceiver());
+  return remote;
+}
+
 void NetworkHealthService::BindHealthReceiver(
     mojo::PendingReceiver<mojom::NetworkHealthService> receiver) {
   network_health_.BindReceiver(std::move(receiver));

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

@@ -7,6 +7,7 @@
 
 #include "chrome/browser/chromeos/net/network_diagnostics/network_diagnostics_impl.h"
 #include "chrome/browser/chromeos/net/network_health/network_health.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 
 namespace chromeos {
 namespace network_health {
@@ -18,6 +19,9 @@ class NetworkHealthService {
   NetworkHealthService();
   ~NetworkHealthService() = delete;
 
+  mojo::PendingRemote<mojom::NetworkHealthService>
+  GetHealthRemoteAndBindReceiver();
+
   void BindHealthReceiver(
       mojo::PendingReceiver<mojom::NetworkHealthService> receiver);
   void BindDiagnosticsReceiver(

chromeos/dbus/cros_healthd/DEPS

 include_rules = [
   "+chromeos/services/cros_healthd/public/mojom",
+  "+chromeos/services/network_health/public/mojom",
   "+mojo/public",
-]
\ No newline at end of file
+]

chromeos/dbus/cros_healthd/fake_cros_healthd_client.cc

@@ -88,5 +88,14 @@ void FakeCrosHealthdClient::EmitLidClosedEventForTesting() {
   fake_service_.EmitLidClosedEventForTesting();
 }
 
+void FakeCrosHealthdClient::RequestNetworkHealthForTesting(
+    chromeos::network_health::mojom::NetworkHealthService::
+        GetHealthSnapshotCallback callback) {
+  // Flush the receiver, so any requests to send the NetworkHealthService remote
+  // are processed before the request is emitted.
+  receiver_.FlushForTesting();
+  fake_service_.RequestNetworkHealthForTesting(std::move(callback));
+}
+
 }  // namespace cros_healthd
 }  // namespace chromeos

chromeos/dbus/cros_healthd/fake_cros_healthd_client.h

@@ -68,6 +68,11 @@ class COMPONENT_EXPORT(CROS_HEALTHD) FakeCrosHealthdClient
   // Calls the lid event OnLidClosed on all registered lid observers.
   void EmitLidClosedEventForTesting();
 
+  // Requests the network health state using the NetworkHealthService remote.
+  void RequestNetworkHealthForTesting(
+      chromeos::network_health::mojom::NetworkHealthService::
+          GetHealthSnapshotCallback callback);
+
  private:
   FakeCrosHealthdService fake_service_;
   mojo::Receiver<mojom::CrosHealthdServiceFactory> receiver_{&fake_service_};

chromeos/dbus/cros_healthd/fake_cros_healthd_service.cc

@@ -27,6 +27,12 @@ void FakeCrosHealthdService::GetEventService(
   event_receiver_set_.Add(this, std::move(service));
 }
 
+void FakeCrosHealthdService::SendNetworkHealthService(
+    mojo::PendingRemote<chromeos::network_health::mojom::NetworkHealthService>
+        remote) {
+  network_health_remote_.Bind(std::move(remote));
+}
+
 void FakeCrosHealthdService::GetAvailableRoutines(
     GetAvailableRoutinesCallback callback) {
   std::move(callback).Run(available_routines_);
@@ -194,5 +200,11 @@ void FakeCrosHealthdService::EmitLidClosedEventForTesting() {
     observer->OnLidClosed();
 }
 
+void FakeCrosHealthdService::RequestNetworkHealthForTesting(
+    chromeos::network_health::mojom::NetworkHealthService::
+        GetHealthSnapshotCallback callback) {
+  network_health_remote_->GetHealthSnapshot(std::move(callback));
+}
+
 }  // namespace cros_healthd
 }  // namespace chromeos

chromeos/dbus/cros_healthd/fake_cros_healthd_service.h

@@ -12,6 +12,7 @@
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_diagnostics.mojom.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_events.mojom.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
+#include "chromeos/services/network_health/public/mojom/network_health.mojom.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
 
@@ -36,6 +37,9 @@ class FakeCrosHealthdService final
   void GetDiagnosticsService(
       mojom::CrosHealthdDiagnosticsServiceRequest service) override;
   void GetEventService(mojom::CrosHealthdEventServiceRequest service) override;
+  void SendNetworkHealthService(
+      mojo::PendingRemote<chromeos::network_health::mojom::NetworkHealthService>
+          remote) override;
 
   // CrosHealthdDiagnosticsService overrides:
   void GetAvailableRoutines(GetAvailableRoutinesCallback callback) override;
@@ -128,6 +132,11 @@ class FakeCrosHealthdService final
   // Calls the lid event OnLidClosed for all registered lid observers.
   void EmitLidClosedEventForTesting();
 
+  // Requests the network health state using the network_health_remote_.
+  void RequestNetworkHealthForTesting(
+      chromeos::network_health::mojom::NetworkHealthService::
+          GetHealthSnapshotCallback callback);
+
  private:
   // Used as the response to any GetAvailableRoutines IPCs received.
   std::vector<mojom::DiagnosticRoutineEnum> available_routines_;
@@ -149,6 +158,10 @@ class FakeCrosHealthdService final
       diagnostics_receiver_set_;
   mojo::ReceiverSet<mojom::CrosHealthdEventService> event_receiver_set_;
 
+  // NetworkHealthService remote.
+  mojo::Remote<chromeos::network_health::mojom::NetworkHealthService>
+      network_health_remote_;
+
   // Collection of registered Bluetooth observers.
   mojo::RemoteSet<mojom::CrosHealthdBluetoothObserver> bluetooth_observers_;
   // Collection of registered lid observers.

chromeos/services/cros_healthd/DEPS

 include_rules = [
   "+chromeos/dbus",
+  "+chromeos/services/network_health/public/mojom",
   "+mojo/public",
 ]

chromeos/services/cros_healthd/public/cpp/service_connection.cc

@@ -112,6 +112,12 @@ class ServiceConnectionImpl : public ServiceConnection {
   void GetDiagnosticsService(
       mojom::CrosHealthdDiagnosticsServiceRequest service) override;
   void GetProbeService(mojom::CrosHealthdProbeServiceRequest service) override;
+  void SetBindNetworkHealthServiceCallback(
+      BindNetworkHealthServiceCallback callback) override;
+
+  // Uses |bind_network_health_callback_| if set to bind a remote to the
+  // NetworkHealthService and send the PendingRemote to the CrosHealthdService.
+  void BindAndSendNetworkHealthService();
 
   // Binds the factory interface |cros_healthd_service_factory_| to an
   // implementation in the cros_healthd daemon, if it is not already bound. The
@@ -144,6 +150,10 @@ class ServiceConnectionImpl : public ServiceConnection {
       cros_healthd_diagnostics_service_;
   mojo::Remote<mojom::CrosHealthdEventService> cros_healthd_event_service_;
 
+  // Repeating callback that binds a mojo::PendingRemote to the
+  // NetworkHealthService and returns it.
+  BindNetworkHealthServiceCallback bind_network_health_callback_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::WeakPtrFactory<ServiceConnectionImpl> weak_factory_{this};
@@ -353,6 +363,22 @@ void ServiceConnectionImpl::GetDiagnosticsService(
   cros_healthd_service_factory_->GetDiagnosticsService(std::move(service));
 }
 
+void ServiceConnectionImpl::SetBindNetworkHealthServiceCallback(
+    BindNetworkHealthServiceCallback callback) {
+  bind_network_health_callback_ = std::move(callback);
+  BindAndSendNetworkHealthService();
+}
+
+void ServiceConnectionImpl::BindAndSendNetworkHealthService() {
+  if (bind_network_health_callback_.is_null())
+    return;
+
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  EnsureCrosHealthdServiceFactoryIsBound();
+  auto remote = bind_network_health_callback_.Run();
+  cros_healthd_service_factory_->SendNetworkHealthService(std::move(remote));
+}
+
 void ServiceConnectionImpl::GetProbeService(
     mojom::CrosHealthdProbeServiceRequest service) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -428,6 +454,11 @@ void ServiceConnectionImpl::OnDisconnect() {
   cros_healthd_event_service_.reset();
 
   EnsureCrosHealthdServiceFactoryIsBound();
+  // If the cros_healthd_service_factory_ was able to be rebound, resend the
+  // Chrome services to the CrosHealthd instance.
+  if (cros_healthd_service_factory_.is_bound()) {
+    BindAndSendNetworkHealthService();
+  }
 }
 
 void ServiceConnectionImpl::OnBootstrapMojoConnectionResponse(

chromeos/services/cros_healthd/public/cpp/service_connection.h

@@ -10,9 +10,11 @@
 #include <cstdint>
 #include <string>
 
+#include "base/callback_forward.h"
 #include "base/optional.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_events.mojom.h"
+#include "chromeos/services/network_health/public/mojom/network_health.mojom.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 
 namespace chromeos {
@@ -25,6 +27,10 @@ class ServiceConnection {
  public:
   static ServiceConnection* GetInstance();
 
+  using BindNetworkHealthServiceCallback =
+      base::RepeatingCallback<mojo::PendingRemote<
+          chromeos::network_health::mojom::NetworkHealthService>()>;
+
   // Retrieve a list of available diagnostic routines. See
   // src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
   // details.
@@ -201,6 +207,12 @@ class ServiceConnection {
   virtual void GetProbeService(
       mojom::CrosHealthdProbeServiceRequest service) = 0;
 
+  // Sets a callback to request bind a PendingRemote to the
+  // NetworkHealthService. This callback is invoked once when it is set, and
+  // anytime the mojo connection to CrosHealthd is disconnected.
+  virtual void SetBindNetworkHealthServiceCallback(
+      BindNetworkHealthServiceCallback callback) = 0;
+
  protected:
   ServiceConnection() = default;
   virtual ~ServiceConnection() = default;

chromeos/services/cros_healthd/public/cpp/service_connection_unittest.cc

@@ -24,6 +24,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::chromeos::network_health::mojom::NetworkHealthService;
 using ::testing::_;
 using ::testing::Invoke;
 using ::testing::StrictMock;
@@ -139,6 +140,29 @@ class MockCrosHealthdPowerObserver : public mojom::CrosHealthdPowerObserver {
   mojo::Receiver<mojom::CrosHealthdPowerObserver> receiver_;
 };
 
+class MockNetworkHealthService : public NetworkHealthService {
+ public:
+  MockNetworkHealthService() : receiver_{this} {}
+  MockNetworkHealthService(const MockNetworkHealthService&) = delete;
+  MockNetworkHealthService& operator=(const MockNetworkHealthService&) = delete;
+
+  MOCK_METHOD(void,
+              GetNetworkList,
+              (NetworkHealthService::GetNetworkListCallback),
+              (override));
+  MOCK_METHOD(void,
+              GetHealthSnapshot,
+              (NetworkHealthService::GetHealthSnapshotCallback),
+              (override));
+
+  mojo::PendingRemote<NetworkHealthService> pending_remote() {
+    return receiver_.BindNewPipeAndPassRemote();
+  }
+
+ private:
+  mojo::Receiver<NetworkHealthService> receiver_;
+};
+
 class CrosHealthdServiceConnectionTest : public testing::Test {
  public:
   CrosHealthdServiceConnectionTest() = default;
@@ -451,6 +475,32 @@ TEST_F(CrosHealthdServiceConnectionTest, AddPowerObserver) {
   run_loop.Run();
 }
 
+// Test that we can set the callback to get the NetworkHealthService remote and
+// request the network health state snapshot.
+TEST_F(CrosHealthdServiceConnectionTest, SetBindNetworkHealthService) {
+  MockNetworkHealthService service;
+  ServiceConnection::GetInstance()->SetBindNetworkHealthServiceCallback(
+      base::BindLambdaForTesting(
+          [&service] { return service.pending_remote(); }));
+
+  base::RunLoop run_loop;
+  auto canned_response = network_health::mojom::NetworkHealthState::New();
+  EXPECT_CALL(service, GetHealthSnapshot(_))
+      .WillOnce(
+          Invoke([&](NetworkHealthService::GetHealthSnapshotCallback callback) {
+            std::move(callback).Run(canned_response.Clone());
+          }));
+
+  FakeCrosHealthdClient::Get()->RequestNetworkHealthForTesting(
+      base::BindLambdaForTesting(
+          [&](network_health::mojom::NetworkHealthStatePtr response) {
+            EXPECT_EQ(canned_response, response);
+            run_loop.Quit();
+          }));
+
+  run_loop.Run();
+}
+
 // Test that we can probe telemetry info.
 TEST_F(CrosHealthdServiceConnectionTest, ProbeTelemetryInfo) {
   auto response = mojom::TelemetryInfo::New();

chromeos/services/cros_healthd/public/mojom/BUILD.gn

@@ -5,6 +5,7 @@
 import("//mojo/public/tools/bindings/mojom.gni")
 
 mojom("mojom") {
+  deps = [ "//chromeos/services/network_health/public/mojom" ]
   sources = [
     "cros_healthd.mojom",
     "cros_healthd_diagnostics.mojom",

chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom

@@ -13,9 +13,11 @@ module chromeos.cros_healthd.mojom;
 import "chromeos/services/cros_healthd/public/mojom/cros_healthd_diagnostics.mojom";
 import "chromeos/services/cros_healthd/public/mojom/cros_healthd_events.mojom";
 import "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom";
+import "chromeos/services/network_health/public/mojom/network_health.mojom";
 
 // Factory interface which allows remote ends to request implementations of the
-// probe or diagnostics services.
+// probe or diagnostics services or to receive network services sent from
+// Chrome.
 interface CrosHealthdServiceFactory {
   // Returns a bound interface to the diagnostics service.
   GetDiagnosticsService(CrosHealthdDiagnosticsService& service);
@@ -23,6 +25,10 @@ interface CrosHealthdServiceFactory {
   GetEventService(CrosHealthdEventService& service);
   // Returns a bound interface to the probe service.
   GetProbeService(CrosHealthdProbeService& service);
+  // Sends a bound interface to the Network Health service in Chrome.
+  SendNetworkHealthService(
+      pending_remote<
+          chromeos.network_health.mojom.NetworkHealthService> remote);
 };
 
 // Diagnostics interface exposed by the cros_healthd daemon. Consumed in Chrome