[Telemetry SWX] Introduce probe service converters

Introduce probe service converters.

It's a good practice to use mojo::StructTraits or it's preferred to
use mojo:TypeConverter even when it's deprecated, but in our case we
may need to change converter in the runtime. For example, depending
on some policy value we may/may not provide some fields:
https://chromium.googlesource.com/chromium/src/+/refs/heads/master/docs/security/mojo.md#use-structtraits

Bug: b:158658869
Change-Id: Ibfe1ef41d71e4dcb90581354135b1a9d44d28a9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252038
Commit-Queue: Oleh Lamzin <lamzin@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Mahmoud Gawad <mgawad@google.com>
Cr-Commit-Position: refs/heads/master@{#781783}

chromeos/components/BUILD.gn

@@ -30,6 +30,10 @@ test("chromeos_components_unittests") {
     "//chromeos/components/trial_group:unit_tests",
     "//mojo/core/embedder",
   ]
+
+  if (!is_official_build) {
+    deps += [ "//chromeos/components/telemetry_extension_ui:unit_tests" ]
+  }
 }
 
 group("closure_compile") {

chromeos/components/telemetry_extension_ui/BUILD.gn

@@ -10,6 +10,8 @@ static_library("telemetry_extension_ui") {
   sources = [
     "probe_service.cc",
     "probe_service.h",
+    "probe_service_converters.cc",
+    "probe_service_converters.h",
     "telemetry_extension_ui.cc",
     "telemetry_extension_ui.h",
     "url_constants.cc",
@@ -27,3 +29,18 @@ static_library("telemetry_extension_ui") {
     "//ui/webui",
   ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [ "probe_service_converters_unittest.cc" ]
+  deps = [
+    ":telemetry_extension_ui",
+    "//base",
+    "//base/test:test_support",
+    "//chromeos/components/telemetry_extension_ui/mojom",
+    "//chromeos/dbus/cros_healthd",
+    "//chromeos/services/cros_healthd/public/mojom",
+    "//testing/gmock",
+    "//testing/gtest",
+  ]
+}

chromeos/components/telemetry_extension_ui/probe_service.cc

@@ -7,105 +7,12 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/notreached.h"
+#include "chromeos/components/telemetry_extension_ui/probe_service_converters.h"
 #include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
 
 namespace chromeos {
 
-namespace {
-
-std::vector<cros_healthd::mojom::ProbeCategoryEnum> ConvertCategories(
-    const std::vector<health::mojom::ProbeCategoryEnum>& original_categories) {
-  std::vector<cros_healthd::mojom::ProbeCategoryEnum> categories;
-  for (auto category : original_categories) {
-    switch (category) {
-      case health::mojom::ProbeCategoryEnum::kBattery:
-        categories.push_back(cros_healthd::mojom::ProbeCategoryEnum::kBattery);
-        break;
-    }
-  }
-  return categories;
-}
-
-health::mojom::ErrorType ConvertErrorType(cros_healthd::mojom::ErrorType type) {
-  switch (type) {
-    case cros_healthd::mojom::ErrorType::kFileReadError:
-      return health::mojom::ErrorType::kFileReadError;
-    case cros_healthd::mojom::ErrorType::kParseError:
-      return health::mojom::ErrorType::kParseError;
-    case cros_healthd::mojom::ErrorType::kSystemUtilityError:
-      return health::mojom::ErrorType::kSystemUtilityError;
-  }
-  NOTREACHED();
-}
-
-health::mojom::ProbeErrorPtr ConvertProbeError(
-    cros_healthd::mojom::ProbeErrorPtr ptr) {
-  if (!ptr) {
-    return nullptr;
-  }
-  return health::mojom::ProbeError::New(ConvertErrorType(ptr->type),
-                                        std::move(ptr->msg));
-}
-
-health::mojom::UInt64ValuePtr ConvertUInt64Value(
-    cros_healthd::mojom::UInt64ValuePtr ptr) {
-  if (!ptr) {
-    return nullptr;
-  }
-  return health::mojom::UInt64Value::New(ptr->value);
-}
-
-health::mojom::BatteryInfoPtr ConvertBatteryInfo(
-    cros_healthd::mojom::BatteryInfoPtr ptr) {
-  if (!ptr) {
-    return nullptr;
-  }
-  auto info = health::mojom::BatteryInfo::New();
-
-  info->cycle_count = ptr->cycle_count;
-  info->voltage_now = ptr->voltage_now;
-  info->vendor = std::move(ptr->vendor);
-  info->serial_number = std::move(ptr->serial_number);
-  info->charge_full_design = ptr->charge_full_design;
-  info->charge_full = ptr->charge_full;
-  info->voltage_min_design = ptr->voltage_min_design;
-  info->model_name = std::move(ptr->model_name);
-  info->charge_now = ptr->charge_now;
-  info->current_now = ptr->current_now;
-  info->technology = std::move(ptr->technology);
-  info->status = std::move(ptr->status);
-
-  if (ptr->manufacture_date.has_value()) {
-    info->manufacture_date = std::move(ptr->manufacture_date.value());
-  }
-
-  info->temperature = ConvertUInt64Value(std::move(ptr->temperature));
-
-  return info;
-}
-
-health::mojom::BatteryResultPtr ConvertBatteryResult(
-    cros_healthd::mojom::BatteryResultPtr ptr) {
-  if (!ptr) {
-    return nullptr;
-  }
-
-  auto battery_result = health::mojom::BatteryResult::New();
-
-  if (ptr->is_error()) {
-    battery_result->set_error(ConvertProbeError(std::move(ptr->get_error())));
-  } else if (ptr->is_battery_info()) {
-    battery_result->set_battery_info(
-        ConvertBatteryInfo(std::move(ptr->get_battery_info())));
-  }
-
-  return battery_result;
-}
-
-}  // namespace
-
 ProbeService::ProbeService(
     mojo::PendingReceiver<health::mojom::ProbeService> receiver)
     : receiver_(this, std::move(receiver)) {}
@@ -116,14 +23,12 @@ void ProbeService::ProbeTelemetryInfo(
     const std::vector<health::mojom::ProbeCategoryEnum>& categories,
     ProbeTelemetryInfoCallback callback) {
   GetService()->ProbeTelemetryInfo(
-      ConvertCategories(categories),
+      probe_service_converters::Convert(categories),
       base::BindOnce(
           [](health::mojom::ProbeService::ProbeTelemetryInfoCallback callback,
              cros_healthd::mojom::TelemetryInfoPtr ptr) {
-            auto info = health::mojom::TelemetryInfo::New();
-            info->battery_result =
-                ConvertBatteryResult(std::move(ptr->battery_result));
-            std::move(callback).Run(std::move(info));
+            std::move(callback).Run(
+                probe_service_converters::Convert(std::move(ptr)));
           },
           std::move(callback)));
 }

chromeos/components/telemetry_extension_ui/probe_service_converters.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/telemetry_extension_ui/probe_service_converters.h"
+
+#include <utility>
+
+#include "base/notreached.h"
+#include "chromeos/components/telemetry_extension_ui/mojom/probe_service.mojom.h"
+#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
+
+namespace chromeos {
+namespace probe_service_converters {
+
+cros_healthd::mojom::ProbeCategoryEnum Convert(
+    health::mojom::ProbeCategoryEnum input) {
+  switch (input) {
+    case health::mojom::ProbeCategoryEnum::kBattery:
+      return cros_healthd::mojom::ProbeCategoryEnum::kBattery;
+  }
+  NOTREACHED();
+}
+
+std::vector<cros_healthd::mojom::ProbeCategoryEnum> Convert(
+    const std::vector<health::mojom::ProbeCategoryEnum>& input) {
+  std::vector<cros_healthd::mojom::ProbeCategoryEnum> output;
+  for (auto category : input) {
+    output.push_back(Convert(category));
+  }
+  return output;
+}
+
+health::mojom::ErrorType Convert(cros_healthd::mojom::ErrorType input) {
+  switch (input) {
+    case cros_healthd::mojom::ErrorType::kFileReadError:
+      return health::mojom::ErrorType::kFileReadError;
+    case cros_healthd::mojom::ErrorType::kParseError:
+      return health::mojom::ErrorType::kParseError;
+    case cros_healthd::mojom::ErrorType::kSystemUtilityError:
+      return health::mojom::ErrorType::kSystemUtilityError;
+  }
+  NOTREACHED();
+}
+
+health::mojom::ProbeErrorPtr Convert(cros_healthd::mojom::ProbeErrorPtr input) {
+  if (!input) {
+    return nullptr;
+  }
+  return health::mojom::ProbeError::New(Convert(input->type),
+                                        std::move(input->msg));
+}
+
+health::mojom::UInt64ValuePtr Convert(
+    cros_healthd::mojom::UInt64ValuePtr input) {
+  if (!input) {
+    return nullptr;
+  }
+  return health::mojom::UInt64Value::New(input->value);
+}
+
+health::mojom::BatteryInfoPtr Convert(
+    cros_healthd::mojom::BatteryInfoPtr input) {
+  if (!input) {
+    return nullptr;
+  }
+  auto output = health::mojom::BatteryInfo::New();
+
+  output->cycle_count = input->cycle_count;
+  output->voltage_now = input->voltage_now;
+  output->vendor = std::move(input->vendor);
+  output->serial_number = std::move(input->serial_number);
+  output->charge_full_design = input->charge_full_design;
+  output->charge_full = input->charge_full;
+  output->voltage_min_design = input->voltage_min_design;
+  output->model_name = std::move(input->model_name);
+  output->charge_now = input->charge_now;
+  output->current_now = input->current_now;
+  output->technology = std::move(input->technology);
+  output->status = std::move(input->status);
+
+  if (input->manufacture_date.has_value()) {
+    output->manufacture_date = std::move(input->manufacture_date.value());
+  }
+
+  output->temperature = Convert(std::move(input->temperature));
+
+  return output;
+}
+
+health::mojom::BatteryResultPtr Convert(
+    cros_healthd::mojom::BatteryResultPtr input) {
+  if (!input) {
+    return nullptr;
+  }
+
+  auto output = health::mojom::BatteryResult::New();
+
+  if (input->is_error()) {
+    output->set_error(Convert(std::move(input->get_error())));
+  } else if (input->is_battery_info()) {
+    output->set_battery_info(Convert(std::move(input->get_battery_info())));
+  }
+
+  return output;
+}
+
+health::mojom::TelemetryInfoPtr Convert(
+    cros_healthd::mojom::TelemetryInfoPtr input) {
+  if (!input) {
+    return nullptr;
+  }
+
+  return health::mojom::TelemetryInfo::New(
+      Convert(std::move(input->battery_result)));
+}
+
+}  // namespace probe_service_converters
+}  // namespace chromeos

chromeos/components/telemetry_extension_ui/probe_service_converters.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_TELEMETRY_EXTENSION_UI_PROBE_SERVICE_CONVERTERS_H_
+#define CHROMEOS_COMPONENTS_TELEMETRY_EXTENSION_UI_PROBE_SERVICE_CONVERTERS_H_
+
+#if defined(OFFICIAL_BUILD)
+#error Probe service convertors should only be included in unofficial builds.
+#endif
+
+#include <vector>
+
+#include "chromeos/components/telemetry_extension_ui/mojom/probe_service.mojom-forward.h"
+#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom-forward.h"
+
+namespace chromeos {
+namespace probe_service_converters {
+
+// This file contains helper functions used by ProbeService to convert its
+// types to/from cros_healthd ProbeService types.
+
+cros_healthd::mojom::ProbeCategoryEnum Convert(
+    health::mojom::ProbeCategoryEnum input);
+
+std::vector<cros_healthd::mojom::ProbeCategoryEnum> Convert(
+    const std::vector<health::mojom::ProbeCategoryEnum>& input);
+
+health::mojom::ErrorType Convert(cros_healthd::mojom::ErrorType type);
+
+health::mojom::ProbeErrorPtr Convert(cros_healthd::mojom::ProbeErrorPtr input);
+
+health::mojom::UInt64ValuePtr Convert(
+    cros_healthd::mojom::UInt64ValuePtr input);
+
+health::mojom::BatteryInfoPtr Convert(
+    cros_healthd::mojom::BatteryInfoPtr input);
+
+health::mojom::BatteryResultPtr Convert(
+    cros_healthd::mojom::BatteryResultPtr input);
+
+health::mojom::TelemetryInfoPtr Convert(
+    cros_healthd::mojom::TelemetryInfoPtr input);
+
+}  // namespace probe_service_converters
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_TELEMETRY_EXTENSION_UI_PROBE_SERVICE_CONVERTERS_H_

chromeos/components/telemetry_extension_ui/probe_service_converters_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/telemetry_extension_ui/probe_service_converters.h"
+
+#include <cstdint>
+#include <vector>
+
+#include "chromeos/components/telemetry_extension_ui/mojom/probe_service.mojom.h"
+#include "chromeos/services/cros_healthd/public/mojom/cros_healthd_probe.mojom.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::ElementsAre;
+
+namespace chromeos {
+namespace probe_service_converters {
+
+TEST(ProbeServiceConvertors, ProbeCategoryEnum) {
+  EXPECT_EQ(Convert(health::mojom::ProbeCategoryEnum::kBattery),
+            cros_healthd::mojom::ProbeCategoryEnum::kBattery);
+}
+
+TEST(ProbeServiceConvertors, ProbeCategoryEnumVector) {
+  const std::vector<health::mojom::ProbeCategoryEnum> kInput{
+      health::mojom::ProbeCategoryEnum::kBattery};
+  EXPECT_THAT(Convert(kInput),
+              ElementsAre(cros_healthd::mojom::ProbeCategoryEnum::kBattery));
+}
+
+TEST(ProbeServiceConvertors, ErrorType) {
+  EXPECT_EQ(Convert(cros_healthd::mojom::ErrorType::kFileReadError),
+            health::mojom::ErrorType::kFileReadError);
+
+  EXPECT_EQ(Convert(cros_healthd::mojom::ErrorType::kParseError),
+            health::mojom::ErrorType::kParseError);
+
+  EXPECT_EQ(Convert(cros_healthd::mojom::ErrorType::kSystemUtilityError),
+            health::mojom::ErrorType::kSystemUtilityError);
+}
+
+TEST(ProbeServiceConvertors, ProbeErrorPtrNull) {
+  EXPECT_TRUE(Convert(cros_healthd::mojom::ProbeErrorPtr()).is_null());
+}
+
+TEST(ProbeServiceConvertors, ProbeErrorPtr) {
+  constexpr char kMsg[] = "file not found";
+  EXPECT_EQ(Convert(cros_healthd::mojom::ProbeError::New(
+                cros_healthd::mojom::ErrorType::kFileReadError, kMsg)),
+            health::mojom::ProbeError::New(
+                health::mojom::ErrorType::kFileReadError, kMsg));
+}
+
+TEST(ProbeServiceConvertors, UInt64ValuePtrNull) {
+  EXPECT_TRUE(Convert(cros_healthd::mojom::UInt64ValuePtr()).is_null());
+}
+
+TEST(ProbeServiceConvertors, UInt64ValuePtr) {
+  constexpr uint64_t kValue = 100500;
+  EXPECT_EQ(Convert(cros_healthd::mojom::UInt64Value::New(kValue)),
+            health::mojom::UInt64Value::New(kValue));
+}
+
+TEST(ProbeServiceConvertors, BatteryInfoPtrNull) {
+  EXPECT_TRUE(Convert(cros_healthd::mojom::BatteryInfoPtr()).is_null());
+}
+
+TEST(ProbeServiceConvertors, BatteryInfoPtr) {
+  constexpr int64_t kCycleCount = 512;
+  constexpr double kVoltageNow = 10.2;
+  constexpr char kVendor[] = "Google";
+  constexpr char kSerialNumber[] = "ABCDEF123456";
+  constexpr double kChargeFullDesign = 1000.3;
+  constexpr double kChargeFull = 999.0;
+  constexpr double kVoltageMinDesign = 41.1;
+  constexpr char kModelName[] = "Google Battery";
+  constexpr double kChargeNow = 20.1;
+  constexpr double kCurrentNow = 15.2;
+  constexpr char kTechnology[] = "FastCharge";
+  constexpr char kStatus[] = "Charging";
+  constexpr char kManufactureDate[] = "2018-10-01";
+  constexpr uint64_t kTemperature = 3097;
+
+  // Here we don't use cros_healthd::mojom::BatteryInfo::New because BatteryInfo
+  // may contain some fields that we don't use yet.
+  auto battery_info = cros_healthd::mojom::BatteryInfo::New();
+  battery_info->cycle_count = kCycleCount;
+  battery_info->voltage_now = kVoltageNow;
+  battery_info->vendor = kVendor;
+  battery_info->serial_number = kSerialNumber;
+  battery_info->charge_full_design = kChargeFullDesign;
+  battery_info->charge_full = kChargeFull;
+  battery_info->voltage_min_design = kVoltageMinDesign;
+  battery_info->model_name = kModelName;
+  battery_info->charge_now = kChargeNow;
+  battery_info->current_now = kCurrentNow;
+  battery_info->technology = kTechnology;
+  battery_info->status = kStatus;
+  battery_info->manufacture_date = kManufactureDate;
+  battery_info->temperature =
+      cros_healthd::mojom::UInt64Value::New(kTemperature);
+
+  // Here we intentionaly use health::mojom::BatteryInfo::New to don't
+  // forget to test new fields.
+  EXPECT_EQ(Convert(std::move(battery_info)),
+            health::mojom::BatteryInfo::New(
+                kCycleCount, kVoltageNow, kVendor, kSerialNumber,
+                kChargeFullDesign, kChargeFull, kVoltageMinDesign, kModelName,
+                kChargeNow, kCurrentNow, kTechnology, kStatus, kManufactureDate,
+                health::mojom::UInt64Value::New(kTemperature)));
+}
+
+TEST(ProbeServiceConvertors, BatteryResultPtrNull) {
+  EXPECT_TRUE(Convert(cros_healthd::mojom::BatteryResultPtr()).is_null());
+}
+
+TEST(ProbeServiceConvertors, BatteryResultPtrInfo) {
+  const health::mojom::BatteryResultPtr ptr =
+      Convert(cros_healthd::mojom::BatteryResult::NewBatteryInfo(nullptr));
+  ASSERT_TRUE(ptr);
+  EXPECT_TRUE(ptr->is_battery_info());
+}
+
+TEST(ProbeServiceConvertors, BatteryResultPtrError) {
+  const health::mojom::BatteryResultPtr ptr =
+      Convert(cros_healthd::mojom::BatteryResult::NewError(nullptr));
+  ASSERT_TRUE(ptr);
+  EXPECT_TRUE(ptr->is_error());
+}
+
+TEST(ProbeServiceConvertors, TelemetryInfoPtrHasBatteryResult) {
+  constexpr int64_t kCycleCount = 1;
+
+  auto battery_info_input = cros_healthd::mojom::BatteryInfo::New();
+  battery_info_input->cycle_count = kCycleCount;
+
+  auto telemetry_info_input = cros_healthd::mojom::TelemetryInfo::New();
+
+  telemetry_info_input->battery_result =
+      cros_healthd::mojom::BatteryResult::NewBatteryInfo(
+          std::move(battery_info_input));
+
+  const health::mojom::TelemetryInfoPtr telemetry_info_output =
+      Convert(std::move(telemetry_info_input));
+  ASSERT_TRUE(telemetry_info_output);
+  ASSERT_TRUE(telemetry_info_output->battery_result);
+  ASSERT_TRUE(telemetry_info_output->battery_result->is_battery_info());
+  EXPECT_EQ(
+      telemetry_info_output->battery_result->get_battery_info()->cycle_count,
+      kCycleCount);
+}
+
+TEST(ProbeServiceConvertors, TelemetryInfoPtrWithNullFields) {
+  const health::mojom::TelemetryInfoPtr telemetry_info_output =
+      Convert(cros_healthd::mojom::TelemetryInfo::New());
+  ASSERT_TRUE(telemetry_info_output);
+  EXPECT_FALSE(telemetry_info_output->battery_result);
+}
+
+TEST(ProbeServiceConvertors, TelemetryInfoPtrNull) {
+  EXPECT_TRUE(Convert(cros_healthd::mojom::TelemetryInfoPtr()).is_null());
+}
+
+}  // namespace probe_service_converters
+}  // namespace chromeos