Skip to content

Commit

Permalink
Merge pull request #26083 from brave/issues/41695
Browse files Browse the repository at this point in the history
[ads] Followup to "Smart NTT" #41303
  • Loading branch information
tmancey authored Oct 18, 2024
2 parents 2222894 + 96b7cfc commit 25ddfaf
Show file tree
Hide file tree
Showing 27 changed files with 189 additions and 23 deletions.
1 change: 1 addition & 0 deletions browser/brave_ads/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ source_set("brave_ads") {
"//chrome/browser/profiles:profile",
"//components/keyed_service/content",
"//components/prefs",
"//components/search_engines",
"//components/sessions:session_id",
"//content/public/browser",
]
Expand Down
20 changes: 20 additions & 0 deletions browser/brave_ads/ads_service_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

#include <utility>

#include "base/strings/utf_string_conversions.h"
#include "brave/browser/brave_ads/ad_units/notification_ad/notification_ad_platform_bridge.h"
#include "brave/browser/brave_ads/application_state/notification_helper/notification_helper.h"
#include "brave/browser/ui/brave_ads/notification_ad.h"
#include "brave/components/brave_adaptive_captcha/brave_adaptive_captcha_service.h"
#include "build/build_config.h"
#include "chrome/browser/notifications/notification_display_service.h"
#include "chrome/browser/profiles/profile.h"
#include "components/search_engines/template_url_prepopulate_data.h"

#if BUILDFLAG(IS_ANDROID)
#include "brave/browser/notifications/brave_notification_platform_bridge_helper_android.h"
#include "chrome/browser/android/service_tab_launcher.h"
Expand All @@ -29,12 +32,15 @@ namespace brave_ads {

AdsServiceDelegate::AdsServiceDelegate(
Profile* profile,
PrefService* local_state,
brave_adaptive_captcha::BraveAdaptiveCaptchaService*
adaptive_captcha_service,
NotificationDisplayService* notification_display_service,
std::unique_ptr<NotificationAdPlatformBridge>
notification_ad_platform_bridge)
: profile_(profile),
local_state_(local_state),
search_engine_choice_service_(*profile_->GetPrefs(), local_state_),
adaptive_captcha_service_(adaptive_captcha_service),
notification_display_service_(notification_display_service),
notification_ad_platform_bridge_(
Expand Down Expand Up @@ -132,4 +138,18 @@ bool AdsServiceDelegate::IsFullScreenMode() {
return ::IsFullScreenMode();
}
#endif

base::Value::Dict AdsServiceDelegate::GetVirtualPrefs() {
const auto template_url_data =
TemplateURLPrepopulateData::GetPrepopulatedFallbackSearch(
profile_->GetPrefs(), &search_engine_choice_service_);
if (!template_url_data) {
return {};
}

return base::Value::Dict().Set(
"[virtual]:default_search_engine.name",
base::UTF16ToUTF8(template_url_data->short_name()));
}

} // namespace brave_ads
19 changes: 15 additions & 4 deletions browser/brave_ads/ads_service_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
#include <string>

#include "base/memory/raw_ptr.h"
#include "base/values.h"
#include "brave/components/brave_ads/browser/ads_service.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/search_engine_choice/search_engine_choice_service.h"

class Profile;

Expand All @@ -29,18 +32,21 @@ class AdsServiceDelegate : public AdsService::Delegate {
public:
explicit AdsServiceDelegate(
Profile* profile,
PrefService* local_state,
brave_adaptive_captcha::BraveAdaptiveCaptchaService*
adaptive_captcha_service,
NotificationDisplayService* notification_display_service,
std::unique_ptr<NotificationAdPlatformBridge>
notification_ad_platform_bridge);
~AdsServiceDelegate() override;

AdsServiceDelegate(const AdsServiceDelegate&) = delete;
AdsServiceDelegate& operator=(const AdsServiceDelegate&) = delete;

AdsServiceDelegate(AdsServiceDelegate&&) noexcept = delete;
AdsServiceDelegate& operator=(AdsServiceDelegate&&) noexcept = delete;

~AdsServiceDelegate() override;

// AdsService::Delegate implementation
void InitNotificationHelper() override;
bool CanShowSystemNotificationsWhileBrowserIsBackgrounded() override;
Expand All @@ -64,11 +70,16 @@ class AdsServiceDelegate : public AdsService::Delegate {
#else
bool IsFullScreenMode() override;
#endif

base::Value::Dict GetVirtualPrefs() override;

private:
raw_ptr<Profile> profile_;
raw_ptr<Profile> profile_ = nullptr;
raw_ptr<PrefService> local_state_ = nullptr;
search_engines::SearchEngineChoiceService search_engine_choice_service_;
raw_ptr<brave_adaptive_captcha::BraveAdaptiveCaptchaService>
adaptive_captcha_service_;
raw_ptr<NotificationDisplayService> notification_display_service_;
adaptive_captcha_service_ = nullptr;
raw_ptr<NotificationDisplayService> notification_display_service_ = nullptr;
std::unique_ptr<NotificationAdPlatformBridge>
notification_ad_platform_bridge_;
};
Expand Down
3 changes: 2 additions & 1 deletion browser/brave_ads/ads_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ AdsServiceFactory::BuildServiceInstanceForBrowserContext(
->GetForProfile(profile);
auto* display_service = NotificationDisplayService::GetForProfile(profile);
auto* delegate = new AdsServiceDelegate(
profile, brave_adaptive_captcha_service, display_service,
profile, g_browser_process->local_state(), brave_adaptive_captcha_service,
display_service,
std::make_unique<NotificationAdPlatformBridge>(*profile));

auto* history_service = HistoryServiceFactory::GetInstance()->GetForProfile(
Expand Down
5 changes: 5 additions & 0 deletions components/brave_ads/browser/ads_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ class AdsService : public KeyedService {
#else
virtual bool IsFullScreenMode() = 0;
#endif

virtual base::Value::Dict GetVirtualPrefs() = 0;

protected:
virtual ~Delegate() = default;
};
Expand All @@ -71,6 +74,8 @@ class AdsService : public KeyedService {
virtual void AddObserver(AdsServiceObserver* observer) = 0;
virtual void RemoveObserver(AdsServiceObserver* observer) = 0;

virtual Delegate* GetDelegate() = 0;

// Returns true if a browser upgrade is required to serve ads.
virtual bool IsBrowserUpgradeRequiredToServeAds() const = 0;

Expand Down
8 changes: 8 additions & 0 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,10 @@ void AdsServiceImpl::AddBatAdsObserver(
}
}

AdsService::Delegate* AdsServiceImpl::GetDelegate() {
return delegate_;
}

bool AdsServiceImpl::IsBrowserUpgradeRequiredToServeAds() const {
return browser_upgrade_required_to_serve_ads_;
}
Expand Down Expand Up @@ -1824,6 +1828,10 @@ void AdsServiceImpl::HasLocalStatePrefPath(
std::move(callback).Run(local_state_->HasPrefPath(path));
}

void AdsServiceImpl::GetVirtualPrefs(GetVirtualPrefsCallback callback) {
std::move(callback).Run(delegate_->GetVirtualPrefs());
}

void AdsServiceImpl::Log(const std::string& file,
const int32_t line,
const int32_t verbose_level,
Expand Down
4 changes: 4 additions & 0 deletions components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ class AdsServiceImpl final : public AdsService,
void AddBatAdsObserver(mojo::PendingRemote<bat_ads::mojom::BatAdsObserver>
bat_ads_observer_pending_remote) override;

Delegate* GetDelegate() override;

bool IsBrowserUpgradeRequiredToServeAds() const override;

int64_t GetMaximumNotificationAdsPerHour() const override;
Expand Down Expand Up @@ -386,6 +388,8 @@ class AdsServiceImpl final : public AdsService,
void HasLocalStatePrefPath(const std::string& path,
HasLocalStatePrefPathCallback callback) override;

void GetVirtualPrefs(GetVirtualPrefsCallback callback) override;

void Log(const std::string& file,
int32_t line,
int32_t verbose_level,
Expand Down
2 changes: 2 additions & 0 deletions components/brave_ads/browser/ads_service_mock.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class AdsServiceMock : public AdsService {
MOCK_METHOD(void, AddObserver, (AdsServiceObserver * observer));
MOCK_METHOD(void, RemoveObserver, (AdsServiceObserver * observer));

MOCK_METHOD(AdsService::Delegate*, GetDelegate, ());

MOCK_METHOD(void,
AddBatAdsObserver,
(mojo::PendingRemote<bat_ads::mojom::BatAdsObserver>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ class AdsClientMock : public AdsClient {
MOCK_METHOD(void, ClearLocalStatePref, (const std::string& path));
MOCK_METHOD(bool, HasLocalStatePrefPath, (const std::string& path), (const));

MOCK_METHOD(base::Value::Dict, GetVirtualPrefs, (), (const));

MOCK_METHOD(void,
Log,
(const char* file,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,17 @@ bool AdsClientPrefProvider::HasLocalStatePrefPath(
return GetAdsClient()->HasLocalStatePrefPath(pref_path);
}

std::optional<base::Value> AdsClientPrefProvider::GetVirtualPref(
const std::string& pref_path) const {
if (pref_path.starts_with(kVirtualPrefPathPrefix)) {
const base::Value::Dict virtual_prefs = GetAdsClient()->GetVirtualPrefs();
if (const base::Value* const value = virtual_prefs.Find(pref_path)) {
return value->Clone();
}
}

// The virtual preference does not exist.
return std::nullopt;
}

} // namespace brave_ads
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class AdsClientPrefProvider : public PrefProviderInterface {
std::optional<base::Value> GetLocalStatePref(
const std::string& pref_path) const override;
bool HasLocalStatePrefPath(const std::string& pref_path) const override;

std::optional<base::Value> GetVirtualPref(
const std::string& pref_path) const override;
};

} // namespace brave_ads
Expand Down
21 changes: 19 additions & 2 deletions components/brave_ads/core/internal/prefs/pref_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@

#include "brave/components/brave_ads/core/public/prefs/pref_provider.h"

#include <utility>

#include "components/prefs/pref_service.h"

namespace brave_ads {

PrefProvider::PrefProvider(PrefService* const profile_prefs,
PrefService* const local_state_prefs)
: profile_prefs_(profile_prefs), local_state_prefs_(local_state_prefs) {
PrefService* const local_state_prefs,
base::Value::Dict virtual_prefs)
: profile_prefs_(profile_prefs),
local_state_prefs_(local_state_prefs),
virtual_prefs_(std::move(virtual_prefs)) {
CHECK(profile_prefs_);
CHECK(local_state_prefs_);
}
Expand Down Expand Up @@ -46,4 +51,16 @@ bool PrefProvider::HasLocalStatePrefPath(const std::string& pref_path) const {
return local_state_prefs_->HasPrefPath(pref_path);
}

std::optional<base::Value> PrefProvider::GetVirtualPref(
const std::string& pref_path) const {
if (pref_path.starts_with(kVirtualPrefPathPrefix)) {
if (const base::Value* const value = virtual_prefs_.Find(pref_path)) {
return value->Clone();
}
}

// The virtual preference does not exist.
return std::nullopt;
}

} // namespace brave_ads
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <vector>

#include "base/functional/bind.h"
#include "brave/components/brave_ads/core/internal/ads_client/ads_client_pref_provider.h"
#include "brave/components/brave_ads/core/internal/ads_client/ads_client_util.h"
#include "brave/components/brave_ads/core/internal/common/logging_util.h"
#include "brave/components/brave_ads/core/internal/serving/eligible_ads/eligible_ads_feature.h"
Expand All @@ -26,19 +25,6 @@

namespace brave_ads {

namespace {

void ApplyConditionMatcher(CreativeNewTabPageAdList& creative_ads) {
const AdsClientPrefProvider pref_provider;
std::erase_if(creative_ads, [&pref_provider](const auto& creative_ad) {
return creative_ad.wallpapers.size() != 1 ||
!MatchConditions(&pref_provider,
creative_ad.wallpapers[0].condition_matchers);
});
}

} // namespace

EligibleNewTabPageAdsV2::EligibleNewTabPageAdsV2(
const SubdivisionTargeting& subdivision_targeting,
const AntiTargetingResource& anti_targeting_resource)
Expand Down Expand Up @@ -152,6 +138,15 @@ void EligibleNewTabPageAdsV2::FilterAndMaybePredictCreativeAd(
std::move(callback).Run(/*eligible_ads=*/{});
}

void EligibleNewTabPageAdsV2::ApplyConditionMatcher(
CreativeNewTabPageAdList& creative_ads) {
std::erase_if(creative_ads, [this](const auto& creative_ad) {
return creative_ad.wallpapers.size() != 1 ||
!MatchConditions(&pref_provider_,
creative_ad.wallpapers[0].condition_matchers);
});
}

void EligibleNewTabPageAdsV2::FilterIneligibleCreativeAds(
CreativeNewTabPageAdList& creative_ads,
const AdEventList& ad_events,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_SERVING_ELIGIBLE_ADS_PIPELINES_NEW_TAB_PAGE_ADS_ELIGIBLE_NEW_TAB_PAGE_ADS_V2_H_

#include "base/memory/weak_ptr.h"
#include "brave/components/brave_ads/core/internal/ads_client/ads_client_pref_provider.h"
#include "brave/components/brave_ads/core/internal/creatives/new_tab_page_ads/creative_new_tab_page_ad_info.h"
#include "brave/components/brave_ads/core/internal/creatives/new_tab_page_ads/creative_new_tab_page_ads_database_table.h"
#include "brave/components/brave_ads/core/internal/segments/segment_alias.h"
Expand Down Expand Up @@ -51,6 +52,8 @@ class EligibleNewTabPageAdsV2 final : public EligibleNewTabPageAdsBase {
const SegmentList& segments,
const CreativeNewTabPageAdList& creative_ads);

void ApplyConditionMatcher(CreativeNewTabPageAdList& creative_ads);

void FilterAndMaybePredictCreativeAd(
const UserModelInfo& user_model,
const CreativeNewTabPageAdList& creative_ads,
Expand All @@ -65,6 +68,8 @@ class EligibleNewTabPageAdsV2 final : public EligibleNewTabPageAdsBase {

const database::table::AdEvents ad_events_database_table_;

const AdsClientPrefProvider pref_provider_;

base::WeakPtrFactory<EligibleNewTabPageAdsV2> weak_factory_{this};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ std::optional<base::Value> MaybeGetRootPrefValue(
const std::string& pref_path) {
CHECK(pref_provider);

if (pref_path.starts_with(kVirtualPrefPathPrefix)) {
return pref_provider->GetVirtualPref(pref_path);
}

if (std::optional<base::Value> pref_value =
pref_provider->GetProfilePref(pref_path)) {
return pref_value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,29 @@ TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
EXPECT_FALSE(MatchRegex("foo.baz.bar", "bar.*.foo"));
}

TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
GetVirtualPrefValue) {
// Arrange
ON_CALL(ads_client_mock_, GetVirtualPrefs).WillByDefault([]() {
return base::Value::Dict().Set("[virtual]:matrix", /*room*/ 303);
});

// Act & Assert
EXPECT_EQ(base::Value(/*room*/ 303),
MaybeGetPrefValue(&pref_provider_, "[virtual]:matrix"));
}

TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
DoNotGetUnknownVirtualPrefValue) {
// Arrange
ON_CALL(ads_client_mock_, GetVirtualPrefs).WillByDefault([]() {
return base::Value::Dict().Set("[virtual]:inverse.matrices", /*room*/ 101);
});

// Act & Assert
EXPECT_FALSE(MaybeGetPrefValue(&pref_provider_, "[virtual]:matrix"));
}

TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
GetBooleanProfilePrefValue) {
// Arrange
Expand Down
3 changes: 3 additions & 0 deletions components/brave_ads/core/public/ads_client/ads_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ class ADS_EXPORT AdsClient {
// preference `path`.
virtual bool HasLocalStatePrefPath(const std::string& path) const = 0;

// Get the virtual preferences.
virtual base::Value::Dict GetVirtualPrefs() const = 0;

// Log a `message` to `file` and the console log with `line` and
// `verbose_level`.
virtual void Log(const char* file,
Expand Down
Loading

0 comments on commit 25ddfaf

Please sign in to comment.