Remaining Desktop VPN work

.storybook/BUILD.gn

@@ -11,7 +11,7 @@ group("storybook") {
   # are disabled in a regular brave build due to build flags,
   # they will be generated before storybook is compiled.
   deps = [
-    "//brave/components/brave_vpn:mojom_js",
+    "//brave/components/brave_vpn/mojom:mojom_js",
     "//brave/components/brave_wallet/common:mojom_js",
     "//ui/webui/resources/js:cr.m",
   ]

browser/brave_content_browser_client.cc

@@ -174,8 +174,8 @@ using extensions::ChromeContentBrowserClientExtensionsPart;
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN) && !BUILDFLAG(IS_ANDROID)
 #include "brave/browser/ui/webui/brave_vpn/vpn_panel_ui.h"
-#include "brave/components/brave_vpn/brave_vpn.mojom.h"
 #include "brave/components/brave_vpn/brave_vpn_utils.h"
+#include "brave/components/brave_vpn/mojom/brave_vpn.mojom.h"
 #endif
 
 #if BUILDFLAG(ETHEREUM_REMOTE_CLIENT_ENABLED)

browser/brave_vpn/android/brave_vpn_native_worker.cc

@@ -17,10 +17,12 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 
+using brave_vpn::BraveVpnService;
+
 namespace {
 
 BraveVpnService* GetBraveVpnService() {
-  return BraveVpnServiceFactory::GetForProfile(
+  return brave_vpn::BraveVpnServiceFactory::GetForProfile(
       ProfileManager::GetActiveUserProfile()->GetOriginalProfile());
 }
 

browser/brave_vpn/brave_vpn_service_factory.cc

@@ -6,91 +6,69 @@
 #include "brave/browser/brave_vpn/brave_vpn_service_factory.h"
 
 #include "base/feature_list.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/skus/skus_service_factory.h"
 #include "brave/components/brave_vpn/brave_vpn_service.h"
 #include "brave/components/skus/common/features.h"
+#include "build/build_config.h"
 #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"
 
-// TODO(bsclifton) or TODO(shong):
-// We should be able to consolidate this integration into one implementation
-// which we can share between Android and Desktop.
-//
-// As seen below, Desktop returns BraveVpnServiceDesktop and Android
-// returns BraveVpnService.
-//
-// See https://github.com/brave/brave-browser/issues/20374 for more info.
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-#include "brave/components/brave_vpn/brave_vpn_service_desktop.h"
 #include "brave/components/brave_vpn/brave_vpn_utils.h"
 #endif
 
+namespace brave_vpn {
+
 // static
 BraveVpnServiceFactory* BraveVpnServiceFactory::GetInstance() {
   return base::Singleton<BraveVpnServiceFactory>::get();
 }
 
 // static
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-BraveVpnServiceDesktop* BraveVpnServiceFactory::GetForProfile(
-    Profile* profile) {
-  return static_cast<BraveVpnServiceDesktop*>(
-      GetInstance()->GetServiceForBrowserContext(profile, true));
-}
-#elif BUILDFLAG(IS_ANDROID)
 BraveVpnService* BraveVpnServiceFactory::GetForProfile(Profile* profile) {
   return static_cast<BraveVpnService*>(
       GetInstance()->GetServiceForBrowserContext(profile, true));
 }
-#endif
 
-// TODO(bsclifton) or TODO(shong):
-// BraveVpnServiceDesktop is currently only used on Desktop,
-// which is why there are only OS guards for Windows and macOS.
-// Consolidating the Android/Desktop behaviors is captured with:
-// https://github.com/brave/brave-browser/issues/20374
 BraveVpnServiceFactory::BraveVpnServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "BraveVpnService",
           BrowserContextDependencyManager::GetInstance()) {
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
   DependsOn(skus::SkusServiceFactory::GetInstance());
-#endif
 }
 
 BraveVpnServiceFactory::~BraveVpnServiceFactory() = default;
 
 KeyedService* BraveVpnServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
+  // TODO(simonhong): Can we use this check for android?
+  // For now, vpn is disabled by default on desktop but not sure on
+  // android.
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-  if (!brave_vpn::IsBraveVPNEnabled())
+  if (!IsBraveVPNEnabled())
     return nullptr;
 #endif
 
   auto* default_storage_partition = context->GetDefaultStoragePartition();
   auto shared_url_loader_factory =
       default_storage_partition->GetURLLoaderFactoryForBrowserProcess();
 
-// TODO(bsclifton) or TODO(shong):
-// see same notes above
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
   auto callback = base::BindRepeating(
       [](content::BrowserContext* context) {
         return skus::SkusServiceFactory::GetForContext(context);
       },
       context);
-  return new BraveVpnServiceDesktop(
-      shared_url_loader_factory, user_prefs::UserPrefs::Get(context), callback);
+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
+  return new BraveVpnService(shared_url_loader_factory,
+                             user_prefs::UserPrefs::Get(context), callback);
 #elif BUILDFLAG(IS_ANDROID)
-  return new BraveVpnService(shared_url_loader_factory);
+  return new BraveVpnService(shared_url_loader_factory, callback);
 #endif
 }
 
-content::BrowserContext* BraveVpnServiceFactory::GetBrowserContextToUse(
-    content::BrowserContext* context) const {
-  return chrome::GetBrowserContextRedirectedInIncognito(context);
-}
+}  // namespace brave_vpn

browser/brave_vpn/brave_vpn_service_factory.h

@@ -14,23 +14,13 @@
 
 class Profile;
 
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-class BraveVpnServiceDesktop;
-#endif
+namespace brave_vpn {
 
-#if BUILDFLAG(IS_ANDROID)
 class BraveVpnService;
-#endif
 
 class BraveVpnServiceFactory : public BrowserContextKeyedServiceFactory {
  public:
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
-  static BraveVpnServiceDesktop* GetForProfile(Profile* profile);
-#endif
-
-#if BUILDFLAG(IS_ANDROID)
   static BraveVpnService* GetForProfile(Profile* profile);
-#endif
   static BraveVpnServiceFactory* GetInstance();
 
   BraveVpnServiceFactory(const BraveVpnServiceFactory&) = delete;
@@ -45,8 +35,8 @@ class BraveVpnServiceFactory : public BrowserContextKeyedServiceFactory {
   // BrowserContextKeyedServiceFactory overrides:
   KeyedService* BuildServiceInstanceFor(
       content::BrowserContext* context) const override;
-  content::BrowserContext* GetBrowserContextToUse(
-      content::BrowserContext* context) const override;
 };
 
+}  // namespace brave_vpn
+
 #endif  // BRAVE_BROWSER_BRAVE_VPN_BRAVE_VPN_SERVICE_FACTORY_H_

browser/brave_vpn/sources.gni

@@ -12,16 +12,18 @@ if (enable_brave_vpn) {
   brave_browser_brave_vpn_sources += [
     "//brave/browser/brave_vpn/brave_vpn_service_factory.cc",
     "//brave/browser/brave_vpn/brave_vpn_service_factory.h",
+    "//brave/browser/brave_vpn/vpn_utils.cc",
+    "//brave/browser/brave_vpn/vpn_utils.h",
   ]
 
   brave_browser_brave_vpn_deps += [
     "//base",
+    "//brave/browser/skus",
     "//brave/components/brave_vpn",
     "//chrome/browser/profiles:profile",
     "//components/keyed_service/content",
     "//components/user_prefs",
     "//content/public/browser",
-    "//brave/browser/skus"
   ]
 
   if (is_android) {

browser/brave_vpn/vpn_utils.cc

@@ -0,0 +1,25 @@
+/* Copyright (c) 2022 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/browser/brave_vpn/vpn_utils.h"
+
+#include "brave/browser/profiles/profile_util.h"
+#include "brave/components/brave_vpn/brave_vpn_utils.h"
+#include "build/build_config.h"
+
+namespace brave_vpn {
+
+bool IsBraveVPNEnabled(content::BrowserContext* context) {
+  // TODO(simonhong): Can we use this check for android?
+  // For now, vpn is disabled by default on desktop but not sure on
+  // android.
+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
+  return brave_vpn::IsBraveVPNEnabled() && brave::IsRegularProfile(context);
+#else
+  return brave::IsRegularProfile(context);
+#endif
+}
+
+}  // namespace brave_vpn

browser/brave_vpn/vpn_utils.h

@@ -0,0 +1,19 @@
+/* Copyright (c) 2022 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_BROWSER_BRAVE_VPN_VPN_UTILS_H_
+#define BRAVE_BROWSER_BRAVE_VPN_VPN_UTILS_H_
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace brave_vpn {
+
+bool IsBraveVPNEnabled(content::BrowserContext* context);
+
+}  // namespace brave_vpn
+
+#endif  // BRAVE_BROWSER_BRAVE_VPN_VPN_UTILS_H_

browser/ui/BUILD.gn

@@ -548,7 +548,7 @@ source_set("ui") {
     ]
     deps += [
       "//brave/components/brave_vpn",
-      "//brave/components/brave_vpn:mojom",
+      "//brave/components/brave_vpn/mojom",
       "//brave/components/brave_vpn/resources/panel:brave_vpn_panel_generated",
       "//mojo/public/cpp/bindings",
       "//ui/webui:webui",

browser/ui/brave_browser_command_controller.cc

@@ -31,8 +31,8 @@
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
 #include "brave/browser/brave_vpn/brave_vpn_service_factory.h"
-#include "brave/components/brave_vpn/brave_vpn_service_desktop.h"
-#include "brave/components/brave_vpn/brave_vpn_utils.h"
+#include "brave/browser/brave_vpn/vpn_utils.h"
+#include "brave/components/brave_vpn/brave_vpn_service.h"
 #endif
 
 #if BUILDFLAG(ENABLE_SIDEBAR)
@@ -64,7 +64,10 @@ BraveBrowserCommandController::BraveBrowserCommandController(Browser* browser)
       brave_command_updater_(nullptr) {
   InitBraveCommandState();
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
-  Observe(BraveVpnServiceFactory::GetForProfile(browser_->profile()));
+  if (auto* vpn_service = brave_vpn::BraveVpnServiceFactory::GetForProfile(
+          browser_->profile())) {
+    Observe(vpn_service);
+  }
 #endif
 }
 
@@ -189,7 +192,7 @@ void BraveBrowserCommandController::UpdateCommandForSidebar() {
 
 void BraveBrowserCommandController::UpdateCommandForBraveVPN() {
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
-  if (!brave_vpn::IsBraveVPNEnabled()) {
+  if (!brave_vpn::IsBraveVPNEnabled(browser_->profile())) {
     UpdateCommandEnabled(IDC_SHOW_BRAVE_VPN_PANEL, false);
     UpdateCommandEnabled(IDC_BRAVE_VPN_MENU, false);
     UpdateCommandEnabled(IDC_TOGGLE_BRAVE_VPN_TOOLBAR_BUTTON, false);
@@ -206,8 +209,8 @@ void BraveBrowserCommandController::UpdateCommandForBraveVPN() {
   UpdateCommandEnabled(IDC_ABOUT_BRAVE_VPN, true);
   UpdateCommandEnabled(IDC_MANAGE_BRAVE_VPN_PLAN, true);
 
-  if (auto* vpn_service =
-          BraveVpnServiceFactory::GetForProfile(browser_->profile())) {
+  if (auto* vpn_service = brave_vpn::BraveVpnServiceFactory::GetForProfile(
+          browser_->profile())) {
     // Only show vpn sub menu for purchased user.
     UpdateCommandEnabled(IDC_BRAVE_VPN_MENU, vpn_service->is_purchased_user());
     UpdateCommandEnabled(IDC_TOGGLE_BRAVE_VPN,

browser/ui/brave_browser_command_controller.h

@@ -28,7 +28,7 @@ namespace chrome {
 class BraveBrowserCommandController : public chrome::BrowserCommandController
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
     ,
-                                      public BraveVPNServiceObserver
+                                      public brave_vpn::BraveVPNServiceObserver
 #endif
 {
  public:
@@ -59,7 +59,7 @@ class BraveBrowserCommandController : public chrome::BrowserCommandController
   bool UpdateCommandEnabled(int id, bool state) override;
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
-  // BraveVPNServiceObserver overrides:
+  // brave_vpn::BraveVPNServiceObserver overrides:
   void OnPurchasedStateChanged(brave_vpn::mojom::PurchasedState state) override;
 #endif
 

browser/ui/brave_browser_command_controller_browsertest.cc

@@ -32,7 +32,7 @@
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
 #include "brave/browser/brave_vpn/brave_vpn_service_factory.h"
-#include "brave/components/brave_vpn/brave_vpn_service_desktop.h"
+#include "brave/components/brave_vpn/brave_vpn_service.h"
 #include "brave/components/brave_vpn/features.h"
 #endif
 
@@ -47,9 +47,12 @@ class BraveBrowserCommandControllerTest : public InProcessBrowserTest {
 
 #if BUILDFLAG(ENABLE_BRAVE_VPN)
   void SetPurchasedUserForBraveVPN(Browser* browser, bool purchased) {
-    auto* service = BraveVpnServiceFactory::GetForProfile(browser->profile());
-    auto target_state =
-        purchased ? PurchasedState::PURCHASED : PurchasedState::NOT_PURCHASED;
+    auto* service =
+        brave_vpn::BraveVpnServiceFactory::GetForProfile(browser->profile());
+    ASSERT_TRUE(!!service);
+    auto target_state = purchased
+                            ? brave_vpn::mojom::PurchasedState::PURCHASED
+                            : brave_vpn::mojom::PurchasedState::NOT_PURCHASED;
     service->SetPurchasedState(target_state);
     // Call explicitely to update vpn commands status because mojo works in
     // async way.
@@ -144,10 +147,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
       command_controller->IsCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR));
 #endif
 
-#if BUILDFLAG(ENABLE_BRAVE_VPN)
-  CheckBraveVPNCommands(private_browser);
-#endif
-
   if (syncer::IsSyncAllowedByFlag())
     EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_SYNC));
   else
@@ -182,10 +181,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
       command_controller->IsCommandEnabled(IDC_NEW_OFFTHERECORD_WINDOW_TOR));
 #endif
 
-#if BUILDFLAG(ENABLE_BRAVE_VPN)
-  CheckBraveVPNCommands(guest_browser);
-#endif
-
   EXPECT_FALSE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_SYNC));
 
   EXPECT_FALSE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_WALLET));
@@ -220,10 +215,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
   else
     EXPECT_FALSE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_SYNC));
 
-#if BUILDFLAG(ENABLE_BRAVE_VPN)
-  CheckBraveVPNCommands(tor_browser);
-#endif
-
   EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_WALLET));
   EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_ADD_NEW_PROFILE));
   EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_OPEN_GUEST_PROFILE));