Skip to content

Commit

Permalink
Do not frequency cap on new tab page view event.
Browse files Browse the repository at this point in the history 
fix brave/brave-browser#14015
  • Loading branch information
@aseren
aseren committed Feb 21, 2022
1 parent 2bdce3c commit 73116ec
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 145 deletions.
8 changes: 5 additions & 3 deletions ...at-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ bool AdServing::ServeAd(const NewTabPageAdInfo& ad,
GetNewTabPageAdCallback callback) const {
DCHECK(ad.IsValid());

// TODO(https://github.com/brave/brave-browser/issues/14015): Add logging for
// wallpapers
BLOG(1, "Serving new tab page ad:\n"
<< " uuid: " << ad.uuid << "\n"
<< " creativeInstanceId: " << ad.creative_instance_id << "\n"
Expand All @@ -124,7 +122,11 @@ bool AdServing::ServeAd(const NewTabPageAdInfo& ad,
<< " companyName: " << ad.company_name << "\n"
<< " imageUrl: " << ad.image_url << "\n"
<< " alt: " << ad.alt << "\n"
<< " targetUrl: " << ad.target_url);
<< " targetUrl: " << ad.target_url << "\n"
<< " wallpaperImageUrl: " << ad.wallpapers[0].image_url << "\n"
<< " wallpaperFocalPointX: " << ad.wallpapers[0].focal_point.x
<< "\n"
<< " wallpaperFocalPointY: " << ad.wallpapers[0].focal_point.y);

callback(/* success */ true, ad);

Expand Down
156 changes: 154 additions & 2 deletions ...tive-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@

#include "bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.h"

#include "bat/ads/internal/ad_events/ad_event_unittest_util.h"
#include "bat/ads/internal/ad_serving/ad_serving_features.h"
#include "bat/ads/internal/ad_serving/ad_targeting/geographic/subdivision/subdivision_targeting.h"
#include "bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_builder.h"
#include "bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_permission_rules_unittest_util.h"
#include "bat/ads/internal/bundle/creative_new_tab_page_ad_unittest_util.h"
#include "bat/ads/internal/database/tables/creative_new_tab_page_ads_database_table.h"
#include "bat/ads/internal/frequency_capping/permission_rules/user_activity_permission_rule_unittest_util.h"
#include "bat/ads/internal/resources/frequency_capping/anti_targeting/anti_targeting_resource.h"
#include "bat/ads/internal/unittest_base.h"
#include "bat/ads/internal/unittest_time_util.h"
#include "bat/ads/internal/unittest_util.h"
#include "bat/ads/new_tab_page_ad_info.h"
#include "net/http/http_status_code.h"
Expand Down Expand Up @@ -97,10 +101,10 @@ class BatAdsNewTabPageAdServingTest : public UnitTestBase {

TEST_F(BatAdsNewTabPageAdServingTest, ServeAd) {
// Arrange
ForceUserActivityFrequencyCapPermission();
new_tab_page_ads::frequency_capping::ForcePermissionRules();

CreativeNewTabPageAdList creative_ads;
const CreativeNewTabPageAdInfo& creative_ad = BuildCreativeNewTabPageAd();
CreativeNewTabPageAdInfo creative_ad = BuildCreativeNewTabPageAd();
creative_ads.push_back(creative_ad);
Save(creative_ads);

Expand All @@ -118,6 +122,30 @@ TEST_F(BatAdsNewTabPageAdServingTest, ServeAd) {
// Assert
}

TEST_F(BatAdsNewTabPageAdServingTest,
DoNotServeAdIfExceededPerDayCapFromCatalog) {
// Arrange
new_tab_page_ads::frequency_capping::ForcePermissionRules();

CreativeNewTabPageAdList creative_ads;
CreativeNewTabPageAdInfo creative_ad = BuildCreativeNewTabPageAd();
creative_ads.push_back(creative_ad);
Save(creative_ads);

AdEventInfo ad_event = BuildAdEvent(creative_ad, AdType::kNewTabPageAd,
ConfirmationType::kServed, Now());
for (int i = 0; i < creative_ad.per_day; ++i) {
FireAdEvent(ad_event);
}

// Act
ad_serving_->MaybeServeAd([](const bool success, const NewTabPageAdInfo& ad) {
EXPECT_FALSE(success);
});

// Assert
}

TEST_F(BatAdsNewTabPageAdServingTest,
DoNotServeAdIfNotAllowedDueToPermissionRules) {
// Arrange
Expand All @@ -134,4 +162,128 @@ TEST_F(BatAdsNewTabPageAdServingTest,
// Assert
}

TEST_F(BatAdsNewTabPageAdServingTest, ServeAdIfNotExceededAdsPerHourCap) {
// Arrange
new_tab_page_ads::frequency_capping::ForcePermissionRules();

CreativeNewTabPageAdList creative_ads;
CreativeNewTabPageAdInfo creative_ad1 = BuildCreativeNewTabPageAd();
CreativeNewTabPageAdInfo creative_ad2 = BuildCreativeNewTabPageAd();
creative_ads.push_back(creative_ad1);
creative_ads.push_back(creative_ad2);
Save(creative_ads);

AdEventInfo ad_event1 = BuildAdEvent(creative_ad1, AdType::kNewTabPageAd,
ConfirmationType::kServed, Now());

const int ads_per_hour = features::GetMaximumNewTabPageAdsPerHour();
for (int i = 0; i < ads_per_hour - 1; ++i) {
FireAdEvent(ad_event1);
}

// Act
ad_serving_->MaybeServeAd(
[&creative_ad2](const bool success, const NewTabPageAdInfo& ad) {
ASSERT_TRUE(success);

NewTabPageAdInfo expected_ad = BuildNewTabPageAd(creative_ad2);
expected_ad.uuid = ad.uuid;

EXPECT_EQ(expected_ad, ad);
});

// Assert
}

TEST_F(BatAdsNewTabPageAdServingTest, DoNotServeAdIfExceededAdsPerHourCap) {
// Arrange
new_tab_page_ads::frequency_capping::ForcePermissionRules();

CreativeNewTabPageAdList creative_ads;
CreativeNewTabPageAdInfo creative_ad1 = BuildCreativeNewTabPageAd();
CreativeNewTabPageAdInfo creative_ad2 = BuildCreativeNewTabPageAd();
creative_ads.push_back(creative_ad1);
creative_ads.push_back(creative_ad2);
Save(creative_ads);

AdEventInfo ad_event1 = BuildAdEvent(creative_ad1, AdType::kNewTabPageAd,
ConfirmationType::kServed, Now());

const int ads_per_hour = features::GetMaximumNewTabPageAdsPerHour();
for (int i = 0; i < ads_per_hour; ++i) {
FireAdEvent(ad_event1);
}

// Act
ad_serving_->MaybeServeAd([](const bool success, const NewTabPageAdInfo& ad) {
EXPECT_FALSE(success);
});

// Assert
}

TEST_F(BatAdsNewTabPageAdServingTest, ServeAdIfNotExceededAdsPerDayCap) {
// Arrange
new_tab_page_ads::frequency_capping::ForcePermissionRules();

CreativeNewTabPageAdList creative_ads;
CreativeNewTabPageAdInfo creative_ad1 = BuildCreativeNewTabPageAd();
CreativeNewTabPageAdInfo creative_ad2 = BuildCreativeNewTabPageAd();
creative_ads.push_back(creative_ad1);
creative_ads.push_back(creative_ad2);
Save(creative_ads);

AdEventInfo ad_event1 = BuildAdEvent(creative_ad1, AdType::kNewTabPageAd,
ConfirmationType::kServed, Now());

const int ads_per_day = features::GetMaximumNewTabPageAdsPerDay();
for (int i = 0; i < ads_per_day - 1; ++i) {
FireAdEvent(ad_event1);
}

AdvanceClock(base::Hours(1));

// Act
ad_serving_->MaybeServeAd(
[&creative_ad2](const bool success, const NewTabPageAdInfo& ad) {
ASSERT_TRUE(success);

NewTabPageAdInfo expected_ad = BuildNewTabPageAd(creative_ad2);
expected_ad.uuid = ad.uuid;

EXPECT_EQ(expected_ad, ad);
});

// Assert
}

TEST_F(BatAdsNewTabPageAdServingTest, DoNotServeAdIfExceededAdsPerDayCap) {
// Arrange
new_tab_page_ads::frequency_capping::ForcePermissionRules();

CreativeNewTabPageAdList creative_ads;
CreativeNewTabPageAdInfo creative_ad1 = BuildCreativeNewTabPageAd();
CreativeNewTabPageAdInfo creative_ad2 = BuildCreativeNewTabPageAd();
creative_ads.push_back(creative_ad1);
creative_ads.push_back(creative_ad2);
Save(creative_ads);

AdEventInfo ad_event1 = BuildAdEvent(creative_ad1, AdType::kNewTabPageAd,
ConfirmationType::kServed, Now());

const int ads_per_day = features::GetMaximumNewTabPageAdsPerDay();
for (int i = 0; i < ads_per_day; ++i) {
FireAdEvent(ad_event1);
}

AdvanceClock(base::Hours(1));

// Act
ad_serving_->MaybeServeAd([](const bool success, const NewTabPageAdInfo& ad) {
EXPECT_FALSE(success);
});

// Assert
}

} // namespace ads
20 changes: 0 additions & 20 deletions vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ void NewTabPageAd::FireEvent(const std::string& uuid,
return;
}

// TODO(https://github.com/brave/brave-browser/issues/14015): Refactor this
// logic to only apply frequency capping if the new tab page ad was not served
// by the library. |AdServing::MaybeServeAd| is responsible for applying
// frequency caps for new tab page ads served by the library
new_tab_page_ads::frequency_capping::PermissionRules permission_rules;
if (event_type == mojom::NewTabPageAdEventType::kViewed &&
!permission_rules.HasPermission()) {
BLOG(1, "New tab page ad: Not allowed due to permission rules");
NotifyNewTabPageAdEventFailed(uuid, creative_instance_id, event_type);
return;
}

database::table::CreativeNewTabPageAds database_table;
database_table.GetForCreativeInstanceId(
creative_instance_id,
Expand Down Expand Up @@ -100,14 +88,6 @@ void NewTabPageAd::FireEvent(const NewTabPageAdInfo& ad,
return;
}

if (event_type == mojom::NewTabPageAdEventType::kViewed) {
// TODO(https://github.com/brave/brave-browser/issues/14015): We need
// to fire an ad served event until new tab page ads are served by the
// ads library
FireEvent(uuid, creative_instance_id,
mojom::NewTabPageAdEventType::kServed);
}

const auto ad_event =
new_tab_page_ads::AdEventFactory::Build(event_type);
ad_event->FireEvent(ad);
Expand Down
Loading

0 comments on commit 73116ec

Please sign in to comment.