[metrics] Use Delegate for storage limits in Cast

Adds a function to CastMetricsServiceDelegate that Cast embedders can
override to set platform-specific limits for local metrics storage.

Also cleans up changes from https://crrev.com/c/2505741 to use
`constexpr` and multiples of 1024 for default values for consistency.

Bug: b/172356230
Test: metrics_unittests
Test: cast_metrics_unittest
Change-Id: Ib920e4438baf56dff64bcca3f9bcabca77fd354e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518833Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Commit-Queue: Akira Baruah <akirabaruah@google.com>
Cr-Commit-Position: refs/heads/master@{#824678}

chromecast/metrics/BUILD.gn

@@ -39,6 +39,7 @@ test("cast_metrics_unittest") {
     "//base/test:run_all_unittests",
     "//base/test:test_support",
     "//chromecast/base:dummy_cast_sys_info",
+    "//components/metrics",
     "//services/network/public/cpp",
     "//testing/gmock:gmock",
     "//testing/gtest:gtest",

chromecast/metrics/cast_metrics_service_client.cc

@@ -154,7 +154,6 @@ bool CastMetricsServiceClient::GetBrand(std::string* brand_code) {
 }
 
 ::metrics::SystemProfileProto::Channel CastMetricsServiceClient::GetChannel() {
-
 #if defined(OS_ANDROID) || defined(OS_FUCHSIA)
   switch (cast_sys_info_->GetBuildType()) {
     case CastSysInfo::BUILD_ENG:
@@ -240,6 +239,14 @@ base::TimeDelta CastMetricsServiceClient::GetStandardUploadInterval() {
   return base::TimeDelta::FromMinutes(kStandardUploadIntervalMinutes);
 }
 
+::metrics::MetricsLogStore::StorageLimits
+CastMetricsServiceClient::GetStorageLimits() const {
+  auto limits = ::metrics::MetricsServiceClient::GetStorageLimits();
+  if (delegate_)
+    delegate_->ApplyMetricsStorageLimits(&limits);
+  return limits;
+}
+
 bool CastMetricsServiceClient::IsConsentGiven() const {
   return pref_service_->GetBoolean(prefs::kOptInStats);
 }

chromecast/metrics/cast_metrics_service_client.h

@@ -15,6 +15,7 @@
 #include "build/build_config.h"
 #include "chromecast/public/cast_sys_info.h"
 #include "components/metrics/enabled_state_provider.h"
+#include "components/metrics/metrics_log_store.h"
 #include "components/metrics/metrics_log_uploader.h"
 #include "components/metrics/metrics_service_client.h"
 
@@ -42,9 +43,14 @@ class CastMetricsServiceDelegate {
  public:
   // Invoked when the metrics client ID changes.
   virtual void SetMetricsClientId(const std::string& client_id) = 0;
+
   // Allows registration of extra metrics providers.
   virtual void RegisterMetricsProviders(::metrics::MetricsService* service) = 0;
 
+  // Adds Cast embedder-specific storage limits to |limits| object.
+  virtual void ApplyMetricsStorageLimits(
+      ::metrics::MetricsLogStore::StorageLimits* limits) {}
+
  protected:
   virtual ~CastMetricsServiceDelegate() = default;
 };
@@ -95,6 +101,7 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient,
       const ::metrics::MetricsLogUploader::UploadCallback& on_upload_complete)
       override;
   base::TimeDelta GetStandardUploadInterval() override;
+  ::metrics::MetricsLogStore::StorageLimits GetStorageLimits() const override;
 
   // ::metrics::EnabledStateProvider:
   bool IsConsentGiven() const override;

chromecast/metrics/cast_metrics_service_client_unittest.cc

@@ -6,9 +6,31 @@
 
 #include "base/test/task_environment.h"
 #include "chromecast/metrics/mock_cast_sys_info_util.h"
+#include "components/metrics/metrics_log_store.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace chromecast {
+namespace metrics {
+namespace {
+
+class FakeCastMetricsServiceDelegate : public CastMetricsServiceDelegate {
+ public:
+  void SetStorageLimits(::metrics::MetricsLogStore::StorageLimits limits) {
+    storage_limits_ = std::move(limits);
+  }
+
+ private:
+  void SetMetricsClientId(const std::string& client_id) override {}
+  void RegisterMetricsProviders(::metrics::MetricsService* service) override {}
+  void ApplyMetricsStorageLimits(
+      ::metrics::MetricsLogStore::StorageLimits* limits) override {
+    *limits = std::move(storage_limits_);
+  }
+
+  ::metrics::MetricsLogStore::StorageLimits storage_limits_;
+};
+
 class CastMetricsServiceClientTest : public testing::Test {
  public:
   CastMetricsServiceClientTest() {}
@@ -27,13 +49,42 @@ class CastMetricsServiceClientTest : public testing::Test {
 TEST_F(CastMetricsServiceClientTest, CreateSysInfoSingleInvocation) {
   EXPECT_EQ(chromecast::GetSysInfoCreatedCount(), 0);
 
-  chromecast::metrics::CastMetricsServiceClient metrics_client(nullptr, nullptr,
-                                                               nullptr);
+  CastMetricsServiceClient client(nullptr, nullptr, nullptr);
 
-  metrics_client.GetChannel();
-  metrics_client.GetChannel();
+  client.GetChannel();
+  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
+
+TEST_F(CastMetricsServiceClientTest, UsesDelegateToGetStorageLimits) {
+  FakeCastMetricsServiceDelegate delegate;
+  CastMetricsServiceClient client(&delegate, nullptr, nullptr);
+
+  ::metrics::MetricsLogStore::StorageLimits expected_limits = {
+      /*min_initial_log_queue_count=*/10,
+      /*min_initial_log_queue_size=*/2000,
+      /*min_ongoing_log_queue_count=*/30,
+      /*min_ongoing_log_queue_size=*/4000,
+      /*max_ongoing_log_size=*/5000,
+  };
+  delegate.SetStorageLimits(expected_limits);
+  ::metrics::MetricsLogStore::StorageLimits actual_limits =
+      client.GetStorageLimits();
+  EXPECT_EQ(actual_limits.min_initial_log_queue_count,
+            expected_limits.min_initial_log_queue_count);
+  EXPECT_EQ(actual_limits.min_initial_log_queue_size,
+            expected_limits.min_initial_log_queue_size);
+  EXPECT_EQ(actual_limits.min_ongoing_log_queue_count,
+            expected_limits.min_ongoing_log_queue_count);
+  EXPECT_EQ(actual_limits.min_ongoing_log_queue_size,
+            expected_limits.min_ongoing_log_queue_size);
+  EXPECT_EQ(actual_limits.max_ongoing_log_size,
+            expected_limits.max_ongoing_log_size);
+}
+
+}  // namespace
+}  // namespace metrics
+}  // namespace chromecast

components/metrics/metrics_service_client.cc

@@ -25,12 +25,12 @@ constexpr int kMetricsUploadIntervalSecMinimum = 20;
 // then we will discard the log, and not try to retransmit it. We also don't
 // persist the log to the prefs for transmission during the next chrome session
 // if this limit is exceeded.
-const size_t kMaxOngoingLogSize = 100 * 1024;
+constexpr size_t kMaxOngoingLogSize = 100 * 1024;  // 100 KiB
 
 // The number of bytes of logs to save of each type (initial/ongoing). This
 // ensures that a reasonable amount of history will be stored even if there is a
 // long series of very small logs.
-constexpr size_t kMinLogQueueSize = 300 * 1000;  // ~300kB
+constexpr size_t kMinLogQueueSize = 300 * 1024;  // 300 KiB
 
 // The minimum number of "initial" logs to save, and hope to send during a
 // future Chrome session. Initial logs contain crash stats, and are pretty