From ae97205b0e4271466d9bacbab2df66cb97953e97 Mon Sep 17 00:00:00 2001 From: Aleksei Seren Date: Tue, 12 Nov 2024 11:38:29 -0500 Subject: [PATCH] [ads] Update Brave Ads ClearData to enhance reusability --- .../Brave/Frontend/Rewards/BraveRewards.swift | 13 +-- ios/browser/api/ads/ads_client_ios.h | 2 +- ios/browser/api/ads/brave_ads.mm | 20 ++-- ios/browser/brave_ads/ads_service_impl_ios.h | 15 ++- ios/browser/brave_ads/ads_service_impl_ios.mm | 110 +++++++++++------- 5 files changed, 92 insertions(+), 68 deletions(-) diff --git a/ios/brave-ios/Sources/Brave/Frontend/Rewards/BraveRewards.swift b/ios/brave-ios/Sources/Brave/Frontend/Rewards/BraveRewards.swift index d5f0c7d02b5b..b38c8f1e3878 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Rewards/BraveRewards.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Rewards/BraveRewards.swift @@ -239,17 +239,8 @@ public class BraveRewards: PreferencesObserver { /// Clear Brave Ads Data. @MainActor func clearAdsData() async { await withCheckedContinuation { continuation in - ads.shutdownService { [ads] in - ads.clearData { - continuation.resume() - } - } - } - if shouldStartAds { - await withCheckedContinuation { continuation in - ads.initialize { _ in - continuation.resume() - } + ads.clearData { + continuation.resume() } } } diff --git a/ios/browser/api/ads/ads_client_ios.h b/ios/browser/api/ads/ads_client_ios.h index d344d9c6057c..381a74ec3cc9 100644 --- a/ios/browser/api/ads/ads_client_ios.h +++ b/ios/browser/api/ads/ads_client_ios.h @@ -22,7 +22,7 @@ class AdsClientIOS : public brave_ads::AdsClient { ~AdsClientIOS() override; private: - __unsafe_unretained id bridge_; + __weak id bridge_; void AddObserver(brave_ads::AdsClientNotifierObserver* observer) override; void RemoveObserver(brave_ads::AdsClientNotifierObserver* observer) override; diff --git a/ios/browser/api/ads/brave_ads.mm b/ios/browser/api/ads/brave_ads.mm index 6bd8851981a0..3cbb627f440d 100644 --- a/ios/browser/api/ads/brave_ads.mm +++ b/ios/browser/api/ads/brave_ads.mm @@ -101,7 +101,6 @@ - (instancetype)initWithInlineContentAdInfo: @end @interface BraveAds () { - std::unique_ptr adsClient; std::unique_ptr adsClientNotifier; std::unique_ptr adEventCache; raw_ptr adsService; @@ -147,7 +146,6 @@ - (instancetype)initWithStateStoragePath:(NSString*)path { adEventCache = std::make_unique(); - adsClient = std::make_unique(self); adsClientNotifier = std::make_unique(); } return self; @@ -161,7 +159,6 @@ - (void)dealloc { [self stopNetworkMonitor]; [self deallocAdsClientNotifier]; - [self deallocAdsClient]; [self deallocAdEventCache]; @@ -172,10 +169,6 @@ - (void)deallocAdsClientNotifier { adsClientNotifier.reset(); } -- (void)deallocAdsClient { - adsClient.reset(); -} - - (void)deallocAdEventCache { adEventCache.reset(); } @@ -258,9 +251,10 @@ - (void)initServiceWithSysInfo:(BraveAdsSysInfo*)sysInfo CHECK(adsService); adsService->InitializeAds( - base::SysNSStringToUTF8(self.storagePath), *adsClient, - std::move(cppSysInfo), std::move(cppBuildChannelInfo), - std::move(cppWalletInfo), base::BindOnce(^(const bool success) { + base::SysNSStringToUTF8(self.storagePath), + std::make_unique(self), std::move(cppSysInfo), + std::move(cppBuildChannelInfo), std::move(cppWalletInfo), + base::BindOnce(^(const bool success) { if (success) { [self registerAdsResources]; [self periodicallyCheckForAdsResourceUpdates]; @@ -1687,11 +1681,13 @@ - (void)purgeOrphanedAdEventsForType:(BraveAdsAdType)adType } - (void)clearData:(void (^)())completion { - if (adsService == nil) { + if (![self isServiceRunning]) { return completion(); } - adsService->ClearData(base::BindOnce(completion)); + adsService->ClearData(base::BindOnce(^() { + completion(); + })); } #pragma mark - Ads client notifier diff --git a/ios/browser/brave_ads/ads_service_impl_ios.h b/ios/browser/brave_ads/ads_service_impl_ios.h index 384f5204dfa2..16280a9c35b1 100644 --- a/ios/browser/brave_ads/ads_service_impl_ios.h +++ b/ios/browser/brave_ads/ads_service_impl_ios.h @@ -44,7 +44,7 @@ class AdsServiceImplIOS : public KeyedService { bool IsRunning() const; void InitializeAds(const std::string& storage_path, - AdsClient& ads_client, + std::unique_ptr ads_client, mojom::SysInfoPtr mojom_sys_info, mojom::BuildChannelInfoPtr mojom_build_channel, mojom::WalletInfoPtr mojom_wallet, @@ -100,17 +100,24 @@ class AdsServiceImplIOS : public KeyedService { // KeyedService: void Shutdown() override; - void InitializeDatabase(const std::string& storage_path); - void Cleanup(); - + void InitializeAds(InitializeCallback callback); void InitializeAdsCallback(InitializeCallback callback, bool success); + void InitializeDatabase(); + void ShutdownAdsCallback(ShutdownCallback callback, bool success); + void ClearAdsData(base::OnceClosure callback, bool success); + void ClearAdsDataCallback(base::OnceClosure callback); + const raw_ptr prefs_ = nullptr; // Not owned. const scoped_refptr database_queue_; base::FilePath storage_path_; + std::unique_ptr ads_client_; + mojom::SysInfoPtr mojom_sys_info_; + mojom::BuildChannelInfoPtr mojom_build_channel_; + mojom::WalletInfoPtr mojom_wallet_; base::SequenceBound database_; diff --git a/ios/browser/brave_ads/ads_service_impl_ios.mm b/ios/browser/brave_ads/ads_service_impl_ios.mm index 44650660eeae..47c19650a9eb 100644 --- a/ios/browser/brave_ads/ads_service_impl_ios.mm +++ b/ios/browser/brave_ads/ads_service_impl_ios.mm @@ -5,6 +5,7 @@ #include "brave/ios/browser/brave_ads/ads_service_impl_ios.h" +#include #include #include "base/check.h" @@ -12,11 +13,14 @@ #include "base/files/file_util.h" #include "base/functional/bind.h" #include "base/functional/callback.h" +#include "base/functional/callback_helpers.h" #include "base/metrics/histogram_macros.h" #include "base/task/sequenced_task_runner.h" #include "base/task/thread_pool.h" #include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h" #include "brave/components/brave_ads/core/public/ads.h" +#include "brave/components/brave_ads/core/public/ads_callback.h" +#include "brave/components/brave_ads/core/public/ads_client/ads_client.h" #include "brave/components/brave_ads/core/public/ads_constants.h" #include "brave/components/brave_ads/core/public/database/database.h" #include "brave/components/brave_ads/core/public/flags/flags_util.h" @@ -48,25 +52,20 @@ void AdsServiceImplIOS::InitializeAds( const std::string& storage_path, - AdsClient& ads_client, + std::unique_ptr ads_client, mojom::SysInfoPtr mojom_sys_info, mojom::BuildChannelInfoPtr mojom_build_channel, mojom::WalletInfoPtr mojom_wallet, InitializeCallback callback) { CHECK(!IsRunning()); - InitializeDatabase(storage_path); - - ads_ = Ads::CreateInstance(ads_client); - - ads_->SetSysInfo(std::move(mojom_sys_info)); - ads_->SetBuildChannel(std::move(mojom_build_channel)); - ads_->SetFlags(BuildFlags()); + storage_path_ = base::FilePath(storage_path); + ads_client_ = std::move(ads_client); + mojom_sys_info_ = std::move(mojom_sys_info); + mojom_build_channel_ = std::move(mojom_build_channel); + mojom_wallet_ = std::move(mojom_wallet); - ads_->Initialize( - std::move(mojom_wallet), - base::BindOnce(&AdsServiceImplIOS::InitializeAdsCallback, - weak_ptr_factory_.GetWeakPtr(), std::move(callback))); + InitializeAds(std::move(callback)); } void AdsServiceImplIOS::ShutdownAds(ShutdownCallback callback) { @@ -77,6 +76,15 @@ std::move(callback))); } +void AdsServiceImplIOS::ClearData(base::OnceClosure callback) { + UMA_HISTOGRAM_BOOLEAN(kClearDataHistogramName, true); + prefs_->ClearPrefsWithPrefixSilently("brave.brave_ads"); + + ShutdownAds(base::BindOnce(&AdsServiceImplIOS::ClearAdsData, + weak_ptr_factory_.GetWeakPtr(), + std::move(callback))); +} + void AdsServiceImplIOS::RunDBTransaction( mojom::DBTransactionInfoPtr mojom_db_transaction, RunDBTransactionCallback callback) { @@ -85,29 +93,6 @@ .Then(std::move(callback)); } -void AdsServiceImplIOS::ClearData(base::OnceClosure callback) { - // Ensure the Brave Ads service is stopped before clearing data. - CHECK(!IsRunning()); - - UMA_HISTOGRAM_BOOLEAN(kClearDataHistogramName, true); - prefs_->ClearPrefsWithPrefixSilently("brave.brave_ads"); - - database_queue_->PostTaskAndReply( - FROM_HERE, - base::BindOnce( - [](const base::FilePath& storage_path) { - sql::Database::Delete(storage_path.Append(kAdsDatabaseFilename)); - - base::DeleteFile( - storage_path.Append(brave_ads::kClientJsonFilename)); - - base::DeleteFile( - storage_path.Append(brave_ads::kConfirmationsJsonFilename)); - }, - storage_path_), - std::move(callback)); -} - void AdsServiceImplIOS::GetStatementOfAccounts( GetStatementOfAccountsCallback callback) { CHECK(IsRunning()); @@ -207,12 +192,21 @@ database_.Reset(); } -void AdsServiceImplIOS::InitializeDatabase(const std::string& storage_path) { - storage_path_ = base::FilePath(storage_path); +void AdsServiceImplIOS::InitializeAds(InitializeCallback callback) { + CHECK(!IsRunning()); - database_ = base::SequenceBound( - database_queue_, - base::FilePath(storage_path_.Append(kAdsDatabaseFilename))); + InitializeDatabase(); + + ads_ = Ads::CreateInstance(*ads_client_); + + ads_->SetSysInfo(mojom_sys_info_.Clone()); + ads_->SetBuildChannel(mojom_build_channel_.Clone()); + ads_->SetFlags(BuildFlags()); + + ads_->Initialize( + mojom_wallet_.Clone(), + base::BindOnce(&AdsServiceImplIOS::InitializeAdsCallback, + weak_ptr_factory_.GetWeakPtr(), std::move(callback))); } void AdsServiceImplIOS::InitializeAdsCallback(InitializeCallback callback, @@ -224,6 +218,12 @@ std::move(callback).Run(success); } +void AdsServiceImplIOS::InitializeDatabase() { + database_ = base::SequenceBound( + database_queue_, + base::FilePath(storage_path_.Append(kAdsDatabaseFilename))); +} + void AdsServiceImplIOS::ShutdownAdsCallback(ShutdownCallback callback, const bool success) { Shutdown(); @@ -231,4 +231,34 @@ std::move(callback).Run(success); } +void AdsServiceImplIOS::ClearAdsData(base::OnceClosure callback, + const bool success) { + if (!success) { + return std::move(callback).Run(); + } + + // Ensure the Brave Ads service is stopped before clearing data. + CHECK(!IsRunning()); + + database_queue_->PostTaskAndReply( + FROM_HERE, + base::BindOnce( + [](const base::FilePath& storage_path) { + sql::Database::Delete(storage_path.Append(kAdsDatabaseFilename)); + + base::DeleteFile( + storage_path.Append(brave_ads::kClientJsonFilename)); + + base::DeleteFile( + storage_path.Append(brave_ads::kConfirmationsJsonFilename)); + }, + storage_path_), + base::BindOnce(&AdsServiceImplIOS::ClearAdsDataCallback, + weak_ptr_factory_.GetWeakPtr(), std::move(callback))); +} + +void AdsServiceImplIOS::ClearAdsDataCallback(base::OnceClosure callback) { + InitializeAds(base::IgnoreArgs(std::move(callback))); +} + } // namespace brave_ads