From dbd500c527580b5e1fd396b845c334f7e747bcb6 Mon Sep 17 00:00:00 2001 From: Alex <256235273+alekspop26@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:38:42 -0400 Subject: [PATCH] [VPN 2.0] Finalize extraction of BraveVpnService V1 implementation (#36851) To add a new BraveVpnService implementation based on Architecture 2.0, which must co-exist with Architecture 1.0 for quite a while, we need to split service's interface and implementation. All the external components will keep accessing VPN service via the BraveVpnService interface, but the implementation mostly goes into BraveVpnServiceImpl. This change finalizes the BraveVpnService Architecture 1.0 extraction based on GN flags. It also splits "components/brave_vpn/browser" targets in the BUILD.gn into two targets - a shared one and a V1 specific one. The factory is prepared to create different service implementations, and the V1 tests are ensured to run only when V1 architecture is enabled by a GN flag. Implements part of https://github.com/brave/brave-browser/issues/54597 --- BUILD.gn | 9 +- .../brave_vpn/brave_vpn_service_factory.cc | 34 ++++++-- browser/brave_vpn/sources.gni | 45 +++++----- browser/brave_vpn/win/BUILD.gn | 1 + browser/sources.gni | 6 +- browser/ui/BUILD.gn | 14 +++- .../chrome/installer/setup/test/BUILD.gn | 2 +- chromium_src/chrome/test/sources.gni | 2 +- components/brave_vpn/browser/BUILD.gn | 84 +++++++++++++------ .../browser/connection/ikev2/BUILD.gn | 2 +- test/BUILD.gn | 6 +- 11 files changed, 133 insertions(+), 72 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 0cc1e9213ec..8d1e315927d 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -3,8 +3,6 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at https://mozilla.org/MPL/2.0/. -import( - "//brave/browser/brave_vpn/win/brave_vpn_wireguard_service/allowlist.gni") import("//brave/build/config.gni") import("//brave/build/redirect_cc.gni") import("//brave/components/brave_vpn/common/buildflags/buildflags.gni") @@ -20,6 +18,11 @@ import("//tools/grit/repack.gni") import("//tools/v8_context_snapshot/v8_context_snapshot.gni") import("//ui/base/ui_features.gni") +if (enable_brave_vpn_v1) { + import( + "//brave/browser/brave_vpn/win/brave_vpn_wireguard_service/allowlist.gni") +} + if (enable_library_cdms) { import("//media/cdm/library_cdm/cdm_paths.gni") } @@ -771,7 +774,7 @@ if (is_win && enable_resource_allowlist_generation) { # Merge chrome_resource_allowlist and wireguard resources allowlist to keep # wireguard resources in pak files. When VPN is disabled, just copy the chrome # allowlist. - if (enable_brave_vpn) { + if (enable_brave_vpn_v1) { action("merge_allowlists") { deps = [ "//brave/browser/brave_vpn/win/brave_vpn_wireguard_service:wireguard_resource_allowlist", diff --git a/browser/brave_vpn/brave_vpn_service_factory.cc b/browser/brave_vpn/brave_vpn_service_factory.cc index 252a04a1b05..632cf9cb3d8 100644 --- a/browser/brave_vpn/brave_vpn_service_factory.cc +++ b/browser/brave_vpn/brave_vpn_service_factory.cc @@ -9,14 +9,15 @@ #include "base/feature_list.h" #include "base/no_destructor.h" +#include "base/notreached.h" #include "brave/browser/brave_browser_process.h" #include "brave/browser/brave_vpn/vpn_utils.h" #include "brave/browser/misc_metrics/process_misc_metrics.h" #include "brave/browser/misc_metrics/uptime_monitor_impl.h" #include "brave/browser/skus/skus_service_factory.h" #include "brave/components/brave_vpn/browser/brave_vpn_service.h" -#include "brave/components/brave_vpn/browser/brave_vpn_service_impl.h" #include "brave/components/brave_vpn/common/brave_vpn_utils.h" +#include "brave/components/brave_vpn/common/buildflags/buildflags.h" #include "brave/components/skus/common/features.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" @@ -27,23 +28,24 @@ #include "content/public/browser/browser_context.h" #include "content/public/browser/storage_partition.h" +#if BUILDFLAG(ENABLE_BRAVE_VPN_V1) +#include "brave/components/brave_vpn/browser/brave_vpn_service_impl.h" #if BUILDFLAG(IS_WIN) #include "brave/browser/brave_vpn/dns/brave_vpn_dns_observer_factory_win.h" #include "brave/browser/brave_vpn/dns/brave_vpn_dns_observer_service_win.h" #include "brave/browser/brave_vpn/win/brave_vpn_service_delegate_win.h" #include "brave/browser/brave_vpn/win/brave_vpn_wireguard_observer_factory_win.h" #include "brave/browser/brave_vpn/win/brave_vpn_wireguard_observer_service_win.h" -#endif +#endif // IS_WIN +#endif // ENABLE_BRAVE_VPN_V1 namespace brave_vpn { namespace { -std::unique_ptr BuildVpnService( - content::BrowserContext* context) { - if (!brave_vpn::IsAllowedForContext(context)) { - return nullptr; - } +#if BUILDFLAG(ENABLE_BRAVE_VPN_V1) +std::unique_ptr BuildVpnService_V1( + content::BrowserContext* context) { #if !BUILDFLAG(IS_ANDROID) if (!g_brave_browser_process->brave_vpn_connection_manager()) { return nullptr; @@ -93,6 +95,20 @@ std::unique_ptr BuildVpnService( return vpn_service; } +#endif // BUILDFLAG(ENABLE_BRAVE_VPN_V1) + +std::unique_ptr BuildVpnService( + content::BrowserContext* context) { + if (!brave_vpn::IsAllowedForContext(context)) { + return nullptr; + } +#if BUILDFLAG(ENABLE_BRAVE_VPN_V1) + return BuildVpnService_V1(context); +#else + NOTREACHED() << "No VPN implementation available"; +#endif +} + } // namespace // static @@ -138,10 +154,10 @@ BraveVpnServiceFactory::BraveVpnServiceFactory() "BraveVpnService", BrowserContextDependencyManager::GetInstance()) { DependsOn(skus::SkusServiceFactory::GetInstance()); -#if BUILDFLAG(IS_WIN) +#if BUILDFLAG(ENABLE_BRAVE_VPN_V1) && BUILDFLAG(IS_WIN) DependsOn(brave_vpn::BraveVpnWireguardObserverFactory::GetInstance()); DependsOn(brave_vpn::BraveVpnDnsObserverFactory::GetInstance()); -#endif +#endif // BUILDFLAG(ENABLE_BRAVE_VPN_V1) && BUILDFLAG(IS_WIN) } BraveVpnServiceFactory::~BraveVpnServiceFactory() = default; diff --git a/browser/brave_vpn/sources.gni b/browser/brave_vpn/sources.gni index fd3f97f32a6..9c081958c4b 100644 --- a/browser/brave_vpn/sources.gni +++ b/browser/brave_vpn/sources.gni @@ -13,7 +13,31 @@ if (enable_brave_vpn) { "//brave/browser/brave_vpn/brave_vpn_service_factory.cc", "//brave/browser/brave_vpn/brave_vpn_service_factory.h", ] + brave_browser_brave_vpn_deps += [ + "//base", + "//brave/browser/brave_vpn", + "//brave/components/brave_vpn/browser", + "//chrome/browser/profiles:profile", + "//components/keyed_service/content", + "//components/user_prefs", + "//content/public/browser", + ] + if (is_android) { + brave_browser_brave_vpn_sources += [ + "//brave/browser/brave_vpn/android/brave_vpn_native_worker.cc", + "//brave/browser/brave_vpn/android/brave_vpn_native_worker.h", + "//brave/browser/brave_vpn/android/brave_vpn_service_factory_android.cc", + ] + brave_browser_brave_vpn_deps += [ + "//brave/build/android:jni_headers", + "//brave/components/l10n/common", + "//mojo/public/cpp/bindings", + ] + } +} + +if (enable_brave_vpn_v1) { if (is_win) { brave_browser_brave_vpn_sources += [ "//brave/browser/brave_vpn/dns/brave_vpn_dns_observer_factory_win.cc", @@ -40,26 +64,7 @@ if (enable_brave_vpn) { } brave_browser_brave_vpn_deps += [ - "//base", - "//brave/browser/brave_vpn", "//brave/browser/brave_vpn:v1_utils", - "//brave/components/brave_vpn/browser", - "//chrome/browser/profiles:profile", - "//components/keyed_service/content", - "//components/user_prefs", - "//content/public/browser", + "//brave/components/brave_vpn/browser:v1_utils", ] - - if (is_android) { - brave_browser_brave_vpn_sources += [ - "//brave/browser/brave_vpn/android/brave_vpn_native_worker.cc", - "//brave/browser/brave_vpn/android/brave_vpn_native_worker.h", - "//brave/browser/brave_vpn/android/brave_vpn_service_factory_android.cc", - ] - brave_browser_brave_vpn_deps += [ - "//brave/build/android:jni_headers", - "//brave/components/l10n/common", - "//mojo/public/cpp/bindings", - ] - } } diff --git a/browser/brave_vpn/win/BUILD.gn b/browser/brave_vpn/win/BUILD.gn index 6df7c142467..689ea2b86d0 100644 --- a/browser/brave_vpn/win/BUILD.gn +++ b/browser/brave_vpn/win/BUILD.gn @@ -39,6 +39,7 @@ source_set("win") { ":wireguard_utils", "//base", "//brave/components/brave_vpn/browser", + "//brave/components/brave_vpn/browser:v1_utils", "//brave/components/brave_vpn/browser/connection/wireguard", "//brave/components/brave_vpn/browser/connection/wireguard/credentials", "//brave/components/brave_vpn/common", diff --git a/browser/sources.gni b/browser/sources.gni index 2548768427a..a7ed8be2a4d 100644 --- a/browser/sources.gni +++ b/browser/sources.gni @@ -546,13 +546,11 @@ if (enable_web_discovery_native) { if (enable_brave_vpn) { brave_chrome_browser_deps += [ "//brave/components/brave_vpn/browser", + "//brave/components/brave_vpn/browser:v1_utils", + "//brave/components/brave_vpn/browser/connection", "//brave/components/brave_vpn/browser/connection:api", "//brave/components/brave_vpn/common", ] - if (enable_brave_vpn_v1) { - brave_chrome_browser_deps += - [ "//brave/components/brave_vpn/browser/connection" ] - } } if (enable_brave_vpn_v2) { diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index 4a6d14b1330..205fb5ce7ed 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -985,10 +985,12 @@ source_set("ui") { deps += [ "//brave/browser/brave_vpn", "//brave/components/brave_vpn/browser", - "//brave/components/brave_vpn/browser/connection:api", "//brave/components/brave_vpn/common", ] - if (enable_brave_vpn_v1 && is_win) { + } + if (enable_brave_vpn_v1) { + deps += [ "//brave/components/brave_vpn/browser/connection:api" ] + if (is_win) { deps += [ "//brave/components/brave_vpn/common/win", "//brave/components/brave_vpn/common/wireguard", @@ -1491,8 +1493,6 @@ source_set("unit_tests") { deps += [ "//base", "//brave/components/brave_vpn/browser", - "//brave/components/brave_vpn/browser/connection:api", - "//brave/components/brave_vpn/browser/connection/ikev2:sim", "//brave/components/skus/browser", "//chrome/browser/prefs", "//chrome/browser/themes", @@ -1501,6 +1501,12 @@ source_set("unit_tests") { "//third_party/abseil-cpp:absl", ] } + if (enable_brave_vpn_v1) { + deps += [ + "//brave/components/brave_vpn/browser/connection:api", + "//brave/components/brave_vpn/browser/connection/ikev2:sim", + ] + } } } diff --git a/chromium_src/chrome/installer/setup/test/BUILD.gn b/chromium_src/chrome/installer/setup/test/BUILD.gn index b621aa1aee0..da889a48dcb 100644 --- a/chromium_src/chrome/installer/setup/test/BUILD.gn +++ b/chromium_src/chrome/installer/setup/test/BUILD.gn @@ -5,7 +5,7 @@ import("//brave/components/brave_vpn/common/buildflags/buildflags.gni") -if (enable_brave_vpn && is_win) { +if (enable_brave_vpn_v1 && is_win) { source_set("vpn_unit_tests") { testonly = true sources = [ "install_worker_vpn_unittest.cc" ] diff --git a/chromium_src/chrome/test/sources.gni b/chromium_src/chrome/test/sources.gni index d4577b697db..0a1379fa16b 100644 --- a/chromium_src/chrome/test/sources.gni +++ b/chromium_src/chrome/test/sources.gni @@ -26,7 +26,7 @@ if (enable_ai_chat) { [ "//brave/components/ai_chat/core/browser:test_support" ] } -if (enable_brave_vpn) { +if (enable_brave_vpn_v1) { brave_chromium_src_chrome_test_test_support_deps += [ "//brave/components/brave_vpn/browser/connection:api" ] } diff --git a/components/brave_vpn/browser/BUILD.gn b/components/brave_vpn/browser/BUILD.gn index 78de29e37fe..432ea4578a6 100644 --- a/components/brave_vpn/browser/BUILD.gn +++ b/components/brave_vpn/browser/BUILD.gn @@ -7,43 +7,63 @@ import("//brave/components/brave_vpn/common/buildflags/buildflags.gni") assert(enable_brave_vpn) +# Browser sources shared between v1 and v2 implementations. static_library("browser") { + sources = [ + "brave_vpn_service.cc", + "brave_vpn_service.h", + "brave_vpn_service_observer.cc", + "brave_vpn_service_observer.h", + ] + + public_deps = [ + "//base", + "//brave/components/brave_vpn/common/mojom", + "//brave/components/skus/browser", + "//components/keyed_service/core", + "//mojo/public/cpp/bindings", + ] + + deps = [ "//brave/components/brave_vpn/common" ] +} + +static_library("v1_utils") { sources = [ "brave_vpn_metrics.cc", "brave_vpn_metrics.h", - "brave_vpn_service.cc", - "brave_vpn_service.h", "brave_vpn_service_delegate.h", "brave_vpn_service_helper.cc", "brave_vpn_service_helper.h", "brave_vpn_service_impl.cc", "brave_vpn_service_impl.h", - "brave_vpn_service_observer.cc", - "brave_vpn_service_observer.h", ] - deps = [ + public_deps = [ + ":browser", "api", - "connection", "connection:api", - "connection:common", "//base", "//brave/components/brave_vpn/common", "//brave/components/brave_vpn/common/mojom", - "//brave/components/constants", "//brave/components/misc_metrics", - "//brave/components/p3a_utils", - "//brave/components/resources:strings", "//brave/components/skus/browser", "//brave/components/skus/common:mojom", "//brave/components/time_period_storage", - "//brave/components/version_info", "//build:buildflag_header_h", - "//components/keyed_service/core", - "//components/pref_registry", "//components/prefs", - "//components/version_info", + "//mojo/public/cpp/bindings", "//services/network/public/cpp", + ] + + deps = [ + "connection", + "connection:common", + "//brave/components/constants", + "//brave/components/p3a_utils", + "//brave/components/resources:strings", + "//brave/components/version_info", + "//components/pref_registry", + "//components/version_info", "//ui/base", "//url", ] @@ -52,20 +72,18 @@ static_library("browser") { source_set("unit_tests") { testonly = true - sources = [ - "brave_vpn_metrics_unittest.cc", - "brave_vpn_service_impl_unittest.cc", - "brave_vpn_service_unittest.cc", - ] + sources = [ "brave_vpn_service_unittest.cc" ] + + if (enable_brave_vpn_v1) { + sources += [ + "brave_vpn_metrics_unittest.cc", + "brave_vpn_service_impl_unittest.cc", + ] + } deps = [ ":browser", - "api", - "api:unit_tests", "//base", - "//brave/components/brave_vpn/browser/connection:api", - "//brave/components/brave_vpn/browser/connection:common", - "//brave/components/brave_vpn/browser/connection/wireguard/credentials:unit_tests", "//brave/components/brave_vpn/common", "//brave/components/brave_vpn/common/mojom", "//brave/components/constants", @@ -80,10 +98,22 @@ source_set("unit_tests") { "//testing/gmock", "//testing/gtest", ] - if (!is_android) { + + if (enable_brave_vpn_v1) { deps += [ - "connection:unit_tests", - "//brave/components/brave_vpn/browser/connection/ikev2:sim", + ":v1_utils", + "api", + "api:unit_tests", + "//brave/components/brave_vpn/browser/connection:api", + "//brave/components/brave_vpn/browser/connection:common", + "//brave/components/brave_vpn/browser/connection/wireguard/credentials:unit_tests", ] + + if (!is_android) { + deps += [ + "connection:unit_tests", + "//brave/components/brave_vpn/browser/connection/ikev2:sim", + ] + } } } diff --git a/components/brave_vpn/browser/connection/ikev2/BUILD.gn b/components/brave_vpn/browser/connection/ikev2/BUILD.gn index 843ea09597f..b435dc4dd24 100644 --- a/components/brave_vpn/browser/connection/ikev2/BUILD.gn +++ b/components/brave_vpn/browser/connection/ikev2/BUILD.gn @@ -51,7 +51,7 @@ source_set("unit_tests") { deps = [ ":sim", - "//brave/components/brave_vpn/browser", + "//brave/components/brave_vpn/browser:v1_utils", "//brave/components/brave_vpn/browser/connection:api", "//brave/components/brave_vpn/browser/connection:common", "//brave/components/brave_vpn/common", diff --git a/test/BUILD.gn b/test/BUILD.gn index 4eaecc7ff13..64325276fa2 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -416,9 +416,11 @@ test("brave_unit_tests") { deps += [ "//brave/components/brave_vpn/browser:unit_tests", "//brave/components/brave_vpn/common:unit_tests", - "//brave/components/brave_vpn/common/wireguard:unit_tests", "//brave/components/skus/browser:unit_tests", ] + } + if (enable_brave_vpn_v1) { + deps += [ "//brave/components/brave_vpn/common/wireguard:unit_tests" ] if (is_win) { deps += [ "//brave/browser/brave_vpn/dns:unit_tests", @@ -713,7 +715,7 @@ if (!is_android) { "//testing/gtest", ] - if (enable_brave_vpn) { + if (enable_brave_vpn_v1) { deps += [ "//brave/chromium_src/chrome/installer/setup/test:vpn_unit_tests", ]