Cached selected vpn region in prefs

fix https://github.com/brave/brave-browser/issues/18305

Selected VPN region data is stored as dict in prefs.
Moved existing kBraveVPNShowButton pref name to brave_vpn/pref_names to
manage vpn related prefs in one place.

browser/brave_prefs_browsertest.cc

@@ -44,6 +44,10 @@
 #include "brave/components/brave_wayback_machine/pref_names.h"
 #endif
 
+#if BUILDFLAG(ENABLE_BRAVE_VPN)
+#include "brave/components/brave_vpn/pref_names.h"
+#endif
+
 #if defined(OS_ANDROID)
 #include "chrome/test/base/android/android_browser_test.h"
 #else
@@ -127,7 +131,7 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, MiscBravePrefs) {
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
   EXPECT_TRUE(chrome_test_utils::GetProfile(this)->GetPrefs()->GetBoolean(
-      kBraveVPNShowButton));
+      brave_vpn::prefs::kBraveVPNShowButton));
 #endif
 }
 

browser/brave_profile_prefs.cc

@@ -129,6 +129,10 @@
 using extensions::FeatureSwitch;
 #endif
 
+#if BUILDFLAG(ENABLE_BRAVE_VPN)
+#include "brave/components/brave_vpn/pref_names.h"
+#endif
+
 namespace brave {
 
 void RegisterProfilePrefsForMigration(
@@ -174,7 +178,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
   brave_sync::Prefs::RegisterProfilePrefs(registry);
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
-  registry->RegisterBooleanPref(kBraveVPNShowButton, true);
+  brave_vpn::prefs::RegisterProfilePrefs(registry);
 #endif
 
   // TODO(shong): Migrate this to local state also and guard in ENABLE_WIDEVINE.

browser/brave_vpn/brave_vpn_service_factory.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/storage_partition.h"
 
@@ -55,7 +56,8 @@ KeyedService* BraveVpnServiceFactory::BuildServiceInstanceFor(
       default_storage_partition->GetURLLoaderFactoryForBrowserProcess();
 
 #if defined(OS_WIN) || defined(OS_MAC)
-  return new BraveVpnServiceDesktop(shared_url_loader_factory);
+  return new BraveVpnServiceDesktop(shared_url_loader_factory,
+                                    user_prefs::UserPrefs::Get(context));
 #endif
 
 #if defined(OS_ANDROID)

browser/brave_vpn/sources.gni

@@ -19,6 +19,7 @@ if (enable_brave_vpn) {
     "//brave/components/brave_vpn",
     "//chrome/browser/profiles:profile",
     "//components/keyed_service/content",
+    "//components/user_prefs",
     "//content/public/browser",
   ]
 

browser/extensions/api/settings_private/brave_prefs_util.cc

@@ -69,6 +69,10 @@
 #include "brave/components/ftx/common/pref_names.h"
 #endif
 
+#if BUILDFLAG(ENABLE_BRAVE_VPN)
+#include "brave/components/brave_vpn/pref_names.h"
+#endif
+
 namespace extensions {
 
 using ntp_background_images::prefs::kNewTabPageShowBackgroundImage;
@@ -135,7 +139,7 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() {
   (*s_brave_allowlist)[kTabsSearchShow] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
-  (*s_brave_allowlist)[kBraveVPNShowButton] =
+  (*s_brave_allowlist)[brave_vpn::prefs::kBraveVPNShowButton] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
 #endif
 #if BUILDFLAG(ENABLE_SIDEBAR)

browser/sources.gni

@@ -30,6 +30,7 @@ import("//brave/browser/themes/sources.gni")
 import("//brave/chromium_src/chrome/browser/prefs/sources.gni")
 import("//brave/chromium_src/chrome/browser/sources.gni")
 import("//brave/components/brave_referrals/buildflags/buildflags.gni")
+import("//brave/components/brave_vpn/buildflags/buildflags.gni")
 import("//brave/components/brave_wallet/common/buildflags/buildflags.gni")
 import("//brave/components/brave_wayback_machine/buildflags/buildflags.gni")
 import("//brave/components/brave_webtorrent/browser/buildflags/buildflags.gni")
@@ -125,6 +126,7 @@ brave_chrome_browser_deps = [
   "//brave/components/brave_sync:network_time_helper",
   "//brave/components/brave_sync:prefs",
   "//brave/components/brave_talk",
+  "//brave/components/brave_vpn/buildflags",
   "//brave/components/brave_wallet/common/buildflags",
   "//brave/components/brave_wayback_machine:buildflags",
   "//brave/components/brave_webtorrent/browser/buildflags",
@@ -215,6 +217,10 @@ if (brave_wallet_enabled) {
   ]
 }
 
+if (enable_brave_vpn) {
+  brave_chrome_browser_deps += [ "//brave/components/brave_vpn" ]
+}
+
 if (ethereum_remote_client_enabled) {
   brave_chrome_browser_deps +=
       [ "//brave/browser/ethereum_remote_client:browser" ]

browser/ui/browser_commands.cc

@@ -40,6 +40,10 @@
 #include "brave/components/tor/tor_profile_service.h"
 #endif
 
+#if BUILDFLAG(ENABLE_BRAVE_VPN)
+#include "brave/components/brave_vpn/pref_names.h"
+#endif
+
 using content::WebContents;
 
 namespace {
@@ -121,8 +125,8 @@ void ShowBraveVPNBubble(Browser* browser) {
 void ToggleBraveVPNButton(Browser* browser) {
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
   auto* prefs = browser->profile()->GetPrefs();
-  const bool show = prefs->GetBoolean(kBraveVPNShowButton);
-  prefs->SetBoolean(kBraveVPNShowButton, !show);
+  const bool show = prefs->GetBoolean(brave_vpn::prefs::kBraveVPNShowButton);
+  prefs->SetBoolean(brave_vpn::prefs::kBraveVPNShowButton, !show);
 #endif
 }
 

browser/ui/toolbar/brave_vpn_menu_model.cc

@@ -6,7 +6,7 @@
 #include "brave/browser/ui/toolbar/brave_vpn_menu_model.h"
 
 #include "brave/app/brave_command_ids.h"
-#include "brave/common/pref_names.h"
+#include "brave/components/brave_vpn/pref_names.h"
 #include "brave/grit/brave_generated_resources.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -41,5 +41,5 @@ void BraveVPNMenuModel::ExecuteCommand(int command_id, int event_flags) {
 
 bool BraveVPNMenuModel::IsBraveVPNButtonVisible() const {
   auto* prefs = browser_->profile()->GetPrefs();
-  return prefs->GetBoolean(kBraveVPNShowButton);
+  return prefs->GetBoolean(brave_vpn::prefs::kBraveVPNShowButton);
 }

browser/ui/views/toolbar/brave_toolbar_view.cc

@@ -37,6 +37,7 @@
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
 #include "brave/browser/ui/views/toolbar/brave_vpn_button.h"
 #include "brave/components/brave_vpn/brave_vpn_utils.h"
+#include "brave/components/brave_vpn/pref_names.h"
 #endif
 
 namespace {
@@ -171,7 +172,7 @@ void BraveToolbarView::Init() {
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
   if (brave_vpn::IsBraveVPNEnabled()) {
     show_brave_vpn_button_.Init(
-        kBraveVPNShowButton, profile->GetPrefs(),
+        brave_vpn::prefs::kBraveVPNShowButton, profile->GetPrefs(),
         base::BindRepeating(&BraveToolbarView::OnVPNButtonVisibilityChanged,
                             base::Unretained(this)));
     brave_vpn_ = AddChildViewAt(std::make_unique<BraveVPNButton>(browser()),

browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc

@@ -34,6 +34,7 @@
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
 #include "brave/browser/ui/views/toolbar/brave_vpn_button.h"
 #include "brave/components/brave_vpn/features.h"
+#include "brave/components/brave_vpn/pref_names.h"
 #endif
 
 // An observer that returns back to test code after a new profile is
@@ -89,13 +90,13 @@ IN_PROC_BROWSER_TEST_F(BraveToolbarViewTest, VPNButtonVisibility) {
   auto* prefs = browser()->profile()->GetPrefs();
 
   // Button is visible by default.
-  EXPECT_TRUE(prefs->GetBoolean(kBraveVPNShowButton));
+  EXPECT_TRUE(prefs->GetBoolean(brave_vpn::prefs::kBraveVPNShowButton));
   EXPECT_TRUE(toolbar->brave_vpn_button()->GetVisible());
   EXPECT_EQ(browser_view->GetAnchorViewForBraveVPNPanel(),
             toolbar->brave_vpn_button());
 
   // Hide button.
-  prefs->SetBoolean(kBraveVPNShowButton, false);
+  prefs->SetBoolean(brave_vpn::prefs::kBraveVPNShowButton, false);
   EXPECT_FALSE(toolbar->brave_vpn_button()->GetVisible());
   EXPECT_EQ(browser_view->GetAnchorViewForBraveVPNPanel(),
             static_cast<views::View*>(toolbar->app_menu_button()));

common/BUILD.gn

@@ -68,7 +68,6 @@ source_set("pref_names") {
   ]
 
   deps = [
-    "//brave/components/brave_vpn/buildflags",
     "//components/gcm_driver:gcm_buildflags",
     "//extensions/buildflags:buildflags",
   ]

common/pref_names.cc

@@ -101,10 +101,6 @@ const char kMRUCyclingEnabled[] = "brave.mru_cycling_enabled";
 const char kTabsSearchShow[] = "brave.tabs_search_show";
 const char kDontAskForCrashReporting[] = "brave.dont_ask_for_crash_reporting";
 
-#if BUILDFLAG(ENABLE_BRAVE_VPN)
-const char kBraveVPNShowButton[] = "brave.brave_vpn.show_button";
-#endif
-
 #if defined(OS_ANDROID)
 const char kDesktopModeEnabled[] = "brave.desktop_mode_enabled";
 const char kPlayYTVideoInBrowserEnabled[] =

common/pref_names.h

@@ -6,7 +6,6 @@
 #ifndef BRAVE_COMMON_PREF_NAMES_H_
 #define BRAVE_COMMON_PREF_NAMES_H_
 
-#include "brave/components/brave_vpn/buildflags/buildflags.h"
 #include "build/build_config.h"
 #include "components/gcm_driver/gcm_buildflags.h"
 #include "extensions/buildflags/buildflags.h"
@@ -100,8 +99,4 @@ extern const char kDefaultBrowserLaunchingCount[];
 extern const char kTabsSearchShow[];
 extern const char kDontAskForCrashReporting[];
 
-#if BUILDFLAG(ENABLE_BRAVE_VPN)
-extern const char kBraveVPNShowButton[];
-#endif
-
 #endif  // BRAVE_COMMON_PREF_NAMES_H_

components/brave_vpn/BUILD.gn

@@ -20,12 +20,15 @@ static_library("brave_vpn") {
     "brave_vpn_utils.h",
     "features.cc",
     "features.h",
+    "pref_names.cc",
+    "pref_names.h",
   ]
 
   deps = [
     "//base",
     "//brave/components/api_request_helper:api_request_helper",
     "//components/keyed_service/core",
+    "//components/prefs",
     "//services/network/public/cpp",
     "//third_party/abseil-cpp:absl",
     "//url",
@@ -104,6 +107,7 @@ source_set("unit_tests") {
     deps = [
       ":brave_vpn",
       "//base",
+      "//components/prefs:test_support",
       "//content/test:test_support",
       "//services/network:test_support",
       "//testing/gtest",

components/brave_vpn/brave_vpn.mojom

@@ -43,6 +43,8 @@ interface ServiceHandler {
   Disconnect();
   GetAllRegions() => (array<Region> regions);
   GetDeviceRegion() => (Region device_region);
+  GetSelectedRegion() => (Region current_region);
+  SetSelectedRegion(Region region);
 };
 
 // WebUI-side handler for requests from the browser.

components/brave_vpn/brave_vpn_service_desktop.cc

@@ -16,13 +16,20 @@
 #include "base/notreached.h"
 #include "base/strings/string_split.h"
 #include "base/values.h"
+#include "brave/components/brave_vpn/pref_names.h"
 #include "brave/components/brave_vpn/switches.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
 #include "third_party/icu/source/i18n/unicode/timezone.h"
 
 namespace {
 
 constexpr char kBraveVPNEntryName[] = "BraveVPN";
 
+constexpr char kRegionContinentKey[] = "continent";
+constexpr char kRegionNameKey[] = "name";
+constexpr char kRegionNamePrettyKey[] = "name_pretty";
+
 bool GetVPNCredentialsFromSwitch(brave_vpn::BraveVPNConnectionInfo* info) {
   DCHECK(info);
   auto* cmd = base::CommandLine::ForCurrentProcess();
@@ -52,8 +59,9 @@ brave_vpn::BraveVPNOSConnectionAPI* GetBraveVPNConnectionAPI() {
 }  // namespace
 
 BraveVpnServiceDesktop::BraveVpnServiceDesktop(
-    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
-    : BraveVpnService(url_loader_factory) {
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    PrefService* prefs)
+    : BraveVpnService(url_loader_factory), prefs_(prefs) {
   observed_.Observe(GetBraveVPNConnectionAPI());
 
   GetBraveVPNConnectionAPI()->set_target_vpn_entry_name(kBraveVPNEntryName);
@@ -371,3 +379,39 @@ void BraveVpnServiceDesktop::GetAllRegions(GetAllRegionsCallback callback) {
 void BraveVpnServiceDesktop::GetDeviceRegion(GetDeviceRegionCallback callback) {
   std::move(callback).Run(device_region_.Clone());
 }
+
+void BraveVpnServiceDesktop::GetSelectedRegion(
+    GetSelectedRegionCallback callback) {
+  auto* preference =
+      prefs_->FindPreference(brave_vpn::prefs::kBraveVPNSelectedRegion);
+  if (preference->IsDefaultValue()) {
+    // Gives device region if there is no cached selected region.
+    std::move(callback).Run(device_region_.Clone());
+    return;
+  }
+
+  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) {
+    // Gives device region if invalid data is cached.
+    std::move(callback).Run(device_region_.Clone());
+    return;
+  }
+
+  brave_vpn::mojom::Region region(*continent, *name, *name_pretty);
+  std::move(callback).Run(region.Clone());
+}
+
+void BraveVpnServiceDesktop::SetSelectedRegion(
+    brave_vpn::mojom::RegionPtr region_ptr) {
+  DictionaryPrefUpdate update(prefs_,
+                              brave_vpn::prefs::kBraveVPNSelectedRegion);
+  base::Value* dict = update.Get();
+  dict->SetStringKey(kRegionContinentKey, region_ptr->continent);
+  dict->SetStringKey(kRegionNameKey, region_ptr->name);
+  dict->SetStringKey(kRegionNamePrettyKey, region_ptr->name_pretty);
+}

components/brave_vpn/brave_vpn_service_desktop.h

@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "base/scoped_observation.h"
-#include "brave/components/brave_vpn/brave_vpn.mojom-shared.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_os_connection_api.h"
@@ -23,6 +22,8 @@ namespace base {
 class Value;
 }  // namespace base
 
+class PrefService;
+
 typedef brave_vpn::mojom::ConnectionState ConnectionState;
 
 class BraveVpnServiceDesktop
@@ -30,8 +31,9 @@ class BraveVpnServiceDesktop
       public brave_vpn::BraveVPNOSConnectionAPI::Observer,
       public brave_vpn::mojom::ServiceHandler {
  public:
-  explicit BraveVpnServiceDesktop(
-      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
+  BraveVpnServiceDesktop(
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      PrefService* prefs);
   ~BraveVpnServiceDesktop() override;
 
   BraveVpnServiceDesktop(const BraveVpnServiceDesktop&) = delete;
@@ -58,6 +60,8 @@ class BraveVpnServiceDesktop
   void CreateVPNConnection() override;
   void GetAllRegions(GetAllRegionsCallback callback) override;
   void GetDeviceRegion(GetDeviceRegionCallback callback) override;
+  void GetSelectedRegion(GetSelectedRegionCallback callback) override;
+  void SetSelectedRegion(brave_vpn::mojom::RegionPtr region) override;
 
  private:
   friend class BraveAppMenuBrowserTest;
@@ -93,6 +97,7 @@ class BraveVpnServiceDesktop
     is_purchased_user_ = purchased;
   }
 
+  PrefService* prefs_ = nullptr;
   std::vector<brave_vpn::mojom::Region> regions_;
   brave_vpn::mojom::Region device_region_;
   ConnectionState state_ = ConnectionState::DISCONNECTED;

components/brave_vpn/brave_vpn_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "brave/components/brave_vpn/brave_vpn_service_desktop.h"
 #include "brave/components/brave_vpn/features.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"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -17,7 +18,8 @@ class BraveVPNTest : public testing::Test {
  public:
   void SetUp() override {
     service_ = std::make_unique<BraveVpnServiceDesktop>(
-        base::MakeRefCounted<network::TestSharedURLLoaderFactory>());
+        base::MakeRefCounted<network::TestSharedURLLoaderFactory>(),
+        &pref_service_);
   }
 
   std::string GetRegionsData() {
@@ -134,6 +136,7 @@ class BraveVPNTest : public testing::Test {
   }
 
   content::BrowserTaskEnvironment task_environment_;
+  TestingPrefServiceSimple pref_service_;
   std::unique_ptr<BraveVpnServiceDesktop> service_;
 };
 

components/brave_vpn/pref_names.cc

@@ -0,0 +1,20 @@
+/* Copyright (c) 2021 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "brave/components/brave_vpn/pref_names.h"
+
+#include "components/prefs/pref_registry_simple.h"
+
+namespace brave_vpn {
+namespace prefs {
+
+void RegisterProfilePrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(kBraveVPNShowButton, true);
+  registry->RegisterDictionaryPref(kBraveVPNSelectedRegion);
+}
+
+}  // namespace prefs
+
+}  // namespace brave_vpn

components/brave_vpn/pref_names.h

@@ -0,0 +1,23 @@
+/* Copyright (c) 2021 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef BRAVE_COMPONENTS_BRAVE_VPN_PREF_NAMES_H_
+#define BRAVE_COMPONENTS_BRAVE_VPN_PREF_NAMES_H_
+
+class PrefRegistrySimple;
+
+namespace brave_vpn {
+namespace prefs {
+
+constexpr char kBraveVPNSelectedRegion[] = "brave.brave_vpn.selected_region";
+constexpr char kBraveVPNShowButton[] = "brave.brave_vpn.show_button";
+
+void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
+}  // namespace prefs
+
+}  // namespace brave_vpn
+
+#endif  // BRAVE_COMPONENTS_BRAVE_VPN_PREF_NAMES_H_

test/BUILD.gn

@@ -1024,6 +1024,10 @@ if (!is_android) {
       "//testing/android/native_test:native_test_support",
     ]
 
+    if (enable_brave_vpn) {
+      deps += [ "//brave/components/brave_vpn" ]
+    }
+
     # There are three types of tests that need to be rewritten for Android:
     # 1. using extensions
     # 2. using desktop specific UI elements (e.g. TabStripModelObserver)