Skip to content

Commit

Permalink
Additional queue improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
cypt4 committed Apr 10, 2023
1 parent fa156a0 commit 3cea9a8
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 35 deletions.
61 changes: 29 additions & 32 deletions components/brave_wallet/browser/brave_wallet_auto_pin_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ BraveWalletAutoPinService::BraveWalletAutoPinService(
if (IsAutoPinEnabled()) {
Restore();
}

pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
pref_change_registrar_->Init(pref_service_);
pref_change_registrar_->Add(
kAutoPinEnabled,
base::BindRepeating(&BraveWalletAutoPinService::OnAutoPinStatusChanged,
weak_ptr_factory_.GetWeakPtr()));
base::Unretained(this)));
brave_wallet_service->AddObserver(
brave_wallet_service_observer_.BindNewPipeAndPassRemote());
}
Expand All @@ -52,6 +53,7 @@ void BraveWalletAutoPinService::OnResetWallet() {

void BraveWalletAutoPinService::Reset() {
weak_ptr_factory_.InvalidateWeakPtrs();

queue_.clear();
current_.reset();
SetAutoPinEnabled(false);
Expand All @@ -64,6 +66,8 @@ void BraveWalletAutoPinService::OnAutoPinStatusChanged() {
Restore();
} else {
queue_.clear();
current_.reset();
weak_ptr_factory_.InvalidateWeakPtrs();
}
for (const auto& observer : observers_) {
observer->OnAutoPinStatusChanged(enabled);
Expand Down Expand Up @@ -95,8 +99,9 @@ void BraveWalletAutoPinService::OnTokenAdded(BlockchainTokenPtr token) {
base::EraseIf(queue_, [&token](const std::unique_ptr<IntentData>& intent) {
return intent->token == token;
});
AddOrExecute(
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt));
auto intent =
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt);
AddOrExecute(std::move(intent));
}

void BraveWalletAutoPinService::OnTokenRemoved(BlockchainTokenPtr token) {
Expand All @@ -110,8 +115,9 @@ void BraveWalletAutoPinService::OnTokenRemoved(BlockchainTokenPtr token) {
base::EraseIf(queue_, [&token](const std::unique_ptr<IntentData>& intent) {
return intent->token == token;
});
AddOrExecute(
std::make_unique<IntentData>(token, Operation::kDelete, absl::nullopt));
auto intent =
std::make_unique<IntentData>(token, Operation::kDelete, absl::nullopt);
AddOrExecute(std::move(intent));
}

void BraveWalletAutoPinService::Restore() {
Expand Down Expand Up @@ -152,38 +158,30 @@ void BraveWalletAutoPinService::OnTokenListResolved(
mojom::TokenPinStatusPtr status =
brave_wallet_pin_service_->GetTokenStatus(absl::nullopt, token);

if (!status ||
status->code == mojom::TokenPinStatusCode::STATUS_NOT_PINNED) {
AddOrExecute(
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt));
} else if (status->code ==
mojom::TokenPinStatusCode::STATUS_PINNING_FAILED) {
if (ShouldRetryOnError(status->error)) {
AddOrExecute(std::make_unique<IntentData>(token, Operation::kAdd,
absl::nullopt));
}
} else if (status->code ==
mojom::TokenPinStatusCode::STATUS_PINNING_IN_PROGRESS ||
status->code ==
mojom::TokenPinStatusCode::STATUS_PINNING_PENDING) {
AddOrExecute(
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt));
} else if (status->code ==
mojom::TokenPinStatusCode::STATUS_UNPINNING_FAILED ||
status->code ==
mojom::TokenPinStatusCode::STATUS_UNPINNING_IN_PROGRESS ||
status->code ==
mojom::TokenPinStatusCode::STATUS_UNPINNING_PENDING) {
AddOrExecute(std::make_unique<IntentData>(token, Operation::kDelete,
absl::nullopt));
std::unique_ptr<IntentData> intent;
if (!status) {
intent =
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt);
} else if (status->code == mojom::TokenPinStatusCode::STATUS_PINNED) {
// Pinned tokens should be verified for entirety time to time.
// We should check that related CIDs are still pinned.
auto t1 = status->validate_time;
if ((base::Time::Now() - t1) > base::Days(1) || t1 > base::Time::Now()) {
AddOrExecute(std::make_unique<IntentData>(token, Operation::kValidate,
absl::nullopt));
intent = std::make_unique<IntentData>(token, Operation::kValidate,
absl::nullopt);
}
} else if (status->code ==
mojom::TokenPinStatusCode::STATUS_PINNING_FAILED) {
if (ShouldRetryOnError(status->error)) {
intent =
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt);
}
} else {
intent =
std::make_unique<IntentData>(token, Operation::kAdd, absl::nullopt);
}
if (intent) {
AddOrExecute(std::move(intent));
}
}

Expand Down Expand Up @@ -299,7 +297,6 @@ void BraveWalletAutoPinService::CheckQueue() {

void BraveWalletAutoPinService::OnTaskFinished(bool result,
mojom::PinErrorPtr error) {
CHECK(current_);
if (!result &&
(current_->operation != Operation::kAdd || ShouldRetryOnError(error))) {
PostRetry(std::move(current_));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <vector>

#include "base/memory/scoped_refptr.h"
#include "base/task/cancelable_task_tracker.h"
#include "base/task/sequenced_task_runner.h"
#include "brave/components/brave_wallet/browser/blockchain_registry.h"
#include "brave/components/brave_wallet/browser/brave_wallet_pin_service.h"
Expand Down Expand Up @@ -71,6 +72,7 @@ class BraveWalletAutoPinService
Operation operation;
absl::optional<std::string> service;
size_t attempt = 0;

IntentData(const BlockchainTokenPtr& token,
Operation operation,
absl::optional<std::string> service);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,11 @@ TEST_F(BraveWalletAutoPinServiceTest, PinContinue_WhenRestore) {
mojom::TokenPinStatusCode::STATUS_PINNING_IN_PROGRESS;
} else if ("0x3" == token->token_id) {
status->code = mojom::TokenPinStatusCode::STATUS_PINNING_PENDING;
} else if ("0x4" == token->token_id) {
status->code =
mojom::TokenPinStatusCode::STATUS_UNPINNING_PENDING;
} else if ("0x5" == token->token_id) {
status->code = mojom::TokenPinStatusCode::STATUS_UNPINNING_FAILED;
}
return status;
}));
Expand All @@ -342,6 +347,10 @@ TEST_F(BraveWalletAutoPinServiceTest, PinContinue_WhenRestore) {
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x2"));
result.push_back(GetErc721Token(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x3"));
result.push_back(GetErc721Token(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x4"));
result.push_back(GetErc721Token(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x5"));
std::move(callback).Run(std::move(result));
}));

Expand Down Expand Up @@ -371,6 +380,18 @@ TEST_F(BraveWalletAutoPinServiceTest, PinContinue_WhenRestore) {
"0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x3"),
testing::Eq(absl::nullopt), _))
.Times(1);
EXPECT_CALL(
*GetBraveWalletPinService(),
AddPin(TokenPathMatches("nft.local.60.0x1."
"0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x4"),
testing::Eq(absl::nullopt), _))
.Times(1);
EXPECT_CALL(
*GetBraveWalletPinService(),
AddPin(TokenPathMatches("nft.local.60.0x1."
"0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x5"),
testing::Eq(absl::nullopt), _))
.Times(1);

BraveWalletAutoPinService auto_pin_service(
GetPrefs(), GetBraveWalletService(), GetBraveWalletPinService());
Expand All @@ -385,6 +406,8 @@ TEST_F(BraveWalletAutoPinServiceTest, UnpinContinue_WhenRestore) {
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x2");
known_tokens.insert(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x3");
known_tokens.insert(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x4");

ON_CALL(*GetBraveWalletPinService(),
GetTokenStatus(testing::Eq(absl::nullopt), _))
Expand All @@ -401,6 +424,9 @@ TEST_F(BraveWalletAutoPinServiceTest, UnpinContinue_WhenRestore) {
} else if ("0x3" == token->token_id) {
status->code =
mojom::TokenPinStatusCode::STATUS_UNPINNING_PENDING;
} else if ("0x4" == token->token_id) {
status->code =
mojom::TokenPinStatusCode::STATUS_UNPINNING_PENDING;
}
return status;
}));
Expand All @@ -411,9 +437,7 @@ TEST_F(BraveWalletAutoPinServiceTest, UnpinContinue_WhenRestore) {
GetUserAssetsCallback callback) {
std::vector<mojom::BlockchainTokenPtr> result;
result.push_back(GetErc721Token(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x1"));
result.push_back(GetErc721Token(
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x2"));
"nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x4"));
std::move(callback).Run(std::move(result));
}));

Expand All @@ -425,6 +449,14 @@ TEST_F(BraveWalletAutoPinServiceTest, UnpinContinue_WhenRestore) {
std::move(callback).Run(true, nullptr);
}));

ON_CALL(*GetBraveWalletPinService(), AddPin(_, _, _))
.WillByDefault(::testing::Invoke(
[](BlockchainTokenPtr token,
const absl::optional<std::string>& service,
BraveWalletPinService::RemovePinCallback callback) {
std::move(callback).Run(true, nullptr);
}));

EXPECT_CALL(*GetBraveWalletPinService(),
RemovePin(TokenPathMatches(
"nft.local.60.0x1."
Expand All @@ -443,6 +475,12 @@ TEST_F(BraveWalletAutoPinServiceTest, UnpinContinue_WhenRestore) {
"0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x3"),
testing::Eq(absl::nullopt), _))
.Times(1);
EXPECT_CALL(*GetBraveWalletPinService(),
RemovePin(TokenPathMatches(
"nft.local.60.0x1."
"0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x4"),
testing::Eq(absl::nullopt), _))
.Times(0);

BraveWalletAutoPinService auto_pin_service(
GetPrefs(), GetBraveWalletService(), GetBraveWalletPinService());
Expand Down

0 comments on commit 3cea9a8

Please sign in to comment.