diff --git a/components/brave_ads/core/internal/BUILD.gn b/components/brave_ads/core/internal/BUILD.gn index 1146b08d50e1..90a0e47ac084 100644 --- a/components/brave_ads/core/internal/BUILD.gn +++ b/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_queue.cc", + "common/functional/once_closure_queue.h", "common/instance_id.cc", "common/instance_id.h", "common/locale/locale_util.cc", diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier.cc b/components/brave_ads/core/internal/ads_client/ads_client_notifier.cc index 6b3c220acbe3..2184123d5925 100644 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier.cc +++ b/components/brave_ads/core/internal/ads_client/ads_client_notifier.cc @@ -9,13 +9,13 @@ #include #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_queue.h" #include "url/gurl.h" namespace brave_ads { AdsClientNotifier::AdsClientNotifier() - : queue_(std::make_unique()) {} + : pending_task_queue_(std::make_unique()) {} AdsClientNotifier::~AdsClientNotifier() = default; @@ -34,12 +34,12 @@ void AdsClientNotifier::RemoveObserver( void AdsClientNotifier::NotifyPendingObservers() { should_queue_ = false; - queue_->Process(); + pending_task_queue_->Process(); } void AdsClientNotifier::NotifyDidInitializeAds() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyDidInitializeAds, weak_factory_.GetWeakPtr())); } @@ -53,7 +53,7 @@ void AdsClientNotifier::NotifyRewardsWalletDidUpdate( const std::string& payment_id, const std::string& recovery_seed_base64) { if (should_queue_) { - return queue_->Add(base::BindOnce( + return pending_task_queue_->Add(base::BindOnce( &AdsClientNotifier::NotifyRewardsWalletDidUpdate, weak_factory_.GetWeakPtr(), payment_id, recovery_seed_base64)); } @@ -65,8 +65,9 @@ void AdsClientNotifier::NotifyRewardsWalletDidUpdate( void AdsClientNotifier::NotifyLocaleDidChange(const std::string& locale) { if (should_queue_) { - return queue_->Add(base::BindOnce(&AdsClientNotifier::NotifyLocaleDidChange, - weak_factory_.GetWeakPtr(), locale)); + return pending_task_queue_->Add( + base::BindOnce(&AdsClientNotifier::NotifyLocaleDidChange, + weak_factory_.GetWeakPtr(), locale)); } for (auto& observer : observers_) { @@ -76,8 +77,9 @@ 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)); + return pending_task_queue_->Add( + base::BindOnce(&AdsClientNotifier::NotifyPrefDidChange, + weak_factory_.GetWeakPtr(), path)); } for (auto& observer : observers_) { @@ -89,7 +91,7 @@ void AdsClientNotifier::NotifyResourceComponentDidChange( const std::string& manifest_version, const std::string& id) { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyResourceComponentDidChange, weak_factory_.GetWeakPtr(), manifest_version, id)); } @@ -102,7 +104,7 @@ void AdsClientNotifier::NotifyResourceComponentDidChange( void AdsClientNotifier::NotifyDidUnregisterResourceComponent( const std::string& id) { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyDidUnregisterResourceComponent, weak_factory_.GetWeakPtr(), id)); } @@ -117,7 +119,7 @@ void AdsClientNotifier::NotifyTabTextContentDidChange( const std::vector& redirect_chain, const std::string& text) { if (should_queue_) { - return queue_->Add(base::BindOnce( + return pending_task_queue_->Add(base::BindOnce( &AdsClientNotifier::NotifyTabTextContentDidChange, weak_factory_.GetWeakPtr(), tab_id, redirect_chain, text)); } @@ -132,7 +134,7 @@ void AdsClientNotifier::NotifyTabHtmlContentDidChange( const std::vector& redirect_chain, const std::string& html) { if (should_queue_) { - return queue_->Add(base::BindOnce( + return pending_task_queue_->Add(base::BindOnce( &AdsClientNotifier::NotifyTabHtmlContentDidChange, weak_factory_.GetWeakPtr(), tab_id, redirect_chain, html)); } @@ -144,7 +146,7 @@ void AdsClientNotifier::NotifyTabHtmlContentDidChange( void AdsClientNotifier::NotifyTabDidStartPlayingMedia(const int32_t tab_id) { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyTabDidStartPlayingMedia, weak_factory_.GetWeakPtr(), tab_id)); } @@ -156,7 +158,7 @@ void AdsClientNotifier::NotifyTabDidStartPlayingMedia(const int32_t tab_id) { void AdsClientNotifier::NotifyTabDidStopPlayingMedia(const int32_t tab_id) { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyTabDidStopPlayingMedia, weak_factory_.GetWeakPtr(), tab_id)); } @@ -173,7 +175,7 @@ void AdsClientNotifier::NotifyTabDidChange( const bool is_restoring, const bool is_visible) { if (should_queue_) { - return queue_->Add(base::BindOnce( + return pending_task_queue_->Add(base::BindOnce( &AdsClientNotifier::NotifyTabDidChange, weak_factory_.GetWeakPtr(), tab_id, redirect_chain, is_new_navigation, is_restoring, is_visible)); } @@ -199,8 +201,9 @@ 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)); + return pending_task_queue_->Add( + base::BindOnce(&AdsClientNotifier::NotifyDidCloseTab, + weak_factory_.GetWeakPtr(), tab_id)); } for (auto& observer : observers_) { @@ -211,7 +214,7 @@ void AdsClientNotifier::NotifyDidCloseTab(const int32_t tab_id) { void AdsClientNotifier::NotifyUserGestureEventTriggered( const int32_t page_transition_type) { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyUserGestureEventTriggered, weak_factory_.GetWeakPtr(), page_transition_type)); } @@ -223,7 +226,7 @@ void AdsClientNotifier::NotifyUserGestureEventTriggered( void AdsClientNotifier::NotifyUserDidBecomeIdle() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyUserDidBecomeIdle, weak_factory_.GetWeakPtr())); } @@ -237,7 +240,7 @@ void AdsClientNotifier::NotifyUserDidBecomeActive( const base::TimeDelta idle_time, const bool screen_was_locked) { if (should_queue_) { - return queue_->Add(base::BindOnce( + return pending_task_queue_->Add(base::BindOnce( &AdsClientNotifier::NotifyUserDidBecomeActive, weak_factory_.GetWeakPtr(), idle_time, screen_was_locked)); } @@ -249,7 +252,7 @@ void AdsClientNotifier::NotifyUserDidBecomeActive( void AdsClientNotifier::NotifyBrowserDidEnterForeground() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyBrowserDidEnterForeground, weak_factory_.GetWeakPtr())); } @@ -261,7 +264,7 @@ void AdsClientNotifier::NotifyBrowserDidEnterForeground() { void AdsClientNotifier::NotifyBrowserDidEnterBackground() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyBrowserDidEnterBackground, weak_factory_.GetWeakPtr())); } @@ -273,7 +276,7 @@ void AdsClientNotifier::NotifyBrowserDidEnterBackground() { void AdsClientNotifier::NotifyBrowserDidBecomeActive() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyBrowserDidBecomeActive, weak_factory_.GetWeakPtr())); } @@ -285,7 +288,7 @@ void AdsClientNotifier::NotifyBrowserDidBecomeActive() { void AdsClientNotifier::NotifyBrowserDidResignActive() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyBrowserDidResignActive, weak_factory_.GetWeakPtr())); } @@ -297,7 +300,7 @@ void AdsClientNotifier::NotifyBrowserDidResignActive() { void AdsClientNotifier::NotifyDidSolveAdaptiveCaptcha() { if (should_queue_) { - return queue_->Add( + return pending_task_queue_->Add( base::BindOnce(&AdsClientNotifier::NotifyDidSolveAdaptiveCaptcha, weak_factory_.GetWeakPtr())); } diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.cc b/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.cc index 69f62ad50835..9f77e7c6d9e2 100644 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.cc +++ b/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(); diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.h b/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.h index 0482cdda3671..4078d79b38c0 100644 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier_for_testing.h +++ b/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; diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.h b/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.h deleted file mode 100644 index 2c8b735ad33f..000000000000 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.h +++ /dev/null @@ -1,35 +0,0 @@ -/* Copyright (c) 2023 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at https://mozilla.org/MPL/2.0/. */ - -#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_ADS_CLIENT_ADS_CLIENT_NOTIFIER_QUEUE_H_ -#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_ADS_CLIENT_ADS_CLIENT_NOTIFIER_QUEUE_H_ - -#include "base/containers/queue.h" -#include "base/functional/callback.h" - -namespace brave_ads { - -class AdsClientNotifierQueue final { - public: - AdsClientNotifierQueue(); - - AdsClientNotifierQueue(const AdsClientNotifierQueue&) = delete; - AdsClientNotifierQueue& operator=(const AdsClientNotifierQueue&) = delete; - - AdsClientNotifierQueue(AdsClientNotifierQueue&&) noexcept = delete; - AdsClientNotifierQueue& operator=(AdsClientNotifierQueue&&) noexcept = delete; - - ~AdsClientNotifierQueue(); - - void Add(base::OnceClosure notifier); - void Process(); - - private: - base::queue queue_; -}; - -} // namespace brave_ads - -#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_ADS_CLIENT_ADS_CLIENT_NOTIFIER_QUEUE_H_ diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_unittest.cc b/components/brave_ads/core/internal/ads_client/ads_client_notifier_unittest.cc index 3c9051f17c20..bc1c9ed7ac53 100644 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier_unittest.cc +++ b/components/brave_ads/core/internal/ads_client/ads_client_notifier_unittest.cc @@ -177,7 +177,6 @@ class BraveAdsAdsClientNotifierTest : public ::testing::Test { TEST_F(BraveAdsAdsClientNotifierTest, FireQueuedAdsClientNotifications) { // Arrange - ads_client_notifier_.set_should_queue_for_testing(true); // Act & Assert ExpectAdsClientNotifierCallCount(0); @@ -198,7 +197,6 @@ TEST_F( BraveAdsAdsClientNotifierTest, DoNotFireQueuedAdsClientNotificationsIfNotifyPendingObserversIsNotCalled) { // Arrange - ads_client_notifier_.set_should_queue_for_testing(true); // Act & Assert ExpectAdsClientNotifierCallCount(0); @@ -206,9 +204,9 @@ TEST_F( } TEST_F(BraveAdsAdsClientNotifierTest, - FireAdsClientNotificationsImmediatelyIfNotQueued) { + FireAdsClientNotificationsImmediatelyIfNotifyPendingObserversWasCalled) { // Arrange - ads_client_notifier_.set_should_queue_for_testing(false); + ads_client_notifier_.NotifyPendingObservers(); // Act & Assert ExpectAdsClientNotifierCallCount(1); diff --git a/components/brave_ads/core/internal/ads_impl.cc b/components/brave_ads/core/internal/ads_impl.cc index 3696821c0259..0cf4ade8537a 100644 --- a/components/brave_ads/core/internal/ads_impl.cc +++ b/components/brave_ads/core/internal/ads_impl.cc @@ -25,6 +25,7 @@ #include "brave/components/brave_ads/core/internal/legacy_migration/confirmations/legacy_confirmation_migration.h" #include "brave/components/brave_ads/core/internal/user_engagement/ad_events/ad_events.h" #include "brave/components/brave_ads/core/public/ad_units/notification_ad/notification_ad_info.h" +#include "brave/components/brave_ads/core/public/ad_units/notification_ad/notification_ad_value_util.h" #include "brave/components/brave_ads/core/public/ads_client/ads_client.h" namespace brave_ads { @@ -86,7 +87,9 @@ void AdsImpl::Shutdown(ShutdownCallback callback) { if (!is_initialized_) { BLOG(0, "Shutdown failed as not initialized"); - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::Shutdown, weak_factory_.GetWeakPtr(), std::move(callback))); + return; } NotificationAdManager::GetInstance().RemoveAll(/*should_close=*/true); @@ -96,7 +99,10 @@ void AdsImpl::Shutdown(ShutdownCallback callback) { void AdsImpl::GetDiagnostics(GetDiagnosticsCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*diagnostics=*/std::nullopt); + pending_task_queue_.Add(base::BindOnce(&AdsImpl::GetDiagnostics, + weak_factory_.GetWeakPtr(), + std::move(callback))); + return; } DiagnosticManager::GetInstance().GetDiagnostics(std::move(callback)); @@ -104,7 +110,10 @@ void AdsImpl::GetDiagnostics(GetDiagnosticsCallback callback) { void AdsImpl::GetStatementOfAccounts(GetStatementOfAccountsCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*statement=*/nullptr); + pending_task_queue_.Add(base::BindOnce(&AdsImpl::GetStatementOfAccounts, + weak_factory_.GetWeakPtr(), + std::move(callback))); + return; } GetAccount().GetStatement(std::move(callback)); @@ -114,7 +123,10 @@ void AdsImpl::MaybeServeInlineContentAd( const std::string& dimensions, MaybeServeInlineContentAdCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(dimensions, /*ad=*/std::nullopt); + pending_task_queue_.Add(base::BindOnce(&AdsImpl::MaybeServeInlineContentAd, + weak_factory_.GetWeakPtr(), + dimensions, std::move(callback))); + return; } GetAdHandler().MaybeServeInlineContentAd(dimensions, std::move(callback)); @@ -126,7 +138,11 @@ void AdsImpl::TriggerInlineContentAdEvent( const mojom::InlineContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerInlineContentAdEvent, weak_factory_.GetWeakPtr(), + placement_id, creative_instance_id, mojom_ad_event_type, + std::move(callback))); + return; } GetAdHandler().TriggerInlineContentAdEvent(placement_id, creative_instance_id, @@ -136,7 +152,10 @@ void AdsImpl::TriggerInlineContentAdEvent( void AdsImpl::MaybeServeNewTabPageAd(MaybeServeNewTabPageAdCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*ad=*/std::nullopt); + pending_task_queue_.Add(base::BindOnce(&AdsImpl::MaybeServeNewTabPageAd, + weak_factory_.GetWeakPtr(), + std::move(callback))); + return; } GetAdHandler().MaybeServeNewTabPageAd(std::move(callback)); @@ -148,7 +167,11 @@ void AdsImpl::TriggerNewTabPageAdEvent( const mojom::NewTabPageAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerNewTabPageAdEvent, weak_factory_.GetWeakPtr(), + placement_id, creative_instance_id, mojom_ad_event_type, + std::move(callback))); + return; } GetAdHandler().TriggerNewTabPageAdEvent(placement_id, creative_instance_id, @@ -156,10 +179,18 @@ void AdsImpl::TriggerNewTabPageAdEvent( std::move(callback)); } -std::optional AdsImpl::MaybeGetNotificationAd( - const std::string& placement_id) { - return NotificationAdManager::GetInstance().MaybeGetForPlacementId( - placement_id); +void AdsImpl::MaybeGetNotificationAd(const std::string& placement_id, + MaybeGetNotificationAdCallback callback) { + if (!is_initialized_) { + pending_task_queue_.Add(base::BindOnce(&AdsImpl::MaybeGetNotificationAd, + weak_factory_.GetWeakPtr(), + placement_id, std::move(callback))); + return; + } + + std::move(callback).Run( + NotificationAdManager::GetInstance().MaybeGetForPlacementId( + placement_id)); } void AdsImpl::TriggerNotificationAdEvent( @@ -167,7 +198,10 @@ void AdsImpl::TriggerNotificationAdEvent( const mojom::NotificationAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerNotificationAdEvent, weak_factory_.GetWeakPtr(), + placement_id, mojom_ad_event_type, std::move(callback))); + return; } GetAdHandler().TriggerNotificationAdEvent(placement_id, mojom_ad_event_type, @@ -180,7 +214,11 @@ void AdsImpl::TriggerPromotedContentAdEvent( const mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerPromotedContentAdEvent, weak_factory_.GetWeakPtr(), + placement_id, creative_instance_id, mojom_ad_event_type, + std::move(callback))); + return; } GetAdHandler().TriggerPromotedContentAdEvent( @@ -193,7 +231,11 @@ void AdsImpl::TriggerSearchResultAdEvent( const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add( + base::BindOnce(&AdsImpl::TriggerSearchResultAdEvent, + weak_factory_.GetWeakPtr(), std::move(mojom_creative_ad), + mojom_ad_event_type, std::move(callback))); + return; } GetAdHandler().TriggerSearchResultAdEvent( @@ -204,7 +246,10 @@ void AdsImpl::PurgeOrphanedAdEventsForType( const mojom::AdType mojom_ad_type, PurgeOrphanedAdEventsForTypeCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::PurgeOrphanedAdEventsForType, weak_factory_.GetWeakPtr(), + mojom_ad_type, std::move(callback))); + return; } PurgeOrphanedAdEvents( @@ -229,7 +274,10 @@ void AdsImpl::GetAdHistory(const base::Time from_time, const base::Time to_time, GetAdHistoryForUICallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*ad_history=*/std::nullopt); + pending_task_queue_.Add( + base::BindOnce(&AdsImpl::GetAdHistory, weak_factory_.GetWeakPtr(), + from_time, to_time, std::move(callback))); + return; } AdHistoryManager::GetForUI(from_time, to_time, std::move(callback)); @@ -238,7 +286,10 @@ void AdsImpl::GetAdHistory(const base::Time from_time, void AdsImpl::ToggleLikeAd(mojom::ReactionInfoPtr mojom_reaction, ToggleReactionCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleLikeAd, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); + return; } GetReactions().ToggleLikeAd(std::move(mojom_reaction), std::move(callback)); @@ -247,7 +298,10 @@ void AdsImpl::ToggleLikeAd(mojom::ReactionInfoPtr mojom_reaction, void AdsImpl::ToggleDislikeAd(mojom::ReactionInfoPtr mojom_reaction, ToggleReactionCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleDislikeAd, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); + return; } GetReactions().ToggleDislikeAd(std::move(mojom_reaction), @@ -257,7 +311,10 @@ void AdsImpl::ToggleDislikeAd(mojom::ReactionInfoPtr mojom_reaction, void AdsImpl::ToggleLikeSegment(mojom::ReactionInfoPtr mojom_reaction, ToggleReactionCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleLikeSegment, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); + return; } GetReactions().ToggleLikeSegment(std::move(mojom_reaction), @@ -267,7 +324,10 @@ void AdsImpl::ToggleLikeSegment(mojom::ReactionInfoPtr mojom_reaction, void AdsImpl::ToggleDislikeSegment(mojom::ReactionInfoPtr mojom_reaction, ToggleReactionCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::ToggleDislikeSegment, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); + return; } GetReactions().ToggleDislikeSegment(std::move(mojom_reaction), @@ -277,7 +337,10 @@ void AdsImpl::ToggleDislikeSegment(mojom::ReactionInfoPtr mojom_reaction, void AdsImpl::ToggleSaveAd(mojom::ReactionInfoPtr mojom_reaction, ToggleReactionCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleSaveAd, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); + return; } GetReactions().ToggleSaveAd(std::move(mojom_reaction), std::move(callback)); @@ -286,7 +349,10 @@ void AdsImpl::ToggleSaveAd(mojom::ReactionInfoPtr mojom_reaction, void AdsImpl::ToggleMarkAdAsInappropriate(mojom::ReactionInfoPtr mojom_reaction, ToggleReactionCallback callback) { if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + pending_task_queue_.Add(base::BindOnce( + &AdsImpl::ToggleMarkAdAsInappropriate, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); + return; } GetReactions().ToggleMarkAdAsInappropriate(std::move(mojom_reaction), @@ -329,6 +395,8 @@ void AdsImpl::SuccessfullyInitialized(mojom::WalletInfoPtr mojom_wallet, GetAdsClient()->NotifyPendingObservers(); + pending_task_queue_.Process(); + std::move(callback).Run(/*success=*/true); } diff --git a/components/brave_ads/core/internal/ads_impl.h b/components/brave_ads/core/internal/ads_impl.h index 30c8b5943114..619ea661f6f7 100644 --- a/components/brave_ads/core/internal/ads_impl.h +++ b/components/brave_ads/core/internal/ads_impl.h @@ -7,12 +7,12 @@ #define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_ADS_IMPL_H_ #include -#include #include #include "base/memory/weak_ptr.h" #include "brave/components/brave_ads/browser/ads_service_callback.h" #include "brave/components/brave_ads/core/internal/account/tokens/token_generator_interface.h" +#include "brave/components/brave_ads/core/internal/common/functional/once_closure_queue.h" #include "brave/components/brave_ads/core/internal/global_state/global_state.h" #include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" #include "brave/components/brave_ads/core/public/ads.h" @@ -24,8 +24,6 @@ class Time; namespace brave_ads { -struct NotificationAdInfo; - namespace database { class Maintenance; } // namespace database @@ -74,8 +72,8 @@ class AdsImpl final : public Ads { mojom::NewTabPageAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) override; - std::optional MaybeGetNotificationAd( - const std::string& placement_id) override; + void MaybeGetNotificationAd(const std::string& placement_id, + MaybeGetNotificationAdCallback callback) override; void TriggerNotificationAdEvent( const std::string& placement_id, mojom::NotificationAdEventType mojom_ad_event_type, @@ -143,6 +141,8 @@ class AdsImpl final : public Ads { // state. GlobalState global_state_; + OnceClosureQueue pending_task_queue_; + // Handles database maintenance tasks, such as purging. std::unique_ptr database_maintenance_; diff --git a/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.cc b/components/brave_ads/core/internal/common/functional/once_closure_queue.cc similarity index 55% rename from components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.cc rename to components/brave_ads/core/internal/common/functional/once_closure_queue.cc index af0de6884cf3..5c987a3586b4 100644 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.cc +++ b/components/brave_ads/core/internal/common/functional/once_closure_queue.cc @@ -3,21 +3,21 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#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_queue.h" #include namespace brave_ads { -AdsClientNotifierQueue::AdsClientNotifierQueue() = default; +OnceClosureQueue::OnceClosureQueue() = default; -AdsClientNotifierQueue::~AdsClientNotifierQueue() = default; +OnceClosureQueue::~OnceClosureQueue() = default; -void AdsClientNotifierQueue::Add(base::OnceClosure notifier) { - queue_.push(std::move(notifier)); +void OnceClosureQueue::Add(base::OnceClosure closure) { + queue_.push(std::move(closure)); } -void AdsClientNotifierQueue::Process() { +void OnceClosureQueue::Process() { while (!queue_.empty()) { std::move(queue_.front()).Run(); queue_.pop(); diff --git a/components/brave_ads/core/internal/common/functional/once_closure_queue.h b/components/brave_ads/core/internal/common/functional/once_closure_queue.h new file mode 100644 index 000000000000..33c2da911e5a --- /dev/null +++ b/components/brave_ads/core/internal/common/functional/once_closure_queue.h @@ -0,0 +1,35 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_COMMON_FUNCTIONAL_ONCE_CLOSURE_QUEUE_H_ +#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_COMMON_FUNCTIONAL_ONCE_CLOSURE_QUEUE_H_ + +#include "base/containers/queue.h" +#include "base/functional/callback.h" + +namespace brave_ads { + +class OnceClosureQueue final { + public: + OnceClosureQueue(); + + OnceClosureQueue(const OnceClosureQueue&) = delete; + OnceClosureQueue& operator=(const OnceClosureQueue&) = delete; + + OnceClosureQueue(OnceClosureQueue&&) noexcept = delete; + OnceClosureQueue& operator=(OnceClosureQueue&&) noexcept = delete; + + ~OnceClosureQueue(); + + void Add(base::OnceClosure closure); + void Process(); + + private: + base::queue queue_; +}; + +} // namespace brave_ads + +#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_COMMON_FUNCTIONAL_ONCE_CLOSURE_QUEUE_H_ diff --git a/components/brave_ads/core/internal/common/test/test_base.cc b/components/brave_ads/core/internal/common/test/test_base.cc index bf325dc985d9..227c1abce1eb 100644 --- a/components/brave_ads/core/internal/common/test/test_base.cc +++ b/components/brave_ads/core/internal/common/test/test_base.cc @@ -323,6 +323,8 @@ void TestBase::SetUpIntegrationTestCallback(const bool success) { NotifyBrowserDidBecomeActive(); NotifyDidInitializeAds(); + + NotifyPendingObservers(); } void TestBase::SetUpUnitTest() { @@ -337,6 +339,8 @@ void TestBase::SetUpUnitTest() { Mock(); MockDefaultAdsServiceState(); + + NotifyPendingObservers(); } } // namespace brave_ads::test diff --git a/components/brave_ads/core/public/ads.h b/components/brave_ads/core/public/ads.h index 182306320f49..9da26b8e1f38 100644 --- a/components/brave_ads/core/public/ads.h +++ b/components/brave_ads/core/public/ads.h @@ -109,10 +109,11 @@ class ADS_EXPORT Ads { mojom::NewTabPageAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) = 0; - // Called to get the notification ad specified by `placement_id`. Returns - // `NotificationAdInfo` containing the info of the ad. - virtual std::optional MaybeGetNotificationAd( - const std::string& placement_id) = 0; + // Called to get the notification ad specified by `placement_id`. The callback + // takes one argument - `NotificationAdInfo` containing the info of the ad. + virtual void MaybeGetNotificationAd( + const std::string& placement_id, + MaybeGetNotificationAdCallback callback) = 0; // Called when a user views or interacts with a notification ad or the ad // notification times out to trigger a `mojom_ad_event_type` event for the diff --git a/components/brave_ads/core/public/ads_callback.h b/components/brave_ads/core/public/ads_callback.h index 2033f6c7190c..4e82c54a5e69 100644 --- a/components/brave_ads/core/public/ads_callback.h +++ b/components/brave_ads/core/public/ads_callback.h @@ -14,6 +14,7 @@ #include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h" #include "brave/components/brave_ads/core/public/ad_units/inline_content_ad/inline_content_ad_info.h" #include "brave/components/brave_ads/core/public/ad_units/new_tab_page_ad/new_tab_page_ad_info.h" +#include "brave/components/brave_ads/core/public/ad_units/notification_ad/notification_ad_info.h" #include "brave/components/brave_ads/core/public/history/ad_history_item_info.h" namespace brave_ads { @@ -34,6 +35,9 @@ using MaybeServeInlineContentAdCallback = base::OnceCallback& ad)>; +using MaybeGetNotificationAdCallback = + base::OnceCallback& ad)>; + using TriggerAdEventCallback = base::OnceCallback; using PurgeOrphanedAdEventsForTypeCallback = diff --git a/components/brave_ads/core/public/ads_client/ads_client.h b/components/brave_ads/core/public/ads_client/ads_client.h index b22e43a7d77f..e1962fc9d53d 100644 --- a/components/brave_ads/core/public/ads_client/ads_client.h +++ b/components/brave_ads/core/public/ads_client/ads_client.h @@ -32,7 +32,7 @@ class ADS_EXPORT AdsClient { // Called to remove an ads client observer. virtual void RemoveObserver(AdsClientNotifierObserver* observer) = 0; - // Called to bind pending ads client observers. + // Called to fire all pending observer events. virtual void NotifyPendingObservers() = 0; // Returns `true` if there is an available network connection. diff --git a/components/brave_ads/core/public/ads_client/ads_client_notifier.h b/components/brave_ads/core/public/ads_client/ads_client_notifier.h index 156526fc2864..264e6d1e2ba4 100644 --- a/components/brave_ads/core/public/ads_client/ads_client_notifier.h +++ b/components/brave_ads/core/public/ads_client/ads_client_notifier.h @@ -19,7 +19,7 @@ class GURL; namespace brave_ads { -class AdsClientNotifierQueue; +class OnceClosureQueue; class AdsClientNotifier { public: @@ -33,10 +33,6 @@ class AdsClientNotifier { virtual ~AdsClientNotifier(); - void set_should_queue_for_testing(bool should_queue) { - should_queue_ = should_queue; - } - void AddObserver(AdsClientNotifierObserver* observer); void RemoveObserver(AdsClientNotifierObserver* observer); @@ -151,13 +147,9 @@ class AdsClientNotifier { private: base::ObserverList observers_; - std::unique_ptr queue_; + std::unique_ptr pending_task_queue_; -#if BUILDFLAG(IS_IOS) bool should_queue_ = true; -#else - bool should_queue_ = false; -#endif // BUILDFLAG(IS_IOS) base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/services/bat_ads/bat_ads_client_mojo_bridge.cc b/components/services/bat_ads/bat_ads_client_mojo_bridge.cc index 79233c8a79eb..626c7a7c1587 100644 --- a/components/services/bat_ads/bat_ads_client_mojo_bridge.cc +++ b/components/services/bat_ads/bat_ads_client_mojo_bridge.cc @@ -40,7 +40,7 @@ void BatAdsClientMojoBridge::RemoveObserver( } void BatAdsClientMojoBridge::NotifyPendingObservers() { - bat_ads_client_notifier_impl_.BindReceiver(); + bat_ads_client_notifier_impl_.NotifyPendingObservers(); } bool BatAdsClientMojoBridge::CanShowNotificationAdsWhileBrowserIsBackgrounded() diff --git a/components/services/bat_ads/bat_ads_client_notifier_impl.cc b/components/services/bat_ads/bat_ads_client_notifier_impl.cc index 8077d50af879..9945f3854867 100644 --- a/components/services/bat_ads/bat_ads_client_notifier_impl.cc +++ b/components/services/bat_ads/bat_ads_client_notifier_impl.cc @@ -13,7 +13,10 @@ BatAdsClientNotifierImpl::BatAdsClientNotifierImpl( mojo::PendingReceiver bat_ads_client_notifier_pending_receiver) : bat_ads_client_notifier_pending_receiver_( - std::move(bat_ads_client_notifier_pending_receiver)) {} + std::move(bat_ads_client_notifier_pending_receiver)) { + bat_ads_client_notifier_receiver_.Bind( + std::move(bat_ads_client_notifier_pending_receiver_)); +} BatAdsClientNotifierImpl::~BatAdsClientNotifierImpl() = default; @@ -31,10 +34,8 @@ void BatAdsClientNotifierImpl::RemoveObserver( ads_client_notifier_.RemoveObserver(observer); } -void BatAdsClientNotifierImpl::BindReceiver() { - CHECK(bat_ads_client_notifier_pending_receiver_.is_valid()); - bat_ads_client_notifier_receiver_.Bind( - std::move(bat_ads_client_notifier_pending_receiver_)); +void BatAdsClientNotifierImpl::NotifyPendingObservers() { + ads_client_notifier_.NotifyPendingObservers(); } void BatAdsClientNotifierImpl::NotifyDidInitializeAds() { diff --git a/components/services/bat_ads/bat_ads_client_notifier_impl.h b/components/services/bat_ads/bat_ads_client_notifier_impl.h index 0aff62cfd5a3..17c6e12970e8 100644 --- a/components/services/bat_ads/bat_ads_client_notifier_impl.h +++ b/components/services/bat_ads/bat_ads_client_notifier_impl.h @@ -37,8 +37,8 @@ class BatAdsClientNotifierImpl : public bat_ads::mojom::BatAdsClientNotifier { void AddObserver(brave_ads::AdsClientNotifierObserver* observer); void RemoveObserver(brave_ads::AdsClientNotifierObserver* observer); - // Binds the receiver by consuming the pending receiver swhich was created. - void BindReceiver(); + // Invoked to fire all pending observer events. + void NotifyPendingObservers(); // Invoked when ads did initialize. void NotifyDidInitializeAds() override; diff --git a/components/services/bat_ads/bat_ads_impl.cc b/components/services/bat_ads/bat_ads_impl.cc index 0947b34e3bae..f831c1915b19 100644 --- a/components/services/bat_ads/bat_ads_impl.cc +++ b/components/services/bat_ads/bat_ads_impl.cc @@ -96,15 +96,21 @@ void BatAdsImpl::Shutdown(ShutdownCallback callback) { void BatAdsImpl::MaybeGetNotificationAd( const std::string& placement_id, MaybeGetNotificationAdCallback callback) { - const std::optional ad = - GetAds()->MaybeGetNotificationAd(placement_id); - if (!ad) { - std::move(callback).Run(/*ad*/ std::nullopt); - return; - } + GetAds()->MaybeGetNotificationAd( + placement_id, + base::BindOnce( + [](MaybeGetNotificationAdCallback callback, + const std::optional& ad) { + if (!ad) { + std::move(callback).Run(/*ad*/ std::nullopt); + return; + } - std::optional dict = brave_ads::NotificationAdToValue(*ad); - std::move(callback).Run(std::move(dict)); + std::optional dict = + brave_ads::NotificationAdToValue(*ad); + std::move(callback).Run(std::move(dict)); + }, + std::move(callback))); } void BatAdsImpl::TriggerNotificationAdEvent( diff --git a/ios/browser/api/ads/brave_ads.h b/ios/browser/api/ads/brave_ads.h index 174ea1a75fe9..6077d9cfd771 100644 --- a/ios/browser/api/ads/brave_ads.h +++ b/ios/browser/api/ads/brave_ads.h @@ -113,7 +113,8 @@ OBJC_EXPORT eventType:(BraveAdsNewTabPageAdEventType)eventType completion:(void (^)(BOOL success))completion; -- (nullable NotificationAdIOS*)maybeGetNotificationAd:(NSString*)identifier; +- (void)maybeGetNotificationAd:(NSString*)identifier + completion:(void (^)(NotificationAdIOS*))completion; - (void)triggerNotificationAdEvent:(NSString*)placementId eventType:(BraveAdsNotificationAdEventType)eventType diff --git a/ios/browser/api/ads/brave_ads.mm b/ios/browser/api/ads/brave_ads.mm index 4f9780fd8169..a9c86fefba2e 100644 --- a/ios/browser/api/ads/brave_ads.mm +++ b/ios/browser/api/ads/brave_ads.mm @@ -1587,18 +1587,25 @@ - (void)triggerNewTabPageAdEvent:(NSString*)wallpaperId base::BindOnce(completion)); } -- (nullable NotificationAdIOS*)maybeGetNotificationAd:(NSString*)identifier { +- (void)maybeGetNotificationAd:(NSString*)identifier + completion:(void (^)(NotificationAdIOS*))completion { if (![self isServiceRunning]) { - return nil; + completion(nil); + return; } - const std::optional ad = - ads->MaybeGetNotificationAd(identifier.UTF8String); - if (!ad) { - return nil; - } + ads->MaybeGetNotificationAd( + identifier.UTF8String, + base::BindOnce(^(const std::optional& ad) { + if (!ad) { + completion(nil); + return; + } - return [[NotificationAdIOS alloc] initWithNotificationAdInfo:*ad]; + const auto notification_ad = + [[NotificationAdIOS alloc] initWithNotificationAdInfo:*ad]; + completion(notification_ad); + })); } - (void)triggerNotificationAdEvent:(NSString*)placementId