From 716c1ae47d07c9ec1fc421c3867eeeb31284f352 Mon Sep 17 00:00:00 2001 From: Andrew Paseltiner Date: Thu, 9 Dec 2021 01:43:57 +0000 Subject: [PATCH] Replace TestAttributionManager with gmock Change-Id: Ied469dad0ca015b6d9adbc7591d870b168c76001 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3323526 Reviewed-by: Nan Lin Commit-Queue: Andrew Paseltiner Cr-Commit-Position: refs/heads/main@{#949874} --- .../attribution_host_unittest.cc | 215 ++++++++++-------- .../attribution_internals_browsertest.cc | 98 +++++--- .../attribution_reporter_android_unittest.cc | 51 ++--- .../attribution_test_utils.cc | 68 +----- .../attribution_test_utils.h | 68 +++--- 5 files changed, 244 insertions(+), 256 deletions(-) diff --git a/content/browser/attribution_reporting/attribution_host_unittest.cc b/content/browser/attribution_reporting/attribution_host_unittest.cc index a9bb6206c01ef..9cb9afd0c81b6 100644 --- a/content/browser/attribution_reporting/attribution_host_unittest.cc +++ b/content/browser/attribution_reporting/attribution_host_unittest.cc @@ -55,13 +55,14 @@ using ConversionMeasurementOperation = using testing::_; using testing::AllOf; -using testing::ElementsAre; -using testing::IsEmpty; +using testing::InSequence; using testing::IsNull; +using testing::Mock; using testing::Pointee; using testing::Property; using testing::Return; -using testing::SizeIs; + +using Checkpoint = ::testing::MockFunction; const char kConversionUrl[] = "https://b.com"; const char kConversionUrlWithFragment[] = "https://b.com/#fragment"; @@ -84,7 +85,7 @@ class AttributionHostTest : public RenderViewHostTestHarness { RenderViewHostTestHarness::SetUp(); conversion_host_ = AttributionHostTestPeer::CreateAttributionHost( - web_contents(), std::make_unique(&test_manager_)); + web_contents(), std::make_unique(&mock_manager_)); AttributionHost::SetReceiverImplForTesting(conversion_host_.get()); contents()->GetMainFrame()->InitializeRenderFrameIfNeeded(); @@ -111,11 +112,16 @@ class AttributionHostTest : public RenderViewHostTestHarness { } protected: - TestAttributionManager test_manager_; + MockAttributionManager mock_manager_; std::unique_ptr conversion_host_; }; TEST_F(AttributionHostTest, ValidConversionInSubframe_NoBadMessage) { + EXPECT_CALL(mock_manager_, + HandleTrigger(Property( + &StorableTrigger::conversion_destination, + net::SchemefulSite(GURL("https://www.example.com"))))); + contents()->NavigateAndCommit(GURL("https://www.example.com")); // Create a subframe and use it as a target for the conversion registration @@ -138,15 +144,15 @@ TEST_F(AttributionHostTest, ValidConversionInSubframe_NoBadMessage) { // triggered. base::RunLoop().RunUntilIdle(); EXPECT_FALSE(bad_message_observer.got_bad_message()); - - EXPECT_THAT(test_manager_.handled_triggers(), - ElementsAre(Property( - &StorableTrigger::conversion_destination, - net::SchemefulSite(GURL("https://www.example.com"))))); } TEST_F(AttributionHostTest, ConversionInSubframe_ConversionDestinationMatchesMainFrame) { + EXPECT_CALL(mock_manager_, + HandleTrigger(Property( + &StorableTrigger::conversion_destination, + net::SchemefulSite(GURL("https://www.example.com"))))); + contents()->NavigateAndCommit(GURL("https://www.example.com")); // Create a subframe and use it as a target for the conversion registration @@ -171,14 +177,11 @@ TEST_F(AttributionHostTest, // triggered. base::RunLoop().RunUntilIdle(); EXPECT_FALSE(bad_message_observer.got_bad_message()); - - EXPECT_THAT(test_manager_.handled_triggers(), - ElementsAre(Property( - &StorableTrigger::conversion_destination, - net::SchemefulSite(GURL("https://www.example.com"))))); } TEST_F(AttributionHostTest, ConversionInSubframeOnInsecurePage_BadMessage) { + EXPECT_CALL(mock_manager_, HandleTrigger).Times(0); + contents()->NavigateAndCommit(GURL("http://www.example.com")); // Create a subframe and use it as a target for the conversion registration @@ -202,7 +205,6 @@ TEST_F(AttributionHostTest, ConversionInSubframeOnInsecurePage_BadMessage) { "blink.mojom.ConversionHost can only be used in secure contexts with a " "secure conversion registration origin.", bad_message_observer.WaitForBadMessage()); - EXPECT_THAT(test_manager_.handled_triggers(), IsEmpty()); } TEST_F(AttributionHostTest, ConversionInSubframe_ChecksCorrectOrigins) { @@ -221,6 +223,8 @@ TEST_F(AttributionHostTest, ConversionInSubframe_ChecksCorrectOrigins) { ScopedContentBrowserClientSetting setting(&browser_client); for (bool conversion_allowed : {false, true}) { + EXPECT_CALL(mock_manager_, HandleTrigger).Times(conversion_allowed); + contents()->NavigateAndCommit(GURL("https://www.example.com")); // Create a subframe and use it as a target for the conversion registration @@ -237,13 +241,13 @@ TEST_F(AttributionHostTest, ConversionInSubframe_ChecksCorrectOrigins) { url::Origin::Create(GURL("https://report.example")); conversion_host_mojom()->RegisterConversion(std::move(conversion)); - EXPECT_THAT(test_manager_.handled_triggers(), SizeIs(conversion_allowed)); - - test_manager_.Reset(); + Mock::VerifyAndClear(&mock_manager_); } } TEST_F(AttributionHostTest, ConversionOnInsecurePage_BadMessage) { + EXPECT_CALL(mock_manager_, HandleTrigger).Times(0); + // Create a page with an insecure origin. contents()->NavigateAndCommit(GURL("http://www.example.com")); SetCurrentTargetFrameForTesting(main_rfh()); @@ -260,10 +264,11 @@ TEST_F(AttributionHostTest, ConversionOnInsecurePage_BadMessage) { "blink.mojom.ConversionHost can only be used in secure contexts with a " "secure conversion registration origin.", bad_message_observer.WaitForBadMessage()); - EXPECT_THAT(test_manager_.handled_triggers(), IsEmpty()); } TEST_F(AttributionHostTest, ConversionWithInsecureReportingOrigin_BadMessage) { + EXPECT_CALL(mock_manager_, HandleTrigger).Times(0); + contents()->NavigateAndCommit(GURL("https://www.example.com")); SetCurrentTargetFrameForTesting(main_rfh()); @@ -279,10 +284,11 @@ TEST_F(AttributionHostTest, ConversionWithInsecureReportingOrigin_BadMessage) { "blink.mojom.ConversionHost can only be used in secure contexts with a " "secure conversion registration origin.", bad_message_observer.WaitForBadMessage()); - EXPECT_THAT(test_manager_.handled_triggers(), IsEmpty()); } TEST_F(AttributionHostTest, ValidConversion_NoBadMessage) { + EXPECT_CALL(mock_manager_, HandleTrigger); + // Create a page with a secure origin. contents()->NavigateAndCommit(GURL("https://www.example.com")); SetCurrentTargetFrameForTesting(main_rfh()); @@ -300,10 +306,11 @@ TEST_F(AttributionHostTest, ValidConversion_NoBadMessage) { // triggered. base::RunLoop().RunUntilIdle(); EXPECT_FALSE(bad_message_observer.got_bad_message()); - EXPECT_THAT(test_manager_.handled_triggers(), SizeIs(1)); } TEST_F(AttributionHostTest, ValidConversionWithEmbedderDisable_NoConversion) { + EXPECT_CALL(mock_manager_, HandleTrigger).Times(0); + MockAttributionReportingContentBrowserClient browser_client; EXPECT_CALL( browser_client, @@ -322,11 +329,11 @@ TEST_F(AttributionHostTest, ValidConversionWithEmbedderDisable_NoConversion) { conversion->reporting_origin = url::Origin::Create(GURL("https://secure.com")); conversion_host_mojom()->RegisterConversion(std::move(conversion)); - - EXPECT_THAT(test_manager_.handled_triggers(), IsEmpty()); } TEST_F(AttributionHostTest, ValidImpressionWithEmbedderDisable_NoImpression) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + MockAttributionReportingContentBrowserClient browser_client; // This is called twice because the real AttributionHost is still active for // the test. @@ -346,11 +353,16 @@ TEST_F(AttributionHostTest, ValidImpressionWithEmbedderDisable_NoImpression) { navigation->SetInitiatorFrame(main_rfh()); navigation->set_impression(CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } TEST_F(AttributionHostTest, Conversion_AssociatedWithConversionSite) { + // Verify that we use the domain of the page where the conversion occurred + // instead of the origin. + EXPECT_CALL(mock_manager_, + HandleTrigger(Property( + &StorableTrigger::conversion_destination, + net::SchemefulSite(GURL("https://conversion.com"))))); + // Create a page with a secure origin. contents()->NavigateAndCommit(GURL("https://sub.conversion.com")); SetCurrentTargetFrameForTesting(main_rfh()); @@ -359,13 +371,6 @@ TEST_F(AttributionHostTest, Conversion_AssociatedWithConversionSite) { conversion->reporting_origin = url::Origin::Create(GURL("https://secure.com")); conversion_host_mojom()->RegisterConversion(std::move(conversion)); - - // Verify that we use the domain of the page where the conversion occurred - // instead of the origin. - EXPECT_THAT(test_manager_.handled_triggers(), - ElementsAre(Property( - &StorableTrigger::conversion_destination, - net::SchemefulSite(GURL("https://conversion.com"))))); } TEST_F(AttributionHostTest, PerPageConversionMetrics) { @@ -384,16 +389,16 @@ TEST_F(AttributionHostTest, PerPageConversionMetrics) { url::Origin::Create(GURL("https://secure.com")); for (size_t i = 0u; i < 8u; i++) { + EXPECT_CALL(mock_manager_, HandleTrigger); conversion_host_mojom()->RegisterConversion(conversion->Clone()); - EXPECT_THAT(test_manager_.handled_triggers(), SizeIs(1)); - test_manager_.Reset(); + Mock::VerifyAndClear(&mock_manager_); } + EXPECT_CALL(mock_manager_, HandleTrigger); conversion->reporting_origin = url::Origin::Create(GURL("https://anothersecure.com")); conversion_host_mojom()->RegisterConversion(conversion->Clone()); - EXPECT_THAT(test_manager_.handled_triggers(), SizeIs(1)); - test_manager_.Reset(); + Mock::VerifyAndClear(&mock_manager_); // Same document navs should not reset the counter. contents()->NavigateAndCommit(GURL("https://www.example.com#hash")); @@ -453,23 +458,23 @@ TEST_F(AttributionHostTest, PerPageImpressionMetrics) { blink::Impression impression = CreateValidImpression(); for (size_t i = 0u; i < 8u; i++) { + EXPECT_CALL(mock_manager_, HandleSource); conversion_host_mojom()->RegisterImpression(impression); // Run loop to allow the bad message code to run if a bad message was // triggered. base::RunLoop().RunUntilIdle(); - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); - test_manager_.Reset(); + Mock::VerifyAndClear(&mock_manager_); } + EXPECT_CALL(mock_manager_, HandleSource); impression.reporting_origin = url::Origin::Create(GURL("https://anothersecure.com")); conversion_host_mojom()->RegisterImpression(impression); // Run loop to allow the bad message code to run if a bad message was // triggered. base::RunLoop().RunUntilIdle(); - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); - test_manager_.Reset(); + Mock::VerifyAndClear(&mock_manager_); // Same document navs should not reset the counter. contents()->NavigateAndCommit(GURL("https://www.example.com#hash")); @@ -529,22 +534,22 @@ TEST_F(AttributionHostTest, NavigationWithImpression_PerPageImpressionMetrics) { } TEST_F(AttributionHostTest, NavigationWithNoImpression_Ignored) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + contents()->NavigateAndCommit(GURL("https://secure_impression.com")); NavigationSimulatorImpl::NavigateAndCommitFromDocument(GURL(kConversionUrl), main_rfh()); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } TEST_F(AttributionHostTest, ValidImpression_ForwardedToManager) { + EXPECT_CALL(mock_manager_, HandleSource); + contents()->NavigateAndCommit(GURL("https://secure_impression.com")); auto navigation = NavigationSimulatorImpl::CreateRendererInitiated( GURL(kConversionUrl), main_rfh()); navigation->SetInitiatorFrame(main_rfh()); navigation->set_impression(CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); } TEST_F(AttributionHostTest, ImpressionWithNoManagerAvilable_NoCrash) { @@ -562,6 +567,8 @@ TEST_F(AttributionHostTest, ImpressionWithNoManagerAvilable_NoCrash) { } TEST_F(AttributionHostTest, ImpressionInSubframe_Ignored) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + contents()->NavigateAndCommit(GURL("https://secure_impression.com")); // Create a subframe and use it as a target for the conversion registration @@ -575,13 +582,13 @@ TEST_F(AttributionHostTest, ImpressionInSubframe_Ignored) { navigation->SetInitiatorFrame(main_rfh()); navigation->set_impression(CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } // Test that if we cannot access the initiator frame of the navigation, we // ignore the associated impression. TEST_F(AttributionHostTest, ImpressionNavigationWithDeadInitiator_Ignored) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + base::HistogramTester histograms; contents()->NavigateAndCommit(GURL("https://secure_impression.com")); @@ -593,13 +600,13 @@ TEST_F(AttributionHostTest, ImpressionNavigationWithDeadInitiator_Ignored) { navigation->set_impression(CreateValidImpression()); navigation->Commit(); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); - histograms.ExpectUniqueSample( "Conversions.ImpressionNavigationHasDeadInitiator", true, 2); } TEST_F(AttributionHostTest, ImpressionNavigationCommitsToErrorPage_Ignored) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + contents()->NavigateAndCommit(GURL("https://secure_impression.com")); auto navigation = NavigationSimulatorImpl::CreateRendererInitiated( @@ -608,11 +615,11 @@ TEST_F(AttributionHostTest, ImpressionNavigationCommitsToErrorPage_Ignored) { navigation->set_impression(CreateValidImpression()); navigation->Fail(net::ERR_FAILED); navigation->CommitErrorPage(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } TEST_F(AttributionHostTest, ImpressionNavigationAborts_Ignored) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + contents()->NavigateAndCommit(GURL("https://secure_impression.com")); auto navigation = NavigationSimulatorImpl::CreateRendererInitiated( @@ -620,12 +627,12 @@ TEST_F(AttributionHostTest, ImpressionNavigationAborts_Ignored) { navigation->SetInitiatorFrame(main_rfh()); navigation->set_impression(CreateValidImpression()); navigation->AbortCommit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } TEST_F(AttributionHostTest, CommittedOriginDiffersFromConversionDesintation_Ignored) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + contents()->NavigateAndCommit(GURL("https://secure_impression.com")); auto navigation = NavigationSimulatorImpl::CreateRendererInitiated( @@ -633,8 +640,6 @@ TEST_F(AttributionHostTest, navigation->SetInitiatorFrame(main_rfh()); navigation->set_impression(CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } TEST_F(AttributionHostTest, @@ -674,6 +679,9 @@ TEST_F(AttributionHostTest, }; for (const auto& test_case : kTestCases) { + EXPECT_CALL(mock_manager_, HandleSource) + .Times(test_case.impression_expected); + contents()->NavigateAndCommit(GURL(test_case.impression_origin)); auto navigation = NavigationSimulatorImpl::CreateRendererInitiated( GURL(test_case.conversion_origin), main_rfh()); @@ -687,16 +695,17 @@ TEST_F(AttributionHostTest, navigation->SetInitiatorFrame(main_rfh()); navigation->Commit(); - EXPECT_THAT(test_manager_.handled_sources(), - SizeIs(test_case.impression_expected)) - << "For test case: " << test_case.impression_origin << " | " - << test_case.conversion_origin << " | " << test_case.reporting_origin; - test_manager_.Reset(); + Mock::VerifyAndClear(&mock_manager_); } } TEST_F(AttributionHostTest, ImpressionInSubframe_ImpressionOriginMatchesTopPageOrigin) { + EXPECT_CALL(mock_manager_, + HandleSource(Property( + &StorableSource::impression_origin, + url::Origin::Create(GURL("https://www.example.com"))))); + contents()->NavigateAndCommit(GURL("https://www.example.com")); // Create a subframe and use it as a target for the impression registration @@ -719,14 +728,14 @@ TEST_F(AttributionHostTest, // triggered. base::RunLoop().RunUntilIdle(); EXPECT_FALSE(bad_message_observer.got_bad_message()); - - EXPECT_THAT(test_manager_.handled_sources(), - ElementsAre(Property( - &StorableSource::impression_origin, - url::Origin::Create(GURL("https://www.example.com"))))); } TEST_F(AttributionHostTest, ValidImpression_NoBadMessage) { + EXPECT_CALL(mock_manager_, + HandleSource(AllOf(Property(&StorableSource::source_type, + StorableSource::SourceType::kEvent), + Property(&StorableSource::priority, 10)))); + // Create a page with a secure origin. contents()->NavigateAndCommit(GURL("https://www.example.com")); SetCurrentTargetFrameForTesting(main_rfh()); @@ -742,10 +751,6 @@ TEST_F(AttributionHostTest, ValidImpression_NoBadMessage) { // triggered. base::RunLoop().RunUntilIdle(); EXPECT_FALSE(bad_message_observer.got_bad_message()); - EXPECT_THAT(test_manager_.handled_sources(), - ElementsAre(AllOf(Property(&StorableSource::source_type, - StorableSource::SourceType::kEvent), - Property(&StorableSource::priority, 10)))); } TEST_F(AttributionHostTest, RegisterImpression_RecordsAllowedMetric) { @@ -813,6 +818,17 @@ TEST_F(AttributionHostTest, RegisterConversion_RecordsAllowedMetric) { // navigation begins but before it's committed. Currently only used on Android // but should work cross-platform. TEST_F(AttributionHostTest, AndroidConversion_DuringNavigation) { + Checkpoint checkpoint; + { + InSequence seq; + + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + EXPECT_CALL(checkpoint, Call(1)); + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + EXPECT_CALL(checkpoint, Call(2)); + EXPECT_CALL(mock_manager_, HandleSource); + } + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -826,22 +842,31 @@ TEST_F(AttributionHostTest, AndroidConversion_DuringNavigation) { GURL(kConversionUrl), contents()); navigation->Start(); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); + checkpoint.Call(1); conversion_host()->ReportAttributionForCurrentNavigation( url::Origin::Create(GURL(origin)), CreateValidImpression()); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); + checkpoint.Call(2); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); } // In pre-loaded CCT navigations, the attribution can arrive after the // navigation completes. Currently only used on Android but should work // cross-platform. TEST_F(AttributionHostTest, AndroidConversion_AfterNavigation) { + Checkpoint checkpoint; + { + InSequence seq; + + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + EXPECT_CALL(checkpoint, Call(1)); + EXPECT_CALL(mock_manager_, HandleSource); + EXPECT_CALL(checkpoint, Call(2)); + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + } + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -855,21 +880,21 @@ TEST_F(AttributionHostTest, AndroidConversion_AfterNavigation) { GURL(kConversionUrl), contents()); navigation->Commit(); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); + checkpoint.Call(1); conversion_host()->ReportAttributionForCurrentNavigation( url::Origin::Create(GURL(origin)), CreateValidImpression()); - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); + checkpoint.Call(2); // Make sure we don't allow repeated attributions for the same navigation. conversion_host()->ReportAttributionForCurrentNavigation( url::Origin::Create(GURL(origin)), CreateValidImpression()); - - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); } TEST_F(AttributionHostTest, AndroidConversion_AfterNavigation_SubDomain) { + EXPECT_CALL(mock_manager_, HandleSource); + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -885,14 +910,14 @@ TEST_F(AttributionHostTest, AndroidConversion_AfterNavigation_SubDomain) { conversion_host()->ReportAttributionForCurrentNavigation( url::Origin::Create(GURL(origin)), CreateValidImpression()); - - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); } // In pre-loaded CCT navigations, the attribution can arrive after the // navigation completes, but the destination must match the attribution. TEST_F(AttributionHostTest, AndroidConversion_AfterNavigation_WrongDestination) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -909,20 +934,18 @@ TEST_F(AttributionHostTest, conversion_host()->ReportAttributionForCurrentNavigation( url::Origin::Create(GURL(origin)), CreateValidImpression()); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); - // Navigating to the correct URL after navigation to the wrong one still // shouldn't allow the attribution. auto good_navigation = NavigationSimulatorImpl::CreateBrowserInitiated( GURL(kConversionUrl), contents()); good_navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } // Ensure we don't re-use pending Impressions after an aborted commit. Currently // only used on Android but should work cross-platform. TEST_F(AttributionHostTest, AndroidConversion_NavigationAborted) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -941,19 +964,17 @@ TEST_F(AttributionHostTest, AndroidConversion_NavigationAborted) { navigation_abort->AbortCommit(); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); - auto navigation_commit = NavigationSimulatorImpl::CreateBrowserInitiated( GURL(kConversionUrl), contents()); navigation_commit->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } // Ensure we don't re-use pending Impressions after an Error page commit. // Currently only used on Android but should work cross-platform. TEST_F(AttributionHostTest, AndroidConversion_NavigationError) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -973,19 +994,17 @@ TEST_F(AttributionHostTest, AndroidConversion_NavigationError) { navigation_error->Fail(net::ERR_UNEXPECTED); navigation_error->CommitErrorPage(); - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); - auto navigation_commit = NavigationSimulatorImpl::CreateBrowserInitiated( GURL(kConversionUrl), contents()); navigation_commit->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } // We don't allow attributions before a navigation begins. Currently only used // on Android but should work cross-platform. TEST_F(AttributionHostTest, AndroidConversion_BeforeNavigation) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -1002,12 +1021,12 @@ TEST_F(AttributionHostTest, AndroidConversion_BeforeNavigation) { url::Origin::Create(GURL(origin)), CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } // We ignore same-document navigations. TEST_F(AttributionHostTest, AndroidConversion_SameDocument) { + EXPECT_CALL(mock_manager_, HandleSource); + std::string origin( #if defined(OS_ANDROID) "android-app:com.any.app"); @@ -1025,12 +1044,12 @@ TEST_F(AttributionHostTest, AndroidConversion_SameDocument) { conversion_host()->ReportAttributionForCurrentNavigation( url::Origin::Create(GURL(origin)), CreateValidImpression()); - - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); } #if defined(OS_ANDROID) TEST_F(AttributionHostTest, AndroidConversion) { + EXPECT_CALL(mock_manager_, HandleSource); + url::ScopedSchemeRegistryForTests scoped_registry; url::AddStandardScheme(kAndroidAppScheme, url::SCHEME_WITH_HOST); auto navigation = NavigationSimulatorImpl::CreateBrowserInitiated( @@ -1039,19 +1058,17 @@ TEST_F(AttributionHostTest, AndroidConversion) { url::Origin::Create(GURL("android-app:com.any.app"))); navigation->set_impression(CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), SizeIs(1)); } TEST_F(AttributionHostTest, AndroidConversion_BadScheme) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + auto navigation = NavigationSimulatorImpl::CreateBrowserInitiated( GURL(kConversionUrl), contents()); navigation->set_initiator_origin( url::Origin::Create(GURL("https://com.any.app"))); navigation->set_impression(CreateValidImpression()); navigation->Commit(); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } #endif diff --git a/content/browser/attribution_reporting/attribution_internals_browsertest.cc b/content/browser/attribution_reporting/attribution_internals_browsertest.cc index 414eed2bb42a4..af916f8951d9c 100644 --- a/content/browser/attribution_reporting/attribution_internals_browsertest.cc +++ b/content/browser/attribution_reporting/attribution_internals_browsertest.cc @@ -4,6 +4,13 @@ #include "content/browser/attribution_reporting/attribution_internals_ui.h" +#include + +#include +#include +#include + +#include "base/callback.h" #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "content/browser/attribution_reporting/attribution_manager.h" @@ -43,11 +50,24 @@ const std::u16string kCompleteTitle3 = u"Complete3"; const std::u16string kMaxInt64String = u"9223372036854775807"; const std::u16string kMaxUint64String = u"18446744073709551615"; +template +auto InvokeCallback(T value) { + return [value = std::move(value)](base::OnceCallback callback) { + std::move(callback).Run(std::move(value)); + }; +} + } // namespace class AttributionInternalsWebUiBrowserTest : public ContentBrowserTest { public: - AttributionInternalsWebUiBrowserTest() = default; + AttributionInternalsWebUiBrowserTest() { + ON_CALL(manager_, GetActiveSourcesForWebUI) + .WillByDefault(InvokeCallback>({})); + + ON_CALL(manager_, GetPendingReportsForWebUI) + .WillByDefault(InvokeCallback>({})); + } void ClickRefreshButton() { EXPECT_TRUE(ExecJsInWebUI("document.getElementById('refresh').click();")); @@ -93,7 +113,7 @@ class AttributionInternalsWebUiBrowserTest : public ContentBrowserTest { // The manager must outlive the `AttributionInternalsHandler` so that the // latter can remove itself as an observer of the former on the latter's // destruction. - TestAttributionManager manager_; + MockAttributionManager manager_; }; IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, @@ -115,7 +135,7 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, OverrideWebUIAttributionManager(); // Create a mutation observer to wait for the content to render to the dom. - // Waiting on calls to TestAttributionManager is not sufficient because the + // Waiting on calls to `MockAttributionManager` is not sufficient because the // results are returned in promises. static constexpr char wait_script[] = R"( let status = document.getElementById("feature-status-content"); @@ -147,7 +167,7 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, OverrideWebUIAttributionManager(); // Create a mutation observer to wait for the content to render to the dom. - // Waiting on calls to TestAttributionManager is not sufficient because the + // Waiting on calls to `MockAttributionManager` is not sufficient because the // results are returned in promises. static constexpr char wait_script[] = R"( let status = document.getElementById("feature-status-content"); @@ -200,16 +220,17 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, // are properly handled as `bigint` values in JS and don't run into issues // with `Number.MAX_SAFE_INTEGER`. - manager_.SetActiveSourcesForWebUI( - {SourceBuilder(now) - .SetSourceEventId(std::numeric_limits::max()) - .SetAttributionLogic(StorableSource::AttributionLogic::kNever) - .Build(), - SourceBuilder(now + base::Hours(1)) - .SetSourceType(StorableSource::SourceType::kEvent) - .SetPriority(std::numeric_limits::max()) - .SetDedupKeys({13, 17}) - .Build()}); + ON_CALL(manager_, GetActiveSourcesForWebUI) + .WillByDefault(InvokeCallback>( + {SourceBuilder(now) + .SetSourceEventId(std::numeric_limits::max()) + .SetAttributionLogic(StorableSource::AttributionLogic::kNever) + .Build(), + SourceBuilder(now + base::Hours(1)) + .SetSourceType(StorableSource::SourceType::kEvent) + .SetPriority(std::numeric_limits::max()) + .SetDedupKeys({13, 17}) + .Build()})); manager_.NotifySourceDeactivated( DeactivatedSource(SourceBuilder(now + base::Hours(2)).Build(), @@ -264,7 +285,7 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, OverrideWebUIAttributionManager(); // Create a mutation observer to wait for the content to render to the dom. - // Waiting on calls to TestAttributionManager is not sufficient because the + // Waiting on calls to `MockAttributionManager` is not sufficient because the // results are returned in promises. static constexpr char wait_script[] = R"( let status = document.getElementById("debug-mode-content"); @@ -291,7 +312,7 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, OverrideWebUIAttributionManager(); // Create a mutation observer to wait for the content to render to the dom. - // Waiting on calls to TestAttributionManager is not sufficient because the + // Waiting on calls to `MockAttributionManager` is not sufficient because the // results are returned in promises. static constexpr char wait_script[] = R"( let status = document.getElementById("debug-mode-content"); @@ -333,15 +354,16 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, .Build(), SentReport::Status::kFailure, /*http_response_code=*/0)); - manager_.SetReportsForWebUI( - {ReportBuilder( - SourceBuilder(now) - .SetSourceType(StorableSource::SourceType::kEvent) - .SetAttributionLogic(StorableSource::AttributionLogic::kFalsely) - .Build()) - .SetReportTime(now) - .SetPriority(13) - .Build()}); + ON_CALL(manager_, GetPendingReportsForWebUI) + .WillByDefault(InvokeCallback>( + {ReportBuilder(SourceBuilder(now) + .SetSourceType(StorableSource::SourceType::kEvent) + .SetAttributionLogic( + StorableSource::AttributionLogic::kFalsely) + .Build()) + .SetReportTime(now) + .SetPriority(13) + .Build()})); manager_.NotifyReportDropped(AttributionStorage::CreateReportResult( CreateReportStatus::kPriorityTooLow, ReportBuilder(SourceBuilder(now).Build()) @@ -466,11 +488,18 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, .SetReportTime(now) .SetPriority(7) .Build(); - manager_.SetReportsForWebUI({report}); + EXPECT_CALL(manager_, GetPendingReportsForWebUI) + .WillOnce(InvokeCallback>({report})); + report.report_time += base::Hours(1); manager_.NotifyReportSent(SentReport(report, SentReport::Status::kSent, /*http_response_code=*/200)); + EXPECT_CALL(manager_, ClearData) + .WillOnce([](base::Time delete_begin, base::Time delete_end, + base::RepeatingCallback filter, + base::OnceClosure done) { std::move(done).Run(); }); + // Verify both rows get rendered. static constexpr char wait_script[] = R"( let table = document.querySelector("#report-table-wrapper tbody"); @@ -499,17 +528,20 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, EXPECT_EQ(kDeleteTitle, delete_title_watcher.WaitAndGetTitle()); } -// TODO(johnidel): Use a real AttributionManager here and verify that the -// reports are actually sent. IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest, WebUISendReports_ReportsRemoved) { EXPECT_TRUE(NavigateToURL(shell(), GURL(kAttributionInternalsUrl))); - AttributionReport report = - ReportBuilder(SourceBuilder(base::Time::Now()).Build()) - .SetPriority(7) - .Build(); - manager_.SetReportsForWebUI({report}); + EXPECT_CALL(manager_, GetPendingReportsForWebUI) + .WillOnce(InvokeCallback>( + {ReportBuilder(SourceBuilder(base::Time::Now()).Build()) + .SetPriority(7) + .Build()})) + .WillOnce(InvokeCallback>({})); + + EXPECT_CALL(manager_, SendReportsForWebUI) + .WillOnce([](base::OnceClosure done) { std::move(done).Run(); }); + OverrideWebUIAttributionManager(); static constexpr char wait_script[] = R"( diff --git a/content/browser/attribution_reporting/attribution_reporter_android_unittest.cc b/content/browser/attribution_reporting/attribution_reporter_android_unittest.cc index a5c3e6371ba74..d376ece3c0e19 100644 --- a/content/browser/attribution_reporting/attribution_reporter_android_unittest.cc +++ b/content/browser/attribution_reporting/attribution_reporter_android_unittest.cc @@ -23,8 +23,6 @@ namespace { using testing::_; using testing::AllOf; -using testing::ElementsAre; -using testing::IsEmpty; using testing::IsNull; using testing::Pointee; using testing::Property; @@ -45,7 +43,7 @@ class AttributionReporterTest : public ::testing::Test { } protected: - TestAttributionManager test_manager_; + MockAttributionManager mock_manager_; private: url::ScopedSchemeRegistryForTests scoped_registry_; @@ -53,30 +51,31 @@ class AttributionReporterTest : public ::testing::Test { TEST_F(AttributionReporterTest, ValidImpression_Allowed) { base::Time time = base::Time::Now() - base::Hours(1); + + EXPECT_CALL( + mock_manager_, + HandleSource(AllOf(Property(&StorableSource::impression_origin, + OriginFromAndroidPackageName(kPackageName)), + Property(&StorableSource::source_type, + StorableSource::SourceType::kEvent), + Property(&StorableSource::impression_time, time)))); + attribution_reporter_android::ReportAppImpression( - test_manager_, nullptr, kPackageName, kEventId, kConversionUrl, + mock_manager_, nullptr, kPackageName, kEventId, kConversionUrl, kReportToUrl, 56789, time); - - EXPECT_THAT( - test_manager_.handled_sources(), - ElementsAre(AllOf(Property(&StorableSource::impression_origin, - OriginFromAndroidPackageName(kPackageName)), - Property(&StorableSource::source_type, - StorableSource::SourceType::kEvent), - Property(&StorableSource::impression_time, time)))); } TEST_F(AttributionReporterTest, ValidImpression_Allowed_NoOptionals) { + EXPECT_CALL( + mock_manager_, + HandleSource(AllOf(Property(&StorableSource::impression_origin, + OriginFromAndroidPackageName(kPackageName)), + Property(&StorableSource::source_type, + StorableSource::SourceType::kEvent)))); + attribution_reporter_android::ReportAppImpression( - test_manager_, nullptr, kPackageName, kEventId, kConversionUrl, "", 0, + mock_manager_, nullptr, kPackageName, kEventId, kConversionUrl, "", 0, base::Time::Now()); - - EXPECT_THAT( - test_manager_.handled_sources(), - ElementsAre(AllOf(Property(&StorableSource::impression_origin, - OriginFromAndroidPackageName(kPackageName)), - Property(&StorableSource::source_type, - StorableSource::SourceType::kEvent)))); } TEST_F(AttributionReporterTest, ValidImpression_Disallowed) { @@ -89,19 +88,19 @@ TEST_F(AttributionReporterTest, ValidImpression_Disallowed) { .WillOnce(Return(false)); ScopedContentBrowserClientSetting setting(&browser_client); + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + attribution_reporter_android::ReportAppImpression( - test_manager_, nullptr, kPackageName, kEventId, kConversionUrl, + mock_manager_, nullptr, kPackageName, kEventId, kConversionUrl, kReportToUrl, 56789, base::Time::Now()); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } TEST_F(AttributionReporterTest, InvalidImpression) { + EXPECT_CALL(mock_manager_, HandleSource).Times(0); + attribution_reporter_android::ReportAppImpression( - test_manager_, nullptr, kPackageName, kEventId, kInvalidUrl, kReportToUrl, + mock_manager_, nullptr, kPackageName, kEventId, kInvalidUrl, kReportToUrl, 56789, base::Time::Now()); - - EXPECT_THAT(test_manager_.handled_sources(), IsEmpty()); } } // namespace diff --git a/content/browser/attribution_reporting/attribution_test_utils.cc b/content/browser/attribution_reporting/attribution_test_utils.cc index 4d72709d56578..3fa1769cd1d23 100644 --- a/content/browser/attribution_reporting/attribution_test_utils.cc +++ b/content/browser/attribution_reporting/attribution_test_utils.cc @@ -113,97 +113,49 @@ AttributionManager* TestManagerProvider::GetManager( return manager_; } -TestAttributionManager::TestAttributionManager() = default; +MockAttributionManager::MockAttributionManager() = default; -TestAttributionManager::~TestAttributionManager() = default; +MockAttributionManager::~MockAttributionManager() = default; -void TestAttributionManager::AddObserver(Observer* observer) { +void MockAttributionManager::AddObserver(Observer* observer) { observers_.AddObserver(observer); } -void TestAttributionManager::RemoveObserver(Observer* observer) { +void MockAttributionManager::RemoveObserver(Observer* observer) { observers_.RemoveObserver(observer); } -void TestAttributionManager::HandleSource(StorableSource source) { - handled_sources_.push_back(std::move(source)); -} - -void TestAttributionManager::HandleTrigger(StorableTrigger trigger) { - handled_triggers_.push_back(std::move(trigger)); -} - -void TestAttributionManager::GetActiveSourcesForWebUI( - base::OnceCallback)> callback) { - std::move(callback).Run(sources_); -} - -void TestAttributionManager::GetPendingReportsForWebUI( - base::OnceCallback)> callback) { - std::move(callback).Run(reports_); -} - -void TestAttributionManager::SendReportsForWebUI(base::OnceClosure done) { - reports_.clear(); - std::move(done).Run(); -} - -const AttributionPolicy& TestAttributionManager::GetAttributionPolicy() const { +const AttributionPolicy& MockAttributionManager::GetAttributionPolicy() const { return policy_; } -void TestAttributionManager::ClearData( - base::Time delete_begin, - base::Time delete_end, - base::RepeatingCallback filter, - base::OnceClosure done) { - sources_.clear(); - reports_.clear(); - std::move(done).Run(); -} - -void TestAttributionManager::SetActiveSourcesForWebUI( - std::vector sources) { - sources_ = std::move(sources); -} - -void TestAttributionManager::SetReportsForWebUI( - std::vector reports) { - reports_ = std::move(reports); -} - -void TestAttributionManager::NotifySourcesChanged() { +void MockAttributionManager::NotifySourcesChanged() { for (Observer& observer : observers_) observer.OnSourcesChanged(); } -void TestAttributionManager::NotifyReportsChanged() { +void MockAttributionManager::NotifyReportsChanged() { for (Observer& observer : observers_) observer.OnReportsChanged(); } -void TestAttributionManager::NotifySourceDeactivated( +void MockAttributionManager::NotifySourceDeactivated( const DeactivatedSource& source) { for (Observer& observer : observers_) observer.OnSourceDeactivated(source); } -void TestAttributionManager::NotifyReportSent(const SentReport& info) { +void MockAttributionManager::NotifyReportSent(const SentReport& info) { for (Observer& observer : observers_) observer.OnReportSent(info); } -void TestAttributionManager::NotifyReportDropped( +void MockAttributionManager::NotifyReportDropped( const AttributionStorage::CreateReportResult& result) { for (Observer& observer : observers_) observer.OnReportDropped(result); } -void TestAttributionManager::Reset() { - handled_sources_.clear(); - handled_triggers_.clear(); -} - // Builds an impression with default values. This is done as a builder because // all values needed to be provided at construction time. SourceBuilder::SourceBuilder(base::Time time) diff --git a/content/browser/attribution_reporting/attribution_test_utils.h b/content/browser/attribution_reporting/attribution_test_utils.h index 644e4a476aead..c5bc8b59e2a69 100644 --- a/content/browser/attribution_reporting/attribution_test_utils.h +++ b/content/browser/attribution_reporting/attribution_test_utils.h @@ -157,32 +157,40 @@ class TestManagerProvider : public AttributionManager::Provider { raw_ptr manager_ = nullptr; }; -// Test AttributionManager which can be injected into tests to monitor calls to -// a AttributionManager instance. -class TestAttributionManager : public AttributionManager { +class MockAttributionManager : public AttributionManager { public: - TestAttributionManager(); - ~TestAttributionManager() override; + MockAttributionManager(); + ~MockAttributionManager() override; // AttributionManager: + MOCK_METHOD(void, HandleSource, (StorableSource source), (override)); + + MOCK_METHOD(void, HandleTrigger, (StorableTrigger trigger), (override)); + + MOCK_METHOD(void, + GetActiveSourcesForWebUI, + (base::OnceCallback)> callback), + (override)); + + MOCK_METHOD( + void, + GetPendingReportsForWebUI, + (base::OnceCallback)> callback), + (override)); + + MOCK_METHOD(void, SendReportsForWebUI, (base::OnceClosure done), (override)); + + MOCK_METHOD(void, + ClearData, + (base::Time delete_begin, + base::Time delete_end, + base::RepeatingCallback filter, + base::OnceClosure done), + (override)); + void AddObserver(Observer* observer) override; void RemoveObserver(Observer* observer) override; - void HandleSource(StorableSource source) override; - void HandleTrigger(StorableTrigger trigger) override; - void GetActiveSourcesForWebUI( - base::OnceCallback)> callback) override; - void GetPendingReportsForWebUI( - base::OnceCallback)> callback) - override; - void SendReportsForWebUI(base::OnceClosure done) override; const AttributionPolicy& GetAttributionPolicy() const override; - void ClearData(base::Time delete_begin, - base::Time delete_end, - base::RepeatingCallback filter, - base::OnceClosure done) override; - - void SetActiveSourcesForWebUI(std::vector sources); - void SetReportsForWebUI(std::vector reports); void NotifySourcesChanged(); void NotifyReportsChanged(); @@ -192,28 +200,8 @@ class TestAttributionManager : public AttributionManager { void NotifyReportDropped( const AttributionStorage::CreateReportResult& result); - // Resets all counters on this. - void Reset(); - - const std::vector& handled_sources() const - WARN_UNUSED_RESULT { - return handled_sources_; - } - - const std::vector& handled_triggers() const - WARN_UNUSED_RESULT { - return handled_triggers_; - } - private: AttributionPolicy policy_; - - std::vector handled_sources_; - std::vector handled_triggers_; - - std::vector sources_; - std::vector reports_; - base::ObserverList observers_; };