Merge pull request #26510 from brave/issues/42242

[ads] Update Brave Ads ClearData to enhance reusability
brave · Nov 12, 2024 · 072f655 · 072f655
2 parents 2fb3da4 + 42dc892
commit 072f655
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 68 deletions.
diff --git 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
@@ -22,7 +22,7 @@ class AdsClientIOS : public brave_ads::AdsClient {
   ~AdsClientIOS() override;
 
  private:
-  __unsafe_unretained id<AdsClientBridge> bridge_;
+  __weak id<AdsClientBridge> 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
@@ -101,7 +101,6 @@ - (instancetype)initWithInlineContentAdInfo:
 @end
 
 @interface BraveAds () <AdsClientBridge> {
-  std::unique_ptr<AdsClientIOS> adsClient;
   std::unique_ptr<brave_ads::AdsClientNotifier> adsClientNotifier;
   std::unique_ptr<brave_ads::AdEventCache> adEventCache;
   raw_ptr<brave_ads::AdsServiceImplIOS> adsService;
@@ -147,7 +146,6 @@ - (instancetype)initWithStateStoragePath:(NSString*)path {
 
     adEventCache = std::make_unique<brave_ads::AdEventCache>();
 
-    adsClient = std::make_unique<AdsClientIOS>(self);
     adsClientNotifier = std::make_unique<brave_ads::AdsClientNotifier>();
   }
   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<AdsClientIOS>(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
@@ -44,7 +44,7 @@ class AdsServiceImplIOS : public KeyedService {
   bool IsRunning() const;
 
   void InitializeAds(const std::string& storage_path,
-                     AdsClient& ads_client,
+                     std::unique_ptr<AdsClient> 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<PrefService> prefs_ = nullptr;  // Not owned.
 
   const scoped_refptr<base::SequencedTaskRunner> database_queue_;
 
   base::FilePath storage_path_;
+  std::unique_ptr<AdsClient> ads_client_;
+  mojom::SysInfoPtr mojom_sys_info_;
+  mojom::BuildChannelInfoPtr mojom_build_channel_;
+  mojom::WalletInfoPtr mojom_wallet_;
 
   base::SequenceBound<Database> database_;
 

diff --git a/ios/browser/brave_ads/ads_service_impl_ios.mm b/ios/browser/brave_ads/ads_service_impl_ios.mm
@@ -5,18 +5,21 @@
 
 #include "brave/ios/browser/brave_ads/ads_service_impl_ios.h"
 
+#include <memory>
 #include <string>
 
 #include "base/check.h"
 #include "base/files/file_path.h"
 #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_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 +51,20 @@
 
 void AdsServiceImplIOS::InitializeAds(
     const std::string& storage_path,
-    AdsClient& ads_client,
+    std::unique_ptr<AdsClient> 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 +75,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 +92,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 +191,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<brave_ads::Database>(
-      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,11 +217,47 @@
   std::move(callback).Run(success);
 }
 
+void AdsServiceImplIOS::InitializeDatabase() {
+  database_ = base::SequenceBound<brave_ads::Database>(
+      database_queue_,
+      base::FilePath(storage_path_.Append(kAdsDatabaseFilename)));
+}
+
 void AdsServiceImplIOS::ShutdownAdsCallback(ShutdownCallback callback,
                                             const bool success) {
   Shutdown();
 
   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<bool>(std::move(callback)));
+}
+
 }  // namespace brave_ads