From 662fa38826f807b34126f26acdfd62ba0457084d Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Wed, 13 May 2026 18:40:00 -0500 Subject: [PATCH] Smart NTT [virtual]:serp_metrics should return default values (#36407) On a clean profile, engine keys are absent from the profile attributes dict, causing virtual pref path traversal to fail with "Unknown pref path". Initialize an empty list for each engine key on first construction of ProfileAttributesTimePeriodStore so the paths always resolve to 0. --- .../profile_attributes_time_period_store.cc | 18 ++++++++++----- .../profile_attributes_time_period_store.h | 2 +- ...e_attributes_time_period_store_unittest.cc | 22 ++++++++++--------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/browser/serp_metrics/profile_attributes_time_period_store.cc b/browser/serp_metrics/profile_attributes_time_period_store.cc index 9ad09fdb7da..6a8c7ce93d3 100644 --- a/browser/serp_metrics/profile_attributes_time_period_store.cc +++ b/browser/serp_metrics/profile_attributes_time_period_store.cc @@ -14,12 +14,19 @@ namespace serp_metrics { ProfileAttributesTimePeriodStore::ProfileAttributesTimePeriodStore( - const base::FilePath& profile_path, + base::FilePath profile_path, ProfileAttributesStorage& profile_attributes_storage, std::string_view metric_name) - : profile_path_(profile_path), + : profile_path_(std::move(profile_path)), profile_attributes_storage_(profile_attributes_storage), - metric_name_(metric_name) {} + metric_name_(metric_name) { + if (!Get()) { + // Initialize an empty list so the engine key always exists in the profile + // attributes dict. This ensures virtual pref paths that query this key + // resolve to 0 rather than failing on a clean profile. + Set(base::ListValue()); + } +} ProfileAttributesTimePeriodStore::~ProfileAttributesTimePeriodStore() = default; @@ -46,9 +53,8 @@ void ProfileAttributesTimePeriodStore::Set(base::ListValue list) { } base::DictValue serp_metrics; - if (const base::DictValue* existing_serp_metrics = entry->GetSerpMetrics(); - existing_serp_metrics) { - serp_metrics = existing_serp_metrics->Clone(); + if (const base::DictValue* const value = entry->GetSerpMetrics()) { + serp_metrics = value->Clone(); } serp_metrics.Set(metric_name_, std::move(list)); diff --git a/browser/serp_metrics/profile_attributes_time_period_store.h b/browser/serp_metrics/profile_attributes_time_period_store.h index c65b8be6c2d..d7730cb07d6 100644 --- a/browser/serp_metrics/profile_attributes_time_period_store.h +++ b/browser/serp_metrics/profile_attributes_time_period_store.h @@ -32,7 +32,7 @@ class ProfileAttributesTimePeriodStore final : public SerpMetricsTimePeriodStore { public: ProfileAttributesTimePeriodStore( - const base::FilePath& profile_path, + base::FilePath profile_path, ProfileAttributesStorage& profile_attributes_storage, std::string_view metric_name); diff --git a/browser/serp_metrics/profile_attributes_time_period_store_unittest.cc b/browser/serp_metrics/profile_attributes_time_period_store_unittest.cc index e65b26f33d4..2f01c5ed89e 100644 --- a/browser/serp_metrics/profile_attributes_time_period_store_unittest.cc +++ b/browser/serp_metrics/profile_attributes_time_period_store_unittest.cc @@ -80,24 +80,26 @@ TEST_F(ProfileAttributesTimePeriodStoreTest, ClearStore) { EXPECT_FALSE(store.Get()); } -TEST_F(ProfileAttributesTimePeriodStoreTest, GetUninitializedStore) { +TEST_F(ProfileAttributesTimePeriodStoreTest, + GetReturnsEmptyListOnCleanProfile) { ProfileAttributesTimePeriodStore store( profile_path(), profile_attributes_storage(), kMetricName); - EXPECT_FALSE(store.Get()); + ASSERT_TRUE(store.Get()); + EXPECT_TRUE(store.Get()->empty()); } TEST_F(ProfileAttributesTimePeriodStoreTest, SetStoresWithDifferentKeys) { - ProfileAttributesTimePeriodStore store1( + ProfileAttributesTimePeriodStore store_1( profile_path(), profile_attributes_storage(), kMetricName); - ProfileAttributesTimePeriodStore store2( + ProfileAttributesTimePeriodStore store_2( profile_path(), profile_attributes_storage(), "other_testing_metric"); - store1.Set(base::ListValue().Append(1)); - store2.Set(base::ListValue().Append(2).Append(3)); + store_1.Set(base::ListValue().Append(1)); + store_2.Set(base::ListValue().Append(2).Append(3)); - ASSERT_TRUE(store1.Get()); - EXPECT_THAT(*store1.Get(), ::testing::ElementsAre(1)); - ASSERT_TRUE(store2.Get()); - EXPECT_THAT(*store2.Get(), ::testing::ElementsAre(2, 3)); + ASSERT_TRUE(store_1.Get()); + EXPECT_THAT(*store_1.Get(), ::testing::ElementsAre(1)); + ASSERT_TRUE(store_2.Get()); + EXPECT_THAT(*store_2.Get(), ::testing::ElementsAre(2, 3)); } } // namespace serp_metrics