Merge pull request #5197 from /issues/7226

Brave Ads server retry logic should backoff exponentially with a ceiling
brave · Apr 11, 2020 · b07bbf6 · b07bbf6
2 parents b1db045 + 856e03f
commit b07bbf6
Show file tree

Hide file tree

Showing 29 changed files with 721 additions and 676 deletions.
diff --git a/vendor/bat-native-ads/BUILD.gn b/vendor/bat-native-ads/BUILD.gn
@@ -205,6 +205,8 @@ source_set("ads") {
     "src/bat/ads/internal/ad_notifications.h",
     "src/bat/ads/internal/reports.cc",
     "src/bat/ads/internal/reports.h",
+    "src/bat/ads/internal/retry_timer.cc",
+    "src/bat/ads/internal/retry_timer.h",
     "src/bat/ads/internal/saved_ad.cc",
     "src/bat/ads/internal/saved_ad.h",
     "src/bat/ads/internal/search_provider_info.cc",
@@ -228,6 +230,8 @@ source_set("ads") {
     "src/bat/ads/internal/static_values.h",
     "src/bat/ads/internal/time.cc",
     "src/bat/ads/internal/time.h",
+    "src/bat/ads/internal/timer.cc",
+    "src/bat/ads/internal/timer.h",
     "src/bat/ads/internal/uri_helper.cc",
     "src/bat/ads/internal/uri_helper.h",
     "src/bat/ads/internal/purchase_intent/funnel_site_info.cc",

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ad_conversions.cc b/vendor/bat-native-ads/src/bat/ads/internal/ad_conversions.cc
@@ -38,7 +38,6 @@ AdConversions::AdConversions(
     AdsClient* ads_client,
     Client* client)
     : is_initialized_(false),
-      timer_id_(0),
       ads_(ads),
       ads_client_(ads_client),
       client_(client) {
@@ -47,9 +46,7 @@ AdConversions::AdConversions(
   DCHECK(client_);
 }
 
-AdConversions::~AdConversions() {
-  StopTimer();
-}
+AdConversions::~AdConversions() = default;
 
 void AdConversions::Initialize(
     InitializeCallback callback) {
@@ -77,7 +74,7 @@ void AdConversions::Check(
 void AdConversions::StartTimerIfReady() {
   DCHECK(is_initialized_);
 
-  if (timer_id_ != 0) {
+  if (timer_.IsRunning()) {
     return;
   }
 
@@ -90,25 +87,6 @@ void AdConversions::StartTimerIfReady() {
   StartTimer(ad_conversion);
 }
 
-bool AdConversions::OnTimer(
-    const uint32_t timer_id) {
-  if (timer_id != timer_id_) {
-    return false;
-  }
-
-  timer_id_ = 0;
-
-  DCHECK(!queue_.empty());
-  if (queue_.empty()) {
-    return true;
-  }
-
-  AdConversionQueueItemInfo ad_conversion = queue_.front();
-  ProcessQueueItem(ad_conversion);
-
-  return true;
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 void AdConversions::OnGetAdConversions(
@@ -282,46 +260,38 @@ void AdConversions::ProcessQueueItem(
   StartTimerIfReady();
 }
 
+void AdConversions::ProcessQueue() {
+  if (queue_.empty()) {
+    return;
+  }
+
+  AdConversionQueueItemInfo ad_conversion = queue_.front();
+  ProcessQueueItem(ad_conversion);
+}
+
 void AdConversions::StartTimer(
     const AdConversionQueueItemInfo& info) {
   DCHECK(is_initialized_);
-  DCHECK_EQ(0UL, timer_id_);
-
-  StopTimer();
+  DCHECK(!timer_.IsRunning());
 
   const uint64_t now = Time::NowInSeconds();
 
-  uint64_t start_timer_in;
+  uint64_t delay;
   if (now < info.timestamp_in_seconds) {
-    start_timer_in = info.timestamp_in_seconds - now;
+    delay = info.timestamp_in_seconds - now;
   } else {
-    start_timer_in = brave_base::random::Geometric(
-        kExpiredAdConversionFrequency);
+    delay = brave_base::random::Geometric(kExpiredAdConversionFrequency);
   }
 
-  timer_id_ = ads_client_->SetTimer(start_timer_in);
-  if (timer_id_ == 0) {
-    BLOG(ERROR) << "Failed to start ad conversion timer";
-    return;
-  }
+  timer_.Start(delay, base::BindOnce(&AdConversions::ProcessQueue,
+      base::Unretained(this)));
 
   BLOG(INFO) << "Started ad conversion timer for creative_instance_id "
       << info.creative_instance_id << " with creative set id "
           << info.creative_set_id << " which will trigger on "
               << Time::FromDoubleT(info.timestamp_in_seconds);
 }
 
-void AdConversions::StopTimer() {
-  if (timer_id_ == 0) {
-    return;
-  }
-
-  BLOG(INFO) << "Stopped ad conversion timer";
-
-  ads_client_->KillTimer(timer_id_);
-  timer_id_ = 0;
-}
-
 void AdConversions::SaveState() {
   if (!is_initialized_) {
     return;

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ad_conversions.h b/vendor/bat-native-ads/src/bat/ads/internal/ad_conversions.h
@@ -13,6 +13,7 @@
 #include "bat/ads/internal/ads_impl.h"
 #include "bat/ads/internal/client.h"
 #include "bat/ads/internal/ad_conversion_queue_item_info.h"
+#include "bat/ads/internal/timer.h"
 
 #include "base/values.h"
 
@@ -37,16 +38,13 @@ class AdConversions {
 
   void StartTimerIfReady();
 
-  bool OnTimer(
-      const uint32_t timer_id);
-
  private:
   bool is_initialized_;
   InitializeCallback callback_;
 
   AdConversionQueueItemList queue_;
 
-  uint32_t timer_id_;
+  Timer timer_;
 
   void OnGetAdConversions(
       const std::string& url,
@@ -71,10 +69,10 @@ class AdConversions {
       const std::string& creative_instance_id);
   void ProcessQueueItem(
       const AdConversionQueueItemInfo& info);
+  void ProcessQueue();
 
   void StartTimer(
       const AdConversionQueueItemInfo& info);
-  void StopTimer();
 
   void SaveState();
   void OnStateSaved(