From b51cc938a0ae3c17e60a7da273684462faac0e9d Mon Sep 17 00:00:00 2001 From: Nicola Tommasi <tommasin@chromium.org> Date: Mon, 3 Mar 2025 05:04:07 -0800 Subject: [PATCH] [Merchant Trust]: Let validation logic handle kNoResult status We were preemptively returning when no data was fetched from the optimization service. We're now letting the validation logic take care of it; in this way the histogram is logged correctly and the total count represents the number of optimization requests. Bug: 400379039 Change-Id: I84015bd29edadeaeab3fe016097ca462b1351ccf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6317305 Reviewed-by: Olesia Marukhno <olesiamarukhno@google.com> Auto-Submit: Nicola Tommasi <tommasin@chromium.org> Commit-Queue: Nicola Tommasi <tommasin@chromium.org> Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com> Cr-Commit-Position: refs/heads/main@{#1427065} --- .../page_info/core/merchant_trust_service.cc | 8 +++---- .../core/merchant_trust_service_unittest.cc | 23 +++++++++++++++++++ .../merchant_trust_validation_unittest.cc | 4 ++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/components/page_info/core/merchant_trust_service.cc b/components/page_info/core/merchant_trust_service.cc index 1dfa6512d05c3..079aa5ad01f66 100644 --- a/components/page_info/core/merchant_trust_service.cc +++ b/components/page_info/core/merchant_trust_service.cc @@ -142,11 +142,9 @@ void MerchantTrustService::OnCanApplyOptimizationComplete( if (decision != optimization_guide::OptimizationGuideDecision::kUnknown) { std::optional<commerce::MerchantTrustSignalsV2> merchant_trust_metadata = metadata.ParsedMetadata<commerce::MerchantTrustSignalsV2>(); - if (merchant_trust_metadata.has_value()) { - std::move(callback).Run( - url, GetMerchantDataFromProto(merchant_trust_metadata)); - return; - } + std::move(callback).Run(url, + GetMerchantDataFromProto(merchant_trust_metadata)); + return; } if (kMerchantTrustEnabledWithSampleData.Get()) { diff --git a/components/page_info/core/merchant_trust_service_unittest.cc b/components/page_info/core/merchant_trust_service_unittest.cc index 34261729d99d9..54b5d6c9953ab 100644 --- a/components/page_info/core/merchant_trust_service_unittest.cc +++ b/components/page_info/core/merchant_trust_service_unittest.cc @@ -229,6 +229,29 @@ TEST_F(MerchantTrustServiceTest, SampleData) { run_loop.Run(); } +// Tests that if the proto is empty, no data is returned. +TEST_F(MerchantTrustServiceTest, NoResult) { + base::HistogramTester t; + OptimizationMetadata metadata; + metadata.set_any_metadata({}); + SetResponse(GURL("https://foo.com"), OptimizationGuideDecision::kTrue, + metadata); + + base::RunLoop run_loop; + service()->GetMerchantTrustInfo( + GURL("https://foo.com"), + base::BindOnce( + [](base::RunLoop* run_loop, const GURL& url, + std::optional<page_info::MerchantData> info) { + ASSERT_FALSE(info.has_value()); + run_loop->Quit(); + }, + &run_loop)); + run_loop.Run(); + t.ExpectUniqueSample("Security.PageInfo.MerchantTrustStatus", + MerchantTrustStatus::kNoResult, 1); +} + // Tests that status is recorded as not valid when a proto is missing a field // and no data is returned. TEST_F(MerchantTrustServiceTest, InvalidProto) { diff --git a/components/page_info/core/merchant_trust_validation_unittest.cc b/components/page_info/core/merchant_trust_validation_unittest.cc index 7fb0ccc208324..03e23ebd36811 100644 --- a/components/page_info/core/merchant_trust_validation_unittest.cc +++ b/components/page_info/core/merchant_trust_validation_unittest.cc @@ -19,6 +19,10 @@ commerce::MerchantTrustSignalsV2 GetSampleProto() { return proto; } +TEST(MerchantTrustValidation, NoResult) { + EXPECT_EQ(ValidateProto(std::nullopt), MerchantTrustStatus::kNoResult); +} + // Tests that correct proto messages are accepted. TEST(MerchantTrustValidation, ValidProto) { auto proto = GetSampleProto();