Decouple Brave Ads conversions and resolve #16090 and #26834

brave · Jul 8, 2023 · f97aaed · f97aaed
1 parent d482f8e
commit f97aaed
Show file tree

Hide file tree

Showing 244 changed files with 7,214 additions and 4,355 deletions.
diff --git a/browser/brave_ads/search_result_ad/search_result_ad_browsertest.cc b/browser/brave_ads/search_result_ad/search_result_ad_browsertest.cc
@@ -10,6 +10,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/time/time.h"
 #include "brave/browser/brave_ads/search_result_ad/search_result_ad_tab_helper.h"
 #include "brave/components/brave_ads/browser/ads_service.h"
 #include "brave/components/brave_ads/browser/ads_service_mock.h"
@@ -237,20 +238,21 @@ class SampleSearchResultAdTest : public SearchResultAdTest {
     EXPECT_DOUBLE_EQ(search_result_ad->value, 0.5 + ad_index);
 
     EXPECT_TRUE(search_result_ad->conversion);
-    EXPECT_EQ(search_result_ad->conversion->type,
-              base::StrCat({"data-conversion-type-value", index}));
     EXPECT_EQ(search_result_ad->conversion->url_pattern,
               base::StrCat({"data-conversion-url-pattern-value", index}));
     if (ad_index == 2) {
-      EXPECT_TRUE(search_result_ad->conversion->advertiser_public_key.empty());
+      EXPECT_FALSE(search_result_ad->conversion->extract_verifiable_id);
+      EXPECT_TRUE(search_result_ad->conversion
+                      ->verifiable_advertiser_public_key_base64.empty());
     } else {
+      EXPECT_TRUE(search_result_ad->conversion->extract_verifiable_id);
       EXPECT_EQ(
-          search_result_ad->conversion->advertiser_public_key,
+          search_result_ad->conversion->verifiable_advertiser_public_key_base64,
           base::StrCat({"data-conversion-advertiser-public-key-value", index}));
     }
-    EXPECT_EQ(
-        static_cast<size_t>(search_result_ad->conversion->observation_window),
-        ad_index);
+    EXPECT_EQ(static_cast<size_t>(
+                  search_result_ad->conversion->observation_window.InDays()),
+              ad_index);
 
     return true;
   }

diff --git a/components/brave_ads/common/interfaces/brave_ads.mojom b/components/brave_ads/common/interfaces/brave_ads.mojom
@@ -62,10 +62,10 @@ enum AdType {
 };
 
 struct ConversionInfo {
-  string type;
   string url_pattern;
-  string advertiser_public_key;
-  int32 observation_window;
+  bool extract_verifiable_id;
+  string verifiable_advertiser_public_key_base64;
+  mojo_base.mojom.TimeDelta observation_window;
 };
 
 enum NotificationAdEventType {

diff --git a/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler_unittest.cc b/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler_unittest.cc
@@ -15,6 +15,7 @@
 #include "brave/components/brave_ads/common/brave_ads_feature.h"
 #include "brave/components/brave_ads/common/search_result_ad_feature.h"
 #include "brave/components/brave_ads/content/browser/search_result_ad/search_result_ad_handler.h"
+#include "brave/components/brave_ads/core/search_result_ad/search_result_ad_converting_util.h"
 #include "brave/components/brave_ads/core/search_result_ad/test_web_page_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -49,6 +50,41 @@ GURL GetSearchResultAdClickedUrl() {
       {kSearchResultAdClickUrl, kPlacementId, "=", kTestWebPagePlacementId}));
 }
 
+void CompareSearchResultAdInfosWithNonEmptyConversion(
+    const mojom::SearchResultAdInfoPtr& search_result_ad_info1,
+    const mojom::SearchResultAdInfoPtr& search_result_ad_info2) {
+  EXPECT_EQ(search_result_ad_info1->placement_id,
+            search_result_ad_info2->placement_id);
+  EXPECT_EQ(search_result_ad_info1->advertiser_id,
+            search_result_ad_info2->advertiser_id);
+  EXPECT_EQ(search_result_ad_info1->campaign_id,
+            search_result_ad_info2->campaign_id);
+  EXPECT_EQ(search_result_ad_info1->creative_instance_id,
+            search_result_ad_info2->creative_instance_id);
+  EXPECT_EQ(search_result_ad_info1->creative_set_id,
+            search_result_ad_info2->creative_set_id);
+  EXPECT_EQ(search_result_ad_info1->description,
+            search_result_ad_info2->description);
+  EXPECT_EQ(search_result_ad_info1->headline_text,
+            search_result_ad_info2->headline_text);
+  EXPECT_EQ(search_result_ad_info1->target_url,
+            search_result_ad_info2->target_url);
+  EXPECT_EQ(search_result_ad_info1->type, search_result_ad_info2->type);
+  EXPECT_EQ(search_result_ad_info1->value, search_result_ad_info2->value);
+  ASSERT_TRUE(search_result_ad_info1->conversion);
+  ASSERT_TRUE(search_result_ad_info2->conversion);
+  EXPECT_EQ(search_result_ad_info1->conversion->extract_verifiable_id,
+            search_result_ad_info2->conversion->extract_verifiable_id);
+  EXPECT_EQ(search_result_ad_info1->conversion->observation_window,
+            search_result_ad_info2->conversion->observation_window);
+  EXPECT_EQ(search_result_ad_info1->conversion->url_pattern,
+            search_result_ad_info2->conversion->url_pattern);
+  EXPECT_EQ(search_result_ad_info1->conversion
+                ->verifiable_advertiser_public_key_base64,
+            search_result_ad_info2->conversion
+                ->verifiable_advertiser_public_key_base64);
+}
+
 }  // namespace
 
 class SearchResultAdHandlerTest : public ::testing::Test {
@@ -209,9 +245,14 @@ TEST_F(SearchResultAdHandlerTest, NotValidSearchResultAd) {
 
 TEST_F(SearchResultAdHandlerTest, EmptyConversions) {
   EXPECT_CALL(ads_service_mock_, IsEnabled()).WillRepeatedly(Return(true));
-  EXPECT_CALL(ads_service_mock_,
-              TriggerSearchResultAdEvent(
-                  _, mojom::SearchResultAdEventType::kViewed, _));
+  EXPECT_CALL(
+      ads_service_mock_,
+      TriggerSearchResultAdEvent(_, mojom::SearchResultAdEventType::kViewed, _))
+      .WillOnce([](mojom::SearchResultAdInfoPtr ad_mojom,
+                   mojom::SearchResultAdEventType /*event_type*/,
+                   TriggerAdEventCallback /*callback*/) {
+        EXPECT_FALSE(ad_mojom->conversion);
+      });
   EXPECT_CALL(ads_service_mock_,
               TriggerSearchResultAdEvent(
                   _, mojom::SearchResultAdEventType::kClicked, _));
@@ -222,7 +263,6 @@ TEST_F(SearchResultAdHandlerTest, EmptyConversions) {
           /*should_trigger_viewed_event*/ true);
   ASSERT_TRUE(search_result_ad_handler.get());
 
-  // "data-conversion-type-value" is missed.
   base::MockCallback<OnRetrieveSearchResultAdCallback> callback;
   EXPECT_CALL(callback, Run(_))
       .WillOnce([&search_result_ad_handler](
@@ -232,9 +272,11 @@ TEST_F(SearchResultAdHandlerTest, EmptyConversions) {
               placement_id);
         }
       });
+  // "data-conversion-url-pattern-value" is missed so conversions won't be
+  // parsed.
   SimulateOnRetrieveSearchResultAdEntities(
       search_result_ad_handler.get(), callback.Get(),
-      CreateTestWebPage({"data-conversion-type-value"}));
+      CreateTestWebPage({"data-conversion-url-pattern-value"}));
 
   search_result_ad_handler->MaybeTriggerSearchResultAdClickedEvent(
       GetSearchResultAdClickedUrl());
@@ -277,14 +319,36 @@ TEST_F(SearchResultAdHandlerTest, BraveAdsBecomeDisabled) {
 }
 
 TEST_F(SearchResultAdHandlerTest, BraveAdsViewedClicked) {
+  blink::mojom::WebPagePtr web_page = CreateTestWebPage();
+  ASSERT_TRUE(web_page);
+
   EXPECT_CALL(ads_service_mock_, IsEnabled()).WillRepeatedly(Return(true));
-  EXPECT_CALL(ads_service_mock_,
-              TriggerSearchResultAdEvent(
-                  _, mojom::SearchResultAdEventType::kViewed, _));
+  EXPECT_CALL(
+      ads_service_mock_,
+      TriggerSearchResultAdEvent(_, mojom::SearchResultAdEventType::kViewed, _))
+      .WillOnce([&web_page](mojom::SearchResultAdInfoPtr ad_mojom,
+                            mojom::SearchResultAdEventType /*event_type*/,
+                            TriggerAdEventCallback /*callback*/) {
+        const auto search_result_ads =
+            ConvertWebPageEntitiesToSearchResultAds(web_page->entities);
+        ASSERT_TRUE(search_result_ads.contains(kTestWebPagePlacementId));
+        CompareSearchResultAdInfosWithNonEmptyConversion(
+            search_result_ads.at(kTestWebPagePlacementId), ad_mojom);
+      });
+
   EXPECT_CALL(ads_service_mock_,
               TriggerSearchResultAdEvent(
                   _, mojom::SearchResultAdEventType::kClicked, _))
-      .Times(2);
+      .Times(2)
+      .WillRepeatedly([&web_page](mojom::SearchResultAdInfoPtr ad_mojom,
+                                  mojom::SearchResultAdEventType /*event_type*/,
+                                  TriggerAdEventCallback /*callback*/) {
+        const auto search_result_ads =
+            ConvertWebPageEntitiesToSearchResultAds(web_page->entities);
+        ASSERT_TRUE(search_result_ads.contains(kTestWebPagePlacementId));
+        CompareSearchResultAdInfosWithNonEmptyConversion(
+            search_result_ads.at(kTestWebPagePlacementId), ad_mojom);
+      });
 
   auto search_result_ad_handler =
       SearchResultAdHandler::MaybeCreateSearchResultAdHandler(
@@ -302,7 +366,7 @@ TEST_F(SearchResultAdHandlerTest, BraveAdsViewedClicked) {
         }
       });
   SimulateOnRetrieveSearchResultAdEntities(search_result_ad_handler.get(),
-                                           callback.Get(), CreateTestWebPage());
+                                           callback.Get(), web_page->Clone());
 
   search_result_ad_handler->MaybeTriggerSearchResultAdClickedEvent(
       GetSearchResultAdClickedUrl());

diff --git a/components/brave_ads/core/internal/BUILD.gn b/components/brave_ads/core/internal/BUILD.gn
@@ -156,8 +156,7 @@ source_set("internal") {
     "account/user_data/catalog_user_data.h",
     "account/user_data/conversion_user_data.cc",
     "account/user_data/conversion_user_data.h",
-    "account/user_data/conversion_user_data_builder.cc",
-    "account/user_data/conversion_user_data_builder.h",
+    "account/user_data/conversion_user_data_constants.h",
     "account/user_data/conversion_user_data_util.cc",
     "account/user_data/conversion_user_data_util.h",
     "account/user_data/created_at_timestamp_user_data.cc",
@@ -192,6 +191,8 @@ source_set("internal") {
     "ad_constants.cc",
     "ad_type.cc",
     "ads.cc",
+    "ads/ad_events/ad_event_builder.cc",
+    "ads/ad_events/ad_event_builder.h",
     "ads/ad_events/ad_event_handler_util.cc",
     "ads/ad_events/ad_event_handler_util.h",
     "ads/ad_events/ad_event_history.cc",
@@ -520,6 +521,8 @@ source_set("internal") {
     "catalog/campaign/catalog_daypart_info.h",
     "catalog/campaign/catalog_geo_target_info.cc",
     "catalog/campaign/catalog_geo_target_info.h",
+    "catalog/campaign/creative_set/catalog_conversion_info.cc",
+    "catalog/campaign/creative_set/catalog_conversion_info.h",
     "catalog/campaign/creative_set/catalog_creative_set_info.cc",
     "catalog/campaign/creative_set/catalog_creative_set_info.h",
     "catalog/campaign/creative_set/catalog_os_info.cc",
@@ -654,35 +657,70 @@ source_set("internal") {
     "common/url/url_util.cc",
     "common/url/url_util.h",
     "confirmation_type.cc",
-    "conversions/conversion_builder.cc",
-    "conversions/conversion_builder.h",
-    "conversions/conversion_info.cc",
-    "conversions/conversion_info.h",
-    "conversions/conversion_queue_database_table.cc",
-    "conversions/conversion_queue_database_table.h",
-    "conversions/conversion_queue_item_info.cc",
-    "conversions/conversion_queue_item_info.h",
+    "conversions/actions/conversion_action_types.h",
+    "conversions/actions/conversion_action_types_constants.h",
+    "conversions/actions/conversion_action_types_util.cc",
+    "conversions/actions/conversion_action_types_util.h",
+    "conversions/conversion/conversion_builder.cc",
+    "conversions/conversion/conversion_builder.h",
+    "conversions/conversion/conversion_info.cc",
+    "conversions/conversion/conversion_info.h",
+    "conversions/conversion/conversion_util.cc",
+    "conversions/conversion/conversion_util.h",
     "conversions/conversions.cc",
     "conversions/conversions.h",
-    "conversions/conversions_database_table.cc",
-    "conversions/conversions_database_table.h",
-    "conversions/conversions_database_util.cc",
-    "conversions/conversions_database_util.h",
     "conversions/conversions_feature.cc",
     "conversions/conversions_feature.h",
     "conversions/conversions_observer.h",
     "conversions/conversions_util.cc",
     "conversions/conversions_util.h",
-    "conversions/conversions_util_constants.h",
-    "conversions/verifiable_conversion_envelope_constants.h",
-    "conversions/verifiable_conversion_envelope_info.cc",
-    "conversions/verifiable_conversion_envelope_info.h",
-    "conversions/verifiable_conversion_info.cc",
-    "conversions/verifiable_conversion_info.h",
+    "conversions/queue/conversion_queue.cc",
+    "conversions/queue/conversion_queue.h",
+    "conversions/queue/conversion_queue_database_table.cc",
+    "conversions/queue/conversion_queue_database_table.h",
+    "conversions/queue/conversion_queue_delegate.h",
+    "conversions/queue/conversion_queue_util.cc",
+    "conversions/queue/conversion_queue_util.h",
+    "conversions/queue/conversion_queue_util_constants.h",
+    "conversions/queue/queue_item/conversion_queue_item_builder.cc",
+    "conversions/queue/queue_item/conversion_queue_item_builder.h",
+    "conversions/queue/queue_item/conversion_queue_item_builder_util.cc",
+    "conversions/queue/queue_item/conversion_queue_item_builder_util.h",
+    "conversions/queue/queue_item/conversion_queue_item_info.cc",
+    "conversions/queue/queue_item/conversion_queue_item_info.h",
+    "conversions/types/default_conversion/creative_set_conversion_url_pattern/creative_set_conversion_url_pattern_util.cc",
+    "conversions/types/default_conversion/creative_set_conversion_url_pattern/creative_set_conversion_url_pattern_util.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_envelope_util_constants.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_meta_tag_parser_util.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_meta_tag_parser_util.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_parser_util.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_parser_util.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_url_redirects_parser_util.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_url_redirects_parser_util.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/verifiable_conversion_id_pattern_util.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/verifiable_conversion_id_pattern_util.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_info.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_info.h",
+    "conversions/types/verifiable_conversion/verifiable_conversion_util.cc",
+    "conversions/types/verifiable_conversion/verifiable_conversion_util.h",
     "creatives/campaigns_database_table.cc",
     "creatives/campaigns_database_table.h",
     "creatives/campaigns_database_util.cc",
     "creatives/campaigns_database_util.h",
+    "creatives/conversions/creative_set_conversion_builder.cc",
+    "creatives/conversions/creative_set_conversion_builder.h",
+    "creatives/conversions/creative_set_conversion_database_table.cc",
+    "creatives/conversions/creative_set_conversion_database_table.h",
+    "creatives/conversions/creative_set_conversion_database_table_util.cc",
+    "creatives/conversions/creative_set_conversion_database_table_util.h",
+    "creatives/conversions/creative_set_conversion_info.cc",
+    "creatives/conversions/creative_set_conversion_info.h",
+    "creatives/conversions/creative_set_conversion_util.cc",
+    "creatives/conversions/creative_set_conversion_util.h",
     "creatives/creative_ad_info.cc",
     "creatives/creative_ad_info.h",
     "creatives/creative_ads_database_table.cc",
@@ -1069,13 +1107,16 @@ source_set("internal") {
     "resources/behavioral/anti_targeting/anti_targeting_resource.cc",
     "resources/behavioral/anti_targeting/anti_targeting_resource.h",
     "resources/behavioral/anti_targeting/anti_targeting_resource_constants.h",
-    "resources/behavioral/conversions/conversion_id_pattern_info.cc",
-    "resources/behavioral/conversions/conversion_id_pattern_info.h",
-    "resources/behavioral/conversions/conversions_info.cc",
-    "resources/behavioral/conversions/conversions_info.h",
-    "resources/behavioral/conversions/conversions_resource.cc",
-    "resources/behavioral/conversions/conversions_resource.h",
-    "resources/behavioral/conversions/conversions_resource_constants.h",
+    "resources/behavioral/conversions/conversion_resource.cc",
+    "resources/behavioral/conversions/conversion_resource.h",
+    "resources/behavioral/conversions/conversion_resource_constants.h",
+    "resources/behavioral/conversions/conversion_resource_id_pattern_info.cc",
+    "resources/behavioral/conversions/conversion_resource_id_pattern_info.h",
+    "resources/behavioral/conversions/conversion_resource_id_pattern_search_in_types.h",
+    "resources/behavioral/conversions/conversion_resource_id_pattern_util.cc",
+    "resources/behavioral/conversions/conversion_resource_id_pattern_util.h",
+    "resources/behavioral/conversions/conversion_resource_info.cc",
+    "resources/behavioral/conversions/conversion_resource_info.h",
     "resources/behavioral/multi_armed_bandits/epsilon_greedy_bandit_resource.cc",
     "resources/behavioral/multi_armed_bandits/epsilon_greedy_bandit_resource.h",
     "resources/behavioral/multi_armed_bandits/epsilon_greedy_bandit_resource_util.cc",

diff --git a/components/brave_ads/core/internal/account/account_unittest.cc b/components/brave_ads/core/internal/account/account_unittest.cc
@@ -346,12 +346,12 @@ TEST_F(BraveAdsAccountTest, DepositForCash) {
   privacy::SetUnblindedTokens(/*count*/ 1);
 
   const CreativeNotificationAdInfo creative_ad =
-      BuildCreativeNotificationAd(/*should_use_random_uuids*/ false);
+      BuildCreativeNotificationAd(/*should_use_random_uuids*/ true);
   database::SaveCreativeNotificationAds({creative_ad});
 
   // Act
   account_->Deposit(creative_ad.creative_instance_id, AdType::kNotificationAd,
-                    kSegment, ConfirmationType::kViewed);
+                    creative_ad.segment, ConfirmationType::kViewed);
 
   // Assert
   EXPECT_TRUE(did_process_deposit_);
@@ -422,7 +422,7 @@ TEST_F(BraveAdsAccountTest, DoNotDepositCashIfCreativeInstanceIdDoesNotExist) {
   MockTokenGenerator(token_generator_mock_, /*count*/ 1);
 
   const CreativeNotificationAdInfo creative_ad =
-      BuildCreativeNotificationAd(/*should_use_random_uuids*/ false);
+      BuildCreativeNotificationAd(/*should_use_random_uuids*/ true);
   database::SaveCreativeNotificationAds({creative_ad});
 
   // Act