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.
This commit is contained in:
Terry Mancey
2026-05-14 00:40:00 +01:00
committed by GitHub
parent be9f126352
commit 662fa38826
3 changed files with 25 additions and 17 deletions
@@ -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));
@@ -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);
@@ -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