Skip to content

Commit

Permalink
Check purchased state when service is launched
Browse files Browse the repository at this point in the history
Connection state is queried when purchased state is loaded and
it's purchased user.

fix brave/brave-browser#22677
fix brave/brave-browser#22726
fix brave/brave-browser#23080
  • Loading branch information
simonhong committed May 30, 2022
1 parent 9531d22 commit deaa3aa
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
17 changes: 17 additions & 0 deletions components/brave_vpn/brave_vpn_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,18 @@ BraveVpnService::BraveVpnService(
observed_.Observe(GetBraveVPNConnectionAPI());

GetBraveVPNConnectionAPI()->set_target_vpn_entry_name(kBraveVPNEntryName);

// To get proper connection state, we need to load purchased state.
// Connection state will be checked after we confirm that this profile
// is purchased user.
// However, purchased state loading makes additional network request.
// We should not make this network request for fresh user.
// To prevent this, we load purchased state at startup only
// when profile has cached region list because region list is fetched
// and cached only when user purchased at least once.
auto* preference = prefs_->FindPreference(prefs::kBraveVPNRegionList);
if (preference && !preference->IsDefaultValue())
LoadPurchasedState();
}
#endif

Expand Down Expand Up @@ -198,6 +210,11 @@ void BraveVpnService::UpdateAndNotifyConnectionStateChange(
bool force) {
// this is a simple state machine for handling connection state
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Ignore connection state change request for non purchased user.
// This can be happened when user controls vpn via os settings.
if (!is_purchased_user())
return;

if (connection_state_ == state)
return;

Expand Down
40 changes: 39 additions & 1 deletion components/brave_vpn/brave_vpn_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,13 @@ class BraveVPNServiceTest : public testing::Test {
base::MakeRefCounted<network::TestSharedURLLoaderFactory>();
skus_service_ = std::make_unique<skus::SkusServiceImpl>(&pref_service_,
url_loader_factory);
ResetVpnService();
}

void ResetVpnService() {
service_ = std::make_unique<BraveVpnService>(
url_loader_factory, &pref_service_,
base::MakeRefCounted<network::TestSharedURLLoaderFactory>(),
&pref_service_,
base::BindRepeating(&BraveVPNServiceTest::GetSkusService,
base::Unretained(this)));
}
Expand Down Expand Up @@ -82,6 +87,10 @@ class BraveVPNServiceTest : public testing::Test {
service_->OnCredentialSummary(summary);
}

void UpdateAndNotifyConnectionStateChange(mojom::ConnectionState state) {
service_->UpdateAndNotifyConnectionStateChange(state);
}

std::vector<mojom::Region>& regions() const { return service_->regions_; }

mojom::Region device_region() const {
Expand Down Expand Up @@ -419,6 +428,9 @@ TEST_F(BraveVPNServiceTest, LoadPurchasedStateTest) {
}

TEST_F(BraveVPNServiceTest, CancelConnectingTest) {
// Connection state can be changed with purchased.
purchased_state() = PurchasedState::PURCHASED;

cancel_connecting() = true;
connection_state() = ConnectionState::CONNECTING;
OnCreated();
Expand Down Expand Up @@ -465,6 +477,29 @@ TEST_F(BraveVPNServiceTest, CancelConnectingTest) {
EXPECT_EQ(ConnectionState::DISCONNECTED, connection_state());
}

TEST_F(BraveVPNServiceTest, ConnectionStateUpdateWithPurchasedStateTest) {
purchased_state() = PurchasedState::PURCHASED;
connection_state() = ConnectionState::CONNECTING;
UpdateAndNotifyConnectionStateChange(ConnectionState::CONNECTED);
EXPECT_EQ(ConnectionState::CONNECTED, connection_state());

purchased_state() = PurchasedState::NOT_PURCHASED;
connection_state() = ConnectionState::CONNECTING;
UpdateAndNotifyConnectionStateChange(ConnectionState::CONNECTED);
EXPECT_NE(ConnectionState::CONNECTED, connection_state());
}

TEST_F(BraveVPNServiceTest, CheckInitialPurchasedStateTest) {
// Purchased state is not checked for fresh user.
EXPECT_EQ(PurchasedState::NOT_PURCHASED, purchased_state());

// Dirty region list prefs to pretend it's already cached.
pref_service_.Set(prefs::kBraveVPNRegionList,
base::Value(base::Value::Type::LIST));
ResetVpnService();
EXPECT_EQ(PurchasedState::LOADING, purchased_state());
}

TEST_F(BraveVPNServiceTest, ConnectionInfoTest) {
// Having skus_credential is pre-requisite before try connecting.
skus_credential() = "test_credentials";
Expand All @@ -487,6 +522,9 @@ TEST_F(BraveVPNServiceTest, ConnectionInfoTest) {
}

TEST_F(BraveVPNServiceTest, NeedsConnectTest) {
// Connection state can be changed with purchased.
purchased_state() = PurchasedState::PURCHASED;

// Check ignore Connect() request while connecting or disconnecting is
// in-progress.
SetDeviceRegion("eu-es");
Expand Down

0 comments on commit deaa3aa

Please sign in to comment.