Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[ads] Fix AdsServiceFactory returning nullptr when Rewards unavailable (#35698)

Browse Source 
AdsServiceFactory::GetForProfile returned nullptr when IsSupported
was false, which includes a managed pref that can change during a
session. The guard is removed from the factory so AdsService is
always created for regular profiles. BAT ads startup is now gated
in CanStartBatAdsService instead. AdsServiceImpl and
AdsServiceDelegate also handle null service dependencies gracefully.

A follow-up will add an ads-specific group policy to allow
independent control of ads without relying on the Rewards policy.
This commit is contained in:
Terry Mancey
2026-04-22 01:19:11 +01:00
committed by GitHub
parent daeb40547c
commit 0dbf7fd2da
9 changed files with 146 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/brave_adaptive_captcha/brave_adaptive_captcha_service_factory.cc
+8 -2
View File
@@ -103,11 +103,17 @@ BraveAdaptiveCaptchaServiceFactory::BuildServiceInstanceForBrowserContext(
content::BrowserContext* context) const {
auto url_loader_factory = context->GetDefaultStoragePartition()
->GetURLLoaderFactoryForBrowserProcess();
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
auto* rewards_service = brave_rewards::RewardsServiceFactory::GetForProfile(
Profile::FromBrowserContext(context));
if (!rewards_service) {
return nullptr;
}
#endif
return std::make_unique<BraveAdaptiveCaptchaService>(
user_prefs::UserPrefs::Get(context), std::move(url_loader_factory),
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
brave_rewards::RewardsServiceFactory::GetForProfile(
Profile::FromBrowserContext(context)),
rewards_service,
#endif
std::make_unique<CaptchaDelegate>(context));
}
browser/brave_ads/ads_service_delegate.cc
+10 -4
View File
@@ -40,7 +40,7 @@ constexpr char kNotificationAdUrlPrefix[] = "https://www.brave.com/ads/?";
AdsServiceDelegate::AdsServiceDelegate(
Profile& profile,
PrefService& local_state,
brave_adaptive_captcha::BraveAdaptiveCaptchaService&
brave_adaptive_captcha::BraveAdaptiveCaptchaService*
adaptive_captcha_service)
: profile_(profile),
local_state_(local_state),
@@ -93,15 +93,21 @@ bool AdsServiceDelegate::ShowOnboardingNotification() {
void AdsServiceDelegate::ShowScheduledCaptcha(const std::string& payment_id,
const std::string& captcha_id) {
adaptive_captcha_service_->ShowScheduledCaptcha(payment_id, captcha_id);
if (adaptive_captcha_service_) {
adaptive_captcha_service_->ShowScheduledCaptcha(payment_id, captcha_id);
}
}
void AdsServiceDelegate::ClearScheduledCaptcha() {
adaptive_captcha_service_->ClearScheduledCaptcha();
if (adaptive_captcha_service_) {
adaptive_captcha_service_->ClearScheduledCaptcha();
}
}
void AdsServiceDelegate::SnoozeScheduledCaptcha() {
adaptive_captcha_service_->SnoozeScheduledCaptcha();
if (adaptive_captcha_service_) {
adaptive_captcha_service_->SnoozeScheduledCaptcha();
}
}
void AdsServiceDelegate::ShowNotificationAd(const std::string& id,
browser/brave_ads/ads_service_delegate.h
+5 -4
View File
@@ -8,7 +8,7 @@
#include <string>
#include "base/memory/raw_ref.h"
#include "base/memory/raw_ptr.h"
#include "brave/components/brave_ads/core/browser/service/ads_service.h"
class GURL;
@@ -25,10 +25,11 @@ namespace brave_ads {
// Singleton that owns all AdsService and associates them with Profiles.
class AdsServiceDelegate : public AdsService::Delegate {
public:
// `adaptive_captcha_service` can be `nullptr` in tests.
explicit AdsServiceDelegate(
Profile& profile,
PrefService& local_state,
brave_adaptive_captcha::BraveAdaptiveCaptchaService&
brave_adaptive_captcha::BraveAdaptiveCaptchaService*
adaptive_captcha_service);
AdsServiceDelegate(const AdsServiceDelegate&) = delete;
@@ -59,8 +60,8 @@ class AdsServiceDelegate : public AdsService::Delegate {
const raw_ref<Profile> profile_;
const raw_ref<PrefService> local_state_;
const raw_ref<brave_adaptive_captcha::BraveAdaptiveCaptchaService>
adaptive_captcha_service_;
const raw_ptr<brave_adaptive_captcha::BraveAdaptiveCaptchaService>
adaptive_captcha_service_; // Not owned.
};
} // namespace brave_ads
browser/brave_ads/ads_service_factory.cc
+1 -7
View File
@@ -23,7 +23,6 @@
#include "brave/components/brave_ads/core/browser/service/ads_service.h"
#include "brave/components/brave_ads/core/public/ads_util.h"
#include "brave/components/brave_rewards/core/buildflags/buildflags.h"
#include "brave/components/brave_rewards/core/rewards_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/history/history_service_factory.h"
@@ -61,10 +60,6 @@ AdsService* AdsServiceFactory::GetForProfile(Profile* profile) {
return nullptr;
}
if (!brave_rewards::IsSupported(profile->GetPrefs())) {
return nullptr;
}
return static_cast<AdsService*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
}
@@ -113,10 +108,9 @@ AdsServiceFactory::BuildServiceInstanceForBrowserContext(
auto* brave_adaptive_captcha_service =
brave_adaptive_captcha::BraveAdaptiveCaptchaServiceFactory::GetForProfile(
profile);
CHECK(brave_adaptive_captcha_service);
auto delegate = std::make_unique<AdsServiceDelegate>(
*profile, *local_state, *brave_adaptive_captcha_service);
*profile, *local_state, brave_adaptive_captcha_service);
auto* history_service = HistoryServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS);
browser/brave_rewards/test/rewards_ofac_browsertest.cc
+1 -5
View File
@@ -72,8 +72,7 @@ IN_PROC_BROWSER_TEST_F(BraveRewardsOFACTest, IsBraveRewardsDisabled) {
}
}
// Verify that Rewards and Ads services don't get created when in an OFAC
// sanctioned region.
// Verify that the Rewards service is not created in an OFAC sanctioned region.
IN_PROC_BROWSER_TEST_F(BraveRewardsOFACTest, GetRewardsAndAdsServices) {
{
const brave_l10n::test::ScopedDefaultLocale locale("en_CA"); // "Canada"
@@ -88,9 +87,6 @@ IN_PROC_BROWSER_TEST_F(BraveRewardsOFACTest, GetRewardsAndAdsServices) {
const brave_l10n::test::ScopedDefaultLocale locale("es_CU"); // "Cuba"
EXPECT_EQ(brave_rewards::RewardsServiceFactory::GetForProfile(profile()),
nullptr);
#if BUILDFLAG(ENABLE_BRAVE_ADS)
EXPECT_EQ(brave_ads::AdsServiceFactory::GetForProfile(profile()), nullptr);
#endif // BUILDFLAG(ENABLE_BRAVE_ADS)
}
}
browser/brave_rewards/test/rewards_policy_browsertest.cc
+6 -8
View File
@@ -89,22 +89,20 @@ IN_PROC_BROWSER_TEST_P(BraveRewardsPolicyTest, IsBraveRewardsDisabled) {
}
}
// Verify that Rewards and Ads services don't get created when Brave Rewards are
// disabled by policy.
// Verify that the Rewards service is not created when Brave Rewards are
// disabled by policy, and that the Ads service is always created for regular
// profiles regardless of the Rewards policy.
IN_PROC_BROWSER_TEST_P(BraveRewardsPolicyTest, GetRewardsAndAdsServices) {
if (IsBraveRewardsDisabledTest()) {
EXPECT_EQ(brave_rewards::RewardsServiceFactory::GetForProfile(profile()),
nullptr);
#if BUILDFLAG(ENABLE_BRAVE_ADS)
EXPECT_EQ(brave_ads::AdsServiceFactory::GetForProfile(profile()), nullptr);
#endif // BUILDFLAG(ENABLE_BRAVE_ADS)
} else {
EXPECT_NE(brave_rewards::RewardsServiceFactory::GetForProfile(profile()),
nullptr);
#if BUILDFLAG(ENABLE_BRAVE_ADS)
EXPECT_NE(brave_ads::AdsServiceFactory::GetForProfile(profile()), nullptr);
#endif // BUILDFLAG(ENABLE_BRAVE_ADS)
}
#if BUILDFLAG(ENABLE_BRAVE_ADS)
EXPECT_NE(brave_ads::AdsServiceFactory::GetForProfile(profile()), nullptr);
#endif // BUILDFLAG(ENABLE_BRAVE_ADS)
}
// Verify that Rewards menu item isn't enabled in the app menu when Brave
components/brave_ads/browser/ads_service_impl.cc
+25 -7
View File
@@ -47,6 +47,7 @@
#include "brave/components/brave_rewards/core/buildflags/buildflags.h"
#include "brave/components/brave_rewards/core/mojom/rewards.mojom.h"
#include "brave/components/brave_rewards/core/pref_names.h"
#include "brave/components/brave_rewards/core/rewards_util.h"
#include "brave/components/ntp_background_images/common/pref_names.h"
#include "brave/components/services/bat_ads/public/interfaces/bat_ads.mojom.h"
#include "build/build_config.h"
@@ -166,8 +167,9 @@ AdsServiceImpl::AdsServiceImpl(
}
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
CHECK(rewards_service);
rewards_service_observation_.Observe(rewards_service);
if (rewards_service) {
rewards_service_observation_.Observe(rewards_service);
}
#endif
if (CanStartBatAdsService()) {
@@ -281,6 +283,12 @@ void AdsServiceImpl::GetDeviceIdAndMaybeStartBatAdsServiceCallback(
}
bool AdsServiceImpl::CanStartBatAdsService() const {
if (!brave_rewards::IsSupported(&*prefs_)) {
// Never start if Rewards is disabled by policy, feature flag, or
// unsupported region, regardless of which ad unit the user has opted into.
return false;
}
if (UserHasJoinedBraveRewards()) {
// Always start the service to update brave://ads-internals,
// brave://rewards, and brave://rewards-internals if the user has joined
@@ -371,9 +379,13 @@ void AdsServiceImpl::InitializeBasePathDirectoryCallback(bool success) {
void AdsServiceImpl::InitializeRewardsWallet() {
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
rewards_service_observation_.GetSource()->GetRewardsWallet(
base::BindOnce(&AdsServiceImpl::InitializeRewardsWalletCallback,
bat_ads_service_weak_ptr_factory_.GetWeakPtr()));
if (rewards_service_observation_.IsObserving()) {
rewards_service_observation_.GetSource()->GetRewardsWallet(
base::BindOnce(&AdsServiceImpl::InitializeRewardsWalletCallback,
bat_ads_service_weak_ptr_factory_.GetWeakPtr()));
} else {
InitializeRewardsWalletCallback(/*mojom_rewards_wallet=*/nullptr);
}
#else
InitializeRewardsWalletCallback(/*mojom_rewards_wallet=*/nullptr);
#endif
@@ -729,6 +741,10 @@ void AdsServiceImpl::NotifyPrefChanged(const std::string& path) const {
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
void AdsServiceImpl::GetRewardsWallet() {
if (!rewards_service_observation_.IsObserving()) {
return;
}
rewards_service_observation_.GetSource()->GetRewardsWallet(
base::BindOnce(&AdsServiceImpl::NotifyRewardsWalletDidUpdate,
weak_ptr_factory_.GetWeakPtr()));
@@ -1548,8 +1564,10 @@ void AdsServiceImpl::Log(const std::string& file,
int32_t verbose_level,
const std::string& message) {
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
rewards_service_observation_.GetSource()->WriteDiagnosticLog(
file, line, verbose_level, message);
if (rewards_service_observation_.IsObserving()) {
rewards_service_observation_.GetSource()->WriteDiagnosticLog(
file, line, verbose_level, message);
}
#endif // BUILDFLAG(ENABLE_BRAVE_REWARDS)
const int vlog_level =
components/brave_ads/browser/ads_service_impl.h
+2 -1
View File
@@ -79,7 +79,8 @@ class AdsServiceImpl final : public AdsService,
public content_settings::Observer {
public:
// `http_client`, `resource_component`, `history_service`, and
// `host_content_settings` can be `nullptr` in tests.
// `host_content_settings` can be `nullptr` in tests. `rewards_service`
// can be `nullptr` when Rewards is unsupported or disabled by policy.
explicit AdsServiceImpl(
std::unique_ptr<Delegate> delegate,
PrefService& prefs,
components/brave_ads/browser/ads_service_impl_unittest.cc
+88
View File
@@ -12,6 +12,7 @@
#include "base/memory/raw_ptr.h"
#include "base/test/run_until.h"
#include "base/test/task_environment.h"
#include "base/values.h"
#include "brave/components/brave_ads/browser/test/fake_ads_service_delegate.h"
#include "brave/components/brave_ads/browser/test/fake_ads_tooltips_delegate.h"
#include "brave/components/brave_ads/browser/test/fake_bat_ads_service_factory.h"
@@ -23,6 +24,7 @@
#include "brave/components/brave_rewards/core/pref_names.h"
#include "brave/components/brave_rewards/core/pref_registry.h"
#include "brave/components/ntp_background_images/common/pref_names.h"
#include "build/build_config.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "components/variations/pref_names.h"
@@ -338,6 +340,92 @@ TEST_F(BraveAdsAdsServiceImplTest, ServiceStartsWhenUserHasJoinedBraveRewards) {
// Assert
EXPECT_EQ(1U, bat_ads_service_factory_->launch_count());
}
TEST_F(
BraveAdsAdsServiceImplTest,
ServiceDoesNotStopWhenOptedOutOfNotificationAdsWhileOptedInToSearchResultAds) {
// Arrange
prefs_.SetBoolean(prefs::kOptedInToSearchResultAds, true);
prefs_.SetBoolean(brave_rewards::prefs::kEnabled, true);
prefs_.SetBoolean(prefs::kOptedInToNotificationAds, true);
Startup();
ASSERT_EQ(1U, bat_ads_service_factory_->launch_count());
// Act
prefs_.SetBoolean(prefs::kOptedInToNotificationAds, false);
// Assert
EXPECT_EQ(1U, bat_ads_service_factory_->launch_count());
EXPECT_EQ(0U, bat_ads_service_factory_->shutdown_count());
}
#endif // BUILDFLAG(ENABLE_BRAVE_REWARDS)
#if !BUILDFLAG(IS_ANDROID)
TEST_F(BraveAdsAdsServiceImplTest,
ServiceDoesNotStartForSearchResultAdsWhenRewardsIsDisabledByPolicy) {
// Arrange
prefs_.SetBoolean(prefs::kOptedInToSearchResultAds, true);
prefs_.SetManagedPref(brave_rewards::prefs::kDisabledByPolicy,
base::Value(true));
// Act
Startup();
// Assert
EXPECT_EQ(0U, bat_ads_service_factory_->launch_count());
}
TEST_F(BraveAdsAdsServiceImplTest,
ServiceDoesNotStartForNewTabPageAdsWhenRewardsIsDisabledByPolicy) {
// Arrange
prefs_.SetBoolean(
ntp_background_images::prefs::kNewTabPageShowBackgroundImage, true);
prefs_.SetBoolean(ntp_background_images::prefs::
kNewTabPageShowSponsoredImagesBackgroundImage,
true);
prefs_.SetManagedPref(brave_rewards::prefs::kDisabledByPolicy,
base::Value(true));
// Act
Startup();
// Assert
EXPECT_EQ(0U, bat_ads_service_factory_->launch_count());
}
#if BUILDFLAG(ENABLE_BRAVE_REWARDS)
TEST_F(BraveAdsAdsServiceImplTest,
ServiceDoesNotStartForNotificationAdsWhenRewardsIsDisabledByPolicy) {
// Arrange
prefs_.SetBoolean(prefs::kOptedInToSearchResultAds, false);
prefs_.SetBoolean(brave_rewards::prefs::kEnabled, true);
prefs_.SetBoolean(prefs::kOptedInToNotificationAds, true);
prefs_.SetManagedPref(brave_rewards::prefs::kDisabledByPolicy,
base::Value(true));
// Act
Startup();
// Assert
EXPECT_EQ(0U, bat_ads_service_factory_->launch_count());
}
TEST_F(
BraveAdsAdsServiceImplTest,
ServiceDoesNotStartWhenUserHasJoinedBraveRewardsButRewardsIsDisabledByPolicy) {
// Arrange
prefs_.SetBoolean(prefs::kOptedInToSearchResultAds, false);
prefs_.SetBoolean(brave_rewards::prefs::kEnabled, true);
prefs_.SetManagedPref(brave_rewards::prefs::kDisabledByPolicy,
base::Value(true));
// Act
Startup();
// Assert
EXPECT_EQ(0U, bat_ads_service_factory_->launch_count());
}
#endif // BUILDFLAG(ENABLE_BRAVE_REWARDS)
#endif // !BUILDFLAG(IS_ANDROID)
} // namespace brave_ads