[ads] Fixes ads fail to Initialise (uplift to 1.65.x) (#23061)

* Uplift of #22989 (squashed) to release * [ads] Follow up to #37390: Failing to Initialize ads due to corrupted confirmations.json * [ads] Deprecate legacy transactions migration path --------- Co-authored-by: Terry Mancey <[email protected]>
brave · Apr 16, 2024 · 251448e · 251448e
1 parent c1a6c6e
commit 251448e
Show file tree

Hide file tree

Showing 38 changed files with 151 additions and 1,081 deletions.
diff --git a/components/brave_ads/core/internal/BUILD.gn b/components/brave_ads/core/internal/BUILD.gn
@@ -723,28 +723,6 @@ static_library("internal") {
     "legacy_migration/database/database_creation.h",
     "legacy_migration/database/database_migration.cc",
     "legacy_migration/database/database_migration.h",
-    "legacy_migration/rewards/legacy_rewards_migration.cc",
-    "legacy_migration/rewards/legacy_rewards_migration.h",
-    "legacy_migration/rewards/legacy_rewards_migration_payment_tokens_json_reader.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_payment_tokens_json_reader.h",
-    "legacy_migration/rewards/legacy_rewards_migration_payment_tokens_json_reader_util.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_payment_tokens_json_reader_util.h",
-    "legacy_migration/rewards/legacy_rewards_migration_payments_json_reader.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_payments_json_reader.h",
-    "legacy_migration/rewards/legacy_rewards_migration_payments_json_reader_util.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_payments_json_reader_util.h",
-    "legacy_migration/rewards/legacy_rewards_migration_payments_util.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_payments_util.h",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_constants.h",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_history_json_reader.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_history_json_reader.h",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_history_json_reader_util.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_history_json_reader_util.h",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_util.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_transaction_util.h",
-    "legacy_migration/rewards/legacy_rewards_migration_util.cc",
-    "legacy_migration/rewards/legacy_rewards_migration_util.h",
-    "legacy_migration/rewards/payment_info.h",
     "ml/data/data.cc",
     "ml/data/data.h",
     "ml/data/data_types.h",

diff --git a/components/brave_ads/core/internal/account/transactions/transactions_database_table.cc b/components/brave_ads/core/internal/account/transactions/transactions_database_table.cc
@@ -21,7 +21,6 @@
 #include "brave/components/brave_ads/core/internal/common/database/database_transaction_util.h"
 #include "brave/components/brave_ads/core/internal/common/logging_util.h"
 #include "brave/components/brave_ads/core/internal/common/time/time_util.h"
-#include "brave/components/brave_ads/core/internal/legacy_migration/rewards/legacy_rewards_migration_transaction_constants.h"
 #include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
 
 namespace brave_ads::database::table {
@@ -302,7 +301,6 @@ void Transactions::Reconcile(const PaymentTokenList& payment_tokens,
   for (const auto& payment_token : payment_tokens) {
     transaction_ids.push_back(payment_token.transaction_id);
   }
-  transaction_ids.emplace_back(rewards::kMigrationUnreconciledTransactionId);
 
   mojom::DBTransactionInfoPtr transaction = mojom::DBTransactionInfo::New();
   mojom::DBCommandInfoPtr command = mojom::DBCommandInfo::New();
@@ -331,8 +329,6 @@ void Transactions::Reconcile(const PaymentTokenList& payment_tokens,
     ++index;
   }
 
-  BindString(&*command, index, rewards::kMigrationUnreconciledTransactionId);
-
   transaction->commands.push_back(std::move(command));
 
   RunTransaction(std::move(transaction), std::move(callback));

diff --git a/components/brave_ads/core/internal/ads_impl.cc b/components/brave_ads/core/internal/ads_impl.cc
@@ -20,7 +20,6 @@
 #include "brave/components/brave_ads/core/internal/history/history_manager.h"
 #include "brave/components/brave_ads/core/internal/legacy_migration/client/legacy_client_migration.h"
 #include "brave/components/brave_ads/core/internal/legacy_migration/confirmations/legacy_confirmation_migration.h"
-#include "brave/components/brave_ads/core/internal/legacy_migration/rewards/legacy_rewards_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/history/ad_content_value_util.h"
@@ -32,11 +31,6 @@ namespace {
 
 void FailedToInitialize(InitializeCallback callback) {
   BLOG(0, "Failed to initialize ads");
-
-  // TODO(https://github.com/brave/brave-browser/issues/32066): Remove migration
-  // failure dumps.
-  base::debug::DumpWithoutCrashing();
-
   std::move(callback).Run(/*success=*/false);
 }
 
@@ -79,6 +73,10 @@ void AdsImpl::Initialize(mojom::WalletInfoPtr wallet,
   BLOG(1, "Initializing ads");
 
   if (is_initialized_) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     BLOG(1, "Already initialized ads");
     return FailedToInitialize(std::move(callback));
   }
@@ -286,6 +284,10 @@ void AdsImpl::CreateOrOpenDatabaseCallback(mojom::WalletInfoPtr wallet,
                                            InitializeCallback callback,
                                            const bool success) {
   if (!success) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     BLOG(0, "Failed to create or open database");
     return FailedToInitialize(std::move(callback));
   }
@@ -299,6 +301,10 @@ void AdsImpl::PurgeExpiredAdEventsCallback(mojom::WalletInfoPtr wallet,
                                            InitializeCallback callback,
                                            const bool success) {
   if (!success) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     BLOG(0, "Failed to purge expired ad events");
     return FailedToInitialize(std::move(callback));
   }
@@ -312,19 +318,11 @@ void AdsImpl::PurgeOrphanedAdEventsCallback(mojom::WalletInfoPtr wallet,
                                             InitializeCallback callback,
                                             const bool success) {
   if (!success) {
-    BLOG(0, "Failed to purge orphaned ad events");
-    return FailedToInitialize(std::move(callback));
-  }
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
 
-  rewards::Migrate(base::BindOnce(&AdsImpl::MigrateRewardsStateCallback,
-                                  weak_factory_.GetWeakPtr(), std::move(wallet),
-                                  std::move(callback)));
-}
-
-void AdsImpl::MigrateRewardsStateCallback(mojom::WalletInfoPtr wallet,
-                                          InitializeCallback callback,
-                                          const bool success) {
-  if (!success) {
+    BLOG(0, "Failed to purge orphaned ad events");
     return FailedToInitialize(std::move(callback));
   }
 
@@ -337,6 +335,10 @@ void AdsImpl::MigrateClientStateCallback(mojom::WalletInfoPtr wallet,
                                          InitializeCallback callback,
                                          const bool success) {
   if (!success) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     return FailedToInitialize(std::move(callback));
   }
 
@@ -349,6 +351,10 @@ void AdsImpl::LoadClientStateCallback(mojom::WalletInfoPtr wallet,
                                       InitializeCallback callback,
                                       const bool success) {
   if (!success) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     return FailedToInitialize(std::move(callback));
   }
 
@@ -361,6 +367,10 @@ void AdsImpl::MigrateConfirmationStateCallback(mojom::WalletInfoPtr wallet,
                                                InitializeCallback callback,
                                                const bool success) {
   if (!success) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     return FailedToInitialize(std::move(callback));
   }
 
@@ -387,6 +397,10 @@ void AdsImpl::LoadConfirmationStateCallback(mojom::WalletInfoPtr wallet,
                                             InitializeCallback callback,
                                             const bool success) {
   if (!success) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     return FailedToInitialize(std::move(callback));
   }
 

diff --git a/components/brave_ads/core/internal/ads_impl.h b/components/brave_ads/core/internal/ads_impl.h
@@ -124,9 +124,6 @@ class AdsImpl final : public Ads {
   void PurgeOrphanedAdEventsCallback(mojom::WalletInfoPtr wallet,
                                      InitializeCallback callback,
                                      bool success);
-  void MigrateRewardsStateCallback(mojom::WalletInfoPtr wallet,
-                                   InitializeCallback callback,
-                                   bool success);
   void MigrateClientStateCallback(mojom::WalletInfoPtr wallet,
                                   InitializeCallback callback,
                                   bool success);

diff --git a/components/brave_ads/core/internal/deprecated/client/client_info.cc b/components/brave_ads/core/internal/deprecated/client/client_info.cc
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/check.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/strings/string_number_conversions.h"
@@ -222,6 +223,10 @@ bool ClientInfo::FromJson(const std::string& json) {
       json, base::JSON_PARSE_CHROMIUM_EXTENSIONS |
                 base::JSONParserOptions::JSON_PARSE_RFC);
   if (!dict) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     return false;
   }
 

diff --git a/components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.cc b/components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.cc
@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/check.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/functional/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
@@ -107,28 +108,31 @@ std::string ConfirmationStateManager::ToJson() {
 bool ConfirmationStateManager::FromJson(const std::string& json) {
   const std::optional<base::Value::Dict> dict =
       base::JSONReader::ReadDict(json);
+  confirmation_tokens_.RemoveAll();
+  payment_tokens_.RemoveAllTokens();
+
   if (!dict) {
+    // TODO(https://github.com/brave/brave-browser/issues/32066):
+    // Remove migration failure dumps.
+    base::debug::DumpWithoutCrashing();
+
     return false;
   }
 
-  if (!ParseConfirmationTokensFromDictionary(*dict)) {
-    BLOG(1, "Failed to parse confirmation tokens");
-  }
+  ParseConfirmationTokensFromDictionary(*dict);
 
-  if (!ParsePaymentTokensFromDictionary(*dict)) {
-    BLOG(1, "Failed to parse payment tokens");
-  }
+  ParsePaymentTokensFromDictionary(*dict);
 
   return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool ConfirmationStateManager::ParseConfirmationTokensFromDictionary(
+void ConfirmationStateManager::ParseConfirmationTokensFromDictionary(
     const base::Value::Dict& dict) {
   const auto* const list = dict.FindList("unblinded_tokens");
   if (!list) {
-    return false;
+    return;
   }
 
   ConfirmationTokenList filtered_confirmation_tokens =
@@ -151,20 +155,13 @@ bool ConfirmationStateManager::ParseConfirmationTokensFromDictionary(
   }
 
   confirmation_tokens_.Set(filtered_confirmation_tokens);
-
-  return true;
 }
 
-bool ConfirmationStateManager::ParsePaymentTokensFromDictionary(
+void ConfirmationStateManager::ParsePaymentTokensFromDictionary(
     const base::Value::Dict& dict) {
-  const auto* const list = dict.FindList("unblinded_payment_tokens");
-  if (!list) {
-    return false;
+  if (const auto* const list = dict.FindList("unblinded_payment_tokens")) {
+    payment_tokens_.SetTokens(PaymentTokensFromValue(*list));
   }
-
-  payment_tokens_.SetTokens(PaymentTokensFromValue(*list));
-
-  return true;
 }
 
 }  // namespace brave_ads
diff --git a/components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.h b/components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.h
@@ -67,9 +67,9 @@ class ConfirmationStateManager final {
   void LoadCallback(InitializeCallback callback,
                     const std::optional<std::string>& json);
 
-  bool ParseConfirmationTokensFromDictionary(const base::Value::Dict& dict);
+  void ParseConfirmationTokensFromDictionary(const base::Value::Dict& dict);
 
-  bool ParsePaymentTokensFromDictionary(const base::Value::Dict& dict);
+  void ParsePaymentTokensFromDictionary(const base::Value::Dict& dict);
 
   bool is_initialized_ = false;
 

diff --git a/components/brave_ads/core/internal/legacy_migration/client/legacy_client_migration.cc b/components/brave_ads/core/internal/legacy_migration/client/legacy_client_migration.cc
@@ -27,6 +27,7 @@ void FailedToMigrate(InitializeCallback callback) {
 }
 
 void SuccessfullyMigrated(InitializeCallback callback) {
+  SetProfileBooleanPref("brave.brave_ads.state.has_migrated.client.v6", true);
   SetProfileBooleanPref(prefs::kHasMigratedClientState, true);
   std::move(callback).Run(/*success=*/true);
 }
@@ -43,12 +44,25 @@ void MigrateClientState(InitializeCallback callback) {
            [](InitializeCallback callback,
               const std::optional<std::string>& json) {
              if (!json) {
-               // Client state does not exist
+               // Client state does not exist.
                return SuccessfullyMigrated(std::move(callback));
              }
+             std::string mutable_json = *json;
 
              ClientInfo client;
-             if (!client.FromJson(*json)) {
+
+             if (!GetProfileBooleanPref(
+                     "brave.brave_ads.state.has_migrated.client.v6") &&
+                 !client.FromJson(mutable_json)) {
+               // The client state is corrupted, therefore, reset it to the
+               // default values for version 6.
+               BLOG(0,
+                    "Client state is corrupted, resetting to default values");
+               mutable_json = "{}";
+             }
+
+             client = {};
+             if (!client.FromJson(mutable_json)) {
                // TODO(https://github.com/brave/brave-browser/issues/32066):
                // Remove migration failure dumps.
                base::debug::DumpWithoutCrashing();
@@ -59,9 +73,7 @@ void MigrateClientState(InitializeCallback callback) {
 
              BLOG(1, "Migrating client state");
 
-             const std::string migrated_json = client.ToJson();
-
-             Save(kClientStateFilename, migrated_json,
+             Save(kClientStateFilename, client.ToJson(),
                   base::BindOnce(
                       [](InitializeCallback callback, const bool success) {
                         if (!success) {

diff --git a/...nents/brave_ads/core/internal/legacy_migration/client/legacy_client_migration_unittest.cc b/...nents/brave_ads/core/internal/legacy_migration/client/legacy_client_migration_unittest.cc
@@ -39,17 +39,17 @@ TEST_F(BraveAdsLegacyClientMigrationTest, Migrate) {
   EXPECT_TRUE(HasMigratedClientState());
 }
 
-TEST_F(BraveAdsLegacyClientMigrationTest, InvalidState) {
+TEST_F(BraveAdsLegacyClientMigrationTest, ResetInvalidState) {
   // Arrange
   ASSERT_TRUE(CopyFileFromTestPathToTempPath(kInvalidJsonFilename,
                                              kClientStateFilename));
 
   // Act & Assert
   base::MockCallback<InitializeCallback> callback;
-  EXPECT_CALL(callback, Run(/*success=*/false));
+  EXPECT_CALL(callback, Run(/*success=*/true));
   MigrateClientState(callback.Get());
 
-  EXPECT_FALSE(HasMigratedClientState());
+  EXPECT_TRUE(HasMigratedClientState());
 }
 
 }  // namespace brave_ads