Merge pull request #17076 from brave/issues/28346

Decouple Brave Ads captcha implementation detail
brave · Feb 7, 2023 · 8447e38 · 8447e38
2 parents fcf61c5 + 93f1cd7
commit 8447e38
Show file tree

Hide file tree

Showing 14 changed files with 39 additions and 69 deletions.
diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc
@@ -2000,32 +2000,28 @@ void AdsServiceImpl::GetScheduledCaptcha(
 
 void AdsServiceImpl::ShowScheduledCaptchaNotification(
     const std::string& payment_id,
-    const std::string& captcha_id,
-    const bool should_show_tooltip_notification) {
+    const std::string& captcha_id) {
+#if BUILDFLAG(IS_ANDROID)
+  ShowScheduledCaptcha(payment_id, captcha_id);
+#else   // BUILDFLAG(IS_ANDROID)
   const PrefService* const pref_service = profile_->GetPrefs();
-  if (should_show_tooltip_notification) {
-    if (pref_service->GetBoolean(
-            brave_adaptive_captcha::prefs::kScheduledCaptchaPaused)) {
-      VLOG(1) << "Ads paused; support intervention required";
-      return;
-    }
 
-// TODO(sergz): made a guard to prevent a potential crash, but need to
-// check as we could have the guard in the higher level for Android
-#if !BUILDFLAG(IS_ANDROID)
-    const int snooze_count = pref_service->GetInteger(
-        brave_adaptive_captcha::prefs::kScheduledCaptchaSnoozeCount);
+  if (pref_service->GetBoolean(
+          brave_adaptive_captcha::prefs::kScheduledCaptchaPaused)) {
+    VLOG(1) << "Ads paused; support intervention required";
+    return;
+  }
 
-    DCHECK(ads_tooltips_delegate_);
+  const int snooze_count = pref_service->GetInteger(
+      brave_adaptive_captcha::prefs::kScheduledCaptchaSnoozeCount);
 
-    ads_tooltips_delegate_->ShowCaptchaTooltip(
-        payment_id, captcha_id, snooze_count == 0,
-        base::BindOnce(&AdsServiceImpl::ShowScheduledCaptcha, AsWeakPtr()),
-        base::BindOnce(&AdsServiceImpl::SnoozeScheduledCaptcha, AsWeakPtr()));
-#endif
-  } else {
-    ShowScheduledCaptcha(payment_id, captcha_id);
-  }
+  DCHECK(ads_tooltips_delegate_);
+
+  ads_tooltips_delegate_->ShowCaptchaTooltip(
+      payment_id, captcha_id, snooze_count == 0,
+      base::BindOnce(&AdsServiceImpl::ShowScheduledCaptcha, AsWeakPtr()),
+      base::BindOnce(&AdsServiceImpl::SnoozeScheduledCaptcha, AsWeakPtr()));
+#endif  // !BUILDFLAG(IS_ANDROID)
 }
 
 void AdsServiceImpl::ClearScheduledCaptcha() {

diff --git a/components/brave_ads/browser/ads_service_impl.h b/components/brave_ads/browser/ads_service_impl.h
@@ -359,10 +359,8 @@ class AdsServiceImpl : public AdsService,
 
   void GetScheduledCaptcha(const std::string& payment_id,
                            ads::GetScheduledCaptchaCallback callback) override;
-  void ShowScheduledCaptchaNotification(
-      const std::string& payment_id,
-      const std::string& captcha_id,
-      bool should_show_tooltip_notification) override;
+  void ShowScheduledCaptchaNotification(const std::string& payment_id,
+                                        const std::string& captcha_id) override;
   void ClearScheduledCaptcha() override;
 
   void RunDBTransaction(ads::mojom::DBTransactionInfoPtr transaction,

diff --git a/components/services/bat_ads/bat_ads_client_mojo_bridge.cc b/components/services/bat_ads/bat_ads_client_mojo_bridge.cc
@@ -244,11 +244,9 @@ void BatAdsClientMojoBridge::GetScheduledCaptcha(
 
 void BatAdsClientMojoBridge::ShowScheduledCaptchaNotification(
     const std::string& payment_id,
-    const std::string& captcha_id,
-    const bool should_show_tooltip_notification) {
+    const std::string& captcha_id) {
   if (bat_ads_client_.is_bound()) {
-    bat_ads_client_->ShowScheduledCaptchaNotification(
-        payment_id, captcha_id, should_show_tooltip_notification);
+    bat_ads_client_->ShowScheduledCaptchaNotification(payment_id, captcha_id);
   }
 }
 

diff --git a/components/services/bat_ads/bat_ads_client_mojo_bridge.h b/components/services/bat_ads/bat_ads_client_mojo_bridge.h
@@ -82,10 +82,8 @@ class BatAdsClientMojoBridge : public ads::AdsClient {
 
   void GetScheduledCaptcha(const std::string& payment_id,
                            ads::GetScheduledCaptchaCallback callback) override;
-  void ShowScheduledCaptchaNotification(
-      const std::string& payment_id,
-      const std::string& captcha_id,
-      bool should_show_tooltip_notification) override;
+  void ShowScheduledCaptchaNotification(const std::string& payment_id,
+                                        const std::string& captcha_id) override;
   void ClearScheduledCaptcha() override;
 
   void RunDBTransaction(ads::mojom::DBTransactionInfoPtr transaction,

diff --git a/components/services/bat_ads/public/cpp/ads_client_mojo_bridge.cc b/components/services/bat_ads/public/cpp/ads_client_mojo_bridge.cc
@@ -177,10 +177,8 @@ void AdsClientMojoBridge::GetScheduledCaptcha(
 
 void AdsClientMojoBridge::ShowScheduledCaptchaNotification(
     const std::string& payment_id,
-    const std::string& captcha_id,
-    const bool should_show_tooltip_notification) {
-  ads_client_->ShowScheduledCaptchaNotification(
-      payment_id, captcha_id, should_show_tooltip_notification);
+    const std::string& captcha_id) {
+  ads_client_->ShowScheduledCaptchaNotification(payment_id, captcha_id);
 }
 
 void AdsClientMojoBridge::ClearScheduledCaptcha() {

diff --git a/components/services/bat_ads/public/cpp/ads_client_mojo_bridge.h b/components/services/bat_ads/public/cpp/ads_client_mojo_bridge.h
@@ -96,10 +96,8 @@ class AdsClientMojoBridge : public mojom::BatAdsClient {
 
   void GetScheduledCaptcha(const std::string& payment_id,
                            GetScheduledCaptchaCallback callback) override;
-  void ShowScheduledCaptchaNotification(
-      const std::string& payment_id,
-      const std::string& captcha_id,
-      bool should_show_tooltip_notification) override;
+  void ShowScheduledCaptchaNotification(const std::string& payment_id,
+                                        const std::string& captcha_id) override;
   void ClearScheduledCaptcha() override;
 
   void RunDBTransaction(ads::mojom::DBTransactionInfoPtr transaction,

diff --git a/components/services/bat_ads/public/interfaces/bat_ads.mojom b/components/services/bat_ads/public/interfaces/bat_ads.mojom
@@ -57,7 +57,7 @@ interface BatAdsClient {
   LoadDataResource(string name) => (mojo_base.mojom.BigString value);
 
   GetScheduledCaptcha(string payment_id) => (string captcha_id);
-  ShowScheduledCaptchaNotification(string payment_id, string captcha_id, bool should_show_tooltip_notification);
+  ShowScheduledCaptchaNotification(string payment_id, string captcha_id);
   ClearScheduledCaptcha();
 
   RunDBTransaction(ads.mojom.DBTransactionInfo transaction) => (ads.mojom.DBCommandResponseInfo response);

diff --git a/ios/browser/api/ads/ads_client_bridge.h b/ios/browser/api/ads/ads_client_bridge.h
@@ -27,9 +27,7 @@
 - (void)getScheduledCaptcha:(const std::string&)payment_id
                    callback:(ads::GetScheduledCaptchaCallback)callback;
 - (void)showScheduledCaptchaNotification:(const std::string&)payment_id
-                               captchaId:(const std::string&)captcha_id
-                  shouldShowNotification:
-                      (const bool)should_show_tooltip_notification;
+                               captchaId:(const std::string&)captcha_id;
 - (void)getBrowsingHistory:(const int)max_count
                    forDays:(const int)days_ago
                   callback:(ads::GetBrowsingHistoryCallback)callback;

diff --git a/ios/browser/api/ads/ads_client_ios.h b/ios/browser/api/ads/ads_client_ios.h
@@ -52,10 +52,8 @@ class AdsClientIOS : public ads::AdsClient {
   void ClearScheduledCaptcha() override;
   void GetScheduledCaptcha(const std::string& payment_id,
                            ads::GetScheduledCaptchaCallback callback) override;
-  void ShowScheduledCaptchaNotification(
-      const std::string& payment_id,
-      const std::string& captcha_id,
-      const bool should_show_tooltip_notification) override;
+  void ShowScheduledCaptchaNotification(const std::string& payment_id,
+                                        const std::string& captcha_id) override;
   void Log(const char* file,
            const int line,
            const int verbose_level,

diff --git a/ios/browser/api/ads/ads_client_ios.mm b/ios/browser/api/ads/ads_client_ios.mm
@@ -113,11 +113,8 @@
 
 void AdsClientIOS::ShowScheduledCaptchaNotification(
     const std::string& payment_id,
-    const std::string& captcha_id,
-    const bool should_show_tooltip_notification) {
-  [bridge_ showScheduledCaptchaNotification:payment_id
-                                  captchaId:captcha_id
-                     shouldShowNotification:should_show_tooltip_notification];
+    const std::string& captcha_id) {
+  [bridge_ showScheduledCaptchaNotification:payment_id captchaId:captcha_id];
 }
 
 void AdsClientIOS::Log(const char* file,

diff --git a/ios/browser/api/ads/brave_ads.mm b/ios/browser/api/ads/brave_ads.mm
@@ -1177,9 +1177,7 @@ - (void)getScheduledCaptcha:(const std::string&)payment_id
 }
 
 - (void)showScheduledCaptchaNotification:(const std::string&)payment_id
-                               captchaId:(const std::string&)captcha_id
-                  shouldShowNotification:
-                      (const bool)should_show_tooltip_notification {
+                               captchaId:(const std::string&)captcha_id {
   [self.captchaHandler
       handleAdaptiveCaptchaForPaymentId:base::SysUTF8ToNSString(payment_id)
                               captchaId:base::SysUTF8ToNSString(captcha_id)];

diff --git a/vendor/bat-native-ads/include/bat/ads/ads_client.h b/vendor/bat-native-ads/include/bat/ads/ads_client.h
@@ -111,8 +111,7 @@ class ADS_EXPORT AdsClient {
   // continue to be served ads.
   virtual void ShowScheduledCaptchaNotification(
       const std::string& payment_id,
-      const std::string& captcha_id,
-      bool should_show_tooltip_notification) = 0;
+      const std::string& captcha_id) = 0;
 
   // Clear the scheduled captcha, if any.
   virtual void ClearScheduledCaptcha() = 0;

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/account/account.cc b/vendor/bat-native-ads/src/bat/ads/internal/account/account.cc
@@ -402,14 +402,9 @@ void Account::OnDidRefillUnblindedTokens() {
 void Account::OnCaptchaRequiredToRefillUnblindedTokens(
     const std::string& captcha_id) {
   const WalletInfo& wallet = GetWallet();
-#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
-  const bool should_show_tooltip_notification = false;
-#else   // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
-  const bool should_show_tooltip_notification = true;
-#endif  // !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS)
 
   AdsClientHelper::GetInstance()->ShowScheduledCaptchaNotification(
-      wallet.payment_id, captcha_id, should_show_tooltip_notification);
+      wallet.payment_id, captcha_id);
 }
 
 }  // namespace ads
diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ads_client_mock.h b/vendor/bat-native-ads/src/bat/ads/internal/ads_client_mock.h
@@ -74,10 +74,9 @@ class AdsClientMock : public AdsClient {
   MOCK_METHOD2(GetScheduledCaptcha,
                void(const std::string& payment_id,
                     GetScheduledCaptchaCallback callback));
-  MOCK_METHOD3(ShowScheduledCaptchaNotification,
+  MOCK_METHOD2(ShowScheduledCaptchaNotification,
                void(const std::string& payment_id,
-                    const std::string& captcha_id,
-                    const bool should_show_tooltip_notification));
+                    const std::string& captcha_id));
   MOCK_METHOD0(ClearScheduledCaptcha, void());
 
   MOCK_METHOD2(RunDBTransaction,