Merge pull request #25998 from brave/issues/41628

[ads] Do not discard API calls during ads initialization

components/brave_ads/core/internal/BUILD.gn

@@ -294,8 +294,6 @@ static_library("internal") {
     "ads.cc",
     "ads_client/ads_client_notifier.cc",
     "ads_client/ads_client_notifier_observer.cc",
-    "ads_client/ads_client_notifier_queue.cc",
-    "ads_client/ads_client_notifier_queue.h",
     "ads_client/ads_client_pref_provider.cc",
     "ads_client/ads_client_pref_provider.h",
     "ads_client/ads_client_util.cc",
@@ -413,6 +411,8 @@ static_library("internal") {
     "common/database/database_table_util.h",
     "common/database/database_transaction_util.cc",
     "common/database/database_transaction_util.h",
+    "common/functional/once_closure_task_queue.cc",
+    "common/functional/once_closure_task_queue.h",
     "common/instance_id.cc",
     "common/instance_id.h",
     "common/locale/locale_util.cc",

components/brave_ads/core/internal/ads_client/ads_client_notifier.cc

@@ -9,13 +9,13 @@
 #include <memory>
 
 #include "base/functional/bind.h"
-#include "brave/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.h"
+#include "brave/components/brave_ads/core/internal/common/functional/once_closure_task_queue.h"
 #include "url/gurl.h"
 
 namespace brave_ads {
 
 AdsClientNotifier::AdsClientNotifier()
-    : queue_(std::make_unique<AdsClientNotifierQueue>()) {}
+    : task_queue_(std::make_unique<OnceClosureTaskQueue>()) {}
 
 AdsClientNotifier::~AdsClientNotifier() = default;
 
@@ -33,13 +33,12 @@ void AdsClientNotifier::RemoveObserver(
 }
 
 void AdsClientNotifier::NotifyPendingObservers() {
-  should_queue_ = false;
-  queue_->Process();
+  task_queue_->FlushAndStopQueueing();
 }
 
 void AdsClientNotifier::NotifyDidInitializeAds() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyDidInitializeAds,
                        weak_factory_.GetWeakPtr()));
   }
@@ -52,8 +51,8 @@ void AdsClientNotifier::NotifyDidInitializeAds() {
 void AdsClientNotifier::NotifyRewardsWalletDidUpdate(
     const std::string& payment_id,
     const std::string& recovery_seed_base64) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(base::BindOnce(
         &AdsClientNotifier::NotifyRewardsWalletDidUpdate,
         weak_factory_.GetWeakPtr(), payment_id, recovery_seed_base64));
   }
@@ -64,9 +63,10 @@ void AdsClientNotifier::NotifyRewardsWalletDidUpdate(
 }
 
 void AdsClientNotifier::NotifyLocaleDidChange(const std::string& locale) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(&AdsClientNotifier::NotifyLocaleDidChange,
-                                      weak_factory_.GetWeakPtr(), locale));
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
+        base::BindOnce(&AdsClientNotifier::NotifyLocaleDidChange,
+                       weak_factory_.GetWeakPtr(), locale));
   }
 
   for (auto& observer : observers_) {
@@ -75,9 +75,10 @@ void AdsClientNotifier::NotifyLocaleDidChange(const std::string& locale) {
 }
 
 void AdsClientNotifier::NotifyPrefDidChange(const std::string& path) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(&AdsClientNotifier::NotifyPrefDidChange,
-                                      weak_factory_.GetWeakPtr(), path));
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
+        base::BindOnce(&AdsClientNotifier::NotifyPrefDidChange,
+                       weak_factory_.GetWeakPtr(), path));
   }
 
   for (auto& observer : observers_) {
@@ -88,8 +89,8 @@ void AdsClientNotifier::NotifyPrefDidChange(const std::string& path) {
 void AdsClientNotifier::NotifyResourceComponentDidChange(
     const std::string& manifest_version,
     const std::string& id) {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyResourceComponentDidChange,
                        weak_factory_.GetWeakPtr(), manifest_version, id));
   }
@@ -101,8 +102,8 @@ void AdsClientNotifier::NotifyResourceComponentDidChange(
 
 void AdsClientNotifier::NotifyDidUnregisterResourceComponent(
     const std::string& id) {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyDidUnregisterResourceComponent,
                        weak_factory_.GetWeakPtr(), id));
   }
@@ -116,8 +117,8 @@ void AdsClientNotifier::NotifyTabTextContentDidChange(
     const int32_t tab_id,
     const std::vector<GURL>& redirect_chain,
     const std::string& text) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(base::BindOnce(
         &AdsClientNotifier::NotifyTabTextContentDidChange,
         weak_factory_.GetWeakPtr(), tab_id, redirect_chain, text));
   }
@@ -131,8 +132,8 @@ void AdsClientNotifier::NotifyTabHtmlContentDidChange(
     const int32_t tab_id,
     const std::vector<GURL>& redirect_chain,
     const std::string& html) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(base::BindOnce(
         &AdsClientNotifier::NotifyTabHtmlContentDidChange,
         weak_factory_.GetWeakPtr(), tab_id, redirect_chain, html));
   }
@@ -143,8 +144,8 @@ void AdsClientNotifier::NotifyTabHtmlContentDidChange(
 }
 
 void AdsClientNotifier::NotifyTabDidStartPlayingMedia(const int32_t tab_id) {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyTabDidStartPlayingMedia,
                        weak_factory_.GetWeakPtr(), tab_id));
   }
@@ -155,8 +156,8 @@ void AdsClientNotifier::NotifyTabDidStartPlayingMedia(const int32_t tab_id) {
 }
 
 void AdsClientNotifier::NotifyTabDidStopPlayingMedia(const int32_t tab_id) {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyTabDidStopPlayingMedia,
                        weak_factory_.GetWeakPtr(), tab_id));
   }
@@ -172,8 +173,8 @@ void AdsClientNotifier::NotifyTabDidChange(
     const bool is_new_navigation,
     const bool is_restoring,
     const bool is_visible) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(base::BindOnce(
         &AdsClientNotifier::NotifyTabDidChange, weak_factory_.GetWeakPtr(),
         tab_id, redirect_chain, is_new_navigation, is_restoring, is_visible));
   }
@@ -186,10 +187,10 @@ void AdsClientNotifier::NotifyTabDidChange(
 
 void AdsClientNotifier::NotifyTabDidLoad(const int32_t tab_id,
                                          const int http_status_code) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(&AdsClientNotifier::NotifyTabDidLoad,
-                                      weak_factory_.GetWeakPtr(), tab_id,
-                                      http_status_code));
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(base::BindOnce(&AdsClientNotifier::NotifyTabDidLoad,
+                                           weak_factory_.GetWeakPtr(), tab_id,
+                                           http_status_code));
   }
 
   for (auto& observer : observers_) {
@@ -198,9 +199,10 @@ void AdsClientNotifier::NotifyTabDidLoad(const int32_t tab_id,
 }
 
 void AdsClientNotifier::NotifyDidCloseTab(const int32_t tab_id) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(&AdsClientNotifier::NotifyDidCloseTab,
-                                      weak_factory_.GetWeakPtr(), tab_id));
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
+        base::BindOnce(&AdsClientNotifier::NotifyDidCloseTab,
+                       weak_factory_.GetWeakPtr(), tab_id));
   }
 
   for (auto& observer : observers_) {
@@ -210,8 +212,8 @@ void AdsClientNotifier::NotifyDidCloseTab(const int32_t tab_id) {
 
 void AdsClientNotifier::NotifyUserGestureEventTriggered(
     const int32_t page_transition_type) {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyUserGestureEventTriggered,
                        weak_factory_.GetWeakPtr(), page_transition_type));
   }
@@ -222,8 +224,8 @@ void AdsClientNotifier::NotifyUserGestureEventTriggered(
 }
 
 void AdsClientNotifier::NotifyUserDidBecomeIdle() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyUserDidBecomeIdle,
                        weak_factory_.GetWeakPtr()));
   }
@@ -236,8 +238,8 @@ void AdsClientNotifier::NotifyUserDidBecomeIdle() {
 void AdsClientNotifier::NotifyUserDidBecomeActive(
     const base::TimeDelta idle_time,
     const bool screen_was_locked) {
-  if (should_queue_) {
-    return queue_->Add(base::BindOnce(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(base::BindOnce(
         &AdsClientNotifier::NotifyUserDidBecomeActive,
         weak_factory_.GetWeakPtr(), idle_time, screen_was_locked));
   }
@@ -248,8 +250,8 @@ void AdsClientNotifier::NotifyUserDidBecomeActive(
 }
 
 void AdsClientNotifier::NotifyBrowserDidEnterForeground() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyBrowserDidEnterForeground,
                        weak_factory_.GetWeakPtr()));
   }
@@ -260,8 +262,8 @@ void AdsClientNotifier::NotifyBrowserDidEnterForeground() {
 }
 
 void AdsClientNotifier::NotifyBrowserDidEnterBackground() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyBrowserDidEnterBackground,
                        weak_factory_.GetWeakPtr()));
   }
@@ -272,8 +274,8 @@ void AdsClientNotifier::NotifyBrowserDidEnterBackground() {
 }
 
 void AdsClientNotifier::NotifyBrowserDidBecomeActive() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyBrowserDidBecomeActive,
                        weak_factory_.GetWeakPtr()));
   }
@@ -284,8 +286,8 @@ void AdsClientNotifier::NotifyBrowserDidBecomeActive() {
 }
 
 void AdsClientNotifier::NotifyBrowserDidResignActive() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyBrowserDidResignActive,
                        weak_factory_.GetWeakPtr()));
   }
@@ -296,8 +298,8 @@ void AdsClientNotifier::NotifyBrowserDidResignActive() {
 }
 
 void AdsClientNotifier::NotifyDidSolveAdaptiveCaptcha() {
-  if (should_queue_) {
-    return queue_->Add(
+  if (task_queue_->should_queue()) {
+    return task_queue_->Add(
         base::BindOnce(&AdsClientNotifier::NotifyDidSolveAdaptiveCaptcha,
                        weak_factory_.GetWeakPtr()));
   }

components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.cc

@@ -9,6 +9,12 @@
 
 namespace brave_ads {
 
+void AdsClientNotifierForTesting::NotifyPendingObservers() {
+  AdsClientNotifier::NotifyPendingObservers();
+
+  RunTaskEnvironmentUntilIdle();
+}
+
 void AdsClientNotifierForTesting::NotifyDidInitializeAds() {
   AdsClientNotifier::NotifyDidInitializeAds();
 

components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.h

@@ -32,6 +32,8 @@ class AdsClientNotifierForTesting : public AdsClientNotifier {
     task_environment_ = task_environment;
   }
 
+  void NotifyPendingObservers() override;
+
   void NotifyDidInitializeAds() override;
 
   void NotifyLocaleDidChange(const std::string& locale) override;