diff --git a/browser/about_flags.cc b/browser/about_flags.cc index ff68a442c9e7..6c9ac83a6a3d 100644 --- a/browser/about_flags.cc +++ b/browser/about_flags.cc @@ -12,6 +12,7 @@ #include "brave/browser/ethereum_remote_client/buildflags/buildflags.h" #include "brave/browser/ethereum_remote_client/features.h" #include "brave/browser/ui/tabs/features.h" +#include "brave/components/brave_ads/common/brave_ads_feature.h" #include "brave/components/brave_ads/common/custom_notification_ad_feature.h" #include "brave/components/brave_ads/common/notification_ad_feature.h" #include "brave/components/brave_component_updater/browser/features.h" @@ -682,6 +683,45 @@ FEATURE_VALUE_TYPE(brave_rewards::features:: \ kAllowUnsupportedWalletProvidersFeature), \ }, \ + { \ + "brave-ads-should-launch-brave-ads-as-an-in-process-service", \ + "Launch Brave Ads as an in-process service", \ + "Launch Brave Ads as an in-process service removing the utility " \ + "process.", \ + kOsAll, \ + FEATURE_VALUE_TYPE( \ + brave_ads::kShouldLaunchBraveAdsAsAnInProcessServiceFeature), \ + }, \ + { \ + "brave-ads-should-always-run-brave-ads-service", \ + "Should always run Brave Ads service", \ + "Always run Brave Ads service to support triggering ad events when " \ + "Brave Private Ads are disabled.", \ + kOsAll, \ + FEATURE_VALUE_TYPE( \ + brave_ads::kShouldAlwaysRunBraveAdsServiceFeature), \ + }, \ + { \ + "brave-ads-should-always-trigger-new-tab-page-ad-events", \ + "Should always trigger new tab page ad events", \ + "Support triggering new tab page ad events if Brave Private Ads " \ + "are disabled. Requires " \ + "#brave-ads-should-always-run-brave-ads-service to be enabled.", \ + kOsAll, \ + FEATURE_VALUE_TYPE( \ + brave_ads::kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature), \ + }, \ + { \ + "brave-ads-should-always-trigger-search-result-ad-events", \ + "Should always trigger search result ad events", \ + "Support triggering search result ad events if Brave Private Ads " \ + "are disabled. Requires " \ + "#brave-ads-should-always-run-brave-ads-service to be enabled.", \ + kOsAll, \ + FEATURE_VALUE_TYPE( \ + brave_ads:: \ + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature), \ + }, \ { \ "brave-ads-custom-push-notifications-ads", \ "Enable Brave Ads custom push notifications", \ @@ -839,7 +879,6 @@ BRAVE_SHARED_PINNED_TABS \ BRAVE_AI_CHAT \ LAST_BRAVE_FEATURE_ENTRIES_ITEM // Keep it as the last item. - namespace flags_ui { namespace { diff --git a/browser/brave_ads/services/bat_ads_service_factory_impl.cc b/browser/brave_ads/services/bat_ads_service_factory_impl.cc index 2305d10a06af..8073c2a52584 100644 --- a/browser/brave_ads/services/bat_ads_service_factory_impl.cc +++ b/browser/brave_ads/services/bat_ads_service_factory_impl.cc @@ -62,9 +62,8 @@ mojo::Remote BatAdsServiceFactoryImpl::Launch() const { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - return kShouldLaunchAsInProcessRunService.Get() - ? LaunchInProcessBatAdsService() - : LaunchOutOfProcessBatAdsService(); + return ShouldLaunchAsInProcessService() ? LaunchInProcessBatAdsService() + : LaunchOutOfProcessBatAdsService(); } } // namespace brave_ads diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc index b9845e571105..881b67521021 100644 --- a/components/brave_ads/browser/ads_service_impl.cc +++ b/components/brave_ads/browser/ads_service_impl.cc @@ -319,7 +319,7 @@ AdsServiceImpl::AdsServiceImpl( rewards_service_->AddObserver(this); - if (kShouldAlwaysRunService.Get()) { + if (ShouldAlwaysRunService()) { RegisterResourceComponentsForDefaultLocale(); } } @@ -379,7 +379,7 @@ void AdsServiceImpl::GetDeviceIdCallback(std::string device_id) { bool AdsServiceImpl::CanStartBatAdsService() const { return IsSupportedRegion() && (UserHasOptedInToBravePrivateAds() || UserHasOptedInToBraveNews() || - kShouldAlwaysRunService.Get()); + ShouldAlwaysRunService()); } void AdsServiceImpl::MaybeStartBatAdsService() { @@ -547,7 +547,7 @@ void AdsServiceImpl::InitializeBatAdsCallback(const bool success) { is_bat_ads_initialized_ = true; - if (!kShouldAlwaysRunService.Get()) { + if (!ShouldAlwaysRunService()) { RegisterResourceComponentsForDefaultLocale(); } diff --git a/components/brave_ads/common/brave_ads_feature.cc b/components/brave_ads/common/brave_ads_feature.cc index 797f0f33632f..98f83a54fcd1 100644 --- a/components/brave_ads/common/brave_ads_feature.cc +++ b/components/brave_ads/common/brave_ads_feature.cc @@ -7,10 +7,39 @@ namespace brave_ads { -BASE_FEATURE(kBraveAdsFeature, "BraveAds", base::FEATURE_ENABLED_BY_DEFAULT); +BASE_FEATURE(kShouldLaunchBraveAdsAsAnInProcessServiceFeature, + "ShouldLaunchBraveAdsAsInProcessService", + base::FEATURE_DISABLED_BY_DEFAULT); -bool IsBraveAdsFeatureEnabled() { - return base::FeatureList::IsEnabled(kBraveAdsFeature); +bool ShouldLaunchAsInProcessService() { + return base::FeatureList::IsEnabled( + kShouldLaunchBraveAdsAsAnInProcessServiceFeature); +} + +BASE_FEATURE(kShouldAlwaysRunBraveAdsServiceFeature, + "ShouldAlwaysRunBraveAdsService", + base::FEATURE_DISABLED_BY_DEFAULT); + +bool ShouldAlwaysRunService() { + return base::FeatureList::IsEnabled(kShouldAlwaysRunBraveAdsServiceFeature); +} + +BASE_FEATURE(kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, + "ShouldAlwaysTriggerBraveNewTabPageAdEvents", + base::FEATURE_DISABLED_BY_DEFAULT); + +bool ShouldAlwaysTriggerNewTabPageAdEvents() { + return base::FeatureList::IsEnabled( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature); +} + +BASE_FEATURE(kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, + "ShouldAlwaysTriggerBraveSearchResultAdEvents", + base::FEATURE_DISABLED_BY_DEFAULT); + +bool ShouldAlwaysTriggerSearchResultAdEvents() { + return base::FeatureList::IsEnabled( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature); } } // namespace brave_ads diff --git a/components/brave_ads/common/brave_ads_feature.h b/components/brave_ads/common/brave_ads_feature.h index 021e72103562..830e82a3ffbb 100644 --- a/components/brave_ads/common/brave_ads_feature.h +++ b/components/brave_ads/common/brave_ads_feature.h @@ -7,34 +7,33 @@ #define BRAVE_COMPONENTS_BRAVE_ADS_COMMON_BRAVE_ADS_FEATURE_H_ #include "base/feature_list.h" -#include "base/metrics/field_trial_params.h" namespace brave_ads { -BASE_DECLARE_FEATURE(kBraveAdsFeature); - -bool IsBraveAdsFeatureEnabled(); - // Set to |true| to launch as an in process service. -constexpr base::FeatureParam kShouldLaunchAsInProcessRunService{ - &kBraveAdsFeature, "should_launch_as_in_process_service", false}; +BASE_DECLARE_FEATURE(kShouldLaunchBraveAdsAsAnInProcessServiceFeature); + +bool ShouldLaunchAsInProcessService(); // Set to |true| to always run the ads service, even if Brave Private Ads are // disabled. -constexpr base::FeatureParam kShouldAlwaysRunService{ - &kBraveAdsFeature, "should_always_run_service", false}; +BASE_DECLARE_FEATURE(kShouldAlwaysRunBraveAdsServiceFeature); + +bool ShouldAlwaysRunService(); // Set to |true| to always trigger new tab page ad events even if Brave Private -// Ads are disabled. |kShouldAlwaysRunService| must be set to |true|, otherwise +// Ads are disabled. |ShouldAlwaysRunService| must be set to |true|, otherwise // this feature param will be ignored. -constexpr base::FeatureParam kShouldAlwaysTriggerNewTabPageAdEvents{ - &kBraveAdsFeature, "should_always_trigger_new_tab_page_ad_events", false}; +BASE_DECLARE_FEATURE(kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature); + +bool ShouldAlwaysTriggerNewTabPageAdEvents(); // Set to |true| to always trigger search result ad events even if Brave Private -// Ads are disabled. |kShouldAlwaysRunService| must be set to |true|, otherwise +// Ads are disabled. |ShouldAlwaysRunService| must be set to |true|, otherwise // this feature param will be ignored. -constexpr base::FeatureParam kShouldAlwaysTriggerSearchResultAdEvents{ - &kBraveAdsFeature, "should_always_trigger_search_result_ad_events", false}; +BASE_DECLARE_FEATURE(kShouldAlwaysTriggerBraveSearchResultAdEventsFeature); + +bool ShouldAlwaysTriggerSearchResultAdEvents(); } // namespace brave_ads diff --git a/components/brave_ads/common/brave_ads_feature_unittest.cc b/components/brave_ads/common/brave_ads_feature_unittest.cc index e0358c27ff91..d50bf37321f6 100644 --- a/components/brave_ads/common/brave_ads_feature_unittest.cc +++ b/components/brave_ads/common/brave_ads_feature_unittest.cc @@ -14,38 +14,12 @@ namespace brave_ads { -TEST(BraveAdsBraveAdsFeatureTest, IsEnabled) { - // Arrange - - // Act - - // Assert - EXPECT_TRUE(IsBraveAdsFeatureEnabled()); -} - -TEST(BraveAdsBraveAdsFeatureTest, IsDisabled) { - // Arrange - const std::vector enabled_features; - - std::vector disabled_features; - disabled_features.emplace_back(kBraveAdsFeature); - - base::test::ScopedFeatureList scoped_feature_list; - scoped_feature_list.InitWithFeaturesAndParameters(enabled_features, - disabled_features); - - // Act - - // Assert - EXPECT_FALSE(IsBraveAdsFeatureEnabled()); -} - TEST(BraveAdsBraveAdsFeatureTest, ShouldLaunchAsInProcessService) { // Arrange - base::FieldTrialParams params; - params["should_launch_as_in_process_service"] = "true"; std::vector enabled_features; - enabled_features.emplace_back(kBraveAdsFeature, params); + base::FieldTrialParams params; + enabled_features.emplace_back( + kShouldLaunchBraveAdsAsAnInProcessServiceFeature, params); const std::vector disabled_features; @@ -56,41 +30,23 @@ TEST(BraveAdsBraveAdsFeatureTest, ShouldLaunchAsInProcessService) { // Act // Assert - EXPECT_TRUE(kShouldLaunchAsInProcessRunService.Get()); -} - -TEST(BraveAdsBraveAdsFeatureTest, DefaultShouldLaunchAsInProcessService) { - // Arrange - - // Act - - // Assert - EXPECT_FALSE(kShouldLaunchAsInProcessRunService.Get()); + EXPECT_TRUE(ShouldLaunchAsInProcessService()); } -TEST(BraveAdsBraveAdsFeatureTest, ShouldLaunchAsInProcessServiceWhenDisabled) { +TEST(BraveAdsBraveAdsFeatureTest, ShouldNotLaunchAsInProcessService) { // Arrange - const std::vector enabled_features; - - std::vector disabled_features; - disabled_features.emplace_back(kBraveAdsFeature); - - base::test::ScopedFeatureList scoped_feature_list; - scoped_feature_list.InitWithFeaturesAndParameters(enabled_features, - disabled_features); // Act // Assert - EXPECT_FALSE(kShouldLaunchAsInProcessRunService.Get()); + EXPECT_FALSE(ShouldLaunchAsInProcessService()); } TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysRunService) { // Arrange - base::FieldTrialParams params; - params["should_always_run_service"] = "true"; std::vector enabled_features; - enabled_features.emplace_back(kBraveAdsFeature, params); + base::FieldTrialParams params; + enabled_features.emplace_back(kShouldAlwaysRunBraveAdsServiceFeature, params); const std::vector disabled_features; @@ -101,41 +57,24 @@ TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysRunService) { // Act // Assert - EXPECT_TRUE(kShouldAlwaysRunService.Get()); + EXPECT_TRUE(ShouldAlwaysRunService()); } -TEST(BraveAdsBraveAdsFeatureTest, DefaultShouldAlwaysRunService) { +TEST(BraveAdsBraveAdsFeatureTest, ShouldNotAlwaysRunService) { // Arrange // Act // Assert - EXPECT_FALSE(kShouldAlwaysRunService.Get()); -} - -TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysRunServiceWhenDisabled) { - // Arrange - const std::vector enabled_features; - - std::vector disabled_features; - disabled_features.emplace_back(kBraveAdsFeature); - - base::test::ScopedFeatureList scoped_feature_list; - scoped_feature_list.InitWithFeaturesAndParameters(enabled_features, - disabled_features); - - // Act - - // Assert - EXPECT_FALSE(kShouldAlwaysRunService.Get()); + EXPECT_FALSE(ShouldAlwaysRunService()); } TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysTriggerNewTabPageAdEvents) { // Arrange std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); const std::vector disabled_features; @@ -146,43 +85,24 @@ TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysTriggerNewTabPageAdEvents) { // Act // Assert - EXPECT_TRUE(kShouldAlwaysTriggerNewTabPageAdEvents.Get()); + EXPECT_TRUE(ShouldAlwaysTriggerNewTabPageAdEvents()); } -TEST(BraveAdsBraveAdsFeatureTest, - DefaultShouldAlwaysTriggerNewTabPageAdEvents) { +TEST(BraveAdsBraveAdsFeatureTest, ShouldNotAlwaysTriggerNewTabPageAdEvents) { // Arrange // Act // Assert - EXPECT_FALSE(kShouldAlwaysTriggerNewTabPageAdEvents.Get()); -} - -TEST(BraveAdsBraveAdsFeatureTest, - DefaulShouldAlwaysTriggerNewTabPageAdEventsWhenDisabled) { - // Arrange - const std::vector enabled_features; - - std::vector disabled_features; - disabled_features.emplace_back(kBraveAdsFeature); - - base::test::ScopedFeatureList scoped_feature_list; - scoped_feature_list.InitWithFeaturesAndParameters(enabled_features, - disabled_features); - - // Act - - // Assert - EXPECT_FALSE(kShouldAlwaysTriggerNewTabPageAdEvents.Get()); + EXPECT_FALSE(ShouldAlwaysTriggerNewTabPageAdEvents()); } TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysTriggerSearchResultAdEvents) { // Arrange std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); const std::vector disabled_features; @@ -193,35 +113,16 @@ TEST(BraveAdsBraveAdsFeatureTest, ShouldAlwaysTriggerSearchResultAdEvents) { // Act // Assert - EXPECT_TRUE(kShouldAlwaysTriggerSearchResultAdEvents.Get()); + EXPECT_TRUE(ShouldAlwaysTriggerSearchResultAdEvents()); } -TEST(BraveAdsBraveAdsFeatureTest, - DefaultShouldAlwaysTriggerSearchResultAdEvents) { +TEST(BraveAdsBraveAdsFeatureTest, ShouldNotAlwaysTriggerSearchResultAdEvents) { // Arrange // Act // Assert - EXPECT_FALSE(kShouldAlwaysTriggerSearchResultAdEvents.Get()); -} - -TEST(BraveAdsSearchResultAdFeatureTest, - DefaulShouldAlwaysTriggerSearchResultAdEventsWhenDisabled) { - // Arrange - const std::vector enabled_features; - - std::vector disabled_features; - disabled_features.emplace_back(kBraveAdsFeature); - - base::test::ScopedFeatureList scoped_feature_list; - scoped_feature_list.InitWithFeaturesAndParameters(enabled_features, - disabled_features); - - // Act - - // Assert - EXPECT_FALSE(kShouldAlwaysTriggerSearchResultAdEvents.Get()); + EXPECT_FALSE(ShouldAlwaysTriggerSearchResultAdEvents()); } } // namespace brave_ads diff --git a/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler.cc b/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler.cc index 81963cfd23e7..759cb258c483 100644 --- a/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler.cc +++ b/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler.cc @@ -45,8 +45,7 @@ SearchResultAdHandler::MaybeCreateSearchResultAdHandler( return {}; } - const bool is_enabled = - ads_service->IsEnabled() || kShouldAlwaysRunService.Get(); + const bool is_enabled = ads_service->IsEnabled() || ShouldAlwaysRunService(); if (!IsSearchResultAdFeatureEnabled() || !is_enabled || !brave_search::IsAllowedHost(url)) { @@ -111,7 +110,7 @@ void SearchResultAdHandler::MaybeTriggerSearchResultAdClickedEvent( bool SearchResultAdHandler::IsEnabled() { return ads_service_ && - (ads_service_->IsEnabled() || kShouldAlwaysRunService.Get()); + (ads_service_->IsEnabled() || ShouldAlwaysRunService()); } void SearchResultAdHandler::OnRetrieveSearchResultAdEntities( diff --git a/components/brave_ads/core/internal/ads/new_tab_page_ad_handler.cc b/components/brave_ads/core/internal/ads/new_tab_page_ad_handler.cc index 7dd8ab39f33a..2e0f38055b69 100644 --- a/components/brave_ads/core/internal/ads/new_tab_page_ad_handler.cc +++ b/components/brave_ads/core/internal/ads/new_tab_page_ad_handler.cc @@ -79,15 +79,9 @@ void NewTabPageAdHandler::TriggerEvent( const mojom::NewTabPageAdEventType event_type, TriggerAdEventCallback callback) { CHECK(mojom::IsKnownEnumValue(event_type)); - CHECK_NE(mojom::NewTabPageAdEventType::kServed, event_type) - << " should not be called with kServed as this event is handled when " - "calling MaybeServe if the user has opted-in or when triggering a " - "kViewed event if the user has not opted-in to Brave Private Ads"; if (!UserHasOptedInToBravePrivateAds() && - !kShouldAlwaysTriggerNewTabPageAdEvents.Get()) { - // Do not trigger events when the user has not opted-in to Brave Private - // Ads if |kShouldAlwaysTriggerNewTabPageAdEvents| is set to |false|. + !ShouldAlwaysTriggerNewTabPageAdEvents()) { return std::move(callback).Run(/*success*/ false); } diff --git a/components/brave_ads/core/internal/ads/new_tab_page_ad_test.cc b/components/brave_ads/core/internal/ads/new_tab_page_ad_test.cc index 33f9802e6ed2..46e13763db5d 100644 --- a/components/brave_ads/core/internal/ads/new_tab_page_ad_test.cc +++ b/components/brave_ads/core/internal/ads/new_tab_page_ad_test.cc @@ -71,8 +71,8 @@ TEST_F(BraveAdsNewTabPageAdIntegrationTest, Serve) { std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); std::vector disabled_features; @@ -98,8 +98,8 @@ TEST_F(BraveAdsNewTabPageAdIntegrationTest, DoNotServe) { // Arrange std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); std::vector disabled_features; @@ -121,8 +121,8 @@ TEST_F(BraveAdsNewTabPageAdIntegrationTest, TriggerViewedEvent) { std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); std::vector disabled_features; @@ -161,8 +161,8 @@ TEST_F(BraveAdsNewTabPageAdIntegrationTest, std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); std::vector disabled_features; @@ -210,8 +210,8 @@ TEST_F(BraveAdsNewTabPageAdIntegrationTest, TriggerClickedEvent) { std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); std::vector disabled_features; @@ -256,8 +256,8 @@ TEST_F(BraveAdsNewTabPageAdIntegrationTest, std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_new_tab_page_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature, params); std::vector disabled_features; diff --git a/components/brave_ads/core/internal/ads/search_result_ad_handler.cc b/components/brave_ads/core/internal/ads/search_result_ad_handler.cc index f3e836544d5d..c2e1117a2471 100644 --- a/components/brave_ads/core/internal/ads/search_result_ad_handler.cc +++ b/components/brave_ads/core/internal/ads/search_result_ad_handler.cc @@ -56,9 +56,7 @@ void SearchResultAd::TriggerEvent( "calling TriggerEvent with kViewed"; if (!UserHasOptedInToBravePrivateAds() && - !kShouldAlwaysTriggerSearchResultAdEvents.Get()) { - // Do not trigger events when the user has not opted-in to Brave Private - // Ads if |kShouldAlwaysTriggerSearchResultAdEvents| is set to |false|. + !ShouldAlwaysTriggerSearchResultAdEvents()) { return std::move(callback).Run(/*success*/ false); } diff --git a/components/brave_ads/core/internal/ads/search_result_ad_test.cc b/components/brave_ads/core/internal/ads/search_result_ad_test.cc index be683f218b84..507b72013692 100644 --- a/components/brave_ads/core/internal/ads/search_result_ad_test.cc +++ b/components/brave_ads/core/internal/ads/search_result_ad_test.cc @@ -60,8 +60,8 @@ TEST_F(BraveAdsSearchResultAdIntegrationTest, TriggerViewedEvents) { // Arrange std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); std::vector disabled_features; @@ -93,8 +93,8 @@ TEST_F(BraveAdsSearchResultAdIntegrationTest, TriggerQueuedViewedEvents) { // Arrange std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); std::vector disabled_features; @@ -140,8 +140,8 @@ TEST_F(BraveAdsSearchResultAdIntegrationTest, TriggerClickedEvent) { // Arrange std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); std::vector disabled_features; @@ -179,8 +179,8 @@ TEST_F(BraveAdsSearchResultAdIntegrationTest, std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); std::vector disabled_features; @@ -236,8 +236,8 @@ TEST_F(BraveAdsSearchResultAdIntegrationTest, std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); std::vector disabled_features; @@ -286,8 +286,8 @@ TEST_F(BraveAdsSearchResultAdIntegrationTest, std::vector enabled_features; base::FieldTrialParams params; - params["should_always_trigger_search_result_ad_events"] = "true"; - enabled_features.emplace_back(kBraveAdsFeature, params); + enabled_features.emplace_back( + kShouldAlwaysTriggerBraveSearchResultAdEventsFeature, params); std::vector disabled_features; diff --git a/components/brave_ads/core/internal/ads/serving/targeting/behavioral/multi_armed_bandits/epsilon_greedy_bandit_feature_unittest.cc b/components/brave_ads/core/internal/ads/serving/targeting/behavioral/multi_armed_bandits/epsilon_greedy_bandit_feature_unittest.cc index 73ff274dda13..34dc8bce57ad 100644 --- a/components/brave_ads/core/internal/ads/serving/targeting/behavioral/multi_armed_bandits/epsilon_greedy_bandit_feature_unittest.cc +++ b/components/brave_ads/core/internal/ads/serving/targeting/behavioral/multi_armed_bandits/epsilon_greedy_bandit_feature_unittest.cc @@ -16,8 +16,8 @@ namespace brave_ads { TEST(BraveAdsEpsilonGreedyBanditFeatureTest, IsEnabled) { // Arrange - base::FieldTrialParams params; std::vector enabled_features; + base::FieldTrialParams params; enabled_features.emplace_back(kEpsilonGreedyBanditFeatures, params); const std::vector disabled_features; diff --git a/components/brave_ads/core/internal/ads/serving/targeting/contextual/text_embedding/text_embedding_feature_unittest.cc b/components/brave_ads/core/internal/ads/serving/targeting/contextual/text_embedding/text_embedding_feature_unittest.cc index ca1be30ffb40..115c422829de 100644 --- a/components/brave_ads/core/internal/ads/serving/targeting/contextual/text_embedding/text_embedding_feature_unittest.cc +++ b/components/brave_ads/core/internal/ads/serving/targeting/contextual/text_embedding/text_embedding_feature_unittest.cc @@ -16,8 +16,8 @@ namespace brave_ads { TEST(BraveAdsTextEmbeddingFeatureTest, IsEnabled) { // Arrange - base::FieldTrialParams params; std::vector enabled_features; + base::FieldTrialParams params; enabled_features.emplace_back(kTextEmbeddingFeature, params); const std::vector disabled_features; diff --git a/components/brave_ads/core/internal/catalog/catalog.cc b/components/brave_ads/core/internal/catalog/catalog.cc index 033ad9aeacfc..14f2b5012d40 100644 --- a/components/brave_ads/core/internal/catalog/catalog.cc +++ b/components/brave_ads/core/internal/catalog/catalog.cc @@ -24,8 +24,7 @@ namespace brave_ads { namespace { bool DoesRequireResourceForNewTabPageAds() { - return kShouldAlwaysRunService.Get() && - kShouldAlwaysTriggerNewTabPageAdEvents.Get() && + return ShouldAlwaysRunService() && ShouldAlwaysTriggerNewTabPageAdEvents() && UserHasOptedInToNewTabPageAds(); } diff --git a/components/brave_ads/core/internal/resources/behavioral/conversions/conversions_resource.cc b/components/brave_ads/core/internal/resources/behavioral/conversions/conversions_resource.cc index 1a28f46c4cab..5cb200d9c416 100644 --- a/components/brave_ads/core/internal/resources/behavioral/conversions/conversions_resource.cc +++ b/components/brave_ads/core/internal/resources/behavioral/conversions/conversions_resource.cc @@ -46,7 +46,7 @@ void ConversionsResource::LoadCallback( } if (result.value().version == 0) { - BLOG(7, kConversionsResourceId << " conversions resource is not available"); + BLOG(1, kConversionsResourceId << " conversions resource is not available"); is_initialized_ = false; return; } diff --git a/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager.cc b/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager.cc index d1055508e97a..e8b6b0f24a45 100644 --- a/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager.cc +++ b/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager.cc @@ -10,6 +10,7 @@ #include "base/ranges/algorithm.h" #include "base/strings/string_number_conversions.h" #include "base/time/time.h" +#include "brave/components/brave_ads/core/internal/account/account_util.h" #include "brave/components/brave_ads/core/internal/ads_client_helper.h" #include "brave/components/brave_ads/core/internal/browser/browser_manager.h" #include "brave/components/brave_ads/core/internal/common/logging_util.h" @@ -62,6 +63,10 @@ UserActivityManager& UserActivityManager::GetInstance() { } void UserActivityManager::RecordEvent(const UserActivityEventType event_type) { + if (!UserHasOptedInToBravePrivateAds()) { + return; + } + UserActivityEventInfo user_activity_event; user_activity_event.type = event_type; user_activity_event.created_at = base::Time::Now(); diff --git a/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager_unittest.cc b/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager_unittest.cc index 48061998e00e..78b2e871cfe7 100644 --- a/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager_unittest.cc +++ b/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager_unittest.cc @@ -6,6 +6,7 @@ #include "brave/components/brave_ads/core/internal/user_attention/user_activity/user_activity_manager.h" #include "base/ranges/algorithm.h" +#include "brave/components/brave_ads/core/internal/ads/ad_unittest_util.h" #include "brave/components/brave_ads/core/internal/common/unittest/unittest_base.h" #include "brave/components/brave_ads/core/internal/common/unittest/unittest_time_util.h" #include "brave/components/brave_ads/core/internal/user_attention/user_activity/user_activity_constants.h" @@ -24,11 +25,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordInitializedAdsEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -46,11 +47,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordBrowserDidEnterForegroundEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -68,11 +69,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordBrowserDidEnterBackgroundEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -91,11 +92,11 @@ TEST_F(BraveAdsUserActivityManagerTest, // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -113,11 +114,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordClickedBookmarkEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -135,11 +136,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordClickedHomePageButtonEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -156,11 +157,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordClickedLinkEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -178,11 +179,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordClickedReloadButtonEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -199,11 +200,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordClosedTabEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -221,11 +222,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordFocusedOnExistingTabEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -243,11 +244,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordGeneratedKeywordEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -265,11 +266,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordNewNavigationEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -288,11 +289,11 @@ TEST_F(BraveAdsUserActivityManagerTest, // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -309,11 +310,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordOpenedNewTabEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -331,11 +332,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordPlayedMediaEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -353,11 +354,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordStoppedPlayingMediaEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -375,11 +376,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordSubmittedFormEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -396,11 +397,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordTabUpdatedEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -418,11 +419,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordTypedAndSelectedNonUrlEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -441,11 +442,11 @@ TEST_F(BraveAdsUserActivityManagerTest, // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -462,11 +463,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordTypedUrlEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -484,11 +485,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordUsedAddressBarEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -506,11 +507,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordBrowserDidBecomeActiveEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -528,11 +529,11 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordBrowserDidResignActiveEvent) { // Act UserActivityManager::GetInstance().RecordEvent(event_type); + // Assert const UserActivityEventList events = UserActivityManager::GetInstance().GetHistoryForTimeWindow( base::Hours(1)); - // Assert UserActivityEventList expected_events; UserActivityEventInfo event; event.type = event_type; @@ -542,6 +543,25 @@ TEST_F(BraveAdsUserActivityManagerTest, RecordBrowserDidResignActiveEvent) { EXPECT_TRUE(base::ranges::equal(expected_events, events)); } +TEST_F(BraveAdsUserActivityManagerTest, + DoNotRecordEventIfBravePrivateAdsAreDisabled) { + // Arrange + DisableBravePrivateAds(); + + const UserActivityEventType event_type = + UserActivityEventType::kInitializedAds; + + // Act + UserActivityManager::GetInstance().RecordEvent(event_type); + + // Assert + const UserActivityEventList events = + UserActivityManager::GetInstance().GetHistoryForTimeWindow( + base::Hours(1)); + + EXPECT_TRUE(events.empty()); +} + TEST_F(BraveAdsUserActivityManagerTest, GetHistoryForTimeWindow) { // Arrange UserActivityManager::GetInstance().RecordEvent( diff --git a/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection.cc b/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection.cc index 2307c360c8e8..b545c3385ea8 100644 --- a/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection.cc +++ b/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection.cc @@ -6,6 +6,7 @@ #include "brave/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection.h" #include "base/time/time.h" +#include "brave/components/brave_ads/core/internal/account/account_util.h" #include "brave/components/brave_ads/core/internal/ads_client_helper.h" #include "brave/components/brave_ads/core/internal/common/logging_util.h" #include "brave/components/brave_ads/core/internal/diagnostics/entries/last_unidle_time_diagnostic_util.h" @@ -28,6 +29,10 @@ UserIdleDetection::~UserIdleDetection() { void UserIdleDetection::OnNotifyUserDidBecomeActive( const base::TimeDelta idle_time, const bool screen_was_locked) { + if (!UserHasOptedInToBravePrivateAds()) { + return; + } + BLOG(1, "User is active after " << idle_time); if (screen_was_locked) { BLOG(1, "Screen was locked before the user become active"); @@ -39,6 +44,10 @@ void UserIdleDetection::OnNotifyUserDidBecomeActive( } void UserIdleDetection::OnNotifyUserDidBecomeIdle() { + if (!UserHasOptedInToBravePrivateAds()) { + return; + } + BLOG(1, "User is idle"); } diff --git a/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection_unittest.cc b/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection_unittest.cc new file mode 100644 index 000000000000..5cbc7001b632 --- /dev/null +++ b/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection_unittest.cc @@ -0,0 +1,80 @@ +/* Copyright (c) 2023 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 + +#include "brave/components/brave_ads/core/internal/ads/ad_unittest_util.h" +#include "brave/components/brave_ads/core/internal/common/unittest/unittest_base.h" + +// npm run test -- brave_unit_tests --filter=BraveAds* + +namespace brave_ads { + +class BraveAdsUserIdleDetectionTest : public UnitTestBase {}; + +TEST_F(BraveAdsUserIdleDetectionTest, + UserDidBecomeActiveIfBravePrivateAdsAreEnabled) { + // Arrange + EXPECT_CALL(ads_client_mock_, Log); + + // Act + NotifyUserDidBecomeActive(/*idle_time*/ base::Seconds(10), + /*screen_was_locked*/ false); + + // Assert +} + +TEST_F(BraveAdsUserIdleDetectionTest, + UserDidBecomeActiveIfScreenWasLockedAndBravePrivateAdsAreEnabled) { + // Arrange + EXPECT_CALL(ads_client_mock_, Log).Times(2); + + // Act + NotifyUserDidBecomeActive(/*idle_time*/ base::Seconds(10), + /*screen_was_locked*/ true); + + // Assert +} + +TEST_F(BraveAdsUserIdleDetectionTest, + UserDidBecomeActiveIfBravePrivateAdsAreDisabled) { + // Arrange + DisableBravePrivateAds(); + + EXPECT_CALL(ads_client_mock_, Log).Times(0); + + // Act + NotifyUserDidBecomeActive(/*idle_time*/ base::Seconds(10), + /*screen_was_locked*/ false); + + // Assert +} + +TEST_F(BraveAdsUserIdleDetectionTest, + UserDidBecomeIdleIfBravePrivateAdsAreEnabled) { + // Arrange + + EXPECT_CALL(ads_client_mock_, Log); + + // Act + NotifyUserDidBecomeIdle(); + + // Assert +} + +TEST_F(BraveAdsUserIdleDetectionTest, + UserDidBecomeIdleIfBravePrivateAdsAreDisabled) { + // Arrange + DisableBravePrivateAds(); + + EXPECT_CALL(ads_client_mock_, Log).Times(0); + + // Act + NotifyUserDidBecomeIdle(); + + // Assert +} + +} // namespace brave_ads diff --git a/components/brave_ads/core/test/BUILD.gn b/components/brave_ads/core/test/BUILD.gn index 0d7bab109087..4f21e9b08a43 100644 --- a/components/brave_ads/core/test/BUILD.gn +++ b/components/brave_ads/core/test/BUILD.gn @@ -496,6 +496,7 @@ source_set("brave_ads_unit_tests") { "//brave/components/brave_ads/core/internal/user_attention/user_activity/user_activity_scoring_util_unittest.cc", "//brave/components/brave_ads/core/internal/user_attention/user_activity/user_activity_util_unittest.cc", "//brave/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection_feature_unittest.cc", + "//brave/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection_unittest.cc", "//brave/components/brave_ads/core/internal/user_attention/user_idle_detection/user_idle_detection_util_unittest.cc", "//brave/components/brave_ads/core/internal/user_attention/user_reactions/user_reactions_unittest.cc", ]