[PUMA] Plumb profile country ID to PumaService This change provides a mechanism for the PumaService to access the profile-specific country ID. This is a prerequisite for populating private metrics reports like RcCoarseSystemProfile. To decouple the profile-agnostic PumaService from platform-specific profile management, this CL introduces a `PumaService::PrivateMetricsProfileClient` interface, which use the `RegionalCapabilitiesService` to retrieve the country. Bug: b:452034784, 461922774 Change-Id: Ic45ee57e1378ad6685f19c5c2b46fce80d08ad06 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7168657 Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Commit-Queue: Victor Tan <victortan@chromium.org> Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org> Reviewed-by: Matt Dembski <dembski@google.com> Reviewed-by: James Lee <ljjlee@google.com> Cr-Commit-Position: refs/heads/main@{#1552945} NOKEYCHECK=True GitOrigin-RevId: e9bec6cb8b78e5ce96bdadaf2a7fef0c22fd0fd3
diff --git a/BUILD.gn b/BUILD.gn index a0c56e2..a568d28 100644 --- a/BUILD.gn +++ b/BUILD.gn
@@ -267,8 +267,10 @@ public_deps = [ ":metrics_component", + "//components/country_codes", "//components/metrics:client_info", "//components/metrics:metrics_pref_names", + "//components/regional_capabilities:country_id", "//components/variations", "//third_party/metrics_proto", "//third_party/zlib/google:compression_utils",
diff --git a/DEPS b/DEPS index 1d99105..b762de9 100644 --- a/DEPS +++ b/DEPS
@@ -2,6 +2,8 @@ # dependencies to a minimal set. include_rules = [ "-components", + "+components/country_codes", + "+components/regional_capabilities", "+components/component_updater", "+components/keep_alive_registry", "+components/metrics",
diff --git a/metrics_service_client.cc b/metrics_service_client.cc index d991157..beae0aa 100644 --- a/metrics_service_client.cc +++ b/metrics_service_client.cc
@@ -17,6 +17,7 @@ #include "components/metrics/metrics_features.h" #include "components/metrics/metrics_switches.h" #include "components/metrics/server_urls.h" +#include "components/regional_capabilities/regional_capabilities_country_id.h" namespace metrics { @@ -251,4 +252,9 @@ return std::nullopt; } +std::optional<regional_capabilities::CountryIdHolder> +MetricsServiceClient::GetProfileCountryIdForPrivateMetricsReporting() { + return std::nullopt; +} + } // namespace metrics
diff --git a/metrics_service_client.h b/metrics_service_client.h index 5464d89..cd230b2 100644 --- a/metrics_service_client.h +++ b/metrics_service_client.h
@@ -24,6 +24,10 @@ class UkmService; } +namespace regional_capabilities { +class CountryIdHolder; +} + namespace metrics::dwa { class DwaService; } @@ -254,6 +258,11 @@ // supported, this function should return std::nullopt. virtual std::optional<std::string> GetCurrentUserId() const; + // Returns the country ID associated with the profile used for metrics. + // Returns std::nullopt if it's not available. + virtual std::optional<regional_capabilities::CountryIdHolder> + GetProfileCountryIdForPrivateMetricsReporting(); + private: base::RepeatingClosure update_running_services_; };
diff --git a/private_metrics/BUILD.gn b/private_metrics/BUILD.gn index 3de0ee6..99809ed 100644 --- a/private_metrics/BUILD.gn +++ b/private_metrics/BUILD.gn
@@ -80,6 +80,8 @@ ":private_metrics_recorders", "//components/metrics/dwa/mojom", "//components/metrics/private_metrics/mojom", + "//components/regional_capabilities:country_access_reason", + "//components/regional_capabilities:country_id", "//third_party/federated_compute:confidential_compute_proto", "//third_party/metrics_proto", ] @@ -118,6 +120,7 @@ ":private_metrics_features", "//base", "//base/test:test_support", + "//components/country_codes", "//components/metrics", "//components/metrics:test_support", "//components/prefs",
diff --git a/private_metrics/DEPS b/private_metrics/DEPS index 016beba..d0d391b 100644 --- a/private_metrics/DEPS +++ b/private_metrics/DEPS
@@ -6,3 +6,9 @@ "+services/network/public/mojom", "+third_party/federated_compute/src/fcp/protos/confidentialcompute", ] + +specific_include_rules = { + "puma_service\.cc": [ + "+components/regional_capabilities/access/country_access_reason.h", + ], +}
diff --git a/private_metrics/puma_service.cc b/private_metrics/puma_service.cc index 67af34c..7ca2306 100644 --- a/private_metrics/puma_service.cc +++ b/private_metrics/puma_service.cc
@@ -14,6 +14,8 @@ #include "components/metrics/private_metrics/private_metrics_features.h" #include "components/metrics/private_metrics/private_metrics_pref_names.h" #include "components/metrics/private_metrics/puma_histogram_encoder.h" +#include "components/regional_capabilities/access/country_access_reason.h" +#include "components/regional_capabilities/regional_capabilities_country_id.h" #include "components/version_info/version_info.h" #include "third_party/metrics_proto/private_metrics/private_metrics.pb.h" #include "third_party/metrics_proto/private_metrics/private_user_metrics.pb.h" @@ -201,6 +203,21 @@ rc_profile->set_platform(GetCurrentPlatform()); rc_profile->set_milestone(version_info::GetMajorVersionNumberAsInt()); + + int country_id; + const std::optional<regional_capabilities::CountryIdHolder> + profile_country_id = + client_->GetProfileCountryIdForPrivateMetricsReporting(); + if (profile_country_id.has_value()) { + country_id = profile_country_id.value() + .GetRestricted(regional_capabilities::CountryAccessKey( + regional_capabilities::CountryAccessReason:: + kPrivateUserMetricsReporting)) + .Serialize(); + } else { + country_id = country_codes::CountryId().Serialize(); + } + rc_profile->set_profile_country_id(country_id); } std::optional<::private_metrics::PrivateUserMetrics> @@ -272,4 +289,9 @@ PumaService::ReportStoringOutcome::kStored); } +regional_capabilities::CountryIdHolder +PumaService::GetCountryIdHolderForTesting() { + return client_->GetProfileCountryIdForPrivateMetricsReporting().value(); +} + } // namespace metrics::private_metrics
diff --git a/private_metrics/puma_service.h b/private_metrics/puma_service.h index b6d4324..a95f048 100644 --- a/private_metrics/puma_service.h +++ b/private_metrics/puma_service.h
@@ -74,6 +74,8 @@ static void RegisterPrefs(PrefRegistrySimple* registry); + regional_capabilities::CountryIdHolder GetCountryIdHolderForTesting(); + private: FRIEND_TEST_ALL_PREFIXES(PumaServiceRcTest, RcBuildReportAndStore_DoesCreateAndStoreReport);
diff --git a/private_metrics/puma_service_unittest.cc b/private_metrics/puma_service_unittest.cc index 322d4e8..132bfff 100644 --- a/private_metrics/puma_service_unittest.cc +++ b/private_metrics/puma_service_unittest.cc
@@ -10,10 +10,12 @@ #include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" #include "base/test/task_environment.h" +#include "components/country_codes/country_codes.h" #include "components/metrics/private_metrics/private_metrics_features.h" #include "components/metrics/private_metrics/private_metrics_pref_names.h" #include "components/metrics/test/test_metrics_service_client.h" #include "components/prefs/testing_pref_service.h" +#include "components/regional_capabilities/regional_capabilities_country_id.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/metrics_proto/private_metrics/system_profiles/coarse_system_profile.pb.h" #include "third_party/metrics_proto/private_metrics/system_profiles/rc_coarse_system_profile.pb.h" @@ -37,6 +39,8 @@ PumaService::RegisterPrefs(prefs_.registry()); PrivateMetricsReportingService::RegisterPrefs(prefs_.registry()); + client_.set_country_id_holder( + regional_capabilities::CountryIdHolder(country_codes::CountryId("BE"))); puma_service_ = std::make_unique<PumaService>(&client_, &prefs_); } @@ -132,8 +136,9 @@ EXPECT_TRUE(rc_profile.has_milestone()); EXPECT_TRUE(rc_profile.has_platform()); - // TODO(b:452034784) Implement profile_country_id mapping. - EXPECT_FALSE(rc_profile.has_profile_country_id()); + EXPECT_TRUE(rc_profile.has_profile_country_id()); + EXPECT_EQ(rc_profile.profile_country_id(), + country_codes::CountryId("BE").Serialize()); } TEST_F(PumaServiceTest, RcClientId_IsNullWhenPumaRcIsDisabled) {
diff --git a/test/test_metrics_service_client.cc b/test/test_metrics_service_client.cc index e66b13f..89d182c 100644 --- a/test/test_metrics_service_client.cc +++ b/test/test_metrics_service_client.cc
@@ -11,6 +11,7 @@ #include "base/containers/contains.h" #include "base/functional/callback.h" #include "components/metrics/metrics_log_uploader.h" +#include "components/regional_capabilities/regional_capabilities_country_id.h" #include "third_party/metrics_proto/chrome_user_metrics_extension.pb.h" namespace metrics { @@ -111,6 +112,11 @@ return storage_limits_; } +std::optional<regional_capabilities::CountryIdHolder> +TestMetricsServiceClient::GetProfileCountryIdForPrivateMetricsReporting() { + return country_id_holder_; +} + void TestMetricsServiceClient::AllowMetricUploadForUserId(uint64_t user_id) { allowed_user_ids_.insert(user_id); }
diff --git a/test/test_metrics_service_client.h b/test/test_metrics_service_client.h index 50f003d..7e1ba23 100644 --- a/test/test_metrics_service_client.h +++ b/test/test_metrics_service_client.h
@@ -16,6 +16,7 @@ #include "components/metrics/metrics_log_uploader.h" #include "components/metrics/metrics_service_client.h" #include "components/metrics/test/test_metrics_log_uploader.h" +#include "components/regional_capabilities/regional_capabilities_country_id.h" namespace variations { class SyntheticTrialRegistry; @@ -59,6 +60,8 @@ std::string GetAppPackageNameIfLoggable() override; bool ShouldResetClientIdsOnClonedInstall() override; MetricsLogStore::StorageLimits GetStorageLimits() const override; + std::optional<regional_capabilities::CountryIdHolder> + GetProfileCountryIdForPrivateMetricsReporting() override; // Adds/removes |user_id| from the set of user ids that have metrics consent // as true. @@ -99,6 +102,10 @@ variations::SyntheticTrialRegistry* registry) { synthetic_trial_registry_ = registry; } + void set_country_id_holder( + const regional_capabilities::CountryIdHolder& country_id_holder) { + country_id_holder_ = country_id_holder; + } private: std::string client_id_{"0a94430b-18e5-43c8-a657-580f7e855ce1"}; @@ -111,6 +118,8 @@ MetricsLogStore::StorageLimits storage_limits_ = MetricsServiceClient::GetStorageLimits(); std::set<uint64_t> allowed_user_ids_; + std::optional<regional_capabilities::CountryIdHolder> country_id_holder_ = + MetricsServiceClient::GetProfileCountryIdForPrivateMetricsReporting(); raw_ptr<variations::SyntheticTrialRegistry> synthetic_trial_registry_ = nullptr;