cros_healthd: Send NetworkDiagnosticsRoutines interface

In order for cros_healthd to invoke network diagnostics routines in the
browser, it needs a remote sent to it by Chrome.

BUG=chromium:956783
TEST=1) chromeos_unittests
--gtest_filter=CrosHealthdServiceConnectionTest.*
2) Applied the LanConnectivity related changes on top of this,
and executed the following:
"cros-health-tool diag --action=run_routine
--routine=lan_connectivity" (no problems - expected resulted)

Change-Id: Ifa15c19c7cb61305ff4ca1f76d8c6350d0ccd5ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2321004
Commit-Queue: Kartik Hegde <khegde@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797814}

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -912,6 +912,14 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
         ->GetHealthRemoteAndBindReceiver();
   }));
 
+  // Pass a callback to the CrosHealthd service connection that binds a pending
+  // remote to the interface.
+  cros_healthd->SetBindNetworkDiagnosticsRoutinesCallback(
+      base::BindRepeating([] {
+        return network_health::NetworkHealthService::GetInstance()
+            ->GetDiagnosticsRemoteAndBindReceiver();
+      }));
+
   // Initialize input methods.
   input_method::InputMethodManager* manager =
       input_method::InputMethodManager::Get();

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

@@ -23,6 +23,14 @@ NetworkHealthService::GetHealthRemoteAndBindReceiver() {
   return remote;
 }
 
+mojo::PendingRemote<network_diagnostics::mojom::NetworkDiagnosticsRoutines>
+NetworkHealthService::GetDiagnosticsRemoteAndBindReceiver() {
+  mojo::PendingRemote<network_diagnostics::mojom::NetworkDiagnosticsRoutines>
+      remote;
+  BindDiagnosticsReceiver(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

@@ -21,6 +21,8 @@ class NetworkHealthService {
 
   mojo::PendingRemote<mojom::NetworkHealthService>
   GetHealthRemoteAndBindReceiver();
+  mojo::PendingRemote<network_diagnostics::mojom::NetworkDiagnosticsRoutines>
+  GetDiagnosticsRemoteAndBindReceiver();
 
   void BindHealthReceiver(
       mojo::PendingReceiver<mojom::NetworkHealthService> receiver);

chromeos/dbus/cros_healthd/BUILD.gn

@@ -10,6 +10,7 @@ component("cros_healthd") {
   deps = [
     "//base",
     "//chromeos/services/cros_healthd/public/mojom",
+    "//chromeos/services/network_health/public/mojom",
     "//dbus",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/platform",

chromeos/dbus/cros_healthd/fake_cros_healthd_client.cc

@@ -97,5 +97,14 @@ void FakeCrosHealthdClient::RequestNetworkHealthForTesting(
   fake_service_.RequestNetworkHealthForTesting(std::move(callback));
 }
 
+void FakeCrosHealthdClient::RunLanConnectivityRoutineForTesting(
+    chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines::
+        LanConnectivityCallback callback) {
+  // Flush the receiver, so the NetworkDiagnosticsRoutines interface is
+  // registered before routines are called on it.
+  receiver_.FlushForTesting();
+  fake_service_.RunLanConnectivityRoutineForTesting(std::move(callback));
+}
+
 }  // namespace cros_healthd
 }  // namespace chromeos

chromeos/dbus/cros_healthd/fake_cros_healthd_client.h

@@ -73,6 +73,12 @@ class COMPONENT_EXPORT(CROS_HEALTHD) FakeCrosHealthdClient
       chromeos::network_health::mojom::NetworkHealthService::
           GetHealthSnapshotCallback callback);
 
+  // Calls the LanConnectivity routine using the NetworkDiagnosticsRoutines
+  // remote.
+  void RunLanConnectivityRoutineForTesting(
+      chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines::
+          LanConnectivityCallback);
+
  private:
   FakeCrosHealthdService fake_service_;
   mojo::Receiver<mojom::CrosHealthdServiceFactory> receiver_{&fake_service_};

chromeos/dbus/cros_healthd/fake_cros_healthd_service.cc

@@ -6,6 +6,8 @@
 
 #include <utility>
 
+#include "base/bind.h"
+
 namespace chromeos {
 namespace cros_healthd {
 
@@ -33,6 +35,13 @@ void FakeCrosHealthdService::SendNetworkHealthService(
   network_health_remote_.Bind(std::move(remote));
 }
 
+void FakeCrosHealthdService::SendNetworkDiagnosticsRoutines(
+    mojo::PendingRemote<
+        chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines>
+        network_diagnostics_routines) {
+  network_diagnostics_routines_.Bind(std::move(network_diagnostics_routines));
+}
+
 void FakeCrosHealthdService::GetAvailableRoutines(
     GetAvailableRoutinesCallback callback) {
   std::move(callback).Run(available_routines_);
@@ -206,5 +215,11 @@ void FakeCrosHealthdService::RequestNetworkHealthForTesting(
   network_health_remote_->GetHealthSnapshot(std::move(callback));
 }
 
+void FakeCrosHealthdService::RunLanConnectivityRoutineForTesting(
+    chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines::
+        LanConnectivityCallback callback) {
+  network_diagnostics_routines_->LanConnectivity(std::move(callback));
+}
+
 }  // namespace cros_healthd
 }  // namespace chromeos

chromeos/dbus/cros_healthd/fake_cros_healthd_service.h

@@ -12,8 +12,11 @@
 #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_diagnostics.mojom.h"
 #include "chromeos/services/network_health/public/mojom/network_health.mojom.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
 
 namespace chromeos {
@@ -40,6 +43,10 @@ class FakeCrosHealthdService final
   void SendNetworkHealthService(
       mojo::PendingRemote<chromeos::network_health::mojom::NetworkHealthService>
           remote) override;
+  void SendNetworkDiagnosticsRoutines(
+      mojo::PendingRemote<
+          chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines>
+          network_diagnostics_routines) override;
 
   // CrosHealthdDiagnosticsService overrides:
   void GetAvailableRoutines(GetAvailableRoutinesCallback callback) override;
@@ -137,6 +144,11 @@ class FakeCrosHealthdService final
       chromeos::network_health::mojom::NetworkHealthService::
           GetHealthSnapshotCallback callback);
 
+  // Calls the LanConnectivity routine on |network_diagnostics_routines_|.
+  void RunLanConnectivityRoutineForTesting(
+      chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines::
+          LanConnectivityCallback callback);
+
  private:
   // Used as the response to any GetAvailableRoutines IPCs received.
   std::vector<mojom::DiagnosticRoutineEnum> available_routines_;
@@ -169,6 +181,11 @@ class FakeCrosHealthdService final
   // Collection of registered power observers.
   mojo::RemoteSet<mojom::CrosHealthdPowerObserver> power_observers_;
 
+  // Allow |this| to call the methods on the NetworkDiagnosticsRoutines
+  // interface.
+  mojo::Remote<chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines>
+      network_diagnostics_routines_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeCrosHealthdService);
 };
 

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

@@ -5,6 +5,7 @@
 #include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/no_destructor.h"
@@ -114,11 +115,18 @@ class ServiceConnectionImpl : public ServiceConnection {
   void GetProbeService(mojom::CrosHealthdProbeServiceRequest service) override;
   void SetBindNetworkHealthServiceCallback(
       BindNetworkHealthServiceCallback callback) override;
+  void SetBindNetworkDiagnosticsRoutinesCallback(
+      BindNetworkDiagnosticsRoutinesCallback 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();
 
+  // Uses |bind_network_diagnostics_callback_| if set to bind a remote to the
+  // NetworkDiagnosticsRoutines interface and send the PendingRemote to
+  // cros_healthd.
+  void BindAndSendNetworkDiagnosticsRoutines();
+
   // Binds the factory interface |cros_healthd_service_factory_| to an
   // implementation in the cros_healthd daemon, if it is not already bound. The
   // binding is accomplished via D-Bus bootstrap.
@@ -154,6 +162,10 @@ class ServiceConnectionImpl : public ServiceConnection {
   // NetworkHealthService and returns it.
   BindNetworkHealthServiceCallback bind_network_health_callback_;
 
+  // Repeating callback that binds a mojo::PendingRemote to the
+  // NetworkDiagnosticsRoutines interface and returns it.
+  BindNetworkDiagnosticsRoutinesCallback bind_network_diagnostics_callback_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::WeakPtrFactory<ServiceConnectionImpl> weak_factory_{this};
@@ -369,6 +381,12 @@ void ServiceConnectionImpl::SetBindNetworkHealthServiceCallback(
   BindAndSendNetworkHealthService();
 }
 
+void ServiceConnectionImpl::SetBindNetworkDiagnosticsRoutinesCallback(
+    BindNetworkDiagnosticsRoutinesCallback callback) {
+  bind_network_diagnostics_callback_ = std::move(callback);
+  BindAndSendNetworkDiagnosticsRoutines();
+}
+
 void ServiceConnectionImpl::BindAndSendNetworkHealthService() {
   if (bind_network_health_callback_.is_null())
     return;
@@ -379,6 +397,17 @@ void ServiceConnectionImpl::BindAndSendNetworkHealthService() {
   cros_healthd_service_factory_->SendNetworkHealthService(std::move(remote));
 }
 
+void ServiceConnectionImpl::BindAndSendNetworkDiagnosticsRoutines() {
+  if (bind_network_diagnostics_callback_.is_null())
+    return;
+
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  EnsureCrosHealthdServiceFactoryIsBound();
+  auto remote = bind_network_diagnostics_callback_.Run();
+  cros_healthd_service_factory_->SendNetworkDiagnosticsRoutines(
+      std::move(remote));
+}
+
 void ServiceConnectionImpl::GetProbeService(
     mojom::CrosHealthdProbeServiceRequest service) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -458,6 +487,7 @@ void ServiceConnectionImpl::OnDisconnect() {
   // Chrome services to the CrosHealthd instance.
   if (cros_healthd_service_factory_.is_bound()) {
     BindAndSendNetworkHealthService();
+    BindAndSendNetworkDiagnosticsRoutines();
   }
 }
 

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

@@ -14,6 +14,7 @@
 #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_diagnostics.mojom.h"
 #include "chromeos/services/network_health/public/mojom/network_health.mojom.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 
@@ -30,6 +31,9 @@ class ServiceConnection {
   using BindNetworkHealthServiceCallback =
       base::RepeatingCallback<mojo::PendingRemote<
           chromeos::network_health::mojom::NetworkHealthService>()>;
+  using BindNetworkDiagnosticsRoutinesCallback =
+      base::RepeatingCallback<mojo::PendingRemote<
+          chromeos::network_diagnostics::mojom::NetworkDiagnosticsRoutines>()>;
 
   // Retrieve a list of available diagnostic routines. See
   // src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
@@ -207,12 +211,18 @@ class ServiceConnection {
   virtual void GetProbeService(
       mojom::CrosHealthdProbeServiceRequest service) = 0;
 
-  // Sets a callback to request bind a PendingRemote to the
+  // Sets a callback to request binding 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;
 
+  // Sets a callback to request binding a PendingRemote to the
+  // NetworkDiagnosticsRoutines interface. This callback is invoked once when it
+  // is set, and anytime the mojo connection to CrosHealthd is disconnected.
+  virtual void SetBindNetworkDiagnosticsRoutinesCallback(
+      BindNetworkDiagnosticsRoutinesCallback callback) = 0;
+
  protected:
   ServiceConnection() = default;
   virtual ~ServiceConnection() = default;

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

@@ -31,6 +31,8 @@ using ::testing::StrictMock;
 using ::testing::WithArgs;
 
 namespace chromeos {
+using network_diagnostics::mojom::NetworkDiagnosticsRoutines;
+using network_diagnostics::mojom::RoutineVerdict;
 namespace cros_healthd {
 namespace {
 
@@ -163,6 +165,58 @@ class MockNetworkHealthService : public NetworkHealthService {
   mojo::Receiver<NetworkHealthService> receiver_;
 };
 
+class MockNetworkDiagnosticsRoutines : public NetworkDiagnosticsRoutines {
+ public:
+  MockNetworkDiagnosticsRoutines() : receiver_{this} {}
+  MockNetworkDiagnosticsRoutines(const MockNetworkDiagnosticsRoutines&) =
+      delete;
+  MockNetworkDiagnosticsRoutines& operator=(
+      const MockNetworkDiagnosticsRoutines&) = delete;
+
+  MOCK_METHOD(void,
+              LanConnectivity,
+              (NetworkDiagnosticsRoutines::LanConnectivityCallback),
+              (override));
+  MOCK_METHOD(void,
+              SignalStrength,
+              (NetworkDiagnosticsRoutines::SignalStrengthCallback),
+              (override));
+  MOCK_METHOD(void,
+              GatewayCanBePinged,
+              (NetworkDiagnosticsRoutines::GatewayCanBePingedCallback),
+              (override));
+  MOCK_METHOD(void,
+              HasSecureWiFiConnection,
+              (NetworkDiagnosticsRoutines::HasSecureWiFiConnectionCallback),
+              (override));
+  MOCK_METHOD(void,
+              DnsResolverPresent,
+              (NetworkDiagnosticsRoutines::DnsResolverPresentCallback),
+              (override));
+  MOCK_METHOD(void,
+              DnsLatency,
+              (NetworkDiagnosticsRoutines::DnsLatencyCallback),
+              (override));
+  MOCK_METHOD(void,
+              DnsResolution,
+              (NetworkDiagnosticsRoutines::DnsResolutionCallback),
+              (override));
+  MOCK_METHOD(void,
+              CaptivePortal,
+              (NetworkDiagnosticsRoutines::CaptivePortalCallback),
+              (override));
+
+  mojo::PendingRemote<NetworkDiagnosticsRoutines> pending_remote() {
+    if (receiver_.is_bound()) {
+      receiver_.reset();
+    }
+    return receiver_.BindNewPipeAndPassRemote();
+  }
+
+ private:
+  mojo::Receiver<NetworkDiagnosticsRoutines> receiver_;
+};
+
 class CrosHealthdServiceConnectionTest : public testing::Test {
  public:
   CrosHealthdServiceConnectionTest() = default;
@@ -501,6 +555,34 @@ TEST_F(CrosHealthdServiceConnectionTest, SetBindNetworkHealthService) {
   run_loop.Run();
 }
 
+// Test that we can set the callback to get the NetworkDiagnosticsRoutines
+// remote and run the lan connectivity routine.
+TEST_F(CrosHealthdServiceConnectionTest, SetBindNetworkDiagnosticsRoutines) {
+  MockNetworkDiagnosticsRoutines network_diagnostics_routines;
+  ServiceConnection::GetInstance()->SetBindNetworkDiagnosticsRoutinesCallback(
+      base::BindLambdaForTesting([&network_diagnostics_routines] {
+        return network_diagnostics_routines.pending_remote();
+      }));
+
+  // Run the LanConnectivity routine so we know that
+  // |network_diagnostics_routines| is connected.
+  base::RunLoop run_loop;
+  RoutineVerdict routine_verdict = RoutineVerdict::kNoProblem;
+  EXPECT_CALL(network_diagnostics_routines, LanConnectivity(_))
+      .WillOnce(Invoke(
+          [&](NetworkDiagnosticsRoutines::LanConnectivityCallback callback) {
+            std::move(callback).Run(routine_verdict);
+          }));
+
+  FakeCrosHealthdClient::Get()->RunLanConnectivityRoutineForTesting(
+      base::BindLambdaForTesting([&](RoutineVerdict response) {
+        EXPECT_EQ(routine_verdict, 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

@@ -12,4 +12,9 @@ mojom("mojom") {
     "cros_healthd_events.mojom",
     "cros_healthd_probe.mojom",
   ]
+
+  public_deps = [
+    "//chromeos/services/network_health/public/mojom",
+    "//mojo/public/mojom/base",
+  ]
 }

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

@@ -13,11 +13,12 @@ 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_diagnostics.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 or to receive network services sent from
-// Chrome.
+// probe, diagnostics, or event services. Additionally, the browser can send
+// network-related implementations to the cros_healthd daemon.
 interface CrosHealthdServiceFactory {
   // Returns a bound interface to the diagnostics service.
   GetDiagnosticsService(CrosHealthdDiagnosticsService& service);
@@ -29,6 +30,12 @@ interface CrosHealthdServiceFactory {
   SendNetworkHealthService(
       pending_remote<
           chromeos.network_health.mojom.NetworkHealthService> remote);
+  // Sends a NetworkDiagnosticsRoutines pending remote to the cros_healthd
+  // daemon.
+  SendNetworkDiagnosticsRoutines(
+      pending_remote<
+          chromeos.network_diagnostics.mojom.NetworkDiagnosticsRoutines>
+              network_diagnostics_routines);
 };
 
 // Diagnostics interface exposed by the cros_healthd daemon. Consumed in Chrome