Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions components/brave_ads/core/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
Expand Down
50 changes: 32 additions & 18 deletions components/brave_ads/core/internal/ads_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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);
}

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand All @@ -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));
}
Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand Down
3 changes: 0 additions & 3 deletions components/brave_ads/core/internal/ads_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 =
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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();
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading
Loading