diff --git a/components/brave_vpn/brave_vpn_service_desktop.cc b/components/brave_vpn/brave_vpn_service_desktop.cc index 8268b3530b24..156e50d755c2 100644 --- a/components/brave_vpn/brave_vpn_service_desktop.cc +++ b/components/brave_vpn/brave_vpn_service_desktop.cc @@ -29,7 +29,7 @@ constexpr char kBraveVPNEntryName[] = "BraveVPN"; constexpr char kRegionContinentKey[] = "continent"; constexpr char kRegionNameKey[] = "name"; -constexpr char kRegionNamePrettyKey[] = "name_pretty"; +constexpr char kRegionNamePrettyKey[] = "name-pretty"; bool GetVPNCredentialsFromSwitch(brave_vpn::BraveVPNConnectionInfo* info) { DCHECK(info); @@ -70,8 +70,14 @@ BraveVpnServiceDesktop::BraveVpnServiceDesktop( GetBraveVPNConnectionAPI()->set_target_vpn_entry_name(kBraveVPNEntryName); GetBraveVPNConnectionAPI()->CheckConnection(kBraveVPNEntryName); + LoadCachedRegionData(); FetchRegionData(); CheckPurchasedStatus(); + + constexpr int kRegionDataUpdateIntervalInHours = 5; + region_data_update_timer_.Start( + FROM_HERE, base::TimeDelta::FromHours(kRegionDataUpdateIntervalInHours), + this, &BraveVpnServiceDesktop::FetchRegionData); } BraveVpnServiceDesktop::~BraveVpnServiceDesktop() = default; @@ -221,57 +227,77 @@ void BraveVpnServiceDesktop::FetchRegionData() { base::Unretained(this))); } +void BraveVpnServiceDesktop::LoadCachedRegionData() { + auto* preference = + prefs_->FindPreference(brave_vpn::prefs::kBraveVPNRegionList); + if (preference && !preference->IsDefaultValue()) + ParseAndCacheRegionList(preference->GetValue()->Clone()); + + preference = prefs_->FindPreference(brave_vpn::prefs::kBraveVPNDeviceRegion); + if (preference && !preference->IsDefaultValue()) { + auto* region_value = preference->GetValue(); + const std::string* continent = + region_value->FindStringKey(kRegionContinentKey); + const std::string* name = region_value->FindStringKey(kRegionNameKey); + const std::string* name_pretty = + region_value->FindStringKey(kRegionNamePrettyKey); + if (continent && name && name_pretty) { + device_region_.continent = *continent; + device_region_.name = *name; + device_region_.name_pretty = *name_pretty; + } + } +} + void BraveVpnServiceDesktop::OnFetchRegionList(const std::string& region_list, bool success) { if (!success) { // TODO(simonhong): Re-try? + VLOG(2) << "Failed to get region list"; return; } - std::string json_string = "{\"response\": " + region_list + "}"; - absl::optional value = base::JSONReader::Read(json_string); - if (value && value->is_dict()) { - ParseAndCacheRegionList(std::move(*value)); - return; + absl::optional value = base::JSONReader::Read(region_list); + if (value && value->is_list()) { + prefs_->Set(brave_vpn::prefs::kBraveVPNRegionList, *value); + + if (ParseAndCacheRegionList(std::move(*value))) { + // Fetch timezones list to determine default region of this device. + GetTimezonesForRegions(base::BindOnce( + &BraveVpnServiceDesktop::OnFetchTimezones, base::Unretained(this))); + return; + } } // TODO(simonhong): Re-try? } -void BraveVpnServiceDesktop::ParseAndCacheRegionList(base::Value region_value) { - base::Value* region_list_value = region_value.FindKey("response"); - DCHECK(region_list_value && region_list_value->is_list()); - - if (!region_list_value || !region_list_value->is_list()) - return; +bool BraveVpnServiceDesktop::ParseAndCacheRegionList(base::Value region_value) { + DCHECK(region_value.is_list()); + if (!region_value.is_list()) + return false; regions_.clear(); - for (const auto& value : region_list_value->GetList()) { + for (const auto& value : region_value.GetList()) { DCHECK(value.is_dict()); if (!value.is_dict()) continue; brave_vpn::mojom::Region region; - if (auto* continent = value.FindStringKey("continent")) + if (auto* continent = value.FindStringKey(kRegionContinentKey)) region.continent = *continent; - if (auto* name = value.FindStringKey("name")) + if (auto* name = value.FindStringKey(kRegionNameKey)) region.name = *name; - if (auto* name_pretty = value.FindStringKey("name-pretty")) + if (auto* name_pretty = value.FindStringKey(kRegionNamePrettyKey)) region.name_pretty = *name_pretty; regions_.push_back(region); } - // If we can't get region list, we can't determine device region. if (regions_.empty()) - return; - - // Make sure device_region_ is not set twice. - DCHECK(device_region_.name.empty()); + return false; - // Fetch timezones list to determine default region of this device. - GetTimezonesForRegions(base::BindOnce( - &BraveVpnServiceDesktop::OnFetchTimezones, base::Unretained(this))); + return true; } void BraveVpnServiceDesktop::OnFetchTimezones(const std::string& timezones_list, @@ -283,10 +309,9 @@ void BraveVpnServiceDesktop::OnFetchTimezones(const std::string& timezones_list, return; } - std::string json_string = "{\"response\": " + timezones_list + "}"; - absl::optional value = base::JSONReader::Read(json_string); - if (value && value->is_dict()) { - ParseAndCacheDefaultRegionName(std::move(*value)); + absl::optional value = base::JSONReader::Read(timezones_list); + if (value && value->is_list()) { + ParseAndCacheDeviceRegionName(std::move(*value)); return; } @@ -294,12 +319,11 @@ void BraveVpnServiceDesktop::OnFetchTimezones(const std::string& timezones_list, SetFallbackDeviceRegion(); } -void BraveVpnServiceDesktop::ParseAndCacheDefaultRegionName( +void BraveVpnServiceDesktop::ParseAndCacheDeviceRegionName( base::Value timezones_value) { - base::Value* timezones_list_value = timezones_value.FindKey("response"); - DCHECK(timezones_list_value && timezones_list_value->is_list()); + DCHECK(timezones_value.is_list()); - if (!timezones_list_value || !timezones_list_value->is_list()) { + if (!timezones_value.is_list()) { SetFallbackDeviceRegion(); return; } @@ -310,7 +334,7 @@ void BraveVpnServiceDesktop::ParseAndCacheDefaultRegionName( return; } - for (const auto& timezones : timezones_list_value->GetList()) { + for (const auto& timezones : timezones_value.GetList()) { DCHECK(timezones.is_dict()); if (!timezones.is_dict()) continue; @@ -339,13 +363,11 @@ void BraveVpnServiceDesktop::ParseAndCacheDefaultRegionName( } void BraveVpnServiceDesktop::SetDeviceRegion(const std::string& name) { - DCHECK(device_region_.name.empty()); - auto it = std::find_if(regions_.begin(), regions_.end(), [&name](const auto& region) { return region.name == name; }); if (it != regions_.end()) { - device_region_ = *it; + SetDeviceRegion(*it); } } @@ -355,7 +377,18 @@ void BraveVpnServiceDesktop::SetFallbackDeviceRegion() { if (regions_.empty()) return; - device_region_ = regions_[0]; + SetDeviceRegion(regions_[0]); +} + +void BraveVpnServiceDesktop::SetDeviceRegion( + const brave_vpn::mojom::Region& region) { + device_region_ = region; + + DictionaryPrefUpdate update(prefs_, brave_vpn::prefs::kBraveVPNDeviceRegion); + base::Value* dict = update.Get(); + dict->SetStringKey(kRegionContinentKey, device_region_.continent); + dict->SetStringKey(kRegionNameKey, device_region_.name); + dict->SetStringKey(kRegionNamePrettyKey, device_region_.name_pretty); } std::string BraveVpnServiceDesktop::GetCurrentTimeZone() { diff --git a/components/brave_vpn/brave_vpn_service_desktop.h b/components/brave_vpn/brave_vpn_service_desktop.h index 94b3265a81d1..fbbdfcf53d74 100644 --- a/components/brave_vpn/brave_vpn_service_desktop.h +++ b/components/brave_vpn/brave_vpn_service_desktop.h @@ -11,6 +11,7 @@ #include "base/containers/flat_map.h" #include "base/scoped_observation.h" +#include "base/timer/timer.h" #include "brave/components/brave_vpn/brave_vpn.mojom.h" #include "brave/components/brave_vpn/brave_vpn_connection_info.h" #include "brave/components/brave_vpn/brave_vpn_data_types.h" @@ -78,6 +79,7 @@ class BraveVpnServiceDesktop friend class BraveBrowserCommandControllerTest; FRIEND_TEST_ALL_PREFIXES(BraveVPNTest, RegionDataTest); FRIEND_TEST_ALL_PREFIXES(BraveVPNTest, HostnamesTest); + FRIEND_TEST_ALL_PREFIXES(BraveVPNTest, LoadRegionDataFromPrefsTest); // BraveVpnService overrides: void Shutdown() override; @@ -92,11 +94,12 @@ class BraveVpnServiceDesktop void OnIsDisconnecting(const std::string& name) override; brave_vpn::BraveVPNConnectionInfo GetConnectionInfo(); + void LoadCachedRegionData(); void FetchRegionData(); void OnFetchRegionList(const std::string& region_list, bool success); - void ParseAndCacheRegionList(base::Value region_value); + bool ParseAndCacheRegionList(base::Value region_value); void OnFetchTimezones(const std::string& timezones_list, bool success); - void ParseAndCacheDefaultRegionName(base::Value timezons_value); + void ParseAndCacheDeviceRegionName(base::Value timezons_value); void FetchHostnamesForRegion(const std::string& name); void OnFetchHostnames(const std::string& region, const std::string& hostnames, @@ -105,6 +108,8 @@ class BraveVpnServiceDesktop base::Value hostnames_value); void SetDeviceRegion(const std::string& name); void SetFallbackDeviceRegion(); + void SetDeviceRegion(const brave_vpn::mojom::Region& region); + std::string GetCurrentTimeZone(); void SetPurchasedState(PurchasedState state); @@ -123,6 +128,7 @@ class BraveVpnServiceDesktop observed_{this}; mojo::ReceiverSet receivers_; mojo::RemoteSet observers_; + base::RepeatingTimer region_data_update_timer_; std::string test_timezone_; }; diff --git a/components/brave_vpn/brave_vpn_unittest.cc b/components/brave_vpn/brave_vpn_unittest.cc index 7ec3e8b123db..83b8f337c601 100644 --- a/components/brave_vpn/brave_vpn_unittest.cc +++ b/components/brave_vpn/brave_vpn_unittest.cc @@ -11,6 +11,7 @@ #include "brave/components/brave_vpn/brave_vpn_service_desktop.h" #include "brave/components/brave_vpn/brave_vpn_utils.h" #include "brave/components/brave_vpn/features.h" +#include "brave/components/brave_vpn/pref_names.h" #include "components/prefs/testing_pref_service.h" #include "content/public/test/browser_task_environment.h" #include "services/network/test/test_shared_url_loader_factory.h" @@ -23,6 +24,7 @@ class BraveVPNTest : public testing::Test { } void SetUp() override { + brave_vpn::prefs::RegisterProfilePrefs(pref_service_.registry()); service_ = std::make_unique( base::MakeRefCounted(), &pref_service_); @@ -223,3 +225,29 @@ TEST_F(BraveVPNTest, HostnamesTest) { service_->OnFetchHostnames("invalid-region-b", "", false); EXPECT_EQ(0UL, service_->hostnames_["invalid-region-b"].size()); } + +TEST_F(BraveVPNTest, LoadRegionDataFromPrefsTest) { + // Initially, prefs doesn't have region data. + EXPECT_EQ(brave_vpn::mojom::Region(), service_->device_region_); + EXPECT_TRUE(service_->regions_.empty()); + + // Set proper data to store them in prefs. + service_->OnFetchRegionList(GetRegionsData(), true); + service_->set_test_timezone("Asia/Seoul"); + service_->OnFetchTimezones(GetTimeZonesData(), true); + + // Check region data is set with above data. + EXPECT_FALSE(brave_vpn::mojom::Region() == service_->device_region_); + EXPECT_FALSE(service_->regions_.empty()); + + // Clear region data. + service_->device_region_ = brave_vpn::mojom::Region(); + service_->regions_.clear(); + EXPECT_EQ(brave_vpn::mojom::Region(), service_->device_region_); + EXPECT_TRUE(service_->regions_.empty()); + + // Check region data is loaded from prefs. + service_->LoadCachedRegionData(); + EXPECT_FALSE(brave_vpn::mojom::Region() == service_->device_region_); + EXPECT_FALSE(service_->regions_.empty()); +} diff --git a/components/brave_vpn/pref_names.cc b/components/brave_vpn/pref_names.cc index 562de9ae2df7..c25418a45187 100644 --- a/components/brave_vpn/pref_names.cc +++ b/components/brave_vpn/pref_names.cc @@ -13,6 +13,8 @@ namespace prefs { void RegisterProfilePrefs(PrefRegistrySimple* registry) { registry->RegisterBooleanPref(kBraveVPNShowButton, true); registry->RegisterDictionaryPref(kBraveVPNSelectedRegion); + registry->RegisterListPref(kBraveVPNRegionList); + registry->RegisterDictionaryPref(kBraveVPNDeviceRegion); } } // namespace prefs diff --git a/components/brave_vpn/pref_names.h b/components/brave_vpn/pref_names.h index ce5b12245619..6a5437c9d5c8 100644 --- a/components/brave_vpn/pref_names.h +++ b/components/brave_vpn/pref_names.h @@ -13,6 +13,8 @@ namespace prefs { constexpr char kBraveVPNSelectedRegion[] = "brave.brave_vpn.selected_region"; constexpr char kBraveVPNShowButton[] = "brave.brave_vpn.show_button"; +constexpr char kBraveVPNRegionList[] = "brave.brave_vpn.region_list"; +constexpr char kBraveVPNDeviceRegion[] = "brave.brave_vpn.device_region"; void RegisterProfilePrefs(PrefRegistrySimple* registry);