From 75ba9d6c78540093f0128ac41b2718fdf29cb33d Mon Sep 17 00:00:00 2001 From: Aleksey Seren Date: Wed, 4 Sep 2024 14:11:56 -0500 Subject: [PATCH] [ads] Close search result ad infobar after the user clicks "Learn more" --- ...arch_result_ad_clicked_infobar_delegate.cc | 18 ++++++++----- ...earch_result_ad_clicked_infobar_delegate.h | 1 + .../creative_search_result_ad_tab_helper.cc | 2 +- .../creative_search_result_ad_tab_helper.h | 6 ++--- .../creative_search_result_ad_handler.cc | 2 +- .../creative_search_result_ad_handler.h | 6 ++--- ...ative_search_result_ad_handler_unittest.cc | 25 +++++++++---------- 7 files changed, 33 insertions(+), 27 deletions(-) diff --git a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.cc b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.cc index cb062cd722e0..1555076e9a9a 100644 --- a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.cc +++ b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.cc @@ -36,17 +36,16 @@ void CreativeSearchResultAdClickedInfoBarDelegate::Create( CHECK(web_contents); CHECK(prefs); - infobars::ContentInfoBarManager* infobar_manager = - infobars::ContentInfoBarManager::FromWebContents(web_contents); - if (!infobar_manager) { - return; - } - if (!prefs->GetBoolean(prefs::kShouldShowSearchResultAdClickedInfoBar)) { return; } prefs->SetBoolean(prefs::kShouldShowSearchResultAdClickedInfoBar, false); + infobars::ContentInfoBarManager* infobar_manager = + infobars::ContentInfoBarManager::FromWebContents(web_contents); + if (!infobar_manager) { + return; + } infobar_manager->AddInfoBar( CreateConfirmInfoBar(std::unique_ptr( new CreativeSearchResultAdClickedInfoBarDelegate()))); @@ -87,4 +86,11 @@ GURL CreativeSearchResultAdClickedInfoBarDelegate::GetLinkURL() const { return GURL(kLearnMoreUrl); } +bool CreativeSearchResultAdClickedInfoBarDelegate::LinkClicked( + WindowOpenDisposition disposition) { + ConfirmInfoBarDelegate::LinkClicked(disposition); + // Return true to immediately close the infobar. + return true; +} + } // namespace brave_ads diff --git a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.h b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.h index ccf02db1e427..55f95c55143f 100644 --- a/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.h +++ b/browser/brave_ads/creatives/search_result_ad/creative_search_result_ad_clicked_infobar_delegate.h @@ -37,6 +37,7 @@ class CreativeSearchResultAdClickedInfoBarDelegate int GetButtons() const override; std::u16string GetLinkText() const override; GURL GetLinkURL() const override; + bool LinkClicked(WindowOpenDisposition disposition) override; }; } // namespace brave_ads 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 5a38b001b133..98ebd8c71fb5 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 @@ -100,7 +100,7 @@ bool CreativeSearchResultAdTabHelper::ShouldHandleCreativeAdEvents() const { void CreativeSearchResultAdTabHelper::MaybeTriggerCreativeAdClickedEvent( const GURL& url, - base::OnceCallback callback) { + TriggerAdEventCallback callback) { if (creative_search_result_ad_handler_) { creative_search_result_ad_handler_->MaybeTriggerCreativeAdClickedEvent( url, std::move(callback)); 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 415536653a7b..5e98caf63478 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 @@ -11,6 +11,7 @@ #include #include "base/memory/weak_ptr.h" +#include "brave/components/brave_ads/core/public/ads_callback.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_user_data.h" @@ -40,9 +41,8 @@ class CreativeSearchResultAdTabHelper bool ShouldHandleCreativeAdEvents() const; - void MaybeTriggerCreativeAdClickedEvent( - const GURL& url, - base::OnceCallback callback); + void MaybeTriggerCreativeAdClickedEvent(const GURL& url, + TriggerAdEventCallback callback); private: friend class content::WebContentsUserData; 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 6b8b6bfa5e0b..ea199517e439 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 @@ -78,7 +78,7 @@ void CreativeSearchResultAdHandler:: void CreativeSearchResultAdHandler::MaybeTriggerCreativeAdClickedEvent( const GURL& url, - base::OnceCallback callback) { + TriggerAdEventCallback callback) { if (!creative_search_result_ads_) { // No creative search result ads are present on the web page. return; 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 5b44005224a7..739023cbd4c5 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 @@ -15,6 +15,7 @@ #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/public/ads_callback.h" #include "mojo/public/cpp/bindings/remote.h" #include "third_party/blink/public/mojom/document_metadata/document_metadata.mojom-forward.h" @@ -53,9 +54,8 @@ class CreativeSearchResultAdHandler final { ExtractCreativeAdPlacementIdsFromWebPageCallback callback); void MaybeTriggerCreativeAdViewedEvent(const std::string& placement_id); - void MaybeTriggerCreativeAdClickedEvent( - const GURL& url, - base::OnceCallback callback); + void MaybeTriggerCreativeAdClickedEvent(const GURL& url, + TriggerAdEventCallback callback); private: friend class BraveAdsCreativeSearchResultAdHandlerTest; 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 bef2443f2a49..c311fe669c3e 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 @@ -18,6 +18,7 @@ #include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_mojom_web_page_entities_test_util.h" #include "brave/components/brave_ads/content/browser/creatives/search_result_ad/creative_search_result_ad_test_constants.h" #include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h" +#include "brave/components/brave_ads/core/public/ads_callback.h" #include "brave/components/brave_ads/core/public/ads_feature.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -162,7 +163,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), std::move(mojom_web_page)); - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/false)); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -195,7 +196,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), blink::mojom::WebPagePtr()); - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/::testing::_)).Times(0); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -228,7 +229,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), blink::mojom::WebPage::New()); - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/::testing::_)).Times(0); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -263,7 +264,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, test::CreativeSearchResultAdMojomWebPage( /*excluded_property_names=*/{kCreativeAdRewardsValuePropertyName})); - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/::testing::_)).Times(0); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -307,7 +308,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, /*excluded_property_names=*/{ kCreativeSetConversionUrlPatternPropertyName})); - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/false)); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -380,15 +381,13 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), mojom_web_page->Clone()); - base::MockCallback> - fist_click_callback; + base::MockCallback fist_click_callback; EXPECT_CALL(fist_click_callback, Run(/*success=*/true)); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( ClickRedirectUrl(), fist_click_callback.Get()); - base::MockCallback> - second_click_callback; + base::MockCallback second_click_callback; EXPECT_CALL(second_click_callback, Run(/*success=*/false)); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -422,7 +421,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, test::CreativeSearchResultAdMojomWebPage(/*excluded_property_names=*/{})); { - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/::testing::_)).Times(0); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -430,7 +429,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, } { - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/::testing::_)).Times(0); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -439,7 +438,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, } { - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/::testing::_)).Times(0); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent( @@ -509,7 +508,7 @@ TEST_F(BraveAdsCreativeSearchResultAdHandlerTest, SimulateMaybeExtractCreativeAdPlacementIdsFromWebPageCallback( creative_search_result_ad_handler.get(), mojom_web_page->Clone()); - base::MockCallback> callback; + base::MockCallback callback; EXPECT_CALL(callback, Run(/*success=*/true)); creative_search_result_ad_handler->MaybeTriggerCreativeAdClickedEvent(