Fire ad events if brave ads are disabled.

aseren · aseren · commit f8e707eb8957 · 2022-03-10T16:41:59.000+07:00
diff --git a/browser/brave_ads/ads_service_impl.cc b/browser/brave_ads/ads_service_impl.cc
@@ -701,12 +701,13 @@ void AdsServiceImpl::OnInitialize(const bool success) {
 void AdsServiceImpl::SetupOnFirstInitialize() {
   DCHECK(!is_setup_on_first_initialize_done_);
 
-  PurgeOrphanedAdEventsForType(
-      ads::mojom::AdType::kNewTabPageAd,
-      base::BindOnce(&AdsServiceImpl::PrefetchNewTabPageAd, AsWeakPtr()));
-
   file_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&RemoveDeprecatedAdsDataFiles, base_path_));
+
+  PurgeOrphanedAdEventsForType(
+      ads::mojom::AdType::kNewTabPageAd,
+      base::BindOnce(&AdsServiceImpl::OnPurgeOrphanedAdEventsForNewTabPageAds,
+                     AsWeakPtr()));
 }
 
 void AdsServiceImpl::ShutdownBatAds() {
@@ -1190,7 +1191,7 @@ void AdsServiceImpl::OnInlineContentAdEvent(
 
 void AdsServiceImpl::PurgeOrphanedAdEventsForType(
     const ads::mojom::AdType ad_type,
-    base::OnceClosure callback) {
+    PurgeOrphanedAdEventsForTypeCallback callback) {
   if (!connected()) {
     return;
   }
@@ -1342,6 +1343,17 @@ void AdsServiceImpl::OnGetInlineContentAd(OnGetInlineContentAdCallback callback,
   std::move(callback).Run(success, dimensions, dictionary);
 }
 
+void AdsServiceImpl::OnPurgeOrphanedAdEventsForNewTabPageAds(
+    const bool success) {
+  if (!success) {
+    VLOG(0) << "Failed to purge orphaned ad events for new tab page ads";
+    return;
+  }
+
+  VLOG(0) << "Successfully purged orphaned ad events for new tab page ads";
+  PrefetchNewTabPageAd();
+}
+
 void AdsServiceImpl::OnGetAdsHistory(OnGetAdsHistoryCallback callback,
                                      const std::string& json) {
   ads::AdsHistoryInfo ads_history;
diff --git a/browser/brave_ads/ads_service_impl.h b/browser/brave_ads/ads_service_impl.h
@@ -177,8 +177,9 @@ class AdsServiceImpl : public AdsService,
 
   absl::optional<ads::NewTabPageAdInfo> GetPrefetchedNewTabPageAd() override;
 
-  void PurgeOrphanedAdEventsForType(const ads::mojom::AdType ad_type,
-                                    base::OnceClosure callback) override;
+  void PurgeOrphanedAdEventsForType(
+      const ads::mojom::AdType ad_type,
+      PurgeOrphanedAdEventsForTypeCallback callback) override;
 
   void GetAdsHistory(const double from_timestamp,
                      const double to_timestamp,
@@ -271,7 +272,6 @@ class AdsServiceImpl : public AdsService,
   void RegisterResourceComponentsForLocale(const std::string& locale);
 
   void PrefetchNewTabPageAd();
-
   void OnPrefetchNewTabPageAd(bool success, const std::string& json);
 
   void OnURLRequestStarted(
@@ -289,6 +289,8 @@ class AdsServiceImpl : public AdsService,
                             const std::string& dimensions,
                             const std::string& json);
 
+  void OnPurgeOrphanedAdEventsForNewTabPageAds(const bool success);
+
   void OnGetAdsHistory(OnGetAdsHistoryCallback callback,
                        const std::string& json);
 
diff --git a/browser/notifications/BUILD.gn b/browser/notifications/BUILD.gn
@@ -15,7 +15,7 @@ source_set("notifications") {
   deps = [
     "//base",
     "//brave/components/brave_adaptive_captcha/buildflags",
-    "//brave/vendor/bat-native-ads",
+    "//brave/components/brave_ads/browser",
     "//chrome/browser/notifications",
     "//third_party/abseil-cpp:absl",
     "//url",
diff --git a/components/brave_ads/browser/ads_service.h b/components/brave_ads/browser/ads_service.h
@@ -56,6 +56,9 @@ using GetAccountStatementCallback = base::OnceCallback<
 using GetAdDiagnosticsCallback =
     base::OnceCallback<void(const bool, const std::string&)>;
 
+using PurgeOrphanedAdEventsForTypeCallback =
+    base::OnceCallback<void(const bool)>;
+
 class AdsService : public KeyedService {
  public:
   AdsService();
@@ -140,8 +143,9 @@ class AdsService : public KeyedService {
 
   virtual absl::optional<ads::NewTabPageAdInfo> GetPrefetchedNewTabPageAd() = 0;
 
-  virtual void PurgeOrphanedAdEventsForType(const ads::mojom::AdType ad_type,
-                                            base::OnceClosure callback) = 0;
+  virtual void PurgeOrphanedAdEventsForType(
+      const ads::mojom::AdType ad_type,
+      PurgeOrphanedAdEventsForTypeCallback callback) = 0;
 
   virtual void GetAdsHistory(const double from_timestamp,
                              const double to_timestamp,
diff --git a/components/brave_ads/browser/mock_ads_service.h b/components/brave_ads/browser/mock_ads_service.h
@@ -93,7 +93,7 @@ class MockAdsService : public AdsService {
                absl::optional<ads::NewTabPageAdInfo>());
 
   MOCK_METHOD2(PurgeOrphanedAdEventsForType,
-               void(ads::mojom::AdType, base::OnceClosure));
+               void(ads::mojom::AdType, PurgeOrphanedAdEventsForTypeCallback));
 
   MOCK_METHOD3(GetAdsHistory, void(double, double, OnGetAdsHistoryCallback));
 
diff --git a/components/brave_ads/test/BUILD.gn b/components/brave_ads/test/BUILD.gn
@@ -93,6 +93,7 @@ source_set("brave_ads_unit_tests") {
     "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/inline_content_ads/inline_content_ad_permission_rules_unittest_util.cc",
     "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/inline_content_ads/inline_content_ad_permission_rules_unittest_util.h",
     "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/inline_content_ads/inline_content_ad_unittest.cc",
+    "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_if_ads_disabled_unittest.cc",
     "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_permission_rules_unittest_util.cc",
     "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_permission_rules_unittest_util.h",
     "//brave/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_unittest.cc",
diff --git a/components/ntp_background_images/browser/view_counter_service.cc b/components/ntp_background_images/browser/view_counter_service.cc
@@ -170,9 +170,9 @@ base::Value ViewCounterService::GetCurrentBrandedWallpaper() const {
     return base::Value();
   }
 
-  const bool should_ads_frequency_cap =
+  const bool should_frequency_cap_ads =
       ads_service_ && ads_service_->IsEnabled();
-  if (should_ads_frequency_cap && !images_data->IsSuperReferral()) {
+  if (should_frequency_cap_ads && !images_data->IsSuperReferral()) {
     return GetCurrentBrandedWallpaperByAdInfo();
   }
 
diff --git a/components/services/bat_ads/bat_ads_impl.cc b/components/services/bat_ads/bat_ads_impl.cc
@@ -186,7 +186,7 @@ void BatAdsImpl::PurgeOrphanedAdEventsForType(
       AsWeakPtr(), std::move(callback));
 
   auto purge_ad_events_for_type_callback =
-      std::bind(BatAdsImpl::OnPurgeOrphanedAdEventsForType, holder);
+      std::bind(BatAdsImpl::OnPurgeOrphanedAdEventsForType, holder, _1);
 
   ads_->PurgeOrphanedAdEventsForType(ad_type,
                                      purge_ad_events_for_type_callback);
@@ -338,9 +338,10 @@ void BatAdsImpl::OnGetInlineContentAd(
 }
 
 void BatAdsImpl::OnPurgeOrphanedAdEventsForType(
-    CallbackHolder<PurgeOrphanedAdEventsForTypeCallback>* holder) {
+    CallbackHolder<PurgeOrphanedAdEventsForTypeCallback>* holder,
+    const bool success) {
   if (holder->is_valid()) {
-    std::move(holder->get()).Run();
+    std::move(holder->get()).Run(success);
   }
 
   delete holder;
diff --git a/components/services/bat_ads/bat_ads_impl.h b/components/services/bat_ads/bat_ads_impl.h
@@ -186,7 +186,8 @@ class BatAdsImpl :
         const ads::InlineContentAdInfo& ad);
 
     static void OnPurgeOrphanedAdEventsForType(
-        CallbackHolder<PurgeOrphanedAdEventsForTypeCallback>* holder);
+        CallbackHolder<PurgeOrphanedAdEventsForTypeCallback>* holder,
+        const bool success);
 
     static void OnRemoveAllHistory(
         CallbackHolder<RemoveAllHistoryCallback>* holder,
diff --git a/components/services/bat_ads/public/interfaces/bat_ads.mojom b/components/services/bat_ads/public/interfaces/bat_ads.mojom
@@ -93,7 +93,7 @@ interface BatAds {
   OnPromotedContentAdEvent(string uuid, string creative_instance_id, ads.mojom.PromotedContentAdEventType event_type);
   GetInlineContentAd(string size) => (bool success, string dimensions, string ad);
   OnInlineContentAdEvent(string uuid, string creative_instance_id, ads.mojom.InlineContentAdEventType event_type);
-  PurgeOrphanedAdEventsForType(ads.mojom.AdType ad_type) => ();
+  PurgeOrphanedAdEventsForType(ads.mojom.AdType ad_type) => (bool success);
   RemoveAllHistory() => (bool success);
   OnWalletUpdated(string payment_id, string seed);
   GetAdsHistory(double from_timestamp, double to_timestamp) => (string json);
diff --git a/ios/browser/api/ads/brave_ads.h b/ios/browser/api/ads/brave_ads.h
@@ -161,7 +161,8 @@ OBJC_EXPORT
                            eventType:(AdsPromotedContentAdEventType)eventType;
 
 /// Purge orphaned ad events for a given ad type
-- (void)purgeOrphanedAdEvents:(AdsAdType)adType;
+- (void)purgeOrphanedAdEvents:(AdsAdType)adType
+                   completion:(void (^)(BOOL success))completion;
 
 /// Get the number of ads received and the estimated earnings of viewing said
 /// ads for this cycle
diff --git a/ios/browser/api/ads/brave_ads.mm b/ios/browser/api/ads/brave_ads.mm
@@ -651,12 +651,13 @@ - (void)reportPromotedContentAdEvent:(NSString*)uuid
       static_cast<ads::mojom::PromotedContentAdEventType>(eventType));
 }
 
-- (void)purgeOrphanedAdEvents:(AdsAdType)adType {
+- (void)purgeOrphanedAdEvents:(AdsAdType)adType
+                   completion:(void (^)(BOOL success))completion {
   if (![self isAdsServiceRunning]) {
     return;
   }
-  ads->PurgeOrphanedAdEventsForType(static_cast<ads::mojom::AdType>(adType), ^{
-                                    });
+  ads->PurgeOrphanedAdEventsForType(static_cast<ads::mojom::AdType>(adType),
+                                    completion);
 }
 
 - (void)detailsForCurrentCycle:(void (^)(NSInteger adsReceived,
diff --git a/vendor/bat-native-ads/include/bat/ads/ads_aliases.h b/vendor/bat-native-ads/include/bat/ads/ads_aliases.h
@@ -32,7 +32,7 @@ using GetAccountStatementCallback =
 using GetAdDiagnosticsCallback =
     std::function<void(const bool, const std::string&)>;
 
-using PurgeOrphanedAdEventsForTypeCallback = std::function<void()>;
+using PurgeOrphanedAdEventsForTypeCallback = std::function<void(const bool)>;
 
 }  // namespace ads
 
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.cc b/vendor/bat-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.cc
@@ -111,6 +111,7 @@ bool AdServing::IsSupported() const {
 bool AdServing::ServeAd(const NewTabPageAdInfo& ad,
                         GetNewTabPageAdCallback callback) const {
   DCHECK(ad.IsValid());
+  DCHECK(!ad.wallpapers.empty());
 
   BLOG(1, "Serving new tab page ad:\n"
               << "  uuid: " << ad.uuid << "\n"
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving_test.cc b/vendor/bat-native-ads/src/bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving_test.cc
@@ -5,6 +5,8 @@
 
 #include "bat/ads/internal/ad_serving/new_tab_page_ads/new_tab_page_ad_serving.h"
 
+#include <algorithm>
+
 #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"
@@ -123,7 +125,7 @@ TEST_F(BatAdsNewTabPageAdServingTest, ServeAd) {
 }
 
 TEST_F(BatAdsNewTabPageAdServingTest,
-       DoNotServeAdIfExceededPerDayCapFromCatalog) {
+       ServeAdIfNotExceededPerDayExclusionRuleFrequencyCap) {
   // Arrange
   new_tab_page_ads::frequency_capping::ForcePermissionRules();
 
@@ -132,9 +134,47 @@ TEST_F(BatAdsNewTabPageAdServingTest,
   creative_ads.push_back(creative_ad);
   Save(creative_ads);
 
+  const int allowed_ad_count_per_day =
+      std::min(creative_ad.per_day, creative_ad.daily_cap);
+
   AdEventInfo ad_event = BuildAdEvent(creative_ad, AdType::kNewTabPageAd,
                                       ConfirmationType::kServed, Now());
-  for (int i = 0; i < creative_ad.per_day; ++i) {
+  for (int i = 0; i < allowed_ad_count_per_day - 1; ++i) {
+    FireAdEvent(ad_event);
+  }
+
+  AdvanceClock(base::Hours(1));
+
+  // Act
+  ad_serving_->MaybeServeAd(
+      [&creative_ad](const bool success, const NewTabPageAdInfo& ad) {
+        ASSERT_TRUE(success);
+
+        NewTabPageAdInfo expected_ad = BuildNewTabPageAd(creative_ad);
+        expected_ad.uuid = ad.uuid;
+
+        EXPECT_EQ(expected_ad, ad);
+      });
+
+  // Assert
+}
+
+TEST_F(BatAdsNewTabPageAdServingTest,
+       DoNotServeAdIfExceededPerDayExclusionRuleFrequencyCap) {
+  // Arrange
+  new_tab_page_ads::frequency_capping::ForcePermissionRules();
+
+  CreativeNewTabPageAdList creative_ads;
+  CreativeNewTabPageAdInfo creative_ad = BuildCreativeNewTabPageAd();
+  creative_ads.push_back(creative_ad);
+  Save(creative_ads);
+
+  const int allowed_ad_count_per_day =
+      std::min(creative_ad.per_day, creative_ad.daily_cap);
+
+  AdEventInfo ad_event = BuildAdEvent(creative_ad, AdType::kNewTabPageAd,
+                                      ConfirmationType::kServed, Now());
+  for (int i = 0; i < allowed_ad_count_per_day; ++i) {
     FireAdEvent(ad_event);
   }
 
@@ -162,7 +202,8 @@ TEST_F(BatAdsNewTabPageAdServingTest,
   // Assert
 }
 
-TEST_F(BatAdsNewTabPageAdServingTest, ServeAdIfNotExceededAdsPerHourCap) {
+TEST_F(BatAdsNewTabPageAdServingTest,
+       ServeAdIfNotExceededAdsPerHourPermissionRuleFrequencyCap) {
   // Arrange
   new_tab_page_ads::frequency_capping::ForcePermissionRules();
 
@@ -195,7 +236,8 @@ TEST_F(BatAdsNewTabPageAdServingTest, ServeAdIfNotExceededAdsPerHourCap) {
   // Assert
 }
 
-TEST_F(BatAdsNewTabPageAdServingTest, DoNotServeAdIfExceededAdsPerHourCap) {
+TEST_F(BatAdsNewTabPageAdServingTest,
+       DoNotServeAdIfExceededAdsPerHourPermissionRuleFrequencyCap) {
   // Arrange
   new_tab_page_ads::frequency_capping::ForcePermissionRules();
 
@@ -222,7 +264,8 @@ TEST_F(BatAdsNewTabPageAdServingTest, DoNotServeAdIfExceededAdsPerHourCap) {
   // Assert
 }
 
-TEST_F(BatAdsNewTabPageAdServingTest, ServeAdIfNotExceededAdsPerDayCap) {
+TEST_F(BatAdsNewTabPageAdServingTest,
+       ServeAdIfNotExceededAdsPerDayPermissionRuleFrequencyCap) {
   // Arrange
   new_tab_page_ads::frequency_capping::ForcePermissionRules();
 
@@ -257,7 +300,8 @@ TEST_F(BatAdsNewTabPageAdServingTest, ServeAdIfNotExceededAdsPerDayCap) {
   // Assert
 }
 
-TEST_F(BatAdsNewTabPageAdServingTest, DoNotServeAdIfExceededAdsPerDayCap) {
+TEST_F(BatAdsNewTabPageAdServingTest,
+       DoNotServeAdIfExceededAdsPerDayPermissionRuleFrequencyCap) {
   // Arrange
   new_tab_page_ads::frequency_capping::ForcePermissionRules();
 
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
@@ -6,6 +6,7 @@
 #include "bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad.h"
 
 #include "base/check.h"
+#include "bat/ads/internal/account/account_util.h"
 #include "bat/ads/internal/ad_events/ad_event.h"
 #include "bat/ads/internal/ad_events/ad_event_info.h"
 #include "bat/ads/internal/ad_events/ad_event_util.h"
@@ -44,6 +45,16 @@ void NewTabPageAd::FireEvent(const std::string& uuid,
     return;
   }
 
+  // Apply permission rules frequency capping for new tab page ad view events
+  // if Brave Ads are disabled.
+  new_tab_page_ads::frequency_capping::PermissionRules permission_rules;
+  if (event_type == mojom::NewTabPageAdEventType::kViewed &&
+      !ShouldRewardUser() && !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,
@@ -88,6 +99,14 @@ void NewTabPageAd::FireEvent(const NewTabPageAdInfo& ad,
           return;
         }
 
+        if (event_type == mojom::NewTabPageAdEventType::kViewed &&
+            !ShouldRewardUser()) {
+          // Fire an ad served event if Brave Ads are disabled and the ad
+          // wasn't served by 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/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_if_ads_disabled_unittest.cc b/vendor/bat-native-ads/src/bat/ads/internal/ads/new_tab_page_ads/new_tab_page_ad_if_ads_disabled_unittest.cc
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ads_impl.cc b/vendor/bat-native-ads/src/bat/ads/internal/ads_impl.cc
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/bundle/bundle.cc b/vendor/bat-native-ads/src/bat/ads/internal/bundle/bundle.cc
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_ad_unittest_util.cc b/vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_ad_unittest_util.cc
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/catalog/catalog_info.cc b/vendor/bat-native-ads/src/bat/ads/internal/catalog/catalog_info.cc

Original file line number	Diff line number	Diff line change
`@@ -170,9 +170,9 @@ base::Value ViewCounterService::GetCurrentBrandedWallpaper() const {`
`170`	`170`	`return base::Value();`
`171`	`171`	`}`
`172`	`172`
`173`		`- const bool should_ads_frequency_cap =`
	`173`	`+ const bool should_frequency_cap_ads =`
`174`	`174`	`ads_service_ && ads_service_->IsEnabled();`
`175`		`- if (should_ads_frequency_cap && !images_data->IsSuperReferral()) {`
	`175`	`+ if (should_frequency_cap_ads && !images_data->IsSuperReferral()) {`
`176`	`176`	`return GetCurrentBrandedWallpaperByAdInfo();`
`177`	`177`	`}`
`178`	`178`