Merge pull request #13460 from /issues/23021

Brave Ads server requests should not retry if a browser upgrade is required

vendor/bat-native-ads/BUILD.gn

@@ -338,6 +338,7 @@ source_set("ads") {
     "src/bat/ads/internal/base/database_transaction_util.h",
     "src/bat/ads/internal/base/field_trial_params_util.cc",
     "src/bat/ads/internal/base/field_trial_params_util.h",
+    "src/bat/ads/internal/base/http_status_code.h",
     "src/bat/ads/internal/base/instance_id_util.cc",
     "src/bat/ads/internal/base/instance_id_util.h",
     "src/bat/ads/internal/base/key_pair_info.cc",

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

@@ -242,7 +242,7 @@ void Account::OnFailedToConfirm(const ConfirmationInfo& confirmation) {
   TopUpUnblindedTokens();
 }
 
-void Account::OnDidGetIssuers(const IssuersInfo& issuers) {
+void Account::OnDidFetchIssuers(const IssuersInfo& issuers) {
   const absl::optional<IssuerInfo>& issuer_optional =
       GetIssuerForType(issuers, IssuerType::kPayments);
   if (!issuer_optional) {
@@ -266,8 +266,8 @@ void Account::OnDidGetIssuers(const IssuersInfo& issuers) {
   TopUpUnblindedTokens();
 }
 
-void Account::OnFailedToGetIssuers() {
-  BLOG(0, "Failed to get issuers");
+void Account::OnFailedToFetchIssuers() {
+  BLOG(0, "Failed to fetch issuers");
 }
 
 void Account::OnDidRedeemUnblindedPaymentTokens(

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

@@ -91,8 +91,8 @@ class Account final : public ConfirmationsDelegate,
   void OnFailedToConfirm(const ConfirmationInfo& confirmation) override;
 
   // IssuersDelegate:
-  void OnDidGetIssuers(const IssuersInfo& issuers) override;
-  void OnFailedToGetIssuers() override;
+  void OnDidFetchIssuers(const IssuersInfo& issuers) override;
+  void OnFailedToFetchIssuers() override;
 
   // RedeemUnblindedPaymentTokensDelegate:
   void OnDidRedeemUnblindedPaymentTokens(

vendor/bat-native-ads/src/bat/ads/internal/account/account_unittest.cc

@@ -16,6 +16,7 @@
 #include "bat/ads/internal/account/transactions/transactions_unittest_util.h"
 #include "bat/ads/internal/account/wallet/wallet_info.h"
 #include "bat/ads/internal/ads_client_helper.h"
+#include "bat/ads/internal/base/http_status_code.h"
 #include "bat/ads/internal/base/unittest_base.h"
 #include "bat/ads/internal/base/unittest_time_util.h"
 #include "bat/ads/internal/base/unittest_util.h"
@@ -27,7 +28,6 @@
 #include "bat/ads/statement_info.h"
 #include "bat/ads/transaction_info.h"
 #include "bat/ads/transaction_info_aliases.h"
-#include "net/http/http_status_code.h"
 
 // npm run test -- brave_unit_tests --filter=BatAds*
 

vendor/bat-native-ads/src/bat/ads/internal/account/confirmations/confirmations_util.cc

@@ -19,9 +19,8 @@
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace ads {
-namespace security {
 
-bool Verify(const ConfirmationInfo& confirmation) {
+bool VerifyConfirmation(const ConfirmationInfo& confirmation) {
   std::string credential;
   base::Base64Decode(confirmation.credential, &credential);
 
@@ -63,5 +62,4 @@ bool Verify(const ConfirmationInfo& confirmation) {
   return verification_key.Verify(verification_signature, payload);
 }
 
-}  // namespace security
 }  // namespace ads

vendor/bat-native-ads/src/bat/ads/internal/account/confirmations/confirmations_util.h

@@ -10,11 +10,8 @@ namespace ads {
 
 struct ConfirmationInfo;
 
-namespace security {
+bool VerifyConfirmation(const ConfirmationInfo& confirmation);
 
-bool Verify(const ConfirmationInfo& confirmation);
-
-}  // namespace security
 }  // namespace ads
 
 #endif  // BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_ACCOUNT_CONFIRMATIONS_CONFIRMATIONS_UTIL_H_

vendor/bat-native-ads/src/bat/ads/internal/account/deposits/cash_deposit_unittest.cc

@@ -5,9 +5,9 @@
 
 #include "bat/ads/internal/account/deposits/cash_deposit.h"
 
+#include "bat/ads/internal/base/http_status_code.h"
 #include "bat/ads/internal/base/unittest_base.h"
 #include "bat/ads/internal/base/unittest_util.h"
-#include "net/http/http_status_code.h"
 
 // npm run test -- brave_unit_tests --filter=BatAds*
 

vendor/bat-native-ads/src/bat/ads/internal/account/issuers/issuers.cc

@@ -16,12 +16,12 @@
 #include "bat/ads/internal/account/issuers/issuers_json_reader.h"
 #include "bat/ads/internal/account/issuers/issuers_url_request_builder.h"
 #include "bat/ads/internal/ads_client_helper.h"
+#include "bat/ads/internal/base/http_status_code.h"
 #include "bat/ads/internal/base/logging_util.h"
 #include "bat/ads/internal/base/time_formatting_util.h"
 #include "bat/ads/internal/server/url/url_request_string_util.h"
 #include "bat/ads/internal/server/url/url_response_string_util.h"
 #include "bat/ads/pref_names.h"
-#include "net/http/http_status_code.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace ads {
@@ -63,7 +63,7 @@ void Issuers::Fetch() {
 
   is_fetching_ = true;
 
-  BLOG(1, "GetIssuers");
+  BLOG(1, "FetchIssuers");
   BLOG(2, "GET /v1/issuers/");
 
   IssuersUrlRequestBuilder url_request_builder;
@@ -77,77 +77,91 @@ void Issuers::Fetch() {
 }
 
 void Issuers::OnFetch(const mojom::UrlResponse& url_response) {
-  BLOG(1, "OnGetIssuers");
+  BLOG(1, "OnFetchIssuers");
 
   BLOG(6, UrlResponseToString(url_response));
   BLOG(7, UrlResponseHeadersToString(url_response));
 
-  if (url_response.status_code != net::HTTP_OK) {
-    OnFailedToGetIssuers();
+  if (url_response.status_code == net::HTTP_UPGRADE_REQUIRED) {
+    BLOG(1, "Failed to fetch issuers as a browser upgrade is required");
+    OnFailedToFetchIssuers(/* should_retry */ false);
+    return;
+  } else if (url_response.status_code != net::HTTP_OK) {
+    OnFailedToFetchIssuers(/* should_retry */ true);
     return;
   }
 
   const absl::optional<IssuersInfo>& issuers_optional =
       ParseJson(url_response.body);
   if (!issuers_optional) {
     BLOG(3, "Failed to parse response: " << url_response.body);
-    OnFailedToGetIssuers();
+    OnFailedToFetchIssuers(/* should_retry */ true);
     return;
   }
 
   const IssuersInfo& issuers = issuers_optional.value();
 
-  OnDidGetIssuers(issuers);
+  OnDidFetchIssuers(issuers);
 }
 
-void Issuers::OnDidGetIssuers(const IssuersInfo& issuers) {
+void Issuers::OnDidFetchIssuers(const IssuersInfo& issuers) {
   StopRetrying();
 
   is_fetching_ = false;
 
   if (delegate_) {
-    delegate_->OnDidGetIssuers(issuers);
+    delegate_->OnDidFetchIssuers(issuers);
   }
 
   FetchAfterDelay();
 }
 
-void Issuers::OnFailedToGetIssuers() {
+void Issuers::OnFailedToFetchIssuers(const bool should_retry) {
   is_fetching_ = false;
 
   if (delegate_) {
-    delegate_->OnFailedToGetIssuers();
+    delegate_->OnFailedToFetchIssuers();
   }
 
-  Retry();
+  if (should_retry) {
+    RetryAfterDelay();
+  }
 }
 
 void Issuers::FetchAfterDelay() {
   DCHECK(!retry_timer_.IsRunning());
 
-  const base::Time time = timer_.StartWithPrivacy(
+  const base::Time fetch_at = timer_.StartWithPrivacy(
       GetFetchDelay(), base::BindOnce(&Issuers::Fetch, base::Unretained(this)));
 
-  BLOG(1, "Fetch issuers " << FriendlyDateAndTime(time));
+  BLOG(1, "Fetch issuers " << FriendlyDateAndTime(fetch_at));
 }
 
 base::TimeDelta Issuers::GetFetchDelay() const {
   const int ping = AdsClientHelper::Get()->GetIntegerPref(prefs::kIssuerPing);
   return base::Milliseconds(ping);
 }
 
-void Issuers::Retry() {
+void Issuers::RetryAfterDelay() {
   DCHECK(!timer_.IsRunning());
 
-  const base::Time time = retry_timer_.StartWithPrivacy(
+  const base::Time retry_at = retry_timer_.StartWithPrivacy(
       kRetryAfter, base::BindOnce(&Issuers::OnRetry, base::Unretained(this)));
 
-  BLOG(1, "Retry fetching issuers " << FriendlyDateAndTime(time));
+  if (delegate_) {
+    delegate_->OnWillRetryFetchingIssuers(retry_at);
+  }
+
+  BLOG(1, "Retry fetching issuers " << FriendlyDateAndTime(retry_at));
 }
 
 void Issuers::OnRetry() {
   BLOG(1, "Retry fetching issuers");
 
+  if (delegate_) {
+    delegate_->OnDidRetryFetchingIssuers();
+  }
+
   Fetch();
 }
 

vendor/bat-native-ads/src/bat/ads/internal/account/issuers/issuers.h

@@ -33,13 +33,13 @@ class Issuers {
   void Fetch();
   void OnFetch(const mojom::UrlResponse& url_response);
 
-  void OnDidGetIssuers(const IssuersInfo& issuers);
-  void OnFailedToGetIssuers();
+  void OnDidFetchIssuers(const IssuersInfo& issuers);
+  void OnFailedToFetchIssuers(const bool should_retry);
 
   void FetchAfterDelay();
   base::TimeDelta GetFetchDelay() const;
 
-  void Retry();
+  void RetryAfterDelay();
   void OnRetry();
   void StopRetrying();
 

vendor/bat-native-ads/src/bat/ads/internal/account/issuers/issuers_delegate.h

@@ -6,6 +6,8 @@
 #ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_ACCOUNT_ISSUERS_ISSUERS_DELEGATE_H_
 #define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_ACCOUNT_ISSUERS_ISSUERS_DELEGATE_H_
 
+#include "base/time/time.h"
+
 namespace ads {
 
 struct IssuersInfo;
@@ -14,11 +16,18 @@ class IssuersDelegate {
  public:
   virtual ~IssuersDelegate() = default;
 
-  // Invoked to tell the delegate we successfuly retrieved the issuers
-  virtual void OnDidGetIssuers(const IssuersInfo& issuers) {}
+  // Invoked to tell the delegate we successfuly fetched the issuers
+  virtual void OnDidFetchIssuers(const IssuersInfo& issuers) {}
+
+  // Invoked to tell the delegate we failed to fetch the issuers
+  virtual void OnFailedToFetchIssuers() {}
+
+  // Invoked to tell the delegate we will retry fetching issuers at the
+  // specified time
+  virtual void OnWillRetryFetchingIssuers(const base::Time retry_at) {}
 
-  // Invoked to tell the delegate we failed to retrieve the issuers
-  virtual void OnFailedToGetIssuers() {}
+  // Invoked to tell the delegate we did retry fetching issuers
+  virtual void OnDidRetryFetchingIssuers() {}
 };
 
 }  // namespace ads

vendor/bat-native-ads/src/bat/ads/internal/account/issuers/issuers_delegate_mock.h

@@ -21,9 +21,13 @@ class IssuersDelegateMock : public IssuersDelegate {
   IssuersDelegateMock(const IssuersDelegateMock&) = delete;
   IssuersDelegateMock& operator=(const IssuersDelegateMock&) = delete;
 
-  MOCK_METHOD(void, OnDidGetIssuers, (const IssuersInfo& issuers));
+  MOCK_METHOD(void, OnDidFetchIssuers, (const IssuersInfo& issuers));
 
-  MOCK_METHOD(void, OnFailedToGetIssuers, ());
+  MOCK_METHOD(void, OnFailedToFetchIssuers, ());
+
+  MOCK_METHOD(void, OnWillRetryFetchingIssuers, (const base::Time retry_at));
+
+  MOCK_METHOD(void, OnDidRetryFetchingIssuers, ());
 };
 
 }  // namespace ads