Skip to content

Commit 54106dd

Browse files
committed
Try trigger NTPSI ad on each NTP refresh.
fix brave/brave-browser#24107
1 parent ce08868 commit 54106dd

8 files changed

+72
-36
lines changed

components/brave_ads/browser/ads_service.h

+3
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ class AdsService : public KeyedService {
186186
const ads::mojom::InlineContentAdEventType event_type) = 0;
187187

188188
// Called to prefetch the next new tab page ad.
189+
virtual void PrefetchNewTabPageAd() = 0;
190+
191+
// Called to get a prefetched new tab page ad.
189192
virtual absl::optional<ads::NewTabPageAdInfo> GetPrefetchedNewTabPageAd() = 0;
190193

191194
// Called when failing to prefetch a new tab page ad for |placement_id| and

components/brave_ads/browser/ads_service_impl.cc

+33-26
Original file line numberDiff line numberDiff line change
@@ -724,8 +724,21 @@ void AdsServiceImpl::CleanUpOnFirstRun() {
724724

725725
RemoveDeprecatedFiles();
726726

727-
PurgeOrphanedAdEventsForType(ads::mojom::AdType::kNewTabPageAd,
728-
base::DoNothing());
727+
// Purge orphaned new tab page ad events which may have remained from the
728+
// previous browser startup. If prefetch_new_tab_page_ad_on_first_run_ value
729+
// is true, then it means that there was an attempt to prefetch a new tab page
730+
// ad prior to CleanUpOnFirstRun() call. In this case, repeating the prefetch
731+
// of a new tab page ad is needed after the purge.
732+
if (prefetch_new_tab_page_ad_on_first_run_) {
733+
prefetch_new_tab_page_ad_on_first_run_ = false;
734+
PurgeOrphanedAdEventsForType(
735+
ads::mojom::AdType::kNewTabPageAd,
736+
base::BindOnce(&AdsServiceImpl::OnPurgeOrphanedNewTabPageAdEvents,
737+
AsWeakPtr()));
738+
} else {
739+
PurgeOrphanedAdEventsForType(ads::mojom::AdType::kNewTabPageAd,
740+
base::DoNothing());
741+
}
729742
}
730743

731744
void AdsServiceImpl::RemoveDeprecatedFiles() const {
@@ -1309,27 +1322,13 @@ AdsServiceImpl::GetPrefetchedNewTabPageAd() {
13091322
prefetched_new_tab_page_ad_info_.reset();
13101323
}
13111324

1312-
if (purge_orphaned_new_tab_page_ad_events_time_ &&
1313-
*purge_orphaned_new_tab_page_ad_events_time_ <= base::Time::Now()) {
1314-
purge_orphaned_new_tab_page_ad_events_time_.reset();
1315-
PurgeOrphanedAdEventsForType(
1316-
ads::mojom::AdType::kNewTabPageAd,
1317-
base::BindOnce(&AdsServiceImpl::OnPurgeOrphanedNewTabPageAdEvents,
1318-
AsWeakPtr()));
1319-
} else {
1320-
PrefetchNewTabPageAd();
1321-
}
1322-
13231325
return ad_info;
13241326
}
13251327

13261328
void AdsServiceImpl::OnFailedToPrefetchNewTabPageAd(
13271329
const std::string& placement_id,
13281330
const std::string& creative_instance_id) {
1329-
if (!purge_orphaned_new_tab_page_ad_events_time_) {
1330-
purge_orphaned_new_tab_page_ad_events_time_ =
1331-
base::Time::Now() + base::Hours(1);
1332-
}
1331+
need_purge_orphaned_new_tab_page_ad_events_ = true;
13331332
}
13341333

13351334
void AdsServiceImpl::TriggerNewTabPageAdEvent(
@@ -1922,6 +1921,13 @@ void AdsServiceImpl::OnResourceComponentUpdated(const std::string& id) {
19221921
}
19231922

19241923
void AdsServiceImpl::PrefetchNewTabPageAd() {
1924+
if (!did_cleanup_on_first_run_) {
1925+
// Postpone prefetching of a new tab page ad at a later time during
1926+
// CleanUpOnFirstRun() call.
1927+
prefetch_new_tab_page_ad_on_first_run_ = true;
1928+
return;
1929+
}
1930+
19251931
if (!IsBatAdsBound()) {
19261932
return;
19271933
}
@@ -1932,8 +1938,16 @@ void AdsServiceImpl::PrefetchNewTabPageAd() {
19321938
return;
19331939
}
19341940

1935-
bat_ads_->MaybeServeNewTabPageAd(
1936-
base::BindOnce(&AdsServiceImpl::OnPrefetchNewTabPageAd, AsWeakPtr()));
1941+
if (need_purge_orphaned_new_tab_page_ad_events_) {
1942+
need_purge_orphaned_new_tab_page_ad_events_ = false;
1943+
PurgeOrphanedAdEventsForType(
1944+
ads::mojom::AdType::kNewTabPageAd,
1945+
base::BindOnce(&AdsServiceImpl::OnPurgeOrphanedNewTabPageAdEvents,
1946+
AsWeakPtr()));
1947+
} else {
1948+
bat_ads_->MaybeServeNewTabPageAd(
1949+
base::BindOnce(&AdsServiceImpl::OnPrefetchNewTabPageAd, AsWeakPtr()));
1950+
}
19371951
}
19381952

19391953
void AdsServiceImpl::OnPrefetchNewTabPageAd(bool success,
@@ -1942,13 +1956,6 @@ void AdsServiceImpl::OnPrefetchNewTabPageAd(bool success,
19421956
return;
19431957
}
19441958

1945-
// The previous successfully prefetched new tab page ad was not served.
1946-
if (prefetched_new_tab_page_ad_info_ &&
1947-
!purge_orphaned_new_tab_page_ad_events_time_) {
1948-
purge_orphaned_new_tab_page_ad_events_time_ =
1949-
base::Time::Now() + base::Hours(1);
1950-
}
1951-
19521959
ads::NewTabPageAdInfo ad_info;
19531960
ad_info.FromJson(json);
19541961
prefetched_new_tab_page_ad_info_ = ad_info;

components/brave_ads/browser/ads_service_impl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class AdsServiceImpl : public AdsService,
182182
bool StopNotificationAdTimeOutTimer(const std::string& placement_id);
183183
void NotificationAdTimedOut(const std::string& placement_id);
184184

185-
void PrefetchNewTabPageAd();
185+
void PrefetchNewTabPageAd() override;
186186
void OnPrefetchNewTabPageAd(bool success, const std::string& json);
187187

188188
void OnTriggerSearchResultAdEvent(
@@ -482,7 +482,8 @@ class AdsServiceImpl : public AdsService,
482482
notification_ad_timers_;
483483

484484
absl::optional<ads::NewTabPageAdInfo> prefetched_new_tab_page_ad_info_;
485-
absl::optional<base::Time> purge_orphaned_new_tab_page_ad_events_time_;
485+
bool need_purge_orphaned_new_tab_page_ad_events_ = false;
486+
bool prefetch_new_tab_page_ad_on_first_run_ = false;
486487

487488
std::string retry_opening_new_tab_for_ad_with_placement_id_;
488489

components/brave_ads/browser/mock_ads_service.h

+1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class MockAdsService : public AdsService {
8484

8585
MOCK_METHOD0(GetPrefetchedNewTabPageAd,
8686
absl::optional<ads::NewTabPageAdInfo>());
87+
MOCK_METHOD0(PrefetchNewTabPageAd, void());
8788
MOCK_METHOD3(TriggerNewTabPageAdEvent,
8889
void(const std::string&,
8990
const std::string&,

components/ntp_background_images/browser/ntp_sponsored_images_data.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ base::Value NTPSponsoredImagesData::GetBackgroundByAdInfo(
347347
}
348348
}
349349
if (campaign_index == campaigns.size()) {
350-
VLOG(0) << "Ad campaign wasn't found in NTP sposored images data: "
350+
VLOG(0) << "The ad campaign wasn't found in the NTP sponsored images data: "
351351
<< ad_info.campaign_id;
352352
return base::Value();
353353
}

components/ntp_background_images/browser/view_counter_service.cc

+12
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ void ViewCounterService::RegisterPageView() {
301301
// This will be no-op when component is not ready.
302302
service_->CheckNTPSIComponentUpdateIfNeeded();
303303
model_.RegisterPageView();
304+
MaybePrefetchNewTabPageAd();
304305
}
305306

306307
void ViewCounterService::BrandedWallpaperLogoClicked(
@@ -374,6 +375,17 @@ std::string ViewCounterService::GetSuperReferralCode() const {
374375
return service_->GetSuperReferralCode();
375376
}
376377

378+
void ViewCounterService::MaybePrefetchNewTabPageAd() {
379+
NTPSponsoredImagesData* images_data = GetCurrentBrandedWallpaperData();
380+
if (!IsBrandedWallpaperActive() || !ads_service_ ||
381+
!ads_service_->IsEnabled() || !images_data ||
382+
images_data->IsSuperReferral()) {
383+
return;
384+
}
385+
386+
ads_service_->PrefetchNewTabPageAd();
387+
}
388+
377389
void ViewCounterService::UpdateP3AValues() const {
378390
uint64_t new_tab_count = new_tab_count_state_->GetHighestValueInWeek();
379391
p3a_utils::RecordToHistogramBucket("Brave.NTP.NewTabsCreated",

components/ntp_background_images/browser/view_counter_service.h

+2
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@ class ViewCounterService : public KeyedService,
145145

146146
void ResetModel();
147147

148+
void MaybePrefetchNewTabPageAd();
149+
148150
void UpdateP3AValues() const;
149151

150152
raw_ptr<NTPBackgroundImagesService> service_ = nullptr;

components/ntp_background_images/browser/view_counter_service_unittest.cc

+17-7
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,13 @@ class NTPBackgroundImagesViewCounterTest : public testing::Test {
207207
return ad_info;
208208
}
209209

210+
int GetInitialCountToBrandedWallpaper() const {
211+
return ViewCounterModel::kInitialCountToBrandedWallpaper;
212+
}
213+
210214
base::Value TryGetFirstSponsoredImageWallpaper() {
211215
// Loading initial count times.
212-
for (int i = 0; i < ViewCounterModel::kInitialCountToBrandedWallpaper;
213-
++i) {
216+
for (int i = 0; i < GetInitialCountToBrandedWallpaper(); ++i) {
214217
base::Value wallpaper = view_counter_->GetCurrentWallpaperForDisplay();
215218
EXPECT_TRUE(wallpaper.FindBoolKey(ntp_background_images::kIsBackgroundKey)
216219
.value_or(false));
@@ -367,7 +370,7 @@ TEST_F(NTPBackgroundImagesViewCounterTest, ModelTest) {
367370

368371
// Initial count is not changed because branded wallpaper is always
369372
// visible in SR mode.
370-
int expected_count = ViewCounterModel::kInitialCountToBrandedWallpaper;
373+
int expected_count = GetInitialCountToBrandedWallpaper();
371374
view_counter_->RegisterPageView();
372375
view_counter_->RegisterPageView();
373376
EXPECT_EQ(expected_count, view_counter_->model_.count_to_branded_wallpaper_);
@@ -409,8 +412,9 @@ TEST_F(NTPBackgroundImagesViewCounterTest,
409412
GetSposoredImageWallpaperAdsServiceDisabled) {
410413
InitBackgroundAndSponsoredImageWallpapers();
411414

412-
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(false));
415+
EXPECT_CALL(ads_service_, IsEnabled()).WillRepeatedly(Return(false));
413416
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd()).Times(0);
417+
EXPECT_CALL(ads_service_, PrefetchNewTabPageAd()).Times(0);
414418

415419
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();
416420
EXPECT_FALSE(si_wallpaper.FindBoolKey(ntp_background_images::kIsBackgroundKey)
@@ -430,9 +434,11 @@ TEST_F(NTPBackgroundImagesViewCounterTest,
430434
TEST_F(NTPBackgroundImagesViewCounterTest, SponsoredImageAdFrequencyCapped) {
431435
InitBackgroundAndSponsoredImageWallpapers();
432436

433-
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(true));
437+
EXPECT_CALL(ads_service_, IsEnabled()).WillRepeatedly(Return(true));
434438
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd())
435439
.WillOnce(Return(absl::nullopt));
440+
EXPECT_CALL(ads_service_, PrefetchNewTabPageAd())
441+
.Times(GetInitialCountToBrandedWallpaper());
436442
EXPECT_CALL(ads_service_, OnFailedToPrefetchNewTabPageAd(_, _)).Times(0);
437443

438444
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();
@@ -450,9 +456,11 @@ TEST_F(NTPBackgroundImagesViewCounterTest, SponsoredImageAdServed) {
450456
ads::NewTabPageAdInfo ad_info = CreateNewTabPageAdInfo();
451457
EXPECT_TRUE(AdInfoMatchesSponsoredImage(ad_info, 0, 1));
452458

453-
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(true));
459+
EXPECT_CALL(ads_service_, IsEnabled()).WillRepeatedly(Return(true));
454460
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd())
455461
.WillOnce(Return(ad_info));
462+
EXPECT_CALL(ads_service_, PrefetchNewTabPageAd())
463+
.Times(GetInitialCountToBrandedWallpaper());
456464
EXPECT_CALL(ads_service_, OnFailedToPrefetchNewTabPageAd(_, _)).Times(0);
457465

458466
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();
@@ -477,9 +485,11 @@ TEST_F(NTPBackgroundImagesViewCounterTest, WrongSponsoredImageAdServed) {
477485
ad_info.creative_instance_id = "wrong_creative_instance_id";
478486
EXPECT_FALSE(AdInfoMatchesSponsoredImage(ad_info, 0, 1));
479487

480-
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(true));
488+
EXPECT_CALL(ads_service_, IsEnabled()).WillRepeatedly(Return(true));
481489
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd())
482490
.WillOnce(Return(ad_info));
491+
EXPECT_CALL(ads_service_, PrefetchNewTabPageAd())
492+
.Times(GetInitialCountToBrandedWallpaper());
483493
EXPECT_CALL(ads_service_, OnFailedToPrefetchNewTabPageAd(_, _));
484494

485495
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();

0 commit comments

Comments
 (0)