From 5ba3023b42d599040453c0ba4ab249e30e3abac3 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Fri, 31 Jul 2020 09:59:17 +0100 Subject: [PATCH] Fixes when rewards is reset ads service should shut down --- components/brave_ads/browser/ads_service.h | 3 +- .../brave_ads/browser/ads_service_impl.cc | 51 +++++++++++++++++-- .../brave_ads/browser/ads_service_impl.h | 7 ++- .../browser/rewards_service_impl.cc | 9 ++-- 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/components/brave_ads/browser/ads_service.h b/components/brave_ads/browser/ads_service.h index ec4d5416d50a..a1efe81f24b1 100644 --- a/components/brave_ads/browser/ads_service.h +++ b/components/brave_ads/browser/ads_service.h @@ -133,7 +133,8 @@ class AdsService : public KeyedService { const bool flagged, OnToggleFlagAdCallback callback) = 0; - virtual void ResetAllState() = 0; + virtual void ResetAllState( + const bool should_shutdown) = 0; virtual void OnUserModelUpdated( const std::string& id) = 0; diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc index bc7f8612f89d..e3cf2f0a4acf 100644 --- a/components/brave_ads/browser/ads_service_impl.cc +++ b/components/brave_ads/browser/ads_service_impl.cc @@ -204,7 +204,6 @@ AdsServiceImpl::AdsServiceImpl(Profile* profile) : base::TaskPriority::BEST_EFFORT, base::TaskShutdownBehavior::BLOCK_SHUTDOWN})), base_path_(profile_->GetPath().AppendASCII("ads_service")), - database_(new ads::Database(base_path_.AppendASCII("database.sqlite"))), last_idle_state_(ui::IdleState::IDLE_STATE_ACTIVE), display_service_(NotificationDisplayService::GetForProfile(profile_)), rewards_service_(brave_rewards::RewardsServiceFactory::GetForProfile( @@ -547,6 +546,10 @@ void AdsServiceImpl::OnInitialize( void AdsServiceImpl::ShutdownBatAds() { VLOG(1) << "Shutting down ads"; + const bool success = file_task_runner_->DeleteSoon(FROM_HERE, + database_.release()); + VLOG_IF(1, !success) << "Failed to release database"; + bat_ads_->Shutdown(base::BindOnce(&AdsServiceImpl::OnShutdownBatAds, AsWeakPtr())); } @@ -633,13 +636,50 @@ void AdsServiceImpl::Stop() { ShutdownBatAds(); } -void AdsServiceImpl::ResetAllState() { +void AdsServiceImpl::ResetState() { + VLOG(1) << "Resetting ads state"; + + profile_->GetPrefs()->ClearPrefsWithPrefixSilently("brave.brave_ads"); + base::PostTaskAndReplyWithResult( file_task_runner_.get(), FROM_HERE, base::BindOnce(&ResetOnFileTaskRunner, base_path_), base::BindOnce(&AdsServiceImpl::OnResetAllState, AsWeakPtr())); } +void AdsServiceImpl::ResetAllState( + const bool should_shutdown) { + if (!should_shutdown) { + ResetState(); + return; + } + + VLOG(1) << "Shutting down and resetting ads state"; + + const bool success = file_task_runner_->DeleteSoon(FROM_HERE, + database_.release()); + VLOG_IF(1, !success) << "Failed to release database"; + + bat_ads_->Shutdown(base::BindOnce(&AdsServiceImpl::OnShutdownAndResetBatAds, + AsWeakPtr())); +} + +void AdsServiceImpl::OnShutdownAndResetBatAds( + const int32_t result) { + DCHECK(is_initialized_); + + if (result != ads::Result::SUCCESS) { + VLOG(0) << "Failed to shutdown and reset ads state"; + return; + } + + Shutdown(); + + VLOG(1) << "Successfully shutdown ads"; + + ResetState(); +} + void AdsServiceImpl::OnResetAllState( const bool success) { if (!success) { @@ -647,8 +687,6 @@ void AdsServiceImpl::OnResetAllState( return; } - profile_->GetPrefs()->ClearPrefsWithPrefixSilently("brave.brave_ads"); - VLOG(1) << "Successfully reset ads state"; } @@ -674,6 +712,9 @@ void AdsServiceImpl::OnEnsureBaseDirectoryExists( bat_ads_.BindNewEndpointAndPassReceiver(), base::BindOnce(&AdsServiceImpl::OnCreate, AsWeakPtr())); + database_ = std::make_unique( + base_path_.AppendASCII("database.sqlite")); + const std::string locale = GetLocale(); RegisterUserModelComponentsForLocale(locale); @@ -2008,6 +2049,8 @@ std::string AdsServiceImpl::LoadResourceForId( ads::DBCommandResponsePtr RunDBTransactionOnFileTaskRunner( ads::DBTransactionPtr transaction, ads::Database* database) { + DCHECK(database); + auto response = ads::DBCommandResponse::New(); if (!database) { diff --git a/components/brave_ads/browser/ads_service_impl.h b/components/brave_ads/browser/ads_service_impl.h index b321e27da6ea..52f93492aa99 100644 --- a/components/brave_ads/browser/ads_service_impl.h +++ b/components/brave_ads/browser/ads_service_impl.h @@ -141,6 +141,9 @@ class AdsServiceImpl : public AdsService, const bool flagged, OnToggleFlagAdCallback callback) override; + void ResetAllState( + const bool should_shutdown) override; + // AdsClient implementation bool IsEnabled() const override; @@ -189,7 +192,9 @@ class AdsServiceImpl : public AdsService, void Start(); void Stop(); - void ResetAllState() override; + void ResetState(); + void OnShutdownAndResetBatAds( + const int32_t result); void OnResetAllState( const bool success); diff --git a/components/brave_rewards/browser/rewards_service_impl.cc b/components/brave_rewards/browser/rewards_service_impl.cc index 65a61a970111..ed2394db09cc 100644 --- a/components/brave_rewards/browser/rewards_service_impl.cc +++ b/components/brave_rewards/browser/rewards_service_impl.cc @@ -374,9 +374,6 @@ RewardsServiceImpl::RewardsServiceImpl(Profile* profile) rewards_base_path_(profile_->GetPath().Append(kRewardsStatePath)), notification_service_(new RewardsNotificationServiceImpl(profile)), next_timer_id_(0) { - file_task_runner_->PostTask( - FROM_HERE, base::BindOnce(&EnsureRewardsBaseDirectoryExists, - rewards_base_path_)); // Set up the rewards data source content::URLDataSource::Add(profile_, std::make_unique(profile_)); @@ -429,6 +426,10 @@ void RewardsServiceImpl::StartLedger() { return; } + file_task_runner_->PostTask( + FROM_HERE, base::BindOnce(&EnsureRewardsBaseDirectoryExists, + rewards_base_path_)); + rewards_database_ = std::make_unique(publisher_info_db_path_); @@ -3919,7 +3920,7 @@ void RewardsServiceImpl::OnCompleteReset( auto* ads_service = brave_ads::AdsServiceFactory::GetForProfile(profile_); if (ads_service) { - ads_service->ResetAllState(); + ads_service->ResetAllState(/* should_shutdown */ true); } for (auto& observer : observers_) {