Merge pull request #11594 from /issues/20089

New tab page ads should adhere to frequency caps
brave · Dec 14, 2021 · 6a3100c · 6a3100c
2 parents 63a729d + 78b77f5
commit 6a3100c
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 15 deletions.
diff --git a/...at-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.cc b/...at-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.cc
@@ -112,7 +112,8 @@ bool AdServing::ServeAd(const NewTabPageAdInfo& ad,
                         GetNewTabPageAdCallback callback) const {
   DCHECK(ad.IsValid());
 
-  // TODO(tmancey): Add logging for wallpapers
+  // 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"

diff --git a/...-native-ads/src/bat/ads/internal/ads/ad_notifications/ad_notification_permission_rules.cc b/...-native-ads/src/bat/ads/internal/ads/ad_notifications/ad_notification_permission_rules.cc
@@ -54,25 +54,29 @@ bool PermissionRules::HasPermission() const {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   CatalogFrequencyCap catalog_frequency_cap;
   if (!ShouldAllow(&catalog_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   IssuersFrequencyCap issuers_frequency_cap;
   if (!ShouldAllow(&issuers_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   UnblindedTokensFrequencyCap unblinded_tokens_frequency_cap;
   if (!ShouldAllow(&unblinded_tokens_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   UserActivityFrequencyCap user_activity_frequency_cap;
   if (!ShouldAllow(&user_activity_frequency_cap)) {
     return false;

diff --git a/...ive-ads/src/bat/ads/internal/ads/inline_content_ads/inline_content_ad_permission_rules.cc b/...ive-ads/src/bat/ads/internal/ads/inline_content_ads/inline_content_ad_permission_rules.cc
@@ -22,25 +22,29 @@ PermissionRules::PermissionRules() = default;
 PermissionRules::~PermissionRules() = default;
 
 bool PermissionRules::HasPermission() const {
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   CatalogFrequencyCap catalog_frequency_cap;
   if (!ShouldAllow(&catalog_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   IssuersFrequencyCap issuers_frequency_cap;
   if (!ShouldAllow(&issuers_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   UnblindedTokensFrequencyCap unblinded_tokens_frequency_cap;
   if (!ShouldAllow(&unblinded_tokens_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   UserActivityFrequencyCap user_activity_frequency_cap;
   if (!ShouldAllow(&user_activity_frequency_cap)) {
     return false;

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad.cc b/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad.cc
@@ -11,6 +11,7 @@
 #include "bat/ads/internal/ad_events/ad_event_util.h"
 #include "bat/ads/internal/ad_events/new_tab_page_ads/new_tab_page_ad_event_factory.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.h"
 #include "bat/ads/internal/bundle/creative_new_tab_page_ad_info.h"
 #include "bat/ads/internal/database/tables/ad_events_database_table.h"
 #include "bat/ads/internal/database/tables/creative_new_tab_page_ads_database_table.h"
@@ -43,6 +44,18 @@ 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,
@@ -84,6 +97,14 @@ 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);
 

diff --git a/...-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_permission_rules.cc b/...-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_permission_rules.cc
@@ -22,25 +22,29 @@ PermissionRules::PermissionRules() = default;
 PermissionRules::~PermissionRules() = default;
 
 bool PermissionRules::HasPermission() const {
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   CatalogFrequencyCap catalog_frequency_cap;
   if (!ShouldAllow(&catalog_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   IssuersFrequencyCap issuers_frequency_cap;
   if (!ShouldAllow(&issuers_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   UnblindedTokensFrequencyCap unblinded_tokens_frequency_cap;
   if (!ShouldAllow(&unblinded_tokens_frequency_cap)) {
     return false;
   }
 
-  // TODO(tmancey): Move to permission rules base class
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Move to
+  // permission rules base class
   UserActivityFrequencyCap user_activity_frequency_cap;
   if (!ShouldAllow(&user_activity_frequency_cap)) {
     return false;

diff --git a/...ds/src/bat/ads/internal/frequency_capping/permission_rules/user_activity_frequency_cap.cc b/...ds/src/bat/ads/internal/frequency_capping/permission_rules/user_activity_frequency_cap.cc
@@ -5,6 +5,7 @@
 
 #include "bat/ads/internal/frequency_capping/permission_rules/user_activity_frequency_cap.h"
 
+#include "bat/ads/internal/account/account_util.h"
 #include "bat/ads/internal/platform/platform_helper.h"
 #include "bat/ads/internal/user_activity/user_activity_scoring_util.h"
 
@@ -15,6 +16,10 @@ UserActivityFrequencyCap::UserActivityFrequencyCap() = default;
 UserActivityFrequencyCap::~UserActivityFrequencyCap() = default;
 
 bool UserActivityFrequencyCap::ShouldAllow() {
+  if (!ShouldRewardUser()) {
+    return true;
+  }
+
   if (!DoesRespectCap()) {
     last_message_ = "User was inactive";
     return false;

diff --git a/vendor/bat-native-ads/src/bat/ads/new_tab_page_ad_info.cc b/vendor/bat-native-ads/src/bat/ads/new_tab_page_ad_info.cc
@@ -84,7 +84,8 @@ bool NewTabPageAdInfo::FromJson(const std::string& json) {
     alt = document["alt"].GetString();
   }
 
-  // TODO(tmancey): Read wallpapers JSON
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Read wallpapers
+  // JSON
 
   if (document.HasMember("target_url")) {
     target_url = document["target_url"].GetString();
@@ -127,7 +128,8 @@ void SaveToJson(JsonWriter* writer, const NewTabPageAdInfo& info) {
   writer->String("alt");
   writer->String(info.alt.c_str());
 
-  // TODO(tmancey): Write wallpapers JSON
+  // TODO(https://github.com/brave/brave-browser/issues/14015): Write wallpapers
+  // JSON
 
   writer->String("target_url");
   writer->String(info.target_url.c_str());