Skip to content

Commit 25c169e

Browse files
aserenaseren
committed
Handle NTP SI show failed case.
1 parent bc970c0 commit 25c169e

File tree

7 files changed

+88
-12
lines changed

7 files changed

+88
-12
lines changed

‎components/brave_ads/browser/ads_service.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ class AdsService : public KeyedService {
136136
const std::string& placement_id,
137137
const std::string& creative_instance_id,
138138
const ads::mojom::NewTabPageAdEventType event_type) = 0;
139+
virtual void OnFailedToServeNewTabPageAd(
140+
const std::string& placement_id,
141+
const std::string& creative_instance_id) = 0;
139142

140143
virtual void TriggerPromotedContentAdEvent(
141144
const std::string& placement_id,

‎components/brave_ads/browser/ads_service_impl.cc

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,9 @@ void AdsServiceImpl::SetupOnFirstInitialize() {
695695
file_task_runner_->PostTask(
696696
FROM_HERE, base::BindOnce(&RemoveDeprecatedAdsDataFiles, base_path_));
697697

698+
// Initiate prefetching of the new tab page ad. Also need to purge orphaned
699+
// new tab page ad events which may have remained from the previous browser
700+
// startup.
698701
PurgeOrphanedAdEventsForType(
699702
ads::mojom::AdType::kNewTabPageAd,
700703
base::BindOnce(&AdsServiceImpl::OnPurgeOrphanedAdEventsForNewTabPageAds,
@@ -1147,6 +1150,15 @@ void AdsServiceImpl::TriggerNewTabPageAdEvent(
11471150
event_type);
11481151
}
11491152

1153+
void AdsServiceImpl::OnFailedToServeNewTabPageAd(
1154+
const std::string& placement_id,
1155+
const std::string& creative_instance_id) {
1156+
if (!purge_orphaned_new_tab_page_ad_events_time_) {
1157+
purge_orphaned_new_tab_page_ad_events_time_ =
1158+
base::Time::Now() + base::Hours(1);
1159+
}
1160+
}
1161+
11501162
void AdsServiceImpl::TriggerPromotedContentAdEvent(
11511163
const std::string& placement_id,
11521164
const std::string& creative_instance_id,
@@ -1211,7 +1223,16 @@ AdsServiceImpl::GetPrefetchedNewTabPageAd() {
12111223
prefetched_new_tab_page_ad_info_.reset();
12121224
}
12131225

1214-
PrefetchNewTabPageAd();
1226+
if (purge_orphaned_new_tab_page_ad_events_time_ &&
1227+
*purge_orphaned_new_tab_page_ad_events_time_ <= base::Time::Now()) {
1228+
purge_orphaned_new_tab_page_ad_events_time_.reset();
1229+
PurgeOrphanedAdEventsForType(
1230+
ads::mojom::AdType::kNewTabPageAd,
1231+
base::BindOnce(&AdsServiceImpl::OnPurgeOrphanedAdEventsForNewTabPageAds,
1232+
AsWeakPtr()));
1233+
} else {
1234+
PrefetchNewTabPageAd();
1235+
}
12151236

12161237
return ad_info;
12171238
}
@@ -1278,7 +1299,12 @@ void AdsServiceImpl::RegisterResourceComponentsForLocale(
12781299

12791300
void AdsServiceImpl::PrefetchNewTabPageAd() {
12801301
if (!connected()) {
1281-
prefetched_new_tab_page_ad_info_.reset();
1302+
return;
1303+
}
1304+
1305+
// The previous prefetched new tab page ad is available. No need to do
1306+
// prefetch again.
1307+
if (prefetched_new_tab_page_ad_info_) {
12821308
return;
12831309
}
12841310

@@ -1288,6 +1314,13 @@ void AdsServiceImpl::PrefetchNewTabPageAd() {
12881314

12891315
void AdsServiceImpl::OnPrefetchNewTabPageAd(bool success,
12901316
const std::string& json) {
1317+
// The previous prefetched new tab page ad was not served.
1318+
if (prefetched_new_tab_page_ad_info_ &&
1319+
!purge_orphaned_new_tab_page_ad_events_time_) {
1320+
purge_orphaned_new_tab_page_ad_events_time_ =
1321+
base::Time::Now() + base::Hours(1);
1322+
}
1323+
12911324
if (!success) {
12921325
prefetched_new_tab_page_ad_info_.reset();
12931326
return;

‎components/brave_ads/browser/ads_service_impl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ class AdsServiceImpl : public AdsService,
161161
const std::string& placement_id,
162162
const std::string& creative_instance_id,
163163
const ads::mojom::NewTabPageAdEventType event_type) override;
164+
void OnFailedToServeNewTabPageAd(
165+
const std::string& placement_id,
166+
const std::string& creative_instance_id) override;
164167

165168
void TriggerPromotedContentAdEvent(
166169
const std::string& placement_id,
@@ -513,6 +516,7 @@ class AdsServiceImpl : public AdsService,
513516
std::unique_ptr<ads::Database> database_;
514517

515518
absl::optional<ads::NewTabPageAdInfo> prefetched_new_tab_page_ad_info_;
519+
absl::optional<base::Time> purge_orphaned_new_tab_page_ad_events_time_;
516520

517521
ui::IdleState last_idle_state_;
518522
int last_idle_time_;

‎components/brave_ads/browser/mock_ads_service.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ class MockAdsService : public AdsService {
7878
void(const std::string&,
7979
const std::string&,
8080
ads::mojom::NewTabPageAdEventType));
81+
MOCK_METHOD2(OnFailedToServeNewTabPageAd,
82+
void(const std::string&, const std::string&));
8183

8284
MOCK_METHOD3(TriggerPromotedContentAdEvent,
8385
void(const std::string&,

‎components/ntp_background_images/browser/ntp_sponsored_images_data.cc

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,8 @@ base::Value NTPSponsoredImagesData::GetBackgroundByAdInfo(
347347
}
348348
}
349349
if (campaign_index == campaigns.size()) {
350-
LOG(ERROR) << "Ad campaign wasn't found in NTP sposored images data: "
351-
<< ad_info.campaign_id;
350+
VLOG(0) << "Ad campaign wasn't found in NTP sposored images data: "
351+
<< ad_info.campaign_id;
352352
return base::Value();
353353
}
354354

@@ -361,16 +361,19 @@ base::Value NTPSponsoredImagesData::GetBackgroundByAdInfo(
361361
}
362362
}
363363
if (background_index == sponsored_backgrounds.size()) {
364-
LOG(ERROR) << "Creative instance wasn't found in NTP sposored images data: "
365-
<< ad_info.creative_instance_id;
364+
VLOG(0) << "Creative instance wasn't found in NTP sposored images data: "
365+
<< ad_info.creative_instance_id;
366366
return base::Value();
367367
}
368368

369-
if (!AdInfoMatchesSponsoredImage(ad_info, campaign_index, background_index)) {
370-
LOG(WARNING) << "Served creative info does not fully match with NTP "
371-
"sponsored images metadata. Campaign id: "
372-
<< ad_info.campaign_id
373-
<< ". Creative instance id: " << ad_info.creative_instance_id;
369+
if (VLOG_IS_ON(0)) {
370+
if (!AdInfoMatchesSponsoredImage(ad_info, campaign_index,
371+
background_index)) {
372+
VLOG(0) << "Served creative info does not fully match with NTP "
373+
"sponsored images metadata. Campaign id: "
374+
<< ad_info.campaign_id
375+
<< ". Creative instance id: " << ad_info.creative_instance_id;
376+
}
374377
}
375378

376379
base::Value data = GetBackgroundAt(campaign_index, background_index);

‎components/ntp_background_images/browser/view_counter_service.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,14 @@ base::Value ViewCounterService::GetCurrentBrandedWallpaperByAdInfo() const {
187187
return base::Value();
188188
}
189189

190-
return GetCurrentBrandedWallpaperData()->GetBackgroundByAdInfo(*ad_info);
190+
base::Value branded_wallpaper_data =
191+
GetCurrentBrandedWallpaperData()->GetBackgroundByAdInfo(*ad_info);
192+
if (!branded_wallpaper_data.is_dict()) {
193+
ads_service_->OnFailedToServeNewTabPageAd(ad_info->placement_id,
194+
ad_info->creative_instance_id);
195+
}
196+
197+
return branded_wallpaper_data;
191198
}
192199

193200
base::Value ViewCounterService::GetCurrentBrandedWallpaperFromModel() const {

‎components/ntp_background_images/browser/view_counter_service_unittest.cc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "brave/components/ntp_background_images/browser/ntp_custom_background_images_service.h"
3535
#endif
3636

37+
using testing::_;
3738
using testing::Return;
3839

3940
namespace ntp_background_images {
@@ -432,6 +433,7 @@ TEST_F(NTPBackgroundImagesViewCounterTest, SponsoredImageAdFrequencyCapped) {
432433
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(true));
433434
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd())
434435
.WillOnce(Return(absl::nullopt));
436+
EXPECT_CALL(ads_service_, OnFailedToServeNewTabPageAd(_, _)).Times(0);
435437

436438
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();
437439
EXPECT_TRUE(si_wallpaper.FindBoolKey(ntp_background_images::kIsBackgroundKey)
@@ -451,6 +453,7 @@ TEST_F(NTPBackgroundImagesViewCounterTest, SponsoredImageAdServed) {
451453
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(true));
452454
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd())
453455
.WillOnce(Return(ad_info));
456+
EXPECT_CALL(ads_service_, OnFailedToServeNewTabPageAd(_, _)).Times(0);
454457

455458
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();
456459
EXPECT_FALSE(si_wallpaper.FindBoolKey(ntp_background_images::kIsBackgroundKey)
@@ -467,4 +470,25 @@ TEST_F(NTPBackgroundImagesViewCounterTest, SponsoredImageAdServed) {
467470
*(si_wallpaper.FindStringKey(ntp_background_images::kWallpaperIDKey)));
468471
}
469472

473+
TEST_F(NTPBackgroundImagesViewCounterTest, WrongSponsoredImageAdServed) {
474+
InitBackgroundAndSponsoredImageWallpapers();
475+
476+
ads::NewTabPageAdInfo ad_info = CreateNewTabPageAdInfo();
477+
ad_info.creative_instance_id = "wrong_creative_instance_id";
478+
EXPECT_FALSE(AdInfoMatchesSponsoredImage(ad_info, 0, 1));
479+
480+
EXPECT_CALL(ads_service_, IsEnabled()).WillOnce(Return(true));
481+
EXPECT_CALL(ads_service_, GetPrefetchedNewTabPageAd())
482+
.WillOnce(Return(ad_info));
483+
EXPECT_CALL(ads_service_, OnFailedToServeNewTabPageAd(_, _));
484+
485+
base::Value si_wallpaper = TryGetFirstSponsoredImageWallpaper();
486+
EXPECT_TRUE(si_wallpaper.FindBoolKey(ntp_background_images::kIsBackgroundKey)
487+
.value_or(false));
488+
EXPECT_FALSE(si_wallpaper.FindStringKey(
489+
ntp_background_images::kCreativeInstanceIDKey));
490+
EXPECT_FALSE(
491+
si_wallpaper.FindStringKey(ntp_background_images::kWallpaperIDKey));
492+
}
493+
470494
} // namespace ntp_background_images

0 commit comments

Comments
 (0)