Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ads] Followup to "Smart NTT" #41303 #26083

Merged
merged 1 commit into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading