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

Fixes storage not persist after reset (uplift to 1.12.x) #6300

Merged
merged 3 commits into from
Aug 5, 2020
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
3 changes: 2 additions & 1 deletion components/brave_ads/browser/ads_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 47 additions & 4 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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()));
}
Expand Down Expand Up @@ -631,22 +634,57 @@ 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) {
VLOG(0) << "Failed to reset ads state";
return;
}

profile_->GetPrefs()->ClearPrefsWithPrefixSilently("brave.brave_ads");

VLOG(1) << "Successfully reset ads state";
}

Expand All @@ -672,6 +710,9 @@ void AdsServiceImpl::OnEnsureBaseDirectoryExists(
bat_ads_.BindNewEndpointAndPassReceiver(),
base::BindOnce(&AdsServiceImpl::OnCreate, AsWeakPtr()));

database_ = std::make_unique<ads::Database>(
base_path_.AppendASCII("database.sqlite"));

MaybeShowMyFirstAdNotification();
}

Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

Expand Down
111 changes: 71 additions & 40 deletions components/brave_rewards/browser/rewards_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,6 @@ bool ResetOnFileTaskRunner(const base::FilePath& path) {
return base::DeleteFile(path, false);
}

bool ResetOnFilesTaskRunner(const std::vector<base::FilePath>& 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);
Expand Down Expand Up @@ -374,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<BraveRewardsSource>(profile_));
Expand Down Expand Up @@ -429,6 +415,10 @@ void RewardsServiceImpl::StartLedger() {
return;
}

file_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&EnsureRewardsBaseDirectoryExists,
rewards_base_path_));

rewards_database_ =
std::make_unique<RewardsDatabase>(publisher_info_db_path_);

Expand Down Expand Up @@ -1475,7 +1465,7 @@ void RewardsServiceImpl::SetRewardsMainEnabled(bool enabled) {

if (!enabled) {
RecordRewardsDisabledForSomeMetrics();
StopLedger();
StopLedger(base::DoNothing());
}
}

Expand All @@ -1488,25 +1478,70 @@ 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");
Reset();
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<base::FilePath> 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() {
Expand Down Expand Up @@ -2479,6 +2514,10 @@ void RewardsServiceImpl::DiagnosticLog(
return;
}

if (resetting_rewards_) {
return;
}

if (profile_->IsOffTheRecord()) {
return;
}
Expand Down Expand Up @@ -3910,41 +3949,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(/* should_shutdown */ true);
}

StopNotificationTimers();
notification_service_->DeleteAllNotifications(true);
std::vector<base::FilePath> 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);
}

Expand Down
14 changes: 12 additions & 2 deletions components/brave_rewards/browser/rewards_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ using ExternalWalletAuthorizationCallback =
const ledger::Result,
const std::map<std::string, std::string>&)>;

using StopLedgerCallback = base::OnceCallback<void(ledger::Result)>;

class RewardsServiceImpl : public RewardsService,
public ledger::LedgerClient,
public base::SupportsWeakPtr<RewardsServiceImpl> {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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_;

Expand Down
5 changes: 3 additions & 2 deletions components/brave_rewards/resources/page/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ const updatePromotion = (newPromotion: Rewards.Promotion, promotions: Rewards.Pr
}

const promotionReducer: Reducer<Rewards.State | undefined> = (state: Rewards.State, action) => {
if (!state) {
return
}

const payload = action.payload
switch (action.type) {
case types.FETCH_PROMOTIONS: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import { Reducer } from 'redux'
import { types } from '../constants/rewards_types'

const publishersReducer: Reducer<Rewards.State | undefined> = (state: Rewards.State, action) => {
if (!state) {
return
}

switch (action.type) {
case types.ON_CONTRIBUTE_LIST:
state = { ...state }
Expand Down
Loading