Skip to content

Commit

Permalink
Fixed selected region is not updated when it's updated from other panels
Browse files Browse the repository at this point in the history
  • Loading branch information
simonhong committed Oct 20, 2022
1 parent 8dbfe02 commit b551a1c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 3 deletions.
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 @@ -76,6 +76,13 @@ BraveVpnService::BraveVpnService(
if (preference && !preference->IsDefaultValue()) {
ReloadPurchasedState();
}

pref_change_registrar_.Init(local_prefs_);
pref_change_registrar_.Add(
prefs::kBraveVPNSelectedRegion,
base::BindRepeating(&BraveVpnService::OnPreferenceChanged,
base::Unretained(this)));

#endif // !BUILDFLAG(IS_ANDROID)

InitP3A();
Expand Down Expand Up @@ -472,6 +479,16 @@ void BraveVpnService::OnCreateSupportTicket(
<< "\nresponse_code=" << api_request_result.response_code();
std::move(callback).Run(success, api_request_result.body());
}

void BraveVpnService::OnPreferenceChanged(const std::string& pref_name) {
if (pref_name == prefs::kBraveVPNSelectedRegion) {
for (const auto& obs : observers_) {
obs->OnSelectedRegionChanged(
GetRegionPtrWithNameFromRegionList(GetSelectedRegion(), regions_));
}
return;
}
}
#endif // !BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_ANDROID)
Expand Down
5 changes: 5 additions & 0 deletions components/brave_vpn/brave_vpn_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "brave/components/skus/common/skus_sdk.mojom.h"
#include "build/build_config.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_change_registrar.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote_set.h"
Expand Down Expand Up @@ -195,6 +196,8 @@ class BraveVpnService :
void OnCreateSupportTicket(CreateSupportTicketCallback callback,
APIRequestResult api_request_result);

void OnPreferenceChanged(const std::string& pref_name);

BraveVPNOSConnectionAPI* GetBraveVPNConnectionAPI() const;
#endif // !BUILDFLAG(IS_ANDROID)

Expand Down Expand Up @@ -236,6 +239,8 @@ class BraveVpnService :
// Only for testing.
std::string test_timezone_;
bool is_simulation_ = false;

PrefChangeRegistrar pref_change_registrar_;
#endif // !BUILDFLAG(IS_ANDROID)

SEQUENCE_CHECKER(sequence_checker_);
Expand Down
1 change: 1 addition & 0 deletions components/brave_vpn/brave_vpn_service_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class BraveVPNServiceObserver : public mojom::ServiceObserver {
void OnPurchasedStateChanged(mojom::PurchasedState state) override {}
#if !BUILDFLAG(IS_ANDROID)
void OnConnectionStateChanged(mojom::ConnectionState state) override {}
void OnSelectedRegionChanged(mojom::RegionPtr region) override {}
#endif // !BUILDFLAG(IS_ANDROID)

private:
Expand Down
25 changes: 25 additions & 0 deletions components/brave_vpn/brave_vpn_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,22 @@ class TestBraveVPNServiceObserver : public mojom::ServiceObserver {
if (connection_state_callback_)
std::move(connection_state_callback_).Run();
}
void OnSelectedRegionChanged(mojom::RegionPtr region) override {
if (selected_region_callback_)
std::move(selected_region_callback_).Run();
}
#endif
void WaitPurchasedStateChange(base::OnceClosure callback) {
purchased_callback_ = std::move(callback);
}
#if !BUILDFLAG(IS_ANDROID)
void WaitConnectionStateChange(base::OnceClosure callback) {
connection_state_callback_ = std::move(callback);
}
void WaitSelectedRegionStateChange(base::OnceClosure callback) {
selected_region_callback_ = std::move(callback);
}
#endif
mojo::PendingRemote<mojom::ServiceObserver> GetReceiver() {
return observer_receiver_.BindNewPipeAndPassRemote();
}
Expand All @@ -165,6 +174,7 @@ class TestBraveVPNServiceObserver : public mojom::ServiceObserver {
absl::optional<ConnectionState> connection_state_;
base::OnceClosure purchased_callback_;
base::OnceClosure connection_state_callback_;
base::OnceClosure selected_region_callback_;
mojo::Receiver<mojom::ServiceObserver> observer_receiver_{this};
};

Expand Down Expand Up @@ -275,6 +285,10 @@ class BraveVPNServiceTest : public testing::Test {
void Suspend() { GetBraveVPNConnectionAPI()->OnSuspend(); }
void OnDNSChanged() { GetBraveVPNConnectionAPI()->OnDNSChanged(); }

void SetSelectedRegion(const std::string& region) {
service_->SetSelectedRegion(region);
}

void OnFetchRegionList(bool background_fetch,
const std::string& region_list,
bool success) {
Expand Down Expand Up @@ -748,6 +762,17 @@ TEST_F(BraveVPNServiceTest, ConnectionStateUpdateWithPurchasedStateTest) {
EXPECT_EQ(ConnectionState::CONNECTED, observer.GetConnectionState());
}

TEST_F(BraveVPNServiceTest, SelectedRegionChangedUpdateTest) {
TestBraveVPNServiceObserver observer;
AddObserver(observer.GetReceiver());

OnFetchRegionList(false, GetRegionsData(), true);
SetSelectedRegion("asia-sg");
base::RunLoop loop;
observer.WaitSelectedRegionStateChange(loop.QuitClosure());
loop.Run();
}

TEST_F(BraveVPNServiceTest, ConnectionInfoTest) {
// Having skus_credential is pre-requisite before try connecting.
SetSkusCredential("test_credentials");
Expand Down
3 changes: 3 additions & 0 deletions components/brave_vpn/mojom/brave_vpn.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ interface ServiceObserver {
[EnableIfNot=is_android]
OnConnectionStateChanged(ConnectionState state);

[EnableIfNot=is_android]
OnSelectedRegionChanged(Region region);

OnPurchasedStateChanged(PurchasedState state);
};

Expand Down
5 changes: 5 additions & 0 deletions components/brave_vpn/resources/panel/state/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export type initializedPayload = {
productUrls: ProductUrls
}

export type selectedRegionPayload = {
region: Region
}

export const connect = createAction('connect')
export const disconnect = createAction('disconnect')
export const connectionFailed = createAction('connectionFailed')
Expand All @@ -45,3 +49,4 @@ export const showMainView = createAction<showMainViewPayload>('showMainView')
export const toggleRegionSelector = createAction<ToggleRegionSelectorPayload>('toggleRegionSelector', (isSelectingRegion) => ({ isSelectingRegion }))
export const connectionStateChanged = createAction<ConnectionStatePayload>('connectionStateChanged')
export const connectToNewRegion = createAction<ConnectToNewRegionPayload>('connectToNewRegion', (region) => ({ region }))
export const selectedRegionChanged = createAction<selectedRegionPayload>('selectedRegionChanged')
7 changes: 7 additions & 0 deletions components/brave_vpn/resources/panel/state/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ reducer.on(Actions.connectionStateChanged, (state, payload): RootState => {
}
})

reducer.on(Actions.selectedRegionChanged, (state, payload): RootState => {
return {
...state,
currentRegion: payload.region
}
})

reducer.on(Actions.retryConnect, (state): RootState => {
return {
...state,
Expand Down
8 changes: 5 additions & 3 deletions components/brave_vpn/resources/panel/state/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { createStore, applyMiddleware } from 'redux'
import reducer from './reducer'
import asyncHandler from './async'
import * as Actions from './actions'
import getPanelBrowserAPI, { ServiceObserverReceiver, ConnectionState, PurchasedState } from '../api/panel_browser_api'
import getPanelBrowserAPI, { ServiceObserverReceiver, ConnectionState, PurchasedState, Region } from '../api/panel_browser_api'

const store = createStore(
reducer,
Expand All @@ -17,11 +17,13 @@ const store = createStore(

// Register the observer earlier
const observer = {
onConnectionCreated: () => { /**/ },
onConnectionRemoved: () => { /**/ },
onConnectionStateChanged: (connectionStatus: ConnectionState) => {
store.dispatch(Actions.connectionStateChanged({ connectionStatus }))
},
onSelectedRegionChanged: (region: Region) => {
store.dispatch(Actions.selectedRegionChanged({ region }))
},

onPurchasedStateChanged: (state: PurchasedState) => {
switch (state) {
case PurchasedState.PURCHASED:
Expand Down

0 comments on commit b551a1c

Please sign in to comment.