From 6df3655bd8791456ba5e34508a88d883fb8e7a12 Mon Sep 17 00:00:00 2001 From: brave-builds Date: Fri, 31 Jul 2020 19:26:58 +0000 Subject: [PATCH 1/3] Uplift of #6285 (squashed) to release --- components/brave_rewards/resources/page/reducers/index.ts | 5 +++-- .../resources/page/reducers/promotion_reducer.ts | 4 ++++ .../resources/page/reducers/publishers_reducer.ts | 4 ++++ .../brave_rewards/resources/page/reducers/rewards_reducer.ts | 4 ++++ .../brave_rewards/resources/page/reducers/wallet_reducer.ts | 4 ++++ 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/components/brave_rewards/resources/page/reducers/index.ts b/components/brave_rewards/resources/page/reducers/index.ts index 7b33db74bb47..39670e7ddf20 100644 --- a/components/brave_rewards/resources/page/reducers/index.ts +++ b/components/brave_rewards/resources/page/reducers/index.ts @@ -24,8 +24,9 @@ const mergeReducers = (state: Rewards.State | undefined, action: any) => { if (!state) { state = storage.defaultState - storage.save(state) - } else if (state !== startingState) { + } + + if (state !== startingState) { storage.debouncedSave(state) } diff --git a/components/brave_rewards/resources/page/reducers/promotion_reducer.ts b/components/brave_rewards/resources/page/reducers/promotion_reducer.ts index 2e4e160856b1..a099a58c4b58 100644 --- a/components/brave_rewards/resources/page/reducers/promotion_reducer.ts +++ b/components/brave_rewards/resources/page/reducers/promotion_reducer.ts @@ -39,6 +39,10 @@ const updatePromotion = (newPromotion: Rewards.Promotion, promotions: Rewards.Pr } const promotionReducer: Reducer = (state: Rewards.State, action) => { + if (!state) { + return + } + const payload = action.payload switch (action.type) { case types.FETCH_PROMOTIONS: { diff --git a/components/brave_rewards/resources/page/reducers/publishers_reducer.ts b/components/brave_rewards/resources/page/reducers/publishers_reducer.ts index b93673b41d53..36e42e566cbf 100644 --- a/components/brave_rewards/resources/page/reducers/publishers_reducer.ts +++ b/components/brave_rewards/resources/page/reducers/publishers_reducer.ts @@ -8,6 +8,10 @@ import { Reducer } from 'redux' import { types } from '../constants/rewards_types' const publishersReducer: Reducer = (state: Rewards.State, action) => { + if (!state) { + return + } + switch (action.type) { case types.ON_CONTRIBUTE_LIST: state = { ...state } diff --git a/components/brave_rewards/resources/page/reducers/rewards_reducer.ts b/components/brave_rewards/resources/page/reducers/rewards_reducer.ts index 9d28b0b407e4..f75fd1b32738 100644 --- a/components/brave_rewards/resources/page/reducers/rewards_reducer.ts +++ b/components/brave_rewards/resources/page/reducers/rewards_reducer.ts @@ -9,6 +9,10 @@ import { types } from '../constants/rewards_types' import { defaultState } from '../storage' const rewardsReducer: Reducer = (state: Rewards.State, action) => { + if (!state) { + return + } + switch (action.type) { case types.TOGGLE_ENABLE_MAIN: { if (state.initializing) { diff --git a/components/brave_rewards/resources/page/reducers/wallet_reducer.ts b/components/brave_rewards/resources/page/reducers/wallet_reducer.ts index adaabf6f2b82..bbc495fd9a1c 100644 --- a/components/brave_rewards/resources/page/reducers/wallet_reducer.ts +++ b/components/brave_rewards/resources/page/reducers/wallet_reducer.ts @@ -22,6 +22,10 @@ const createWallet = (state: Rewards.State) => { } const walletReducer: Reducer = (state: Rewards.State, action) => { + if (!state) { + return + } + switch (action.type) { case types.CREATE_WALLET: state = { ...state } From 1b4e8d3e7ed0d74699159d8e0e880a557e8e7a63 Mon Sep 17 00:00:00 2001 From: Emerick Rogul Date: Thu, 30 Jul 2020 16:15:16 -0400 Subject: [PATCH 2/3] Fix Rewards reset failing on Windows --- .../browser/rewards_service_impl.cc | 104 +++++++++++------- .../browser/rewards_service_impl.h | 14 ++- 2 files changed, 79 insertions(+), 39 deletions(-) diff --git a/components/brave_rewards/browser/rewards_service_impl.cc b/components/brave_rewards/browser/rewards_service_impl.cc index c2c74619d418..b71850d0194b 100644 --- a/components/brave_rewards/browser/rewards_service_impl.cc +++ b/components/brave_rewards/browser/rewards_service_impl.cc @@ -216,17 +216,6 @@ bool ResetOnFileTaskRunner(const base::FilePath& path) { return base::DeleteFile(path, false); } -bool ResetOnFilesTaskRunner(const std::vector& paths) { - bool res = true; - for (size_t i = 0; i < paths.size(); i++) { - if (!base::DeleteFileRecursively(paths[i])) { - res = false; - } - } - - return res; -} - void EnsureRewardsBaseDirectoryExists(const base::FilePath& path) { if (!DirectoryExists(path)) base::CreateDirectory(path); @@ -1475,7 +1464,7 @@ void RewardsServiceImpl::SetRewardsMainEnabled(bool enabled) { if (!enabled) { RecordRewardsDisabledForSomeMetrics(); - StopLedger(); + StopLedger(base::DoNothing()); } } @@ -1488,7 +1477,7 @@ void RewardsServiceImpl::EnableGreaseLion(const bool enabled) { #endif } -void RewardsServiceImpl::StopLedger() { +void RewardsServiceImpl::StopLedger(StopLedgerCallback callback) { BLOG(1, "Shutting down ledger process"); if (!Connected()) { BLOG(1, "Ledger process not running"); @@ -1496,17 +1485,62 @@ void RewardsServiceImpl::StopLedger() { return; } - bat_ledger_->Shutdown( - base::BindOnce(&RewardsServiceImpl::OnStopLedger, AsWeakPtr())); + bat_ledger_->Shutdown(base::BindOnce( + &RewardsServiceImpl::OnStopLedger, + AsWeakPtr(), + std::move(callback))); } -void RewardsServiceImpl::OnStopLedger(const ledger::Result result) { +void RewardsServiceImpl::OnStopLedger( + StopLedgerCallback callback, + const ledger::Result result) { BLOG_IF( 1, result != ledger::Result::LEDGER_OK, "Ledger process was not shut down successfully"); Reset(); BLOG(1, "Successfully shutdown ledger"); + std::move(callback).Run(result); +} + +void RewardsServiceImpl::OnStopLedgerForCompleteReset( + SuccessCallback callback, + const ledger::Result result) { + profile_->GetPrefs()->ClearPrefsWithPrefixSilently(pref_prefix); + + base::PostTaskAndReplyWithResult( + file_task_runner_.get(), + FROM_HERE, + base::BindOnce( + &RewardsServiceImpl::ResetOnFilesTaskRunner, + base::Unretained(this)), + base::BindOnce( + &RewardsServiceImpl::OnCompleteReset, + AsWeakPtr(), + std::move(callback))); +} + +bool RewardsServiceImpl::ResetOnFilesTaskRunner() { + // Close any open files before deleting them (required on Windows) + diagnostic_log_.Close(); + + const std::vector paths = { + ledger_state_path_, + publisher_state_path_, + publisher_info_db_path_, + diagnostic_log_path_, + publisher_list_path_, + rewards_base_path_ + }; + + bool res = true; + for (size_t i = 0; i < paths.size(); i++) { + if (!base::DeleteFileRecursively(paths[i])) { + res = false; + } + } + + return res; } void RewardsServiceImpl::Reset() { @@ -2479,6 +2513,10 @@ void RewardsServiceImpl::DiagnosticLog( return; } + if (resetting_rewards_) { + return; + } + if (profile_->IsOffTheRecord()) { return; } @@ -3910,41 +3948,33 @@ void RewardsServiceImpl::ClearAllNotifications() { } void RewardsServiceImpl::CompleteReset(SuccessCallback callback) { + resetting_rewards_ = true; + + auto* ads_service = brave_ads::AdsServiceFactory::GetForProfile(profile_); + if (ads_service) { + ads_service->ResetAllState(); + } + + StopNotificationTimers(); notification_service_->DeleteAllNotifications(true); - std::vector paths; - paths.push_back(ledger_state_path_); - paths.push_back(publisher_state_path_); - paths.push_back(publisher_info_db_path_); - paths.push_back(diagnostic_log_path_); - paths.push_back(publisher_list_path_); - paths.push_back(rewards_base_path_); - base::PostTaskAndReplyWithResult( - file_task_runner_.get(), - FROM_HERE, - base::BindOnce(&ResetOnFilesTaskRunner, paths), + auto stop_callback = base::BindOnce( - &RewardsServiceImpl::OnCompleteReset, + &RewardsServiceImpl::OnStopLedgerForCompleteReset, AsWeakPtr(), - std::move(callback))); + std::move(callback)); + StopLedger(std::move(stop_callback)); } void RewardsServiceImpl::OnCompleteReset( SuccessCallback callback, const bool success) { - profile_->GetPrefs()->ClearPrefsWithPrefixSilently(pref_prefix); - - auto* ads_service = brave_ads::AdsServiceFactory::GetForProfile(profile_); - if (ads_service) { - ads_service->ResetAllState(); - } + resetting_rewards_ = false; for (auto& observer : observers_) { observer.OnCompleteReset(success); } - StopLedger(); - StopNotificationTimers(); std::move(callback).Run(true); } diff --git a/components/brave_rewards/browser/rewards_service_impl.h b/components/brave_rewards/browser/rewards_service_impl.h index 09318a0fd915..9ce566bf5d11 100644 --- a/components/brave_rewards/browser/rewards_service_impl.h +++ b/components/brave_rewards/browser/rewards_service_impl.h @@ -89,6 +89,8 @@ using ExternalWalletAuthorizationCallback = const ledger::Result, const std::map&)>; +using StopLedgerCallback = base::OnceCallback; + class RewardsServiceImpl : public RewardsService, public ledger::LedgerClient, public base::SupportsWeakPtr { @@ -330,12 +332,19 @@ class RewardsServiceImpl : public RewardsService, void EnableGreaseLion(const bool enabled); - void StopLedger(); + void StopLedger(StopLedgerCallback callback); - void OnStopLedger(const ledger::Result result); + void OnStopLedger( + StopLedgerCallback callback, + const ledger::Result result); + void OnStopLedgerForCompleteReset( + SuccessCallback callback, + const ledger::Result result); void Reset(); + bool ResetOnFilesTaskRunner(); + void OnCreate(); void OnResult(ledger::ResultCallback callback, const ledger::Result result); @@ -767,6 +776,7 @@ class RewardsServiceImpl : public RewardsService, bool reset_states_; bool is_wallet_initialized_ = false; bool ledger_for_testing_ = false; + bool resetting_rewards_ = false; GetTestResponseCallback test_response_callback_; From d3992679dfd23c88db858a8fffbbbd7d225214c6 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Mon, 3 Aug 2020 08:08:10 +0100 Subject: [PATCH 3/3] 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 a1293a4c11eb..4b4b0242ba8d 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; }; } // namespace brave_ads diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc index 107145f8cf51..28aad3deaad8 100644 --- a/components/brave_ads/browser/ads_service_impl.cc +++ b/components/brave_ads/browser/ads_service_impl.cc @@ -217,7 +217,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( @@ -545,6 +544,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())); } @@ -631,13 +634,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) { @@ -645,8 +685,6 @@ void AdsServiceImpl::OnResetAllState( return; } - profile_->GetPrefs()->ClearPrefsWithPrefixSilently("brave.brave_ads"); - VLOG(1) << "Successfully reset ads state"; } @@ -672,6 +710,9 @@ void AdsServiceImpl::OnEnsureBaseDirectoryExists( bat_ads_.BindNewEndpointAndPassReceiver(), base::BindOnce(&AdsServiceImpl::OnCreate, AsWeakPtr())); + database_ = std::make_unique( + base_path_.AppendASCII("database.sqlite")); + MaybeShowMyFirstAdNotification(); } @@ -1997,6 +2038,8 @@ std::string AdsServiceImpl::LoadJsonSchema( 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 7af496c5513f..f87f5d6962c9 100644 --- a/components/brave_ads/browser/ads_service_impl.h +++ b/components/brave_ads/browser/ads_service_impl.h @@ -138,6 +138,9 @@ class AdsServiceImpl : public AdsService, const bool flagged, OnToggleFlagAdCallback callback) override; + void ResetAllState( + const bool should_shutdown) override; + // AdsClient implementation bool IsEnabled() const override; @@ -182,7 +185,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 b71850d0194b..2c761bb11a1e 100644 --- a/components/brave_rewards/browser/rewards_service_impl.cc +++ b/components/brave_rewards/browser/rewards_service_impl.cc @@ -363,9 +363,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_)); @@ -418,6 +415,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_); @@ -3952,7 +3953,7 @@ void RewardsServiceImpl::CompleteReset(SuccessCallback callback) { auto* ads_service = brave_ads::AdsServiceFactory::GetForProfile(profile_); if (ads_service) { - ads_service->ResetAllState(); + ads_service->ResetAllState(/* should_shutdown */ true); } StopNotificationTimers();