Skip to content

Commit

Permalink
Reduce VPN manager instances (#3097)
Browse files Browse the repository at this point in the history 
Task/Issue URL: https://app.asana.com/0/72649045549333/1207151621945908/f
Tech Design URL:
CC:

Description:

This PR reduces the number of VPN manager instances that we create through the lifetime of the app.
  • Loading branch information
@samsymons
samsymons authored Jul 19, 2024
1 parent 4486729 commit ba71738
Show file tree
Hide file tree
Showing 10 changed files with 125 additions and 88 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10182,7 +10182,7 @@
repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 173.0.0;
version = 174.0.0;
};
};
9F8FE9472BAE50E50071E372 /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
Expand Down
6 changes: 3 additions & 3 deletions DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/DuckDuckGo/BrowserServicesKit",
"state" : {
"revision" : "16686ec1d3a8641b47c55d5271e29c7dbe4c9e73",
"version" : "173.0.0"
"revision" : "6db80afec11da4f0a36a81dc6030f7e83a524c87",
"version" : "174.0.0"
}
},
{
Expand Down Expand Up @@ -138,7 +138,7 @@
{
"identity" : "swift-argument-parser",
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-argument-parser",
"location" : "https://github.com/apple/swift-argument-parser.git",
"state" : {
"revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b",
"version" : "1.4.0"
Expand Down
2 changes: 2 additions & 0 deletions DuckDuckGo/AppDependencyProvider.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ protocol DependencyProvider {
var networkProtectionKeychainTokenStore: NetworkProtectionKeychainTokenStore { get }
var networkProtectionTunnelController: NetworkProtectionTunnelController { get }
var connectionObserver: ConnectionStatusObserver { get }
var serverInfoObserver: ConnectionServerInfoObserver { get }
var vpnSettings: VPNSettings { get }
}

Expand Down Expand Up @@ -88,6 +89,7 @@ class AppDependencyProvider: DependencyProvider {
let subscriptionAppGroup = Bundle.main.appGroup(bundle: .subs)

let connectionObserver: ConnectionStatusObserver = ConnectionStatusObserverThroughSession()
let serverInfoObserver: ConnectionServerInfoObserver = ConnectionServerInfoObserverThroughSession()
let vpnSettings = VPNSettings(defaults: .networkProtectionGroupDefaults)

init() {
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/NetworkProtectionConvenienceInitialisers.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import Subscription

private class DefaultTunnelSessionProvider: TunnelSessionProvider {
func activeSession() async -> NETunnelProviderSession? {
try? await ConnectionSessionUtilities.activeSession()
return await AppDependencyProvider.shared.networkProtectionTunnelController.activeSession()
}
}

Expand Down
8 changes: 4 additions & 4 deletions DuckDuckGo/NetworkProtectionDebugUtilities.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ final class NetworkProtectionDebugUtilities {
// MARK: - Registation Key

func expireRegistrationKeyNow() async {
guard let activeSession = try? await ConnectionSessionUtilities.activeSession() else {
guard let activeSession = await AppDependencyProvider.shared.networkProtectionTunnelController.activeSession() else {
return
}

Expand All @@ -41,7 +41,7 @@ final class NetworkProtectionDebugUtilities {
// MARK: - Notifications

func sendTestNotificationRequest() async throws {
guard let activeSession = try? await ConnectionSessionUtilities.activeSession() else {
guard let activeSession = await AppDependencyProvider.shared.networkProtectionTunnelController.activeSession() else {
return
}

Expand All @@ -51,7 +51,7 @@ final class NetworkProtectionDebugUtilities {
// MARK: - Disable VPN

func disableConnectOnDemandAndShutDown() async {
guard let activeSession = try? await ConnectionSessionUtilities.activeSession() else {
guard let activeSession = await AppDependencyProvider.shared.networkProtectionTunnelController.activeSession() else {
return
}

Expand All @@ -61,7 +61,7 @@ final class NetworkProtectionDebugUtilities {
// MARK: - Failure Simulation

func triggerSimulation(_ option: NetworkProtectionSimulationOption) async {
guard let activeSession = try? await ConnectionSessionUtilities.activeSession() else {
guard let activeSession = await AppDependencyProvider.shared.networkProtectionTunnelController.activeSession() else {
return
}

Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/NetworkProtectionRootView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct NetworkProtectionRootView: View {
statusViewModel = NetworkProtectionStatusViewModel(tunnelController: AppDependencyProvider.shared.networkProtectionTunnelController,
settings: AppDependencyProvider.shared.vpnSettings,
statusObserver: AppDependencyProvider.shared.connectionObserver,
serverInfoObserver: AppDependencyProvider.shared.serverInfoObserver,
locationListRepository: locationListRepository)
}
var body: some View {
Expand Down
8 changes: 3 additions & 5 deletions DuckDuckGo/NetworkProtectionStatusView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,9 @@ struct NetworkProtectionStatusView: View {
@ViewBuilder
private func about() -> some View {
Section {
if statusModel.shouldShowFAQ {
NavigationLink(UserText.netPVPNSettingsFAQ, destination: LazyView(NetworkProtectionFAQView()))
.daxBodyRegular()
.foregroundColor(.init(designSystemColor: .textPrimary))
}
NavigationLink(UserText.netPVPNSettingsFAQ, destination: LazyView(NetworkProtectionFAQView()))
.daxBodyRegular()
.foregroundColor(.init(designSystemColor: .textPrimary))

NavigationLink(UserText.netPVPNSettingsShareFeedback, destination: VPNFeedbackFormCategoryView())
.daxBodyRegular()
Expand Down
67 changes: 35 additions & 32 deletions DuckDuckGo/NetworkProtectionStatusViewModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ final class NetworkProtectionStatusViewModel: ObservableObject {
return formatter
}()

private let tunnelController: TunnelController
private let tunnelController: (TunnelController & TunnelSessionProvider)
private let statusObserver: ConnectionStatusObserver
private let serverInfoObserver: ConnectionServerInfoObserver
private let errorObserver: ConnectionErrorObserver
Expand Down Expand Up @@ -134,16 +134,12 @@ final class NetworkProtectionStatusViewModel: ObservableObject {
@Published public var downloadTotal: String?
private var throughputUpdateTimer: Timer?

var shouldShowFAQ: Bool {
AppDependencyProvider.shared.subscriptionFeatureAvailability.isFeatureAvailable
}

@Published public var animationsOn: Bool = false

public init(tunnelController: TunnelController,
public init(tunnelController: (TunnelController & TunnelSessionProvider),
settings: VPNSettings,
statusObserver: ConnectionStatusObserver,
serverInfoObserver: ConnectionServerInfoObserver = ConnectionServerInfoObserverThroughSession(),
serverInfoObserver: ConnectionServerInfoObserver,
errorObserver: ConnectionErrorObserver = ConnectionErrorObserverThroughSession(),
locationListRepository: NetworkProtectionLocationListRepository) {
self.tunnelController = tunnelController
Expand All @@ -159,6 +155,8 @@ final class NetworkProtectionStatusViewModel: ObservableObject {

self.dnsSettings = settings.dnsSettings

updateViewModel(withStatus: statusObserver.recentValue)

setUpIsConnectedStatePublishers()
setUpToggledStatePublisher()
setUpStatusMessagePublishers()
Expand All @@ -176,30 +174,10 @@ final class NetworkProtectionStatusViewModel: ObservableObject {
}

private func setUpIsConnectedStatePublishers() {
let isConnectedPublisher = statusObserver.publisher
.map { $0.isConnected }
.receive(on: DispatchQueue.main)
.eraseToAnyPublisher()
isConnectedPublisher
.map(Self.titleText(connected:))
.assign(to: \.headerTitle, onWeaklyHeld: self)
.store(in: &cancellables)
isConnectedPublisher
.map(Self.statusImageID(connected:))
.assign(to: \.statusImageID, onWeaklyHeld: self)
.store(in: &cancellables)
isConnectedPublisher
.sink { [weak self] isConnected in
if !isConnected {
self?.uploadTotal = nil
self?.downloadTotal = nil
self?.throughputUpdateTimer?.invalidate()
self?.throughputUpdateTimer = nil
} else {
self?.setUpThroughputRefreshTimer()
}
}
.store(in: &cancellables)
statusObserver.publisher.sink { [weak self] status in
self?.updateViewModel(withStatus: status)
}
.store(in: &cancellables)
}

private func setUpToggledStatePublisher() {
Expand Down Expand Up @@ -292,6 +270,31 @@ final class NetworkProtectionStatusViewModel: ObservableObject {
.store(in: &cancellables)
}

private func updateViewModel(withStatus connectionStatus: ConnectionStatus) {
self.headerTitle = Self.titleText(connected: connectionStatus.isConnected)
self.statusImageID = Self.statusImageID(connected: connectionStatus.isConnected)

if !connectionStatus.isConnected {
self.uploadTotal = nil
self.downloadTotal = nil
self.throughputUpdateTimer?.invalidate()
self.throughputUpdateTimer = nil
} else {
self.setUpThroughputRefreshTimer()
}

switch connectionStatus {
case .connected:
self.isNetPEnabled = true
case .connecting:
self.isNetPEnabled = true
self.resetConnectionInformation()
default:
self.isNetPEnabled = false
self.resetConnectionInformation()
}
}

private func setUpErrorPublishers() {
guard AppDependencyProvider.shared.internalUserDecider.isInternalUser else {
return
Expand Down Expand Up @@ -346,7 +349,7 @@ final class NetworkProtectionStatusViewModel: ObservableObject {
}

private func refreshDataVolumeTotals() async {
guard let activeSession = try? await ConnectionSessionUtilities.activeSession() else {
guard let activeSession = await tunnelController.activeSession() else {
return
}

Expand Down
Loading

0 comments on commit ba71738

Please sign in to comment.