Cache CreateSysInfo() Result

Adds a new member variable to the `CastMetricsServiceClient`,
rather than reinitializing the `CastSysInfo` object each time
the `GetChannel()` method is called.

Bug: 145136478
Change-Id: I04cdc10ecbb7ff6761a24cedd4ab88e8098913c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2123508Reviewed-by: Alex Sakhartchouk <alexst@chromium.org>
Commit-Queue: Aaron Knobloch <aknobloch@google.com>
Cr-Commit-Position: refs/heads/master@{#759784}

chromecast/BUILD.gn

@@ -77,6 +77,7 @@ cast_test_group("cast_tests") {
     "//chromecast/crypto:cast_crypto_unittests",
     "//chromecast/device/bluetooth:cast_bluetooth_unittests",
     "//chromecast/media:cast_media_unittests",
+    "//chromecast/metrics:cast_metrics_unittest",
     "//chromecast/net:cast_net_unittests",
     "//chromecast/system/reboot:cast_reboot_unittests",
     "//content/test:content_unittests",

chromecast/metrics/BUILD.gn

@@ -2,24 +2,49 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//chromecast/build/tests/cast_test.gni")
 import("//chromecast/chromecast.gni")
 
 cast_source_set("metrics") {
-  sources = [
-    "cast_metrics_service_client.cc",
-    "cast_metrics_service_client.h",
+  public = [ "cast_metrics_service_client.h" ]
+
+  sources = [ "cast_metrics_service_client.cc" ]
+
+  public_deps = [
+    "//chromecast/base:cast_sys_info_util",
+    "//third_party/metrics_proto",
   ]
 
   deps = [
     "//base",
     "//base:i18n",
     "//chromecast/base",
-    "//chromecast/base:cast_sys_info_util",
     "//chromecast/base:cast_version",
     "//components/metrics",
     "//components/metrics:net",
     "//components/prefs",
     "//services/network/public/cpp",
-    "//third_party/metrics_proto",
   ]
 }
+
+test("cast_metrics_unittest") {
+  sources = [
+    "cast_metrics_service_client_unittest.cc",
+    "mock_cast_sys_info_util.cc",
+    "mock_cast_sys_info_util.h",
+  ]
+
+  deps = [
+    ":metrics",
+    "//base/test:run_all_unittests",
+    "//base/test:test_support",
+    "//chromecast/base:dummy_cast_sys_info",
+    "//services/network/public/cpp",
+    "//testing/gmock:gmock",
+    "//testing/gtest:gtest",
+  ]
+}
+
+cast_test_group("metrics_cast_test_group") {
+  tests = [ ":cast_metrics_unittest" ]
+}

chromecast/metrics/cast_metrics_service_client.cc

@@ -19,7 +19,6 @@
 #include "chromecast/base/path_utils.h"
 #include "chromecast/base/pref_names.h"
 #include "chromecast/base/version.h"
-#include "chromecast/public/cast_sys_info.h"
 #include "components/metrics/client_info.h"
 #include "components/metrics/enabled_state_provider.h"
 #include "components/metrics/metrics_log_uploader.h"
@@ -155,10 +154,9 @@ bool CastMetricsServiceClient::GetBrand(std::string* brand_code) {
 }
 
 ::metrics::SystemProfileProto::Channel CastMetricsServiceClient::GetChannel() {
-  std::unique_ptr<CastSysInfo> sys_info = CreateSysInfo();
 
 #if defined(OS_ANDROID) || defined(OS_FUCHSIA)
-  switch (sys_info->GetBuildType()) {
+  switch (cast_sys_info_->GetBuildType()) {
     case CastSysInfo::BUILD_ENG:
       return ::metrics::SystemProfileProto::CHANNEL_UNKNOWN;
     case CastSysInfo::BUILD_BETA:
@@ -173,7 +171,7 @@ bool CastMetricsServiceClient::GetBrand(std::string* brand_code) {
   // metrics caused by the virtual channel which could be temporary or
   // arbitrary.
   return GetReleaseChannelFromUpdateChannelName(
-      sys_info->GetSystemReleaseChannel());
+      cast_sys_info_->GetSystemReleaseChannel());
 #endif  // defined(OS_ANDROID) || defined(OS_FUCHSIA)
 }
 
@@ -270,7 +268,8 @@ CastMetricsServiceClient::CastMetricsServiceClient(
       pref_service_(pref_service),
       client_info_loaded_(false),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
-      url_loader_factory_(url_loader_factory) {}
+      url_loader_factory_(url_loader_factory),
+      cast_sys_info_(CreateSysInfo()) {}
 
 CastMetricsServiceClient::~CastMetricsServiceClient() = default;
 

chromecast/metrics/cast_metrics_service_client.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "build/build_config.h"
+#include "chromecast/public/cast_sys_info.h"
 #include "components/metrics/enabled_state_provider.h"
 #include "components/metrics/metrics_log_uploader.h"
 #include "components/metrics/metrics_service_client.h"
@@ -57,6 +58,9 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
   ~CastMetricsServiceClient() override;
 
+  CastMetricsServiceClient(const CastMetricsServiceClient&) = delete;
+  CastMetricsServiceClient& operator=(const CastMetricsServiceClient&) = delete;
+
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
   // Use |client_id| when starting MetricsService instead of generating a new
@@ -122,8 +126,7 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient,
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
   base::RepeatingCallback<void(base::OnceClosure)> collect_final_metrics_cb_;
   base::RepeatingCallback<void(base::OnceClosure)> external_events_cb_;
-
-  DISALLOW_COPY_AND_ASSIGN(CastMetricsServiceClient);
+  const std::unique_ptr<CastSysInfo> cast_sys_info_;
 };
 
 }  // namespace metrics

chromecast/metrics/cast_metrics_service_client_unittest.cc

+// Copyright 2014 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 "chromecast/metrics/cast_metrics_service_client.h"
+
+#include "base/test/task_environment.h"
+#include "chromecast/metrics/mock_cast_sys_info_util.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class CastMetricsServiceClientTest : public testing::Test {
+ public:
+  CastMetricsServiceClientTest() {}
+  CastMetricsServiceClientTest(const CastMetricsServiceClientTest&) = delete;
+  CastMetricsServiceClientTest& operator=(const CastMetricsServiceClientTest&) =
+      delete;
+
+ protected:
+  base::test::SingleThreadTaskEnvironment task_environment_;
+};
+
+/**
+ * Validates that class instatiation and calls to GetChannel() only
+ * result in a single method invocation of CreateSysInfo().
+ */
+TEST_F(CastMetricsServiceClientTest, CreateSysInfoSingleInvocation) {
+  EXPECT_EQ(chromecast::GetSysInfoCreatedCount(), 0);
+
+  chromecast::metrics::CastMetricsServiceClient metrics_client(nullptr, nullptr,
+                                                               nullptr);
+
+  metrics_client.GetChannel();
+  metrics_client.GetChannel();
+
+  // Despite muiltiple calls to GetChannel(),
+  // SysInfo should only be created a single time
+  EXPECT_EQ(chromecast::GetSysInfoCreatedCount(), 1);
+}
\ No newline at end of file

chromecast/metrics/mock_cast_sys_info_util.cc

+// Copyright 2014 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 "chromecast/metrics/mock_cast_sys_info_util.h"
+
+#include "chromecast/base/cast_sys_info_dummy.h"
+
+namespace chromecast {
+
+static int times_sys_info_created_ = 0;
+
+int GetSysInfoCreatedCount() {
+  return times_sys_info_created_;
+}
+
+std::unique_ptr<CastSysInfo> CreateSysInfo() {
+  times_sys_info_created_ += 1;
+  return std::make_unique<CastSysInfoDummy>();
+}
+
+}  // namespace chromecast
\ No newline at end of file

chromecast/metrics/mock_cast_sys_info_util.h

+// Copyright 2014 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 <memory>
+
+#include "chromecast/base/cast_sys_info_util.h"
+
+#ifndef CHROMECAST_METRICS_MOCK_CAST_SYS_INFO_UTIL_H_
+#define CHROMECAST_METRICS_MOCK_CAST_SYS_INFO_UTIL_H_
+
+namespace chromecast {
+
+int GetSysInfoCreatedCount();
+std::unique_ptr<CastSysInfo> CreateSysInfo();
+
+}  // namespace chromecast
+
+#endif  // CHROMECAST_METRICS_MOCK_CAST_SYS_INFO_UTIL_H_
\ No newline at end of file