diff --git a/components/brave_ads/core/internal/BUILD.gn b/components/brave_ads/core/internal/BUILD.gn index 1146b08d50e1..42a9959bffcb 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_task_queue.cc", + "common/functional/once_closure_task_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..c6f206a731c3 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_task_queue.h" #include "url/gurl.h" namespace brave_ads { AdsClientNotifier::AdsClientNotifier() - : queue_(std::make_unique()) {} + : task_queue_(std::make_unique()) {} 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& 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& 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())); } 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.cc b/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.cc deleted file mode 100644 index af0de6884cf3..000000000000 --- a/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.cc +++ /dev/null @@ -1,27 +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/. */ - -#include "brave/components/brave_ads/core/internal/ads_client/ads_client_notifier_queue.h" - -#include - -namespace brave_ads { - -AdsClientNotifierQueue::AdsClientNotifierQueue() = default; - -AdsClientNotifierQueue::~AdsClientNotifierQueue() = default; - -void AdsClientNotifierQueue::Add(base::OnceClosure notifier) { - queue_.push(std::move(notifier)); -} - -void AdsClientNotifierQueue::Process() { - while (!queue_.empty()) { - std::move(queue_.front()).Run(); - queue_.pop(); - } -} - -} // namespace brave_ads 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..3c1bade8e007 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 @@ -176,9 +176,6 @@ class BraveAdsAdsClientNotifierTest : public ::testing::Test { }; TEST_F(BraveAdsAdsClientNotifierTest, FireQueuedAdsClientNotifications) { - // Arrange - ads_client_notifier_.set_should_queue_for_testing(true); - // Act & Assert ExpectAdsClientNotifierCallCount(0); FireAdsClientNotifiers(); // Queue notifications. @@ -197,18 +194,15 @@ TEST_F(BraveAdsAdsClientNotifierTest, FireQueuedAdsClientNotifications) { TEST_F( BraveAdsAdsClientNotifierTest, DoNotFireQueuedAdsClientNotificationsIfNotifyPendingObserversIsNotCalled) { - // Arrange - ads_client_notifier_.set_should_queue_for_testing(true); - // Act & Assert ExpectAdsClientNotifierCallCount(0); FireAdsClientNotifiers(); } 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..ae68ebe33f01 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 { @@ -85,7 +86,6 @@ void AdsImpl::Initialize(mojom::WalletInfoPtr mojom_wallet, void AdsImpl::Shutdown(ShutdownCallback callback) { if (!is_initialized_) { BLOG(0, "Shutdown failed as not initialized"); - return std::move(callback).Run(/*success=*/false); } @@ -95,16 +95,20 @@ void AdsImpl::Shutdown(ShutdownCallback callback) { } void AdsImpl::GetDiagnostics(GetDiagnosticsCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*diagnostics=*/std::nullopt); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce(&AdsImpl::GetDiagnostics, + weak_factory_.GetWeakPtr(), + std::move(callback))); } DiagnosticManager::GetInstance().GetDiagnostics(std::move(callback)); } void AdsImpl::GetStatementOfAccounts(GetStatementOfAccountsCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*statement=*/nullptr); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce(&AdsImpl::GetStatementOfAccounts, + weak_factory_.GetWeakPtr(), + std::move(callback))); } GetAccount().GetStatement(std::move(callback)); @@ -113,8 +117,10 @@ void AdsImpl::GetStatementOfAccounts(GetStatementOfAccountsCallback callback) { void AdsImpl::MaybeServeInlineContentAd( const std::string& dimensions, MaybeServeInlineContentAdCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(dimensions, /*ad=*/std::nullopt); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce(&AdsImpl::MaybeServeInlineContentAd, + weak_factory_.GetWeakPtr(), + dimensions, std::move(callback))); } GetAdHandler().MaybeServeInlineContentAd(dimensions, std::move(callback)); @@ -125,8 +131,11 @@ void AdsImpl::TriggerInlineContentAdEvent( const std::string& creative_instance_id, const mojom::InlineContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerInlineContentAdEvent, weak_factory_.GetWeakPtr(), + placement_id, creative_instance_id, mojom_ad_event_type, + std::move(callback))); } GetAdHandler().TriggerInlineContentAdEvent(placement_id, creative_instance_id, @@ -135,8 +144,10 @@ void AdsImpl::TriggerInlineContentAdEvent( } void AdsImpl::MaybeServeNewTabPageAd(MaybeServeNewTabPageAdCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*ad=*/std::nullopt); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce(&AdsImpl::MaybeServeNewTabPageAd, + weak_factory_.GetWeakPtr(), + std::move(callback))); } GetAdHandler().MaybeServeNewTabPageAd(std::move(callback)); @@ -147,8 +158,11 @@ void AdsImpl::TriggerNewTabPageAdEvent( const std::string& creative_instance_id, const mojom::NewTabPageAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerNewTabPageAdEvent, weak_factory_.GetWeakPtr(), + placement_id, creative_instance_id, mojom_ad_event_type, + std::move(callback))); } GetAdHandler().TriggerNewTabPageAdEvent(placement_id, creative_instance_id, @@ -156,18 +170,27 @@ 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 (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce(&AdsImpl::MaybeGetNotificationAd, + weak_factory_.GetWeakPtr(), + placement_id, std::move(callback))); + } + + std::move(callback).Run( + NotificationAdManager::GetInstance().MaybeGetForPlacementId( + placement_id)); } void AdsImpl::TriggerNotificationAdEvent( const std::string& placement_id, const mojom::NotificationAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerNotificationAdEvent, weak_factory_.GetWeakPtr(), + placement_id, mojom_ad_event_type, std::move(callback))); } GetAdHandler().TriggerNotificationAdEvent(placement_id, mojom_ad_event_type, @@ -179,8 +202,11 @@ void AdsImpl::TriggerPromotedContentAdEvent( const std::string& creative_instance_id, const mojom::PromotedContentAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::TriggerPromotedContentAdEvent, weak_factory_.GetWeakPtr(), + placement_id, creative_instance_id, mojom_ad_event_type, + std::move(callback))); } GetAdHandler().TriggerPromotedContentAdEvent( @@ -192,8 +218,11 @@ void AdsImpl::TriggerSearchResultAdEvent( mojom::CreativeSearchResultAdInfoPtr mojom_creative_ad, const mojom::SearchResultAdEventType mojom_ad_event_type, TriggerAdEventCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + if (task_queue_.should_queue()) { + return task_queue_.Add( + base::BindOnce(&AdsImpl::TriggerSearchResultAdEvent, + weak_factory_.GetWeakPtr(), std::move(mojom_creative_ad), + mojom_ad_event_type, std::move(callback))); } GetAdHandler().TriggerSearchResultAdEvent( @@ -203,8 +232,10 @@ void AdsImpl::TriggerSearchResultAdEvent( void AdsImpl::PurgeOrphanedAdEventsForType( const mojom::AdType mojom_ad_type, PurgeOrphanedAdEventsForTypeCallback callback) { - if (!is_initialized_) { - return std::move(callback).Run(/*success=*/false); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::PurgeOrphanedAdEventsForType, weak_factory_.GetWeakPtr(), + mojom_ad_type, std::move(callback))); } PurgeOrphanedAdEvents( @@ -228,8 +259,10 @@ void AdsImpl::PurgeOrphanedAdEventsForType( 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); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce(&AdsImpl::GetAdHistory, + weak_factory_.GetWeakPtr(), from_time, + to_time, std::move(callback))); } AdHistoryManager::GetForUI(from_time, to_time, std::move(callback)); @@ -237,8 +270,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); + if (task_queue_.should_queue()) { + return task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleLikeAd, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); } GetReactions().ToggleLikeAd(std::move(mojom_reaction), std::move(callback)); @@ -246,8 +281,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); + if (task_queue_.should_queue()) { + return task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleDislikeAd, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); } GetReactions().ToggleDislikeAd(std::move(mojom_reaction), @@ -256,8 +293,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); + if (task_queue_.should_queue()) { + return task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleLikeSegment, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); } GetReactions().ToggleLikeSegment(std::move(mojom_reaction), @@ -266,8 +305,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); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::ToggleDislikeSegment, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); } GetReactions().ToggleDislikeSegment(std::move(mojom_reaction), @@ -276,8 +317,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); + if (task_queue_.should_queue()) { + return task_queue_.Add( + base::BindOnce(&AdsImpl::ToggleSaveAd, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); } GetReactions().ToggleSaveAd(std::move(mojom_reaction), std::move(callback)); @@ -285,8 +328,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); + if (task_queue_.should_queue()) { + return task_queue_.Add(base::BindOnce( + &AdsImpl::ToggleMarkAdAsInappropriate, weak_factory_.GetWeakPtr(), + std::move(mojom_reaction), std::move(callback))); } GetReactions().ToggleMarkAdAsInappropriate(std::move(mojom_reaction), @@ -329,6 +374,8 @@ void AdsImpl::SuccessfullyInitialized(mojom::WalletInfoPtr mojom_wallet, GetAdsClient()->NotifyPendingObservers(); + task_queue_.FlushAndStopQueueing(); + 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..99c2466aa786 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_task_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_; + OnceClosureTaskQueue task_queue_; + // Handles database maintenance tasks, such as purging. std::unique_ptr database_maintenance_; diff --git a/components/brave_ads/core/internal/common/functional/once_closure_task_queue.cc b/components/brave_ads/core/internal/common/functional/once_closure_task_queue.cc new file mode 100644 index 000000000000..77988ab79ab8 --- /dev/null +++ b/components/brave_ads/core/internal/common/functional/once_closure_task_queue.cc @@ -0,0 +1,31 @@ +/* Copyright (c) 2024 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/. */ + +#include "brave/components/brave_ads/core/internal/common/functional/once_closure_task_queue.h" + +#include + +namespace brave_ads { + +OnceClosureTaskQueue::OnceClosureTaskQueue() = default; + +OnceClosureTaskQueue::~OnceClosureTaskQueue() = default; + +void OnceClosureTaskQueue::Add(base::OnceClosure closure) { + CHECK(should_queue_); + + queue_.push(std::move(closure)); +} + +void OnceClosureTaskQueue::FlushAndStopQueueing() { + should_queue_ = false; + + while (!queue_.empty()) { + std::move(queue_.front()).Run(); + queue_.pop(); + } +} + +} // namespace brave_ads diff --git a/components/brave_ads/core/internal/common/functional/once_closure_task_queue.h b/components/brave_ads/core/internal/common/functional/once_closure_task_queue.h new file mode 100644 index 000000000000..7d8412a4d489 --- /dev/null +++ b/components/brave_ads/core/internal/common/functional/once_closure_task_queue.h @@ -0,0 +1,40 @@ +/* Copyright (c) 2024 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_TASK_QUEUE_H_ +#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_COMMON_FUNCTIONAL_ONCE_CLOSURE_TASK_QUEUE_H_ + +#include "base/containers/queue.h" +#include "base/functional/callback.h" + +namespace brave_ads { + +class OnceClosureTaskQueue final { + public: + OnceClosureTaskQueue(); + + OnceClosureTaskQueue(const OnceClosureTaskQueue&) = delete; + OnceClosureTaskQueue& operator=(const OnceClosureTaskQueue&) = delete; + + OnceClosureTaskQueue(OnceClosureTaskQueue&&) noexcept = delete; + OnceClosureTaskQueue& operator=(OnceClosureTaskQueue&&) noexcept = delete; + + ~OnceClosureTaskQueue(); + + void Add(base::OnceClosure closure); + void FlushAndStopQueueing(); + + bool should_queue() const { return should_queue_; } + + bool empty() const { return queue_.empty(); } + + private: + bool should_queue_ = true; + base::queue queue_; +}; + +} // namespace brave_ads + +#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_COMMON_FUNCTIONAL_ONCE_CLOSURE_TASK_QUEUE_H_ diff --git a/components/brave_ads/core/internal/common/functional/once_closure_task_queue_unittest.cc b/components/brave_ads/core/internal/common/functional/once_closure_task_queue_unittest.cc new file mode 100644 index 000000000000..08752cd8cee8 --- /dev/null +++ b/components/brave_ads/core/internal/common/functional/once_closure_task_queue_unittest.cc @@ -0,0 +1,42 @@ +/* Copyright (c) 2024 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/. */ + +#include "brave/components/brave_ads/core/internal/common/functional/once_closure_task_queue.h" + +#include "base/functional/callback.h" +#include "base/test/mock_callback.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +// npm run test -- brave_unit_tests --filter=BraveAds* + +namespace brave_ads { + +TEST(BraveAdsOnceClosureTaskQueueTest, Add) { + // Arrange + OnceClosureTaskQueue task_queue; + base::MockCallback task; + EXPECT_CALL(task, Run()).Times(0); + task_queue.Add(task.Get()); + + // Act & Assert + EXPECT_FALSE(task_queue.empty()); + EXPECT_TRUE(task_queue.should_queue()); +} + +TEST(BraveAdsOnceClosureTaskQueueTest, AddAndFlush) { + // Arrange + OnceClosureTaskQueue task_queue; + base::MockCallback task; + EXPECT_CALL(task, Run()); + task_queue.Add(task.Get()); + + // Act & Assert + task_queue.FlushAndStopQueueing(); + EXPECT_TRUE(task_queue.empty()); + EXPECT_FALSE(task_queue.should_queue()); +} + +} // namespace brave_ads diff --git a/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.cc b/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.cc index 37b6c2f88e34..8d0e92621c20 100644 --- a/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.cc +++ b/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.cc @@ -76,6 +76,13 @@ void MockAdsClientNotifierAddObserver(AdsClientMock& ads_client_mock, })); } +void MockNotifyPendingObservers(AdsClientMock& ads_client_mock, + TestBase& test_base) { + ON_CALL(ads_client_mock, NotifyPendingObservers) + .WillByDefault(::testing::Invoke( + [&test_base]() { test_base.NotifyPendingObservers(); })); +} + void MockShowNotificationAd(AdsClientMock& ads_client_mock) { ON_CALL(ads_client_mock, ShowNotificationAd) .WillByDefault(::testing::Invoke([](const NotificationAdInfo& ad) { diff --git a/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.h b/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.h index bcd391606cd0..9218b670affd 100644 --- a/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.h +++ b/components/brave_ads/core/internal/common/test/internal/mock_test_util_internal.h @@ -18,6 +18,8 @@ void MockFlags(); void MockAdsClientNotifierAddObserver(AdsClientMock& ads_client_mock, TestBase& test_base); +void MockNotifyPendingObservers(AdsClientMock& ads_client_mock, + TestBase& test_base); void MockShowNotificationAd(AdsClientMock& ads_client_mock); void MockCloseNotificationAd(AdsClientMock& ads_client_mock); 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..4f6b1ab58d40 100644 --- a/components/brave_ads/core/internal/common/test/test_base.cc +++ b/components/brave_ads/core/internal/common/test/test_base.cc @@ -213,6 +213,8 @@ void TestBase::MockAdsClient() { // `ShowScheduledCaptcha`, `RecordP2AEvents`, and `Log` are not mocked here; // they should be mocked as needed. + MockNotifyPendingObservers(ads_client_mock_, *this); + MockIsNetworkConnectionAvailable(ads_client_mock_, true); MockIsBrowserActive(ads_client_mock_, true); @@ -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..0121b4cd6262 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 notify pending observers. 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..62611806ab6c 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 OnceClosureTaskQueue; 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,7 @@ class AdsClientNotifier { private: base::ObserverList observers_; - std::unique_ptr queue_; - -#if BUILDFLAG(IS_IOS) - bool should_queue_ = true; -#else - bool should_queue_ = false; -#endif // BUILDFLAG(IS_IOS) + std::unique_ptr task_queue_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/brave_ads/core/test/BUILD.gn b/components/brave_ads/core/test/BUILD.gn index 08827fc57735..45022aba9481 100644 --- a/components/brave_ads/core/test/BUILD.gn +++ b/components/brave_ads/core/test/BUILD.gn @@ -249,6 +249,7 @@ source_set("brave_ads_unit_tests") { "//brave/components/brave_ads/core/internal/common/containers/container_util_unittest.cc", "//brave/components/brave_ads/core/internal/common/country_code/country_code_unittest.cc", "//brave/components/brave_ads/core/internal/common/crypto/crypto_util_unittest.cc", + "//brave/components/brave_ads/core/internal/common/functional/once_closure_task_queue_unittest.cc", "//brave/components/brave_ads/core/internal/common/locale/locale_util_unittest.cc", "//brave/components/brave_ads/core/internal/common/net/http/http_status_code_util_unittest.cc", "//brave/components/brave_ads/core/internal/common/platform/platform_helper_mock.cc", 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..9998fb1df908 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,7 @@ 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(); + 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..cdf003b5cb0e 100644 --- a/components/services/bat_ads/bat_ads_impl.cc +++ b/components/services/bat_ads/bat_ads_impl.cc @@ -96,15 +96,20 @@ 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) { + return std::move(callback).Run(/*ad*/ std::nullopt); + } - 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..d13308e0bc39 100644 --- a/ios/browser/api/ads/brave_ads.h +++ b/ios/browser/api/ads/brave_ads.h @@ -113,7 +113,9 @@ OBJC_EXPORT eventType:(BraveAdsNewTabPageAdEventType)eventType completion:(void (^)(BOOL success))completion; -- (nullable NotificationAdIOS*)maybeGetNotificationAd:(NSString*)identifier; +- (void)maybeGetNotificationAd:(NSString*)identifier + completion: + (void (^)(NotificationAdIOS* _Nullable))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..d987b1bd4a88 100644 --- a/ios/browser/api/ads/brave_ads.mm +++ b/ios/browser/api/ads/brave_ads.mm @@ -782,8 +782,9 @@ - (void)downloadAdsResource:(NSString*)folderName return; } - [strongSelf.commonOps saveContents:response - name:adsResourceId.UTF8String]; + [strongSelf.commonOps + saveContents:response + name:base::SysNSStringToUTF8(adsResourceId)]; BLOG(1, @"Cached %@ ads resource version %@", adsResourceId, version); @@ -1385,7 +1386,7 @@ - (void)loadResourceComponent:(const std::string&)id BLOG(1, @"Loading %@ ads resource descriptor", nsFilePath); - base::FilePath file_path(nsFilePath.UTF8String); + base::FilePath file_path(base::SysNSStringToUTF8(nsFilePath)); base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, base::BindOnce(^base::File { return base::File(file_path, @@ -1408,7 +1409,7 @@ - (void)loadResourceComponent:(const std::string&)id if (!contents || error) { return ""; } - return std::string(contents.UTF8String); + return base::SysNSStringToUTF8(contents); } - (void)showScheduledCaptcha:(const std::string&)payment_id @@ -1533,7 +1534,7 @@ - (void)getStatementOfAccounts:(void (^)(NSInteger adsReceived, - (void)maybeServeInlineContentAd:(NSString*)dimensionsArg completion:(void (^)(NSString* dimensions, - InlineContentAdIOS* ad))completion { + InlineContentAdIOS*))completion { if (![self isServiceRunning]) { return; } @@ -1587,18 +1588,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( + base::SysNSStringToUTF8(identifier), + 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