Handle cros_healthd daemon absence

Wait for the Mojo service discovery for 5 seconds and abort if timeouts.
That shall prevent issues like b/165076656

Bug: b:151176984
Change-Id: I9cc03c6909b27aab98a209cb241ad961aaab52ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2373827
Commit-Queue: Daniil Lunev <dlunev@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Oleh Lamzin <lamzin@google.com>
Cr-Commit-Position: refs/heads/master@{#802130}

chrome/browser/metrics/cros_healthd_metrics_provider.cc

@@ -4,11 +4,15 @@
 
 #include "chrome/browser/metrics/cros_healthd_metrics_provider.h"
 
+#include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/logging.h"
 #include "base/strings/string_util.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
 #include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
@@ -17,41 +21,80 @@
 
 using metrics::SystemProfileProto;
 
+namespace {
+
+constexpr base::TimeDelta kServiceDiscoveryTimeout =
+    base::TimeDelta::FromSeconds(5);
+
+}  // namespace
+
 CrosHealthdMetricsProvider::CrosHealthdMetricsProvider() = default;
 CrosHealthdMetricsProvider::~CrosHealthdMetricsProvider() = default;
 
+base::TimeDelta CrosHealthdMetricsProvider::GetTimeout() {
+  return kServiceDiscoveryTimeout;
+}
+
 void CrosHealthdMetricsProvider::AsyncInit(base::OnceClosure done_callback) {
   const std::vector<chromeos::cros_healthd::mojom::ProbeCategoryEnum>
       categories_to_probe = {chromeos::cros_healthd::mojom::ProbeCategoryEnum::
                                  kNonRemovableBlockDevices};
+  DCHECK(init_callback_.is_null());
+  init_callback_ = std::move(done_callback);
+  initialized_ = false;
+
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&CrosHealthdMetricsProvider::OnProbeTimeout,
+                     weak_ptr_factory_.GetWeakPtr()),
+      GetTimeout());
   GetService()->ProbeTelemetryInfo(
       categories_to_probe,
       base::BindOnce(&CrosHealthdMetricsProvider::OnProbeDone,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(done_callback)));
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+bool CrosHealthdMetricsProvider::IsInitialized() {
+  return initialized_;
+}
+
+void CrosHealthdMetricsProvider::OnProbeTimeout() {
+  base::ScopedClosureRunner runner(std::move(init_callback_));
+  DVLOG(1) << "cros_healthd: endpoint is not found.";
+
+  // Invalidate OnProbeDone callback.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
+  devices_.clear();
+  initialized_ = false;
 }
 
 void CrosHealthdMetricsProvider::OnProbeDone(
-    base::OnceClosure done_callback,
     chromeos::cros_healthd::mojom::TelemetryInfoPtr ptr) {
-  base::ScopedClosureRunner runner(std::move(done_callback));
+  base::ScopedClosureRunner runner(std::move(init_callback_));
+
+  // Invalidate OnProbeTimeout callback.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   devices_.clear();
+  initialized_ = true;
 
   if (ptr.is_null()) {
-    LOG(ERROR) << "cros_healthd: Empty response";
+    DVLOG(1) << "cros_healthd: Empty response";
     return;
   }
 
   const auto& block_device_result = ptr->block_device_result;
   if (block_device_result.is_null()) {
-    LOG(ERROR) << "cros_healthd: No block device info";
+    DVLOG(1) << "cros_healthd: No block device info";
     return;
   }
 
   auto tag = block_device_result->which();
   if (tag == chromeos::cros_healthd::mojom::NonRemovableBlockDeviceResult::Tag::
                  ERROR) {
-    LOG(ERROR) << "cros_healthd: Error getting block device info: "
-               << block_device_result->get_error()->msg;
+    DVLOG(1) << "cros_healthd: Error getting block device info: "
+             << block_device_result->get_error()->msg;
     return;
   }
   DCHECK_EQ(tag, chromeos::cros_healthd::mojom::NonRemovableBlockDeviceResult::

chrome/browser/metrics/cros_healthd_metrics_provider.h

@@ -5,7 +5,10 @@
 #ifndef CHROME_BROWSER_METRICS_CROS_HEALTHD_METRICS_PROVIDER_H_
 #define CHROME_BROWSER_METRICS_CROS_HEALTHD_METRICS_PROVIDER_H_
 
+#include "base/callback_forward.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
 #include "components/metrics/metrics_provider.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -22,17 +25,25 @@ class CrosHealthdMetricsProvider : public metrics::MetricsProvider {
   void AsyncInit(base::OnceClosure done_callback) override;
   void ProvideSystemProfileMetrics(
       metrics::SystemProfileProto* system_profile_proto) override;
+  bool IsInitialized();
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(CrosHealthdMetricsProviderTest, EndToEndTimeout);
+
   chromeos::cros_healthd::mojom::CrosHealthdProbeService* GetService();
   void OnDisconnect();
-  void OnProbeDone(base::OnceClosure done_callback,
-                   chromeos::cros_healthd::mojom::TelemetryInfoPtr ptr);
+  void OnProbeDone(chromeos::cros_healthd::mojom::TelemetryInfoPtr ptr);
+  void OnProbeTimeout();
+
+  static base::TimeDelta GetTimeout();
 
   mojo::Remote<chromeos::cros_healthd::mojom::CrosHealthdProbeService> service_;
   std::vector<metrics::SystemProfileProto::Hardware::InternalStorageDevice>
       devices_;
 
+  base::OnceClosure init_callback_;
+  bool initialized_ = false;
+
   base::WeakPtrFactory<CrosHealthdMetricsProvider> weak_ptr_factory_{this};
 };
 

chrome/browser/metrics/cros_healthd_metrics_provider_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
+#include "base/time/time.h"
 #include "chromeos/dbus/cros_healthd/cros_healthd_client.h"
 #include "chromeos/dbus/cros_healthd/fake_cros_healthd_client.h"
 #include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
@@ -31,8 +32,8 @@ class CrosHealthdMetricsProviderTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
- private:
-  base::test::TaskEnvironment task_environment_;
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
 };
 
 TEST_F(CrosHealthdMetricsProviderTest, EndToEnd) {
@@ -105,3 +106,20 @@ TEST_F(CrosHealthdMetricsProviderTest, EndToEnd) {
   }));
   run_loop.Run();
 }
+
+TEST_F(CrosHealthdMetricsProviderTest, EndToEndTimeout) {
+  chromeos::cros_healthd::FakeCrosHealthdClient::Get()->SetCallbackDelay(
+      CrosHealthdMetricsProvider::GetTimeout() +
+      base::TimeDelta::FromSeconds(5));
+
+  base::RunLoop run_loop;
+  CrosHealthdMetricsProvider provider;
+  provider.AsyncInit(base::BindOnce(
+      [](base::OnceClosure callback) { std::move(callback).Run(); },
+      run_loop.QuitClosure()));
+
+  // FastForward by timeout period.
+  task_environment_.FastForwardBy(CrosHealthdMetricsProvider::GetTimeout());
+  run_loop.Run();
+  ASSERT_FALSE(provider.IsInitialized());
+}