diff --git a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.cc b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.cc index 086d83877151..08e73f947c99 100644 --- a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.cc +++ b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.cc @@ -5,6 +5,9 @@ #include "brave/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.h" +#include +#include + #include "base/check.h" #include "base/check_is_test.h" #include "base/functional/bind.h" @@ -13,6 +16,8 @@ #include "brave/browser/brave_ads/ads_service_factory.h" #include "brave/components/brave_ads/browser/ads_service.h" #include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.h" +#include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_url_placement_id_extractor.h" +#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h" #include "brave/components/brave_ads/core/public/ads_feature.h" #include "brave/components/brave_rewards/common/pref_names.h" #include "brave/components/brave_search/common/brave_search_utils.h" @@ -97,13 +102,6 @@ bool CreativeSearchResultAdTabHelper::ShouldHandleCreativeAdEvents() const { return profile->GetPrefs()->GetBoolean(brave_rewards::prefs::kEnabled); } -void CreativeSearchResultAdTabHelper::MaybeTriggerCreativeAdClickedEvent( - const GURL& url) { - if (creative_search_result_ad_handler_) { - creative_search_result_ad_handler_->MaybeTriggerCreativeAdClickedEvent(url); - } -} - /////////////////////////////////////////////////////////////////////////////// AdsService* CreativeSearchResultAdTabHelper::GetAdsService() const { @@ -156,14 +154,16 @@ void CreativeSearchResultAdTabHelper:: } void CreativeSearchResultAdTabHelper::MaybeHandleCreativeAdViewedEvents( - const std::vector placement_ids) { - for (const auto& placement_id : placement_ids) { - MaybeHandleCreativeAdViewedEvent(placement_id); + std::vector + creative_search_result_ads) { + for (auto& creative_search_result_ad : creative_search_result_ads) { + MaybeHandleCreativeAdViewedEvent(std::move(creative_search_result_ad)); } } void CreativeSearchResultAdTabHelper::MaybeHandleCreativeAdViewedEvent( - const std::string& placement_id) { + mojom::CreativeSearchResultAdInfoPtr creative_search_result_ad) { + const std::string& placement_id = creative_search_result_ad->placement_id; CHECK(!placement_id.empty()); // It is safe to pass `placement_id` directly to the JavaScript function as it @@ -175,12 +175,13 @@ void CreativeSearchResultAdTabHelper::MaybeHandleCreativeAdViewedEvent( base::UTF8ToUTF16(javascript), base::BindOnce(&CreativeSearchResultAdTabHelper:: MaybeHandleCreativeAdViewedEventCallback, - weak_factory_.GetWeakPtr(), placement_id), + weak_factory_.GetWeakPtr(), + std::move(creative_search_result_ad)), ISOLATED_WORLD_ID_BRAVE_INTERNAL); } void CreativeSearchResultAdTabHelper::MaybeHandleCreativeAdViewedEventCallback( - const std::string& placement_id, + mojom::CreativeSearchResultAdInfoPtr creative_search_result_ad, const base::Value value) { const bool is_visible = value.is_bool() && value.GetBool(); if (!is_visible) { @@ -190,7 +191,7 @@ void CreativeSearchResultAdTabHelper::MaybeHandleCreativeAdViewedEventCallback( if (creative_search_result_ad_handler_) { creative_search_result_ad_handler_->MaybeTriggerCreativeAdViewedEvent( - placement_id); + std::move(creative_search_result_ad)); } } @@ -202,35 +203,23 @@ void CreativeSearchResultAdTabHelper::MaybeHandleCreativeAdClickedEvent( return; } - const auto& initiator_origin = navigation_handle->GetInitiatorOrigin(); - if (!navigation_handle->IsInPrimaryMainFrame() || - !ui::PageTransitionCoreTypeIs(navigation_handle->GetPageTransition(), - ui::PAGE_TRANSITION_LINK) || - !initiator_origin || - !brave_search::IsAllowedHost(initiator_origin->GetURL())) { + AdsService* ads_service = GetAdsService(); + if (!ads_service) { return; } - content::WebContents* web_contents = navigation_handle->GetWebContents(); - if (!web_contents) { - return; - } - - if (content::WebContents* original_opener_web_contents = - web_contents->GetFirstWebContentsInLiveOriginalOpenerChain()) { - web_contents = original_opener_web_contents; - } + CHECK(!navigation_handle->GetRedirectChain().empty()); + const GURL url = navigation_handle->GetRedirectChain().back(); - CreativeSearchResultAdTabHelper* const creative_search_result_ad_tab_helper = - CreativeSearchResultAdTabHelper::FromWebContents(web_contents); - if (!creative_search_result_ad_tab_helper) { + const std::optional placement_id = + MaybeExtractCreativeAdPlacementIdFromUrl(url); + if (!placement_id || placement_id->empty()) { + // The URL does not contain a placement id. return; } - CHECK(!navigation_handle->GetRedirectChain().empty()); - const GURL url = navigation_handle->GetRedirectChain().back(); - - creative_search_result_ad_tab_helper->MaybeTriggerCreativeAdClickedEvent(url); + ads_service->TriggerSearchResultAdClickedEvent( + *placement_id, /*intentional*/ base::DoNothing()); } void CreativeSearchResultAdTabHelper::DidStartNavigation( diff --git a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.h b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.h index dbd6dedb3e99..3431eb3c267c 100644 --- a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.h +++ b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper.h @@ -7,15 +7,13 @@ #define BRAVE_BROWSER_BRAVE_ADS_CREATIVES_SEARCH_RESULT_AD_CREATIVE_SEARCH_RESULT_AD_TAB_HELPER_H_ #include -#include #include #include "base/memory/weak_ptr.h" +#include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_user_data.h" -class GURL; - namespace brave_ads { class AdsService; @@ -39,8 +37,6 @@ class CreativeSearchResultAdTabHelper bool ShouldHandleCreativeAdEvents() const; - void MaybeTriggerCreativeAdClickedEvent(const GURL& url); - private: friend class content::WebContentsUserData; @@ -52,10 +48,13 @@ class CreativeSearchResultAdTabHelper void MaybeExtractCreativeAdPlacementIdsFromWebPageAndHandleViewedEvents(); void MaybeHandleCreativeAdViewedEvents( - std::vector placement_ids); - void MaybeHandleCreativeAdViewedEvent(const std::string& placement_id); - void MaybeHandleCreativeAdViewedEventCallback(const std::string& placement_id, - const base::Value value); + std::vector + creative_search_result_ads); + void MaybeHandleCreativeAdViewedEvent( + mojom::CreativeSearchResultAdInfoPtr creative_search_result_ad); + void MaybeHandleCreativeAdViewedEventCallback( + mojom::CreativeSearchResultAdInfoPtr creative_search_result_ad, + const base::Value value); void MaybeHandleCreativeAdClickedEvent( content::NavigationHandle* navigation_handle); diff --git a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper_browsertest.cc b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper_browsertest.cc index 4c25ee55ab18..7957f935d815 100644 --- a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper_browsertest.cc +++ b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_tab_helper_browsertest.cc @@ -47,6 +47,10 @@ constexpr char kTargetDomain[] = "example.com"; constexpr char kTargetPath[] = "/simple.html"; constexpr char kSearchResultUrlPath[] = "/brave_ads/creative_search_result_ad.html"; +constexpr char kSearchResultClickedUrlPath[] = + "/a/" + "redirect?click_url=https%3A%2F%2Fexample.com%2Fsimple.html&placement_id=" + "824657d0-eaed-4b80-8a42-a18c12f2977d"; // Placement IDs are defined in the `search_result_ad_sample.html` file. constexpr auto kCreativeAdPlacementIdToIndex = @@ -129,6 +133,8 @@ class BraveAdsCreativeSearchResultAdTabHelperTest return http_response; } + net::EmbeddedTestServer& https_server() { return https_server_; } + PrefService* GetPrefs() { return browser()->profile()->GetPrefs(); } AdsServiceMock& ads_service() { return ads_service_mock_; } @@ -145,7 +151,8 @@ IN_PROC_BROWSER_TEST_F(BraveAdsCreativeSearchResultAdTabHelperTest, GetPrefs()->SetBoolean(brave_rewards::prefs::kEnabled, false); ScopedTestingAdsServiceSetter scoped_setter(&ads_service()); - EXPECT_CALL(ads_service(), TriggerSearchResultAdEvent).Times(0); + EXPECT_CALL(ads_service(), TriggerSearchResultAdViewedImpressionEvent) + .Times(0); const GURL url = GetURL(kAllowedDomain, kSearchResultUrlPath); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); @@ -160,7 +167,8 @@ IN_PROC_BROWSER_TEST_F(BraveAdsCreativeSearchResultAdTabHelperTest, GetPrefs()->SetBoolean(brave_rewards::prefs::kEnabled, true); - EXPECT_CALL(ads_service(), TriggerSearchResultAdEvent).Times(0); + EXPECT_CALL(ads_service(), TriggerSearchResultAdViewedImpressionEvent) + .Times(0); const GURL url = GetURL(kNotAllowedDomain, kSearchResultUrlPath); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); @@ -175,7 +183,8 @@ IN_PROC_BROWSER_TEST_F(BraveAdsCreativeSearchResultAdTabHelperTest, GetPrefs()->SetBoolean(brave_rewards::prefs::kEnabled, true); - EXPECT_CALL(ads_service(), TriggerSearchResultAdEvent).Times(0); + EXPECT_CALL(ads_service(), TriggerSearchResultAdViewedImpressionEvent) + .Times(0); const GURL url = GetURL(kAllowedDomain, "/brave_ads/invalid_creative_search_result_ad"); @@ -255,16 +264,12 @@ class SampleBraveAdsCreativeSearchResultAdTabHelperTest const GURL& url) { auto run_loop1 = std::make_unique(); auto run_loop2 = std::make_unique(); - EXPECT_CALL( - ads_service(), - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) + EXPECT_CALL(ads_service(), TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .Times(2) .WillRepeatedly( [this, &run_loop1, &run_loop2]( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { ASSERT_TRUE(mojom_creative_ad); @@ -305,22 +310,15 @@ IN_PROC_BROWSER_TEST_F(SampleBraveAdsCreativeSearchResultAdTabHelperTest, LoadAndCheckSampleSearchResultAdWebPage(GetSearchResultUrl()); base::RunLoop run_loop; - EXPECT_CALL(ads_service(), TriggerSearchResultAdEvent) - .WillOnce([this, &run_loop]( - mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, - TriggerAdEventCallback callback) { - EXPECT_EQ(mojom_ad_event_type, - mojom::SearchResultAdEventType::kClicked); - const auto iter = - kCreativeAdPlacementIdToIndex.find(mojom_creative_ad->placement_id); + EXPECT_CALL(ads_service(), TriggerSearchResultAdClickedEvent) + .WillOnce([&run_loop](const std::string& placement_id, + TriggerAdEventCallback callback) { + const auto iter = kCreativeAdPlacementIdToIndex.find(placement_id); ASSERT_TRUE(iter != kCreativeAdPlacementIdToIndex.cend()); const size_t ad_index = iter->second; // We clicked on the first ad in `search_result_ad_sample.html`. EXPECT_EQ(1u, ad_index); - VerifyCreativeAdMetadataExpectations( - mojom_creative_ad, mojom_creative_ad->placement_id, ad_index); run_loop.Quit(); }); @@ -339,22 +337,14 @@ IN_PROC_BROWSER_TEST_F(SampleBraveAdsCreativeSearchResultAdTabHelperTest, LoadAndCheckSampleSearchResultAdWebPage(GetSearchResultUrl()); base::RunLoop run_loop; - EXPECT_CALL(ads_service(), TriggerSearchResultAdEvent) - .WillOnce([this, &run_loop]( - mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, - TriggerAdEventCallback callback) { - EXPECT_EQ(mojom_ad_event_type, - mojom::SearchResultAdEventType::kClicked); - const auto iter = - kCreativeAdPlacementIdToIndex.find(mojom_creative_ad->placement_id); + EXPECT_CALL(ads_service(), TriggerSearchResultAdClickedEvent) + .WillOnce([&run_loop](const std::string& placement_id, + TriggerAdEventCallback callback) { + const auto iter = kCreativeAdPlacementIdToIndex.find(placement_id); ASSERT_TRUE(iter != kCreativeAdPlacementIdToIndex.cend()); const size_t ad_index = iter->second; // We clicked on the second ad in `search_result_ad_sample.html`. EXPECT_EQ(2u, ad_index); - - VerifyCreativeAdMetadataExpectations( - mojom_creative_ad, mojom_creative_ad->placement_id, ad_index); run_loop.Quit(); }); @@ -363,4 +353,62 @@ IN_PROC_BROWSER_TEST_F(SampleBraveAdsCreativeSearchResultAdTabHelperTest, run_loop.Run(); } +IN_PROC_BROWSER_TEST_F(SampleBraveAdsCreativeSearchResultAdTabHelperTest, + SearchResultAdOpenedInNewTabByRightClick) { + ScopedTestingAdsServiceSetter scoped_setter(&ads_service()); + + GetPrefs()->SetBoolean(brave_rewards::prefs::kEnabled, true); + + LoadAndCheckSampleSearchResultAdWebPage(GetSearchResultUrl()); + + base::RunLoop run_loop; + EXPECT_CALL(ads_service(), TriggerSearchResultAdClickedEvent) + .WillOnce([&run_loop](const std::string& placement_id, + TriggerAdEventCallback callback) { + const auto iter = kCreativeAdPlacementIdToIndex.find(placement_id); + ASSERT_TRUE(iter != kCreativeAdPlacementIdToIndex.cend()); + const size_t ad_index = iter->second; + // We clicked on the first ad in `search_result_ad_sample.html`. + EXPECT_EQ(1u, ad_index); + run_loop.Quit(); + }); + + const GURL url = + https_server().GetURL(kAllowedDomain, kSearchResultClickedUrlPath); + ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition( + browser(), url, WindowOpenDisposition::NEW_BACKGROUND_TAB, + ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP)); + + run_loop.Run(); +} + +IN_PROC_BROWSER_TEST_F(SampleBraveAdsCreativeSearchResultAdTabHelperTest, + SearchResultAdOpenedInNewWindow) { + ScopedTestingAdsServiceSetter scoped_setter(&ads_service()); + + GetPrefs()->SetBoolean(brave_rewards::prefs::kEnabled, true); + + LoadAndCheckSampleSearchResultAdWebPage(GetSearchResultUrl()); + + base::RunLoop run_loop; + EXPECT_CALL(ads_service(), TriggerSearchResultAdClickedEvent) + .WillOnce([&run_loop](const std::string& placement_id, + TriggerAdEventCallback callback) { + const auto iter = kCreativeAdPlacementIdToIndex.find(placement_id); + ASSERT_TRUE(iter != kCreativeAdPlacementIdToIndex.cend()); + const size_t ad_index = iter->second; + // We clicked on the first ad in `search_result_ad_sample.html`. + EXPECT_EQ(1u, ad_index); + run_loop.Quit(); + }); + + const GURL url = + https_server().GetURL(kAllowedDomain, kSearchResultClickedUrlPath); + ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition( + browser(), url, WindowOpenDisposition::NEW_WINDOW, + ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP)); + + run_loop.Run(); +} + } // namespace brave_ads diff --git a/components/brave_ads/browser/ads_service.h b/components/brave_ads/browser/ads_service.h index e00a661858c2..1307862d287f 100644 --- a/components/brave_ads/browser/ads_service.h +++ b/components/brave_ads/browser/ads_service.h @@ -133,14 +133,22 @@ class AdsService : public KeyedService { mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) = 0; - // Called when a user views or interacts with a search result ad to trigger a - // `mojom_ad_event_type` event for the ad specified in `mojom_creative_ad`. + // Called when a user views a search result ad to trigger a `viewed + // impression` event for the ad specified in `mojom_creative_ad`. The callback + // takes one argument - `bool` is set to `true` if successful otherwise + // `false`. Must be called before the + // `mojom::CreativeSearchResultAdInfo::target_url` landing page is opened. + virtual void TriggerSearchResultAdViewedImpressionEvent( + mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, + TriggerAdEventCallback callback) = 0; + + // Called when a user interacts with a search result ad to trigger a + // `clicked` event for the ad specified in `mojom_creative_ad`. // The callback takes one argument - `bool` is set to `true` if successful // otherwise `false`. Must be called before the // `mojom::CreativeSearchResultAdInfo::target_url` landing page is opened. - virtual void TriggerSearchResultAdEvent( - mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType mojom_ad_event_type, + virtual void TriggerSearchResultAdClickedEvent( + const std::string& placement_id, TriggerAdEventCallback callback) = 0; // Called to purge orphaned served ad events for the specified `mojom_ad_type` diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc index 8ae3ee00bb5c..40efd223e2f1 100644 --- a/components/brave_ads/browser/ads_service_impl.cc +++ b/components/brave_ads/browser/ads_service_impl.cc @@ -5,6 +5,7 @@ #include "brave/components/brave_ads/browser/ads_service_impl.h" +#include #include #include "base/base64.h" @@ -1314,18 +1315,26 @@ void AdsServiceImpl::TriggerPromotedContentAdEvent( std::move(callback)); } -void AdsServiceImpl::TriggerSearchResultAdEvent( +void AdsServiceImpl::TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - CHECK(mojom::IsKnownEnumValue(mojom_ad_event_type)); + if (!bat_ads_associated_remote_.is_bound()) { + return std::move(callback).Run(/*success*/ false); + } + bat_ads_associated_remote_->TriggerSearchResultAdViewedImpressionEvent( + std::move(mojom_creative_ad), std::move(callback)); +} + +void AdsServiceImpl::TriggerSearchResultAdClickedEvent( + const std::string& placement_id, + TriggerAdEventCallback callback) { if (!bat_ads_associated_remote_.is_bound()) { return std::move(callback).Run(/*success*/ false); } - bat_ads_associated_remote_->TriggerSearchResultAdEvent( - std::move(mojom_creative_ad), mojom_ad_event_type, std::move(callback)); + bat_ads_associated_remote_->TriggerSearchResultAdClickedEvent( + placement_id, std::move(callback)); } void AdsServiceImpl::PurgeOrphanedAdEventsForType( diff --git a/components/brave_ads/browser/ads_service_impl.h b/components/brave_ads/browser/ads_service_impl.h index 34111bc6ed2a..9ad9a6e4fff2 100644 --- a/components/brave_ads/browser/ads_service_impl.h +++ b/components/brave_ads/browser/ads_service_impl.h @@ -252,9 +252,11 @@ class AdsServiceImpl final : public AdsService, mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) override; - void TriggerSearchResultAdEvent( + void TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType mojom_ad_event_type, + TriggerAdEventCallback callback) override; + void TriggerSearchResultAdClickedEvent( + const std::string& placement_id, TriggerAdEventCallback callback) override; void PurgeOrphanedAdEventsForType( diff --git a/components/brave_ads/browser/ads_service_mock.h b/components/brave_ads/browser/ads_service_mock.h index 6761465befc5..95671bceb0f5 100644 --- a/components/brave_ads/browser/ads_service_mock.h +++ b/components/brave_ads/browser/ads_service_mock.h @@ -78,10 +78,12 @@ class AdsServiceMock : public AdsService { TriggerAdEventCallback)); MOCK_METHOD(void, - TriggerSearchResultAdEvent, - (mojom::CreativeSearchResultAdInfoPtr, - const mojom::SearchResultAdEventType, - TriggerAdEventCallback)); + TriggerSearchResultAdViewedImpressionEvent, + (mojom::CreativeSearchResultAdInfoPtr, TriggerAdEventCallback)); + + MOCK_METHOD(void, + TriggerSearchResultAdClickedEvent, + (const std::string& placement_id, TriggerAdEventCallback)); MOCK_METHOD(void, PurgeOrphanedAdEventsForType, diff --git a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.cc b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.cc index fab6bdf18074..dc786aebce15 100644 --- a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.cc +++ b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.cc @@ -5,7 +5,6 @@ #include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.h" -#include #include #include "base/functional/bind.h" @@ -76,34 +75,6 @@ void CreativeSearchResultAdHandler:: std::move(callback))); } -void CreativeSearchResultAdHandler::MaybeTriggerCreativeAdClickedEvent( - const GURL& url) { - if (!creative_search_result_ads_) { - // No creative search result ads are present on the web page. - return; - } - - const std::optional placement_id = - MaybeExtractCreativeAdPlacementIdFromUrl(url); - if (!placement_id || placement_id->empty()) { - // The URL does not contain a placement id. - return; - } - - const auto iter = creative_search_result_ads_->find(*placement_id); - if (iter == creative_search_result_ads_->cend()) { - // The placement id does not match any creative search result ad. - return; - } - const auto& [_, creative_search_result_ad] = *iter; - CHECK(creative_search_result_ad); - - ads_service_->TriggerSearchResultAdEvent( - creative_search_result_ad->Clone(), - mojom::SearchResultAdEventType::kClicked, - /*intentional*/ base::DoNothing()); -} - /////////////////////////////////////////////////////////////////////////////// void CreativeSearchResultAdHandler:: @@ -116,47 +87,26 @@ void CreativeSearchResultAdHandler:: return std::move(callback).Run(/*placement_ids=*/{}); } - creative_search_result_ads_ = + std::vector creative_search_result_ads = ExtractCreativeSearchResultAdsFromMojomWebPageEntities( mojom_web_page->entities); - if (!creative_search_result_ads_) { - return std::move(callback).Run(/*placement_ids=*/{}); - } - std::vector placement_ids; - base::ranges::transform(*creative_search_result_ads_, - std::back_inserter(placement_ids), - [](const auto& creative_search_result_ad) { - return creative_search_result_ad.first; - }); - - std::move(callback).Run(std::move(placement_ids)); + std::move(callback).Run(std::move(creative_search_result_ads)); } void CreativeSearchResultAdHandler::MaybeTriggerCreativeAdViewedEvent( - const std::string& placement_id) { - CHECK(!placement_id.empty()); - + mojom::CreativeSearchResultAdInfoPtr creative_search_result_ad) { if (!should_trigger_creative_ad_viewed_events_) { return; } - if (!creative_search_result_ads_) { + if (!creative_search_result_ad) { // No creative search result ads are present on the web page. return; } - const auto iter = creative_search_result_ads_->find(placement_id); - if (iter == creative_search_result_ads_->cend()) { - // The placement id does not match any creative search result ad. - return; - } - const auto& [_, creative_search_result_ad] = *iter; - CHECK(creative_search_result_ad); - - ads_service_->TriggerSearchResultAdEvent( - creative_search_result_ad->Clone(), - mojom::SearchResultAdEventType::kViewedImpression, + ads_service_->TriggerSearchResultAdViewedImpressionEvent( + std::move(creative_search_result_ad), /*intentional*/ base::DoNothing()); } diff --git a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.h b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.h index ef398375e487..bd09bbe7cb13 100644 --- a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.h +++ b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler.h @@ -7,14 +7,13 @@ #define BRAVE_COMPONENTS_BRAVE_ADS_CONTENT_BROWSER_CREATIVES_SEARCH_RESULT_AD_CREATIVE_SEARCH_RESULT_AD_HANDLER_H_ #include -#include -#include #include #include "base/functional/callback_forward.h" #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" #include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.h" +#include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" #include "mojo/public/cpp/bindings/remote.h" #include "third_party/blink/public/mojom/document_metadata/document_metadata.mojom-forward.h" @@ -27,7 +26,8 @@ class RenderFrameHost; namespace brave_ads { using ExtractCreativeAdPlacementIdsFromWebPageCallback = - base::OnceCallback placement_ids)>; + base::OnceCallback + creative_search_result_ads)>; class AdsService; @@ -52,8 +52,8 @@ class CreativeSearchResultAdHandler final { content::RenderFrameHost* render_frame_host, ExtractCreativeAdPlacementIdsFromWebPageCallback callback); - void MaybeTriggerCreativeAdViewedEvent(const std::string& placement_id); - void MaybeTriggerCreativeAdClickedEvent(const GURL& url); + void MaybeTriggerCreativeAdViewedEvent( + mojom::CreativeSearchResultAdInfoPtr creative_search_result_ad); private: friend class BraveAdsCreativeSearchResultAdHandlerTest; @@ -71,8 +71,6 @@ class CreativeSearchResultAdHandler final { const bool should_trigger_creative_ad_viewed_events_; - std::optional creative_search_result_ads_; - base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler_unittest.cc b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler_unittest.cc index 20d523f0b50d..e707085e2fc0 100644 --- a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler_unittest.cc +++ b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_handler_unittest.cc @@ -33,29 +33,6 @@ namespace { constexpr char kAllowedDomain[] = "https://search.brave.com"; constexpr char kDisallowedDomain[] = "https://brave.com"; -GURL ClickRedirectUrl() { - return GURL(base::StrCat({"https://search.brave.com/a/redirect?placement_id=", - test::kCreativeAdPlacementId})); -} - -GURL ClickRedirectUrlWithUnreservedCharactersInPlacementId() { - return GURL(base::StrCat( - {"https://search.brave.com/a/redirect?placement_id=", - test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters})); -} - -GURL InvalidClickRedirectUrlWithNoQueryValue() { - return GURL("https://search.brave.com/a/redirect?placement_id"); -} - -GURL InvalidClickRedirectUrlWithMismatchingQueryNameAndValue() { - return GURL("https://search.brave.com/a/redirect?foo=bar"); -} - -GURL InvalidClickRedirectUrlWithNoQueryNameOrValue() { - return GURL("https://search.brave.com/a/redirect"); -} - } // namespace class BraveAdsCreativeSearchResultAdHandlerTest : public ::testing::Test { @@ -74,10 +51,12 @@ class BraveAdsCreativeSearchResultAdHandlerTest : public ::testing::Test { callback; EXPECT_CALL(callback, Run) .WillOnce([creative_search_result_ad_handler]( - const std::vector& placement_ids) { - for (const std::string& placement_id : placement_ids) { + std::vector + creative_search_result_ads) { + for (auto& creative_search_result_ad : creative_search_result_ads) { creative_search_result_ad_handler - ->MaybeTriggerCreativeAdViewedEvent(placement_id); + ->MaybeTriggerCreativeAdViewedEvent( + std::move(creative_search_result_ad)); } }); @@ -142,23 +121,12 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(mojom_web_page); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .Times(0); - EXPECT_CALL(ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, - ::testing::_)); - SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), std::move(mojom_web_page)); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); } TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, @@ -171,24 +139,12 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(creative_search_result_ad_handler); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) - .Times(0); - - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .Times(0); SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), blink::mojom::WebPagePtr()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); } TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, @@ -201,24 +157,12 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(creative_search_result_ad_handler); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) - .Times(0); - - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .Times(0); SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), blink::mojom::WebPage::New()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); } TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, @@ -231,26 +175,14 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(creative_search_result_ad_handler); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) - .Times(0); - - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .Times(0); SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), test::CreativeSearchResultAdMojomWebPage( /*excluded_property_names=*/{kCreativeAdRewardsValuePropertyName})); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); } TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, @@ -263,30 +195,18 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(creative_search_result_ad_handler); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .WillOnce([](mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType /*mojom_ad_event_type*/, TriggerAdEventCallback /*callback*/) { EXPECT_FALSE(mojom_creative_ad->creative_set_conversion); }); - EXPECT_CALL(ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, - ::testing::_)); - SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), test::CreativeSearchResultAdMojomWebPage( /*excluded_property_names=*/{ kCreativeSetConversionUrlPatternPropertyName})); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); } TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, @@ -303,90 +223,24 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(mojom_web_page); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .WillOnce([&mojom_web_page]( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType /*mojom_ad_event_type*/, TriggerAdEventCallback /*callback*/) { - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page->entities); - ASSERT_TRUE( - creative_search_result_ads.contains(test::kCreativeAdPlacementId)); - - EXPECT_EQ(creative_search_result_ads.at(test::kCreativeAdPlacementId), - mojom_creative_ad); - }); - - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, ::testing::_)) - .Times(2) - .WillRepeatedly( - [&mojom_web_page]( - mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType /*mojom_ad_event_type*/, - TriggerAdEventCallback /*callback*/) { - const CreativeSearchResultAdMap creative_search_result_ads = + const std::vector + creative_search_result_ads = ExtractCreativeSearchResultAdsFromMojomWebPageEntities( mojom_web_page->entities); - ASSERT_TRUE(creative_search_result_ads.contains( - test::kCreativeAdPlacementId)); + ASSERT_EQ(creative_search_result_ads.size(), 1u); + ASSERT_EQ(creative_search_result_ads[0]->placement_id, + test::kCreativeAdPlacementId); - EXPECT_EQ( - creative_search_result_ads.at(test::kCreativeAdPlacementId), - mojom_creative_ad); - }); + EXPECT_EQ(creative_search_result_ads[0], mojom_creative_ad); + }); SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), mojom_web_page->Clone()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrl()); -} - -TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, - DoNotTriggerClickedAdEventsForInvalidRedirectUrls) { - // Arrange - const auto creative_search_result_ad_handler = - CreativeSearchResultAdHandler::MaybeCreate( - &ads_service_mock_, GURL(kAllowedDomain), - /*should_trigger_creative_ad_viewed_events=*/true); - ASSERT_TRUE(creative_search_result_ad_handler); - - // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)); - - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, ::testing::_)) - .Times(0); - - SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( - creative_search_result_ad_handler.get(), - test::CreativeSearchResultAdMojomWebPage(/*excluded_property_names=*/{})); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - InvalidClickRedirectUrlWithNoQueryValue()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - InvalidClickRedirectUrlWithMismatchingQueryNameAndValue()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - InvalidClickRedirectUrlWithNoQueryNameOrValue()); } TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, @@ -405,52 +259,24 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, ASSERT_TRUE(mojom_web_page); // Act & Assert - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kViewedImpression, - ::testing::_)) + EXPECT_CALL(ads_service_mock_, TriggerSearchResultAdViewedImpressionEvent( + ::testing::_, ::testing::_)) .WillOnce([&mojom_web_page]( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType /*mojom_ad_event_type*/, TriggerAdEventCallback /*callback*/) { - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page->entities); - ASSERT_TRUE(creative_search_result_ads.contains( - test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters)); - - EXPECT_EQ( - creative_search_result_ads.at( - test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters), - mojom_creative_ad); - }); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page->entities); + ASSERT_EQ(creative_search_result_ads.size(), 1u); + ASSERT_EQ(creative_search_result_ads[0]->placement_id, + test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters); - EXPECT_CALL( - ads_service_mock_, - TriggerSearchResultAdEvent( - ::testing::_, mojom::SearchResultAdEventType::kClicked, ::testing::_)) - .WillOnce([&mojom_web_page]( - mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType /*mojom_ad_event_type*/, - TriggerAdEventCallback /*callback*/) { - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page->entities); - ASSERT_TRUE(creative_search_result_ads.contains( - test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters)); - - EXPECT_EQ( - creative_search_result_ads.at( - test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters), - mojom_creative_ad); + EXPECT_EQ(creative_search_result_ads[0], mojom_creative_ad); }); SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), mojom_web_page->Clone()); - - creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( - ClickRedirectUrlWithUnreservedCharactersInPlacementId()); } } // namespace brave_ads diff --git a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.cc b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.cc index b61dc4725004..730c78368381 100644 --- a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.cc +++ b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.cc @@ -6,6 +6,7 @@ #include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.h" #include +#include #include #include "base/check.h" @@ -236,14 +237,12 @@ bool ExtractCreativeSetConversionMojomProperty( return false; } -void ExtractMojomEntity(const schema_org::mojom::EntityPtr& mojom_entity, - CreativeSearchResultAdMap* creative_search_result_ads) { - CHECK(creative_search_result_ads); - +mojom::CreativeSearchResultAdInfoPtr ExtractMojomEntity( + const schema_org::mojom::EntityPtr& mojom_entity) { if (!mojom_entity || mojom_entity->type != kCreativeSearchResultAdMojomEntityType) { // Unsupported type. - return; + return {}; } mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = @@ -258,16 +257,16 @@ void ExtractMojomEntity(const schema_org::mojom::EntityPtr& mojom_entity, for (const auto& mojom_property : mojom_entity->properties) { if (!mojom_property) { - return; + return {}; } const std::string_view property_name = mojom_property->name; if (base::Contains(kRequiredCreativeAdPropertyNames, property_name)) { if (!ExtractCreativeAdMojomProperty(mojom_property, mojom_creative_ad.get())) { - return VLOG(6) - << "Failed to extract creative search result ad property " - << property_name; + VLOG(6) << "Failed to extract creative search result ad property " + << property_name; + return {}; } creative_ad_property_names.insert(property_name); @@ -275,8 +274,9 @@ void ExtractMojomEntity(const schema_org::mojom::EntityPtr& mojom_entity, property_name)) { if (!ExtractCreativeSetConversionMojomProperty( mojom_property, mojom_creative_set_conversion.get())) { - return VLOG(6) << "Failed to extract creative set conversion property " - << property_name; + VLOG(6) << "Failed to extract creative set conversion property " + << property_name; + return {}; } creative_set_conversion_property_names.insert(property_name); @@ -291,9 +291,9 @@ void ExtractMojomEntity(const schema_org::mojom::EntityPtr& mojom_entity, std::back_inserter(missing_creative_ad_property_names)); if (!missing_creative_ad_property_names.empty()) { - return VLOG(6) - << base::JoinString(missing_creative_ad_property_names, ", ") - << " creative search result ad required properties are missing"; + VLOG(6) << base::JoinString(missing_creative_ad_property_names, ", ") + << " creative search result ad required properties are missing"; + return {}; } if (!creative_set_conversion_property_names.empty()) { @@ -318,42 +318,46 @@ void ExtractMojomEntity(const schema_org::mojom::EntityPtr& mojom_entity, if (mojom_creative_ad->placement_id.empty()) { // Invalid placement id. - return; + return {}; } mojom_creative_ad->type = mojom::AdType::kSearchResultAd; - const std::string placement_id = mojom_creative_ad->placement_id; - creative_search_result_ads->emplace(placement_id, - std::move(mojom_creative_ad)); + return mojom_creative_ad; } -void ExtractMojomProperty( - const schema_org::mojom::PropertyPtr& mojom_property, - CreativeSearchResultAdMap* creative_search_result_ads) { +std::vector ExtractMojomProperty( + const schema_org::mojom::PropertyPtr& mojom_property) { + std::vector creative_search_result_ads; + if (!mojom_property || mojom_property->name != kCreativeSearchResultAdsMojomPropertyName) { - return; + return {}; } const schema_org::mojom::ValuesPtr& mojom_values = mojom_property->values; if (!mojom_values || !mojom_values->is_entity_values()) { - return; + return {}; } for (const auto& mojom_entity : mojom_values->get_entity_values()) { - ExtractMojomEntity(mojom_entity, creative_search_result_ads); + if (auto creative_search_result_ad = ExtractMojomEntity(mojom_entity)) { + creative_search_result_ads.push_back( + std::move(creative_search_result_ad)); + } } + + return creative_search_result_ads; } -void Log(const CreativeSearchResultAdMap& creative_search_result_ads) { +void Log(const std::vector& + creative_search_result_ads) { const bool should_log = VLOG_IS_ON(6); if (!should_log) { return; } - for (const auto& [placement_id, mojom_creative_ad] : - creative_search_result_ads) { + for (const auto& mojom_creative_ad : creative_search_result_ads) { VLOG(6) << "Creative search result ad properties:\n" << kCreativeAdPlacementIdPropertyName << ": " << mojom_creative_ad->placement_id << "\n" @@ -392,10 +396,10 @@ void Log(const CreativeSearchResultAdMap& creative_search_result_ads) { } // namespace -CreativeSearchResultAdMap +std::vector ExtractCreativeSearchResultAdsFromMojomWebPageEntities( const std::vector& mojom_entities) { - CreativeSearchResultAdMap creative_search_result_ads; + std::vector creative_search_result_ads; for (const auto& mojom_entity : mojom_entities) { if (!mojom_entity || @@ -404,7 +408,8 @@ ExtractCreativeSearchResultAdsFromMojomWebPageEntities( } for (const auto& mojom_property : mojom_entity->properties) { - ExtractMojomProperty(mojom_property, &creative_search_result_ads); + base::ranges::move(ExtractMojomProperty(mojom_property), + std::back_inserter(creative_search_result_ads)); } } diff --git a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.h b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.h index 5c69601ece80..240630dfafea 100644 --- a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.h +++ b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor.h @@ -6,20 +6,14 @@ #ifndef BRAVE_COMPONENTS_BRAVE_ADS_CONTENT_BROWSER_CREATIVES_SEARCH_RESULT_AD_CREATIVE_SEARCH_RESULT_AD_MOJOM_WEB_PAGE_ENTITIES_EXTRACTOR_H_ #define BRAVE_COMPONENTS_BRAVE_ADS_CONTENT_BROWSER_CREATIVES_SEARCH_RESULT_AD_CREATIVE_SEARCH_RESULT_AD_MOJOM_WEB_PAGE_ENTITIES_EXTRACTOR_H_ -#include #include -#include "base/containers/flat_map.h" #include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" #include "components/schema_org/common/metadata.mojom-forward.h" namespace brave_ads { -using CreativeSearchResultAdMap = - base::flat_map; - -CreativeSearchResultAdMap +std::vector ExtractCreativeSearchResultAdsFromMojomWebPageEntities( const std::vector& mojom_entities); diff --git a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor_unittest.cc b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor_unittest.cc index 4ea431a99bb5..259855d6ff0d 100644 --- a/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor_unittest.cc +++ b/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_extractor_unittest.cc @@ -57,14 +57,16 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, Extract) { test::CreativeSearchResultAdMojomWebPageEntities( /*excluded_property_names=*/{}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); ASSERT_THAT(creative_search_result_ads, ::testing::SizeIs(1)); const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad = - creative_search_result_ads.at(test::kCreativeAdPlacementId); + creative_search_result_ads[0]; ASSERT_TRUE(mojom_creative_ad); ASSERT_TRUE(mojom_creative_ad->creative_set_conversion); + EXPECT_EQ(test::kCreativeAdPlacementId, mojom_creative_ad->placement_id); VerifyRequiredMojomCreativeAdExpectations(mojom_creative_ad); VerifyRequiredMojomCreativeSetConversionExpectations(mojom_creative_ad); @@ -82,9 +84,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, const auto& mojom_entity = mojom_property->values->get_entity_values()[0]; mojom_entity->type = "unsupported"; - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -93,9 +96,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, { const std::vector mojom_web_page_entities; - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -105,9 +109,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, /*excluded_property_names=*/{}); mojom_web_page_entities[0]->type = "unsupported"; - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -117,9 +122,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, /*excluded_property_names=*/{}); mojom_web_page_entities[0]->properties.clear(); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -130,9 +136,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, const auto& mojom_property = mojom_web_page_entities[0]->properties[0]; mojom_property->name = "unsupported"; - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -143,9 +150,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, const auto& mojom_property = mojom_web_page_entities[0]->properties[0]; mojom_property->values = schema_org::mojom::Values::NewEntityValues({}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -157,9 +165,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, mojom_property->values = schema_org::mojom::Values::NewStringValues({"unsupported"}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } } @@ -170,9 +179,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntities( /*excluded_property_names=*/{"data-placement-id"}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -183,13 +193,13 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, kCreativeAdPlacementIdPropertyName, test::kCreativeAdPlacementIdWithUnreservedCharacters); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); ASSERT_THAT(creative_search_result_ads, ::testing::SizeIs(1)); const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad = - creative_search_result_ads.at( - test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters); + creative_search_result_ads[0]; ASSERT_TRUE(mojom_creative_ad); EXPECT_EQ(test::kEscapedCreativeAdPlacementIdWithUnreservedCharacters, mojom_creative_ad->placement_id); @@ -202,15 +212,17 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, /*name=*/"foo", /*value=*/"bar"); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); ASSERT_THAT(creative_search_result_ads, ::testing::SizeIs(1)); const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad = - creative_search_result_ads.at(test::kCreativeAdPlacementId); + creative_search_result_ads[0]; ASSERT_TRUE(mojom_creative_ad); ASSERT_TRUE(mojom_creative_ad->creative_set_conversion); + EXPECT_EQ(test::kCreativeAdPlacementId, mojom_creative_ad->placement_id); VerifyRequiredMojomCreativeAdExpectations(mojom_creative_ad); VerifyRequiredMojomCreativeSetConversionExpectations(mojom_creative_ad); @@ -235,9 +247,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntities( /*excluded_property_names=*/{property_name}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } } @@ -250,9 +263,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntitiesWithProperty( kCreativeAdLandingPagePropertyName, /*value=*/"http://brave.com"); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -262,9 +276,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntitiesWithProperty( kCreativeAdRewardsValuePropertyName, /*value=*/"0-5"); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -274,9 +289,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntitiesWithProperty( kCreativeSetConversionObservationWindowPropertyName, /*value=*/"1"); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } @@ -292,9 +308,10 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, kCreativeAdCreativeInstanceIdPropertyName, /*value=*/101); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); EXPECT_THAT(creative_search_result_ads, ::testing::IsEmpty()); } } @@ -305,14 +322,16 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntitiesWithProperty( kCreativeSetConversionAdvertiserPublicKeyPropertyName, /*value=*/""); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); ASSERT_THAT(creative_search_result_ads, ::testing::SizeIs(1)); const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad = - creative_search_result_ads.at(test::kCreativeAdPlacementId); + creative_search_result_ads[0]; ASSERT_TRUE(mojom_creative_ad); ASSERT_TRUE(mojom_creative_ad->creative_set_conversion); + EXPECT_EQ(test::kCreativeAdPlacementId, mojom_creative_ad->placement_id); VerifyRequiredMojomCreativeAdExpectations(mojom_creative_ad); VerifyRequiredMojomCreativeSetConversionExpectations(mojom_creative_ad); @@ -333,13 +352,15 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntities( /*excluded_property_names=*/{property_name}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); ASSERT_THAT(creative_search_result_ads, ::testing::SizeIs(1)); const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad = - creative_search_result_ads.at(test::kCreativeAdPlacementId); + creative_search_result_ads[0]; ASSERT_TRUE(mojom_creative_ad); + EXPECT_EQ(test::kCreativeAdPlacementId, mojom_creative_ad->placement_id); VerifyRequiredMojomCreativeAdExpectations(mojom_creative_ad); EXPECT_FALSE(mojom_creative_ad->creative_set_conversion); @@ -358,13 +379,15 @@ TEST(BraveAdsCreativeSearchResultAdMojomWebPageEntitiesExtractorTest, test::CreativeSearchResultAdMojomWebPageEntities( /*excluded_property_names=*/{property_name}); - const CreativeSearchResultAdMap creative_search_result_ads = - ExtractCreativeSearchResultAdsFromMojomWebPageEntities( - mojom_web_page_entities); + const std::vector + creative_search_result_ads = + ExtractCreativeSearchResultAdsFromMojomWebPageEntities( + mojom_web_page_entities); ASSERT_THAT(creative_search_result_ads, ::testing::SizeIs(1)); const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad = - creative_search_result_ads.at(test::kCreativeAdPlacementId); + creative_search_result_ads[0]; ASSERT_TRUE(mojom_creative_ad); + EXPECT_EQ(test::kCreativeAdPlacementId, mojom_creative_ad->placement_id); VerifyRequiredMojomCreativeAdExpectations(mojom_creative_ad); VerifyRequiredMojomCreativeSetConversionExpectations(mojom_creative_ad); diff --git a/components/brave_ads/core/internal/BUILD.gn b/components/brave_ads/core/internal/BUILD.gn index 633478f2e3b4..57a3729ed167 100644 --- a/components/brave_ads/core/internal/BUILD.gn +++ b/components/brave_ads/core/internal/BUILD.gn @@ -286,6 +286,8 @@ static_library("internal") { "ad_units/promoted_content_ad/promoted_content_ad_info.h", "ad_units/search_result_ad/search_result_ad_builder.cc", "ad_units/search_result_ad/search_result_ad_builder.h", + "ad_units/search_result_ad/search_result_ad_cache.cc", + "ad_units/search_result_ad/search_result_ad_cache.h", "ad_units/search_result_ad/search_result_ad_feature.cc", "ad_units/search_result_ad/search_result_ad_handler.cc", "ad_units/search_result_ad/search_result_ad_handler.h", diff --git a/components/brave_ads/core/internal/ad_units/ad_handler.cc b/components/brave_ads/core/internal/ad_units/ad_handler.cc index 5e13ac2fe74d..ac987916773e 100644 --- a/components/brave_ads/core/internal/ad_units/ad_handler.cc +++ b/components/brave_ads/core/internal/ad_units/ad_handler.cc @@ -107,14 +107,20 @@ void AdHandler::TriggerInlineContentAdEvent( std::move(callback)); } -void AdHandler::TriggerSearchResultAdEvent( +void AdHandler::TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { CHECK(mojom_creative_ad); - search_result_ad_handler_.TriggerEvent( - std::move(mojom_creative_ad), mojom_ad_event_type, std::move(callback)); + search_result_ad_handler_.TriggerViewedImpressionEvent( + std::move(mojom_creative_ad), std::move(callback)); +} + +void AdHandler::TriggerSearchResultAdClickedEvent( + const std::string& placement_id, + TriggerAdEventCallback callback) { + search_result_ad_handler_.TriggerClickedEvent(placement_id, + std::move(callback)); } /////////////////////////////////////////////////////////////////////////////// diff --git a/components/brave_ads/core/internal/ad_units/ad_handler.h b/components/brave_ads/core/internal/ad_units/ad_handler.h index d9c5a8619b6d..0a058468cd61 100644 --- a/components/brave_ads/core/internal/ad_units/ad_handler.h +++ b/components/brave_ads/core/internal/ad_units/ad_handler.h @@ -74,11 +74,13 @@ class AdHandler final : public ConversionsObserver, SiteVisitObserver { mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback); - void TriggerSearchResultAdEvent( + void TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback); + void TriggerSearchResultAdClickedEvent(const std::string& placement_id, + TriggerAdEventCallback callback); + private: // ConversionsObserver: void OnDidConvertAd(const ConversionInfo& conversion) override; diff --git a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.cc b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.cc new file mode 100644 index 000000000000..d7636f8594de --- /dev/null +++ b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.cc @@ -0,0 +1,59 @@ +/* Copyright (c) 2024 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.h" + +#include + +#include "base/check.h" +#include "brave/components/brave_ads/core/internal/tabs/tab_info.h" +#include "brave/components/brave_ads/core/internal/tabs/tab_manager.h" +#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h" + +namespace brave_ads { + +SearchResultAdCache::SearchResultAdCache() { + TabManager::GetInstance().AddObserver(this); +} + +SearchResultAdCache::~SearchResultAdCache() = default; + +void SearchResultAdCache::CacheCreativeSearchResultAd( + const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad) { + CHECK(mojom_creative_ad); + + const std::optional tab = + TabManager::GetInstance().MaybeGetVisible(); + if (!tab) { + return; + } + creative_search_result_ads_by_tab_[tab->id][mojom_creative_ad->placement_id] = + mojom_creative_ad->Clone(); +} + +std::optional +SearchResultAdCache::GetCachedCreativeSearchResultAd( + const std::string& placement_id) { + for (const auto& [_, creative_search_result_ads] : + creative_search_result_ads_by_tab_) { + const auto it = creative_search_result_ads.find(placement_id); + if (it != creative_search_result_ads.cend()) { + return it->second->Clone(); + } + } + return std::nullopt; +} + +void SearchResultAdCache::OnTabDidChange(const TabInfo& tab) { + LOG(ERROR) << "FOOBAR.OnTabDidChange"; + creative_search_result_ads_by_tab_.erase(tab.id); +} + +void SearchResultAdCache::OnDidCloseTab(const int32_t tab_id) { + LOG(ERROR) << "FOOBAR.OnDidCloseTab"; + creative_search_result_ads_by_tab_.erase(tab_id); +} + +} // namespace brave_ads diff --git a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.h b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.h new file mode 100644 index 000000000000..e5e552b1f218 --- /dev/null +++ b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.h @@ -0,0 +1,49 @@ +/* Copyright (c) 2022 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_AD_UNITS_SEARCH_RESULT_AD_SEARCH_RESULT_AD_CACHE_H_ +#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_AD_UNITS_SEARCH_RESULT_AD_SEARCH_RESULT_AD_CACHE_H_ + +#include +#include + +#include "base/containers/flat_map.h" +#include "brave/components/brave_ads/core/internal/tabs/tab_manager_observer.h" +#include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" + +namespace brave_ads { + +using CreativeSearchResultAdMap = + base::flat_map; + +class SearchResultAdCache final : public TabManagerObserver { + public: + SearchResultAdCache(); + ~SearchResultAdCache() override; + + SearchResultAdCache(const SearchResultAdCache&) = delete; + const SearchResultAdCache& operator=(const SearchResultAdCache&) = delete; + SearchResultAdCache(SearchResultAdCache&&) = delete; + const SearchResultAdCache& operator=(SearchResultAdCache&&) = delete; + + void CacheCreativeSearchResultAd( + const mojom::CreativeSearchResultAdInfoPtr& mojom_creative_ad); + + std::optional + GetCachedCreativeSearchResultAd(const std::string& placement_id); + + private: + // TabManagerObserver: + void OnTabDidChange(const TabInfo& tab) override; + void OnDidCloseTab(int32_t tab_id) override; + + std::map + creative_search_result_ads_by_tab_; +}; + +} // namespace brave_ads + +#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_AD_UNITS_SEARCH_RESULT_AD_SEARCH_RESULT_AD_CACHE_H_ diff --git a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_non_rewards_test.cc b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_non_rewards_test.cc index 6beedbd8c81d..143782d3a1da 100644 --- a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_non_rewards_test.cc +++ b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_non_rewards_test.cc @@ -5,6 +5,7 @@ #include "base/test/mock_callback.h" #include "base/test/scoped_feature_list.h" +#include "brave/components/brave_ads/core/internal/ad_units/ad_test_constants.h" #include "brave/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.h" #include "brave/components/brave_ads/core/internal/common/test/test_base.h" #include "brave/components/brave_ads/core/internal/creatives/search_result_ads/creative_search_result_ad_test_util.h" @@ -17,6 +18,15 @@ namespace brave_ads { +namespace { + +constexpr int32_t kBraveSearchTabId = 1; +constexpr int32_t kTargetUrlTabId = 2; +constexpr char kBraveSearchURL[] = "https://search.brave.com/"; +constexpr char kNonBraveSearchURL[] = "https://www.google.com/"; + +} // namespace + class BraveAdsSearchResultAdForNonRewardsIntegrationTest : public test::TestBase { protected: @@ -29,20 +39,52 @@ class BraveAdsSearchResultAdForNonRewardsIntegrationTest test::ForcePermissionRules(); test::DisableBraveRewards(); + + OpenBraveSearchTab(); } void SetUpMocks() override { EXPECT_CALL(ads_client_mock_, RecordP2AEvents).Times(0); } - void TriggerSearchResultAdEventAndVerifyExpectations( + void TriggerSearchResultAdViewedEventAndVerifyExpectations( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, const bool should_fire_event) { base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/should_fire_event)); - GetAds().TriggerSearchResultAdEvent(std::move(mojom_creative_ad), - mojom_ad_event_type, callback.Get()); + GetAds().TriggerSearchResultAdViewedImpressionEvent( + std::move(mojom_creative_ad), callback.Get()); + } + + void TriggerSearchResultAdClickedEventAndVerifyExpectations( + const std::string& placement_id, + const bool should_fire_event) { + base::MockCallback callback; + EXPECT_CALL(callback, Run(/*success=*/should_fire_event)); + GetAds().TriggerSearchResultAdClickedEvent(placement_id, callback.Get()); + } + + void OpenBraveSearchTab() { + NotifyTabDidChange(kBraveSearchTabId, + /*redirect_chain=*/{GURL(kBraveSearchURL)}, + /*is_new_navigation=*/true, /*is_restoring=*/false, + /*is_error_page=*/false, /*is_visible=*/true); + } + + void CloseBraveSearchTab() { NotifyDidCloseTab(kBraveSearchTabId); } + + void NavigateToTargetUrlInNewTab() { + NotifyTabDidChange(kTargetUrlTabId, + /*redirect_chain=*/{GURL(test::kTargetUrl)}, + /*is_new_navigation=*/true, /*is_restoring=*/false, + /*is_error_page=*/false, /*is_visible=*/true); + } + + void NavigateToNonBraveSearchUrl() { + NotifyTabDidChange(kBraveSearchTabId, + /*redirect_chain=*/{GURL(kNonBraveSearchURL)}, + /*is_new_navigation=*/true, /*is_restoring=*/false, + /*is_error_page=*/false, /*is_visible=*/true); } base::test::ScopedFeatureList scoped_feature_list_; @@ -51,10 +93,9 @@ class BraveAdsSearchResultAdForNonRewardsIntegrationTest TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, DoNotTriggerViewedEvent) { // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/false); } @@ -64,10 +105,9 @@ TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, scoped_feature_list_.Reset(); // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/false); } @@ -76,50 +116,118 @@ TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, // Arrange SearchResultAdHandler::DeferTriggeringAdViewedEventForTesting(); - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( // This viewed impression ad event will be deferred. test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/false); // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( // This viewed impression ad event will be deferred as the previous viewed // impression ad event has not fired. test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/false); SearchResultAdHandler::TriggerDeferredAdViewedEventForTesting(); } TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, - TriggerClickedEvent) { + TriggerClickedEventInTheSameTab) { // Arrange const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true); + TriggerSearchResultAdViewedEventAndVerifyExpectations( + mojom_creative_ad.Clone(), + /*should_fire_event=*/false); // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( - mojom_creative_ad.Clone(), mojom::SearchResultAdEventType::kClicked, + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, /*should_fire_event=*/true); } +TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, + TriggerClickedEventInNewTab) { + // Arrange + const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = + test::BuildCreativeSearchResultAdWithConversion( + /*should_generate_random_uuids=*/true); + TriggerSearchResultAdViewedEventAndVerifyExpectations( + mojom_creative_ad.Clone(), + /*should_fire_event=*/false); + NavigateToTargetUrlInNewTab(); + + // Act & Assert + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, + /*should_fire_event=*/true); +} + +TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, + DoNotTriggerClickedEventIfTabWasClosed) { + // Arrange + const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = + test::BuildCreativeSearchResultAdWithConversion( + /*should_generate_random_uuids=*/true); + TriggerSearchResultAdViewedEventAndVerifyExpectations( + mojom_creative_ad.Clone(), + /*should_fire_event=*/false); + CloseBraveSearchTab(); + + // Act & Assert + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, + /*should_fire_event=*/false); +} + +TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, + DoNotTriggerClickedEventIfTabWasNavigatedToNonBraveSearchUrl) { + // Arrange + const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = + test::BuildCreativeSearchResultAdWithConversion( + /*should_generate_random_uuids=*/true); + TriggerSearchResultAdViewedEventAndVerifyExpectations( + mojom_creative_ad.Clone(), + /*should_fire_event=*/false); + NavigateToNonBraveSearchUrl(); + + // Act & Assert + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, + /*should_fire_event=*/false); +} + TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, DoNotTriggerClickedEventIfShouldNotAlwaysTriggerAdEvents) { // Arrange scoped_feature_list_.Reset(); + const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = + test::BuildCreativeSearchResultAdWithConversion( + /*should_generate_random_uuids=*/true); + TriggerSearchResultAdViewedEventAndVerifyExpectations( + mojom_creative_ad.Clone(), + /*should_fire_event=*/false); + + // Act & Assert + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, + /*should_fire_event=*/false); +} + +TEST_F(BraveAdsSearchResultAdForNonRewardsIntegrationTest, + DoNotTriggerClickedEventIfAdWasNotViewed) { + // Arrange const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true); // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( - mojom_creative_ad.Clone(), mojom::SearchResultAdEventType::kClicked, + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, /*should_fire_event=*/false); } diff --git a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_rewards_test.cc b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_rewards_test.cc index 27b213ca782c..d02dd2642024 100644 --- a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_rewards_test.cc +++ b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_for_rewards_test.cc @@ -16,6 +16,13 @@ namespace brave_ads { +namespace { + +constexpr int32_t kBraveSearchTabId = 1; +constexpr char kBraveSearchURL[] = "https://search.brave.com/"; + +} // namespace + class BraveAdsSearchResultAdForRewardsIntegrationTest : public test::TestBase { protected: void SetUp() override { @@ -25,20 +32,36 @@ class BraveAdsSearchResultAdForRewardsIntegrationTest : public test::TestBase { kShouldAlwaysTriggerBraveSearchResultAdEventsFeature); test::ForcePermissionRules(); + + OpenBraveSearchTab(); } void SetUpMocks() override { EXPECT_CALL(ads_client_mock_, RecordP2AEvents).Times(0); } - void TriggerSearchResultAdEventAndVerifyExpectations( + void TriggerSearchResultAdViewedEventAndVerifyExpectations( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, const bool should_fire_event) { base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/should_fire_event)); - GetAds().TriggerSearchResultAdEvent(std::move(mojom_creative_ad), - mojom_ad_event_type, callback.Get()); + GetAds().TriggerSearchResultAdViewedImpressionEvent( + std::move(mojom_creative_ad), callback.Get()); + } + + void TriggerSearchResultAdClickedEventAndVerifyExpectations( + const std::string& placement_id, + const bool should_fire_event) { + base::MockCallback callback; + EXPECT_CALL(callback, Run(/*success=*/should_fire_event)); + GetAds().TriggerSearchResultAdClickedEvent(placement_id, callback.Get()); + } + + void OpenBraveSearchTab() { + NotifyTabDidChange(kBraveSearchTabId, + /*redirect_chain=*/{GURL(kBraveSearchURL)}, + /*is_new_navigation=*/true, /*is_restoring=*/false, + /*is_error_page=*/false, /*is_visible=*/true); } base::test::ScopedFeatureList scoped_feature_list_; @@ -46,15 +69,13 @@ class BraveAdsSearchResultAdForRewardsIntegrationTest : public test::TestBase { TEST_F(BraveAdsSearchResultAdForRewardsIntegrationTest, TriggerViewedEvents) { // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/true); - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( test::BuildCreativeSearchResultAd(/*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/true); } @@ -63,19 +84,17 @@ TEST_F(BraveAdsSearchResultAdForRewardsIntegrationTest, // Arrange SearchResultAdHandler::DeferTriggeringAdViewedEventForTesting(); - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( // This viewed impression ad event will be deferred. test::BuildCreativeSearchResultAdWithConversion( /*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/true); // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( // This viewed impression ad event will be deferred as the previous viewed // impression ad event has not fired. test::BuildCreativeSearchResultAd(/*should_generate_random_uuids=*/true), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/true); SearchResultAdHandler::TriggerDeferredAdViewedEventForTesting(); @@ -86,14 +105,13 @@ TEST_F(BraveAdsSearchResultAdForRewardsIntegrationTest, TriggerClickedEvent) { const mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad = test::BuildCreativeSearchResultAd(/*should_generate_random_uuids=*/true); - TriggerSearchResultAdEventAndVerifyExpectations( + TriggerSearchResultAdViewedEventAndVerifyExpectations( mojom_creative_ad.Clone(), - mojom::SearchResultAdEventType::kViewedImpression, /*should_fire_event=*/true); // Act & Assert - TriggerSearchResultAdEventAndVerifyExpectations( - mojom_creative_ad.Clone(), mojom::SearchResultAdEventType::kClicked, + TriggerSearchResultAdClickedEventAndVerifyExpectations( + mojom_creative_ad->placement_id, /*should_fire_event=*/true); } diff --git a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.cc b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.cc index 7e7372b1ae37..8361f362043b 100644 --- a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.cc +++ b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.cc @@ -11,6 +11,7 @@ #include "base/check_is_test.h" #include "base/functional/bind.h" #include "base/functional/callback_helpers.h" +#include "base/notreached.h" #include "brave/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_info.h" #include "brave/components/brave_ads/core/internal/ads_core/ads_core_util.h" #include "brave/components/brave_ads/core/internal/common/logging_util.h" @@ -70,35 +71,46 @@ void SearchResultAdHandler::TriggerDeferredAdViewedEventForTesting() { } } -void SearchResultAdHandler::TriggerEvent( +void SearchResultAdHandler::TriggerViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - CHECK_NE(mojom::SearchResultAdEventType::kServedImpression, - mojom_ad_event_type) - << "Should not be called with kServedImpression as this event is handled " - "when calling TriggerEvent with kViewedImpression"; + if (!UserHasOptedInToSearchResultAds()) { + // No-op if the user has not opted into search result ads. + return std::move(callback).Run(/*success=*/false); + } + + creative_search_result_ads_cache_.CacheCreativeSearchResultAd( + mojom_creative_ad); + + mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad_copy = + mojom_creative_ad.Clone(); + return event_handler_.FireEvent( + std::move(mojom_creative_ad_copy), + mojom::SearchResultAdEventType::kServedImpression, + base::BindOnce(&SearchResultAdHandler::FireServedEventCallback, + weak_factory_.GetWeakPtr(), std::move(mojom_creative_ad), + std::move(callback))); +} + +void SearchResultAdHandler::TriggerClickedEvent( + const std::string& placement_id, + TriggerAdEventCallback callback) { if (!UserHasOptedInToSearchResultAds()) { // No-op if the user has not opted into search result ads. return std::move(callback).Run(/*success=*/false); } - if (mojom_ad_event_type == - mojom::SearchResultAdEventType::kViewedImpression) { - mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad_copy = - mojom_creative_ad.Clone(); - - return event_handler_.FireEvent( - std::move(mojom_creative_ad_copy), - mojom::SearchResultAdEventType::kServedImpression, - base::BindOnce(&SearchResultAdHandler::FireServedEventCallback, - weak_factory_.GetWeakPtr(), std::move(mojom_creative_ad), - std::move(callback))); + std::optional mojom_creative_ad = + creative_search_result_ads_cache_.GetCachedCreativeSearchResultAd( + placement_id); + + if (!mojom_creative_ad) { + return std::move(callback).Run(/*success=*/false); } event_handler_.FireEvent( - std::move(mojom_creative_ad), mojom_ad_event_type, + std::move(*mojom_creative_ad), mojom::SearchResultAdEventType::kClicked, base::BindOnce(&FireEventCallback, std::move(callback))); } diff --git a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.h b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.h index 18277f8cb543..978a1435343f 100644 --- a/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.h +++ b/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_handler.h @@ -11,6 +11,7 @@ #include "base/containers/circular_deque.h" #include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" +#include "brave/components/brave_ads/core/internal/ad_units/search_result_ad/search_result_ad_cache.h" #include "brave/components/brave_ads/core/internal/user_engagement/ad_events/search_result_ads/search_result_ad_event_handler.h" #include "brave/components/brave_ads/core/internal/user_engagement/ad_events/search_result_ads/search_result_ad_event_handler_delegate.h" #include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" @@ -38,9 +39,12 @@ class SearchResultAdHandler final : public SearchResultAdEventHandlerDelegate { // You must call this if `DeferTriggeringAdViewedEventForTesting` is called. static void TriggerDeferredAdViewedEventForTesting(); - void TriggerEvent(mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType mojom_ad_event_type, - TriggerAdEventCallback callback); + void TriggerViewedImpressionEvent( + mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, + TriggerAdEventCallback callback); + + void TriggerClickedEvent(const std::string& placement_id, + TriggerAdEventCallback callback); private: void FireServedEventCallback( @@ -69,6 +73,8 @@ class SearchResultAdHandler final : public SearchResultAdEventHandlerDelegate { SearchResultAdEventHandler event_handler_; + SearchResultAdCache creative_search_result_ads_cache_; + base::circular_deque ad_viewed_event_queue_; diff --git a/components/brave_ads/core/internal/ads_impl.cc b/components/brave_ads/core/internal/ads_impl.cc index 3696821c0259..9990fa689318 100644 --- a/components/brave_ads/core/internal/ads_impl.cc +++ b/components/brave_ads/core/internal/ads_impl.cc @@ -188,16 +188,26 @@ void AdsImpl::TriggerPromotedContentAdEvent( std::move(callback)); } -void AdsImpl::TriggerSearchResultAdEvent( +void AdsImpl::TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { if (!is_initialized_) { return std::move(callback).Run(/*success=*/false); } - GetAdHandler().TriggerSearchResultAdEvent( - std::move(mojom_creative_ad), mojom_ad_event_type, std::move(callback)); + GetAdHandler().TriggerSearchResultAdViewedImpressionEvent( + std::move(mojom_creative_ad), std::move(callback)); +} + +void AdsImpl::TriggerSearchResultAdClickedEvent( + const std::string& placement_id, + TriggerAdEventCallback callback) { + if (!is_initialized_) { + return std::move(callback).Run(/*success=*/false); + } + + GetAdHandler().TriggerSearchResultAdClickedEvent(placement_id, + std::move(callback)); } void AdsImpl::PurgeOrphanedAdEventsForType( diff --git a/components/brave_ads/core/internal/ads_impl.h b/components/brave_ads/core/internal/ads_impl.h index 30c8b5943114..3b9a53b94cd6 100644 --- a/components/brave_ads/core/internal/ads_impl.h +++ b/components/brave_ads/core/internal/ads_impl.h @@ -87,9 +87,11 @@ class AdsImpl final : public Ads { mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) override; - void TriggerSearchResultAdEvent( + void TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType mojom_ad_event_type, + TriggerAdEventCallback callback) override; + void TriggerSearchResultAdClickedEvent( + const std::string& placement_id, TriggerAdEventCallback callback) override; void PurgeOrphanedAdEventsForType( diff --git a/components/brave_ads/core/public/ads.h b/components/brave_ads/core/public/ads.h index 182306320f49..8e608e25b4ba 100644 --- a/components/brave_ads/core/public/ads.h +++ b/components/brave_ads/core/public/ads.h @@ -142,13 +142,21 @@ class ADS_EXPORT Ads { TriggerAdEventCallback callback) = 0; // Called when a user views or interacts with a search result ad to trigger a - // `mojom_ad_event_type` event for the ad specified in `mojom_creative_ad`. + // `viewed impression` event for the ad specified in `mojom_creative_ad`. // The callback takes one argument - `bool` is set to `true` if successful // otherwise `false`. Must be called before the // `mojom::CreativeSearchResultAdInfo::target_url` landing page is opened. - virtual void TriggerSearchResultAdEvent( + virtual void TriggerSearchResultAdViewedImpressionEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - mojom::SearchResultAdEventType mojom_ad_event_type, + TriggerAdEventCallback callback) = 0; + + // Called when a user views or interacts with a search result ad to trigger a + // `clicked` event for the ad specified in `mojom_creative_ad`. + // The callback takes one argument - `bool` is set to `true` if successful + // otherwise `false`. Must be called before the + // `mojom::CreativeSearchResultAdInfo::target_url` landing page is opened. + virtual void TriggerSearchResultAdClickedEvent( + const std::string& placement_id, TriggerAdEventCallback callback) = 0; // Called to purge orphaned served ad events for the specified `mojom_ad_type` diff --git a/components/services/bat_ads/bat_ads_impl.cc b/components/services/bat_ads/bat_ads_impl.cc index 0947b34e3bae..957f181c9bf9 100644 --- a/components/services/bat_ads/bat_ads_impl.cc +++ b/components/services/bat_ads/bat_ads_impl.cc @@ -191,14 +191,18 @@ void BatAdsImpl::TriggerInlineContentAdEvent( std::move(callback)); } -void BatAdsImpl::TriggerSearchResultAdEvent( +void BatAdsImpl::TriggerSearchResultAdViewedImpressionEvent( brave_ads::mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - const brave_ads::mojom::SearchResultAdEventType mojom_ad_event_type, - TriggerSearchResultAdEventCallback callback) { - CHECK(brave_ads::mojom::IsKnownEnumValue(mojom_ad_event_type)); + TriggerSearchResultAdViewedImpressionEventCallback callback) { + GetAds()->TriggerSearchResultAdViewedImpressionEvent( + std::move(mojom_creative_ad), std::move(callback)); +} - GetAds()->TriggerSearchResultAdEvent( - std::move(mojom_creative_ad), mojom_ad_event_type, std::move(callback)); +void BatAdsImpl::TriggerSearchResultAdClickedEvent( + const std::string& placement_id, + TriggerSearchResultAdClickedEventCallback callback) { + GetAds()->TriggerSearchResultAdClickedEvent(placement_id, + std::move(callback)); } void BatAdsImpl::PurgeOrphanedAdEventsForType( diff --git a/components/services/bat_ads/bat_ads_impl.h b/components/services/bat_ads/bat_ads_impl.h index f0c6ebb417f0..7f7d1c054f27 100644 --- a/components/services/bat_ads/bat_ads_impl.h +++ b/components/services/bat_ads/bat_ads_impl.h @@ -84,10 +84,13 @@ class BatAdsImpl : public mojom::BatAds { brave_ads::mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerPromotedContentAdEventCallback callback) override; - void TriggerSearchResultAdEvent( + void TriggerSearchResultAdViewedImpressionEvent( brave_ads::mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, - brave_ads::mojom::SearchResultAdEventType mojom_ad_event_type, - TriggerSearchResultAdEventCallback callback) override; + TriggerSearchResultAdViewedImpressionEventCallback callback) override; + + void TriggerSearchResultAdClickedEvent( + const std::string& placement_id, + TriggerSearchResultAdClickedEventCallback callback) override; void PurgeOrphanedAdEventsForType( brave_ads::mojom::AdType mojom_ad_type, diff --git a/components/services/bat_ads/public/interfaces/bat_ads.mojom b/components/services/bat_ads/public/interfaces/bat_ads.mojom index a39ca85144cb..c57c28b33bda 100644 --- a/components/services/bat_ads/public/interfaces/bat_ads.mojom +++ b/components/services/bat_ads/public/interfaces/bat_ads.mojom @@ -188,11 +188,12 @@ interface BatAds { brave_ads.mojom.PromotedContentAdEventType mojom_ad_event_type) => (bool success); - TriggerSearchResultAdEvent( - brave_ads.mojom.CreativeSearchResultAdInfo mojom_creative_ad, - brave_ads.mojom.SearchResultAdEventType mojom_ad_event_type) => + TriggerSearchResultAdViewedImpressionEvent( + brave_ads.mojom.CreativeSearchResultAdInfo mojom_creative_ad) => (bool success); + TriggerSearchResultAdClickedEvent(string placement_id) => (bool success); + PurgeOrphanedAdEventsForType(brave_ads.mojom.AdType mojom_ad_type) => (bool success);