Fixes setting to disable Brave Ads ad conversion detection isn't working

components/brave_ads/browser/ads_service_impl.cc

@@ -227,15 +227,14 @@ ads::CreativeAdNotificationList GetCreativeAdNotificationsOnFileTaskRunner(
 }
 
 ads::AdConversionList GetAdConversionsOnFileTaskRunner(
- const std::string& url,
  BundleStateDatabase* backend) {
  ads::AdConversionList ad_conversions;
 
  if (!backend) {
  return ad_conversions;
  }
 
- backend->GetAdConversions(url, &ad_conversions);
+ backend->GetAdConversions(&ad_conversions);
 
  return ad_conversions;
 }
@@ -974,7 +973,6 @@ void AdsServiceImpl::OnGetCreativeAdNotifications(
 
 void AdsServiceImpl::OnGetAdConversions(
  const ads::OnGetAdConversionsCallback& callback,
- const std::string& url,
  const ads::AdConversionList& ad_conversions) {
  if (!connected()) {
  return;
@@ -983,7 +981,7 @@ void AdsServiceImpl::OnGetAdConversions(
  const auto result = ad_conversions.empty() ?
  ads::Result::FAILED : ads::Result::SUCCESS;
 
- callback(result, url, ad_conversions);
+ callback(result, ad_conversions);
 }
 
 void AdsServiceImpl::OnGetAdsHistory(
@@ -2118,13 +2116,12 @@ void AdsServiceImpl::GetCreativeAdNotifications(
 }
 
 void AdsServiceImpl::GetAdConversions(
- const std::string& url,
  ads::OnGetAdConversionsCallback callback) {
  base::PostTaskAndReplyWithResult(file_task_runner_.get(), FROM_HERE,
- base::BindOnce(&GetAdConversionsOnFileTaskRunner, url,
+ base::BindOnce(&GetAdConversionsOnFileTaskRunner,
  bundle_state_backend_.get()),
  base::BindOnce(&AdsServiceImpl::OnGetAdConversions, AsWeakPtr(),
- std::move(callback), url));
+ std::move(callback)));
 }
 
 void AdsServiceImpl::EventLog(

components/brave_ads/browser/ads_service_impl.h

@@ -227,7 +227,6 @@ class AdsServiceImpl : public AdsService,
 
  void OnGetAdConversions(
  const ads::OnGetAdConversionsCallback& callback,
- const std::string& url,
  const ads::AdConversionList& ad_conversions);
 
  void OnGetAdsHistory(
@@ -437,7 +436,6 @@ class AdsServiceImpl : public AdsService,
  ads::OnGetCreativeAdNotificationsCallback callback) override;
 
  void GetAdConversions(
- const std::string& url,
  ads::OnGetAdConversionsCallback callback) override;
 
  void EventLog(

components/brave_ads/browser/bundle_state_database.cc

@@ -490,7 +490,6 @@ bool BundleStateDatabase::GetCreativeAdNotifications(
 }
 
 bool BundleStateDatabase::GetAdConversions(
- const std::string& url,
  ads::AdConversionList* ad_conversions) {
  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
@@ -502,7 +501,8 @@ bool BundleStateDatabase::GetAdConversions(
  const std::string sql =
  "SELECT "
  "ac.creative_set_id, "
- "ac.type, ac.url_pattern, "
+ "ac.type, "
+ "ac.url_pattern, "
  "ac.observation_window "
  "FROM ad_conversions AS ac";
 

components/brave_ads/browser/bundle_state_database.h

@@ -47,7 +47,6 @@ class BundleStateDatabase {
  ads::CreativeAdNotificationList* ads);
 
  bool GetAdConversions(
- const std::string& url,
  ads::AdConversionList* ad_conversions);
 
  // Returns the current version of the bundle state database

components/services/bat_ads/bat_ads_client_mojo_bridge.cc

@@ -459,32 +459,30 @@ void BatAdsClientMojoBridge::GetCreativeAdNotifications(
 void OnGetAdConversions(
  const ads::OnGetAdConversionsCallback& callback,
  const int32_t result,
- const std::string& url,
  const std::vector<std::string>& json_list) {
  ads::AdConversionList ad_conversions;
 
  for (const auto& json : json_list) {
  ads::AdConversionInfo ad_conversion;
  if (ad_conversion.FromJson(json) != ads::Result::SUCCESS) {
- callback(ads::Result::FAILED, url, {});
+ callback(ads::Result::FAILED, {});
  return;
  }
 
  ad_conversions.push_back(ad_conversion);
  }
 
- callback(ToAdsResult(result), url, ad_conversions);
+ callback(ToAdsResult(result), ad_conversions);
 }
 
 void BatAdsClientMojoBridge::GetAdConversions(
- const std::string& url,
  ads::OnGetAdConversionsCallback callback) {
  if (!connected()) {
- callback(ads::Result::FAILED, url, {});
+ callback(ads::Result::FAILED, {});
  return;
  }
 
- bat_ads_client_->GetAdConversions(url,
+ bat_ads_client_->GetAdConversions(
  base::BindOnce(&OnGetAdConversions, std::move(callback)));
 }
 

components/services/bat_ads/bat_ads_client_mojo_bridge.h

@@ -107,7 +107,6 @@ class BatAdsClientMojoBridge
  ads::OnGetCreativeAdNotificationsCallback callback) override;
 
  void GetAdConversions(
- const std::string& url,
  ads::OnGetAdConversionsCallback callback) override;
 
  void EventLog(

components/services/bat_ads/public/cpp/ads_client_mojo_bridge.cc

@@ -501,7 +501,6 @@ void AdsClientMojoBridge::GetCreativeAdNotifications(
 void AdsClientMojoBridge::OnGetAdConversions(
  CallbackHolder<GetAdConversionsCallback>* holder,
  const ads::Result result,
- const std::string& url,
  const ads::AdConversionList& ad_conversions) {
  DCHECK(holder);
 
@@ -512,21 +511,20 @@ void AdsClientMojoBridge::OnGetAdConversions(
  json_list.push_back(ad_conversion.ToJson());
  }
 
- std::move(holder->get()).Run(ToMojomResult(result), url, json_list);
+ std::move(holder->get()).Run(ToMojomResult(result), json_list);
  }
 
  delete holder;
 }
 
 void AdsClientMojoBridge::GetAdConversions(
- const std::string& url,
  GetAdConversionsCallback callback) {
  // this gets deleted in OnGetAdConversions
  auto* holder = new CallbackHolder<GetAdConversionsCallback>(
  AsWeakPtr(), std::move(callback));
 
- ads_client_->GetAdConversions(url,
- std::bind(AdsClientMojoBridge::OnGetAdConversions, holder, _1, _2, _3));
+ ads_client_->GetAdConversions(
+ std::bind(AdsClientMojoBridge::OnGetAdConversions, holder, _1, _2));
 }
 
 } // namespace bat_ads

components/services/bat_ads/public/cpp/ads_client_mojo_bridge.h

@@ -134,7 +134,6 @@ class AdsClientMojoBridge
  const std::vector<std::string>& categories,
  GetCreativeAdNotificationsCallback callback) override;
  void GetAdConversions(
- const std::string& url,
  GetAdConversionsCallback callback) override;
 
  private:
@@ -197,7 +196,6 @@ class AdsClientMojoBridge
  static void OnGetAdConversions(
  CallbackHolder<GetAdConversionsCallback>* holder,
  const ads::Result result,
- const std::string& url,
  const ads::AdConversionList& ad_conversions);
 
  ads::AdsClient* ads_client_; // NOT OWNED

components/services/bat_ads/public/interfaces/bat_ads.mojom

@@ -60,7 +60,7 @@ interface BatAdsClient {
  LoadSampleBundle() => (int32 result, string value);
  Reset(string name) => (int32 result);
  GetCreativeAdNotifications(array<string> categories) => (int32 result, array<string> categories, array<string> json_list);
- GetAdConversions(string url) => (int32 result, string url, array<string> json_list);
+ GetAdConversions() => (int32 result, array<string> json_list);
  EventLog(string json);
 };
 

vendor/bat-native-ads/include/bat/ads/ads_client.h

@@ -53,7 +53,7 @@ using OnGetCreativeAdNotificationsCallback = std::function<void(const Result,
  const std::vector<std::string>&, const CreativeAdNotificationList&)>;
 
 using OnGetAdConversionsCallback = std::function<void(const Result,
- const std::string&, const AdConversionList&)>;
+ const AdConversionList&)>;
 
 using OnLoadSampleBundleCallback = std::function<void(const Result,
  const std::string&)>;
@@ -240,13 +240,11 @@ class ADS_EXPORT AdsClient {
  const std::vector<std::string>& categories,
  OnGetCreativeAdNotificationsCallback callback) = 0;
 
- // Should fetch all ad conversions for the specified |url| from the
- // previously persisted bundle state. The callback takes 3 arguments —
- // |Result| should be set to |SUCCESS| if successful; otherwise, should be set
- // to |FAILED|. |url| should contain the url. |ad_conversions| should
+ // Should fetch all ad conversions from the previously persisted bundle state.
+ // The callback takes 2 arguments — |Result| should be set to |SUCCESS| if
+ // successful; otherwise, should be set to |FAILED|. |ad_conversions| should
  // contain an array of ad conversions
  virtual void GetAdConversions(
- const std::string& url,
  OnGetAdConversionsCallback callback) = 0;
 
  // Should log an event

vendor/bat-native-ads/src/bat/ads/internal/ads_client_mock.h

@@ -127,8 +127,7 @@ class MockAdsClient : public AdsClient {
  const std::vector<std::string>& categories,
  OnGetCreativeAdNotificationsCallback callback));
 
- MOCK_METHOD2(GetAdConversions, void(
- const std::string& url,
+ MOCK_METHOD1(GetAdConversions, void(
  OnGetAdConversionsCallback callback));
 
  MOCK_CONST_METHOD1(EventLog, void(

vendor/bat-native-ads/src/bat/ads/internal/ads_impl.cc

@@ -715,13 +715,13 @@ void AdsImpl::CheckAdConversion(
  return;
  }
 
- auto callback = std::bind(&AdsImpl::OnGetAdConversions, this, _1, _2, _3);
- ads_client_->GetAdConversions(url, callback);
+ auto callback = std::bind(&AdsImpl::OnGetAdConversions, this, url, _1, _2);
+ ads_client_->GetAdConversions(callback);
 }
 
 void AdsImpl::OnGetAdConversions(
- const Result result,
  const std::string& url,
+ const Result result,
  const AdConversionList& ad_conversions) {
  for (const auto& ad_conversion : ad_conversions) {
  if (!helper::Uri::MatchesWildcard(url, ad_conversion.url_pattern)) {
@@ -1198,8 +1198,24 @@ void AdsImpl::OnServeUntargetedAdNotification(
 
 void AdsImpl::ServeAdNotification(
  const CreativeAdNotificationList& ads) {
- auto rand = base::RandInt(0, ads.size() - 1);
- auto ad = ads.at(rand);
+ auto callback =
+ std::bind(&AdsImpl::OnServeAdNotification, this, _1, _2, ads);
+ ads_client_->GetAdConversions(callback);
+}
+
+void AdsImpl::OnServeAdNotification(
+ const Result result,
+ const AdConversionList& ad_conversions,
+ const CreativeAdNotificationList& ads) {
+ CreativeAdNotificationList eligible_ads =
+ GetEligibleAdsForConversions(ads, ad_conversions);
+ if (eligible_ads.empty()) {
+ FailedToServeAdNotification("No eligible ads found");
+ return;
+ }
+
+ const int rand = base::RandInt(0, eligible_ads.size() - 1);
+ const CreativeAdNotificationInfo ad = eligible_ads.at(rand);
  ShowAdNotification(ad);
 
  SuccessfullyServedAd();
@@ -1288,6 +1304,27 @@ CreativeAdNotificationList AdsImpl::GetEligibleAds(
  return eligible_ads;
 }
 
+CreativeAdNotificationList AdsImpl::GetEligibleAdsForConversions(
+ const CreativeAdNotificationList& ads,
+ const AdConversionList& ad_conversions) {
+ CreativeAdNotificationList eligible_ads = ads;
+
+ if (ads_client_->ShouldAllowAdConversionTracking()) {
+ return eligible_ads;
+ }
+
+ for (const auto& ad_conversion : ad_conversions) {
+ const auto iter = std::remove_if(eligible_ads.begin(), eligible_ads.end(),
+ [&ad_conversion](CreativeAdNotificationInfo& ad_notification) {
+ return ad_notification.creative_set_id == ad_conversion.creative_set_id;
+ });
+
+ eligible_ads.erase(iter, eligible_ads.end());
+ }
+
+ return eligible_ads;
+}
+
 CreativeAdNotificationList AdsImpl::GetUnseenAdsAndRoundRobinIfNeeded(
  const CreativeAdNotificationList& ads) const {
  if (ads.empty()) {

vendor/bat-native-ads/src/bat/ads/internal/ads_impl.h

@@ -187,8 +187,8 @@ class AdsImpl : public Ads {
  void CheckAdConversion(
  const std::string& url);
  void OnGetAdConversions(
- const Result result,
  const std::string& url,
+ const Result result,
  const AdConversionList& ad_conversions);
 
  void MaybeServeAdNotification(
@@ -210,12 +210,19 @@ class AdsImpl : public Ads {
  const CreativeAdNotificationList& ads);
  void ServeAdNotification(
  const CreativeAdNotificationList& ads);
+ void OnServeAdNotification(
+ const Result result,
+ const AdConversionList& ad_conversions,
+ const CreativeAdNotificationList& ads);
  void SuccessfullyServedAd();
  void FailedToServeAdNotification(
  const std::string& reason);
 
  CreativeAdNotificationList GetEligibleAds(
  const CreativeAdNotificationList& ads);
+ CreativeAdNotificationList GetEligibleAdsForConversions(
+ const CreativeAdNotificationList& ads,
+ const AdConversionList& ad_conversions);
  CreativeAdNotificationList GetUnseenAdsAndRoundRobinIfNeeded(
  const CreativeAdNotificationList& ads) const;
  CreativeAdNotificationList GetUnseenAds(

vendor/brave-ios/Ads/BATBraveAds.mm

@@ -396,12 +396,12 @@ - (void)getCreativeAdNotifications:(const std::vector<std::string> &)categories
  callback(ads::Result::SUCCESS, categories, found_ads);
 }
 
-- (void)getAdConversions:(const std::string &)url callback:(ads::OnGetAdConversionsCallback)callback
+- (void)getAdConversions:(ads::OnGetAdConversionsCallback)callback
 {
  // TODO(khickinson): To be implemented
  if (![self isAdsServiceRunning]) { return; }
 
- callback(ads::Result::SUCCESS, url, {});
+ callback(ads::Result::SUCCESS, {});
 }
 
 - (void)setCatalogIssuers:(std::unique_ptr<ads::IssuersInfo>)info

vendor/brave-ios/Ads/Generated/NativeAdsClient.h

@@ -45,7 +45,7 @@ class NativeAdsClient : public ads::AdsClient {
  void LoadSampleBundle(ads::OnLoadSampleBundleCallback callback) override;
  void SaveBundleState(std::unique_ptr<ads::BundleState> state, ads::OnSaveCallback callback) override;
  void GetCreativeAdNotifications(const std::vector<std::string> & categories, ads::OnGetCreativeAdNotificationsCallback callback) override;
- void GetAdConversions(const std::string & url, ads::OnGetAdConversionsCallback callback) override;
+ void GetAdConversions(ads::OnGetAdConversionsCallback callback) override;
  void EventLog(const std::string & json) const override;
  std::unique_ptr<ads::LogStream> Log(const char * file, const int line, const ads::LogLevel log_level) const override;
 };

vendor/brave-ios/Ads/Generated/NativeAdsClient.mm

@@ -127,8 +127,8 @@
  [bridge_ getCreativeAdNotifications:categories callback:callback];
 }
 
-void NativeAdsClient::GetAdConversions(const std::string & url, ads::OnGetAdConversionsCallback callback) {
- [bridge_ getAdConversions:url callback:callback];
+void NativeAdsClient::GetAdConversions(ads::OnGetAdConversionsCallback callback) {
+ [bridge_ getAdConversions:callback];
 }
 
 void NativeAdsClient::EventLog(const std::string & json) const {

vendor/brave-ios/Ads/Generated/NativeAdsClientBridge.h

@@ -14,7 +14,7 @@
 - (void)confirmAction:(const std::string &)uuid creativeSetId:(const std::string &)creative_set_id confirmationType:(const ads::ConfirmationType &)confirmation_type;
 - (void)eventLog:(const std::string &)json;
 - (void)getCreativeAdNotifications:(const std::vector<std::string> &)categories callback:(ads::OnGetCreativeAdNotificationsCallback)callback;
-- (void)getAdConversions:(const std::string &)url callback:(ads::OnGetAdConversionsCallback)callback;
+- (void)getAdConversions:(ads::OnGetAdConversionsCallback)callback;
 - (std::string)getLocale;
 - (uint64_t)getAdsPerDay;
 - (uint64_t)getAdsPerHour;