From 29026e335233dbb32cb8b9721ef43d8ca2e09ec1 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Wed, 20 Mar 2024 20:58:00 -0700 Subject: [PATCH 1/4] Update the last version run store usage (#2441) Task/Issue URL: https://app.asana.com/0/414235014887631/1206860499985742/f Tech Design URL: CC: Description: This PR updates the last version run store to include up-to-date versions for both agent and extension. --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 4 +-- DuckDuckGo/LoginItems/LoginItemsManager.swift | 6 ++++ .../NetworkProtectionAppEvents.swift | 12 +++---- .../VPNMetadataCollector.swift | 8 +++-- .../NetworkExtensionController.swift | 9 ++++-- DuckDuckGoVPN/VPNAppEventsHandler.swift | 8 ++--- .../DataBrokerProtection/Package.swift | 2 +- .../NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- .../SystemExtensionManager.swift | 31 +++++++++++++++++-- .../VPNFeedbackFormViewModelTests.swift | 3 +- 12 files changed, 62 insertions(+), 27 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 9b45f377ad..3d58848dbb 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -14109,7 +14109,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 127.1.1; + version = 128.0.0; }; }; AA06B6B52672AF8100F541C5 /* XCRemoteSwiftPackageReference "Sparkle" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index b3a01e6cd1..ae1bff9287 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "1ba3e3682b9231a1d8685f1d16f950e7910b8593", - "version" : "127.1.1" + "revision" : "6c5ed334b3ea7b4394bec697067654182f75c877", + "version" : "128.0.0" } }, { diff --git a/DuckDuckGo/LoginItems/LoginItemsManager.swift b/DuckDuckGo/LoginItems/LoginItemsManager.swift index 8fef5f953b..e0a44a4fb4 100644 --- a/DuckDuckGo/LoginItems/LoginItemsManager.swift +++ b/DuckDuckGo/LoginItems/LoginItemsManager.swift @@ -59,6 +59,12 @@ final class LoginItemsManager { } } + func isAnyEnabled(_ items: Set) -> Bool { + return items.contains(where: { item in + item.status == .enabled + }) + } + private func handleError(for item: LoginItem, action: Action, error: NSError) { let event = Pixel.Event.Debug.loginItemUpdateError( loginItemBundleID: item.agentBundleID, diff --git a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionAppEvents.swift b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionAppEvents.swift index 2d240b6c0d..e639721f17 100644 --- a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionAppEvents.swift +++ b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionAppEvents.swift @@ -85,19 +85,15 @@ final class NetworkProtectionAppEvents { } private func restartNetworkProtectionIfVersionChanged(using loginItemsManager: LoginItemsManager) { - let versionStore = NetworkProtectionLastVersionRunStore() - - // should‘ve been run at least once with NetP enabled - guard versionStore.lastVersionRun != nil else { - os_log(.info, log: .networkProtection, "No last version found for the NetP login items, skipping update") - return - } - // We want to restart the VPN menu app to make sure it's always on the latest. restartNetworkProtectionMenu(using: loginItemsManager) } private func restartNetworkProtectionMenu(using loginItemsManager: LoginItemsManager) { + guard loginItemsManager.isAnyEnabled(LoginItemsManager.networkProtectionLoginItems) else { + return + } + loginItemsManager.restartLoginItems(LoginItemsManager.networkProtectionLoginItems, log: .networkProtection) } diff --git a/DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift b/DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift index 58849d4211..e1c071de4e 100644 --- a/DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift +++ b/DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift @@ -31,7 +31,8 @@ struct VPNMetadata: Encodable { struct AppInfo: Encodable { let appVersion: String - let lastVersionRun: String + let lastAgentVersionRun: String + let lastExtensionVersionRun: String let isInternalUser: Bool let isInApplicationsDirectory: Bool } @@ -154,13 +155,14 @@ final class DefaultVPNMetadataCollector: VPNMetadataCollector { private func collectAppInfoMetadata() -> VPNMetadata.AppInfo { let appVersion = AppVersion.shared.versionAndBuildNumber - let versionStore = NetworkProtectionLastVersionRunStore() + let versionStore = NetworkProtectionLastVersionRunStore(userDefaults: .netP) let isInternalUser = NSApp.delegateTyped.internalUserDecider.isInternalUser let isInApplicationsDirectory = Bundle.main.isInApplicationsDirectory return .init( appVersion: appVersion, - lastVersionRun: versionStore.lastVersionRun ?? "Unknown", + lastAgentVersionRun: versionStore.lastAgentVersionRun ?? "none", + lastExtensionVersionRun: versionStore.lastExtensionVersionRun ?? "none", isInternalUser: isInternalUser, isInApplicationsDirectory: isInApplicationsDirectory ) diff --git a/DuckDuckGoVPN/NetworkExtensionController.swift b/DuckDuckGoVPN/NetworkExtensionController.swift index 49960fdb8a..d850bfbb80 100644 --- a/DuckDuckGoVPN/NetworkExtensionController.swift +++ b/DuckDuckGoVPN/NetworkExtensionController.swift @@ -17,6 +17,7 @@ // import Foundation +import NetworkProtection import NetworkProtectionUI #if NETP_SYSTEM_EXTENSION @@ -32,11 +33,13 @@ final class NetworkExtensionController { #if NETP_SYSTEM_EXTENSION private let systemExtensionManager: SystemExtensionManager + private let defaults: UserDefaults #endif - init(extensionBundleID: String) { + init(extensionBundleID: String, defaults: UserDefaults = .netP) { #if NETP_SYSTEM_EXTENSION systemExtensionManager = SystemExtensionManager(extensionBundleID: extensionBundleID) + self.defaults = defaults #endif } @@ -46,9 +49,11 @@ extension NetworkExtensionController { func activateSystemExtension(waitingForUserApproval: @escaping () -> Void) async throws { #if NETP_SYSTEM_EXTENSION - try await systemExtensionManager.activate( + let extensionVersion = try await systemExtensionManager.activate( waitingForUserApproval: waitingForUserApproval) + NetworkProtectionLastVersionRunStore(userDefaults: defaults).lastExtensionVersionRun = extensionVersion + try? await Task.sleep(nanoseconds: 300 * NSEC_PER_MSEC) #endif } diff --git a/DuckDuckGoVPN/VPNAppEventsHandler.swift b/DuckDuckGoVPN/VPNAppEventsHandler.swift index 0dc1b0364f..5ac88e4a2b 100644 --- a/DuckDuckGoVPN/VPNAppEventsHandler.swift +++ b/DuckDuckGoVPN/VPNAppEventsHandler.swift @@ -30,20 +30,20 @@ final class VPNAppEventsHandler { func appDidFinishLaunching() { let currentVersion = AppVersion.shared.versionAndBuildNumber - let versionStore = NetworkProtectionLastVersionRunStore() + let versionStore = NetworkProtectionLastVersionRunStore(userDefaults: .netP) defer { - versionStore.lastVersionRun = currentVersion + versionStore.lastAgentVersionRun = currentVersion } let restartTunnel = { - os_log(.info, log: .networkProtection, "App updated from %{public}s to %{public}s: updating login items", versionStore.lastVersionRun ?? "null", currentVersion) + os_log(.info, log: .networkProtection, "App updated from %{public}s to %{public}s: updating login items", versionStore.lastAgentVersionRun ?? "null", currentVersion) self.restartTunnel() } #if DEBUG || REVIEW // Since DEBUG and REVIEW builds may not change version No. we want them to always reset. restartTunnel() #else - if versionStore.lastVersionRun != currentVersion { + if versionStore.lastAgentVersionRun != currentVersion { restartTunnel() } #endif diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 6cad03cbe8..9b47fd9774 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "127.1.1"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "128.0.0"), .package(path: "../PixelKit"), .package(path: "../SwiftUIExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index 8ae5ac757b..afa4ba1c24 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -31,7 +31,7 @@ let package = Package( .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "127.1.1"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "128.0.0"), .package(path: "../XPCHelper"), .package(path: "../SwiftUIExtensions"), .package(path: "../LoginItems"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 0e5e192074..1d35dab0e0 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "127.1.1"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "128.0.0"), .package(path: "../SwiftUIExtensions") ], targets: [ diff --git a/LocalPackages/SystemExtensionManager/Sources/SystemExtensionManager/SystemExtensionManager.swift b/LocalPackages/SystemExtensionManager/Sources/SystemExtensionManager/SystemExtensionManager.swift index 956b79e3af..6b9f61a851 100644 --- a/LocalPackages/SystemExtensionManager/Sources/SystemExtensionManager/SystemExtensionManager.swift +++ b/LocalPackages/SystemExtensionManager/Sources/SystemExtensionManager/SystemExtensionManager.swift @@ -44,7 +44,9 @@ public struct SystemExtensionManager { self.workspace = workspace } - public func activate(waitingForUserApproval: @escaping () -> Void) async throws { + /// - Returns: The system extension version. + /// + public func activate(waitingForUserApproval: @escaping () -> Void) async throws -> String { /// Documenting a workaround for the issue discussed in https://app.asana.com/0/0/1205275221447702/f /// Background: For a lot of users, the system won't show the system-extension-blocked alert if there's a previous request /// to activate the extension. You can see active requests in your console using command `systemextensionsctl list`. @@ -62,11 +64,14 @@ public struct SystemExtensionManager { openSystemSettingsSecurity() } - return try await SystemExtensionRequest.activationRequest( + let activationRequest = SystemExtensionRequest.activationRequest( forExtensionWithIdentifier: extensionBundleID, manager: manager, waitingForUserApproval: waitingForUserApproval) - .submit() + + try await activationRequest.submit() + + return activationRequest.version ?? "unknown" } public func deactivate() async throws { @@ -113,6 +118,7 @@ final class SystemExtensionRequest: NSObject { private let request: OSSystemExtensionRequest private let manager: OSSystemExtensionManager private let waitingForUserApproval: (() -> Void)? + private(set) var version: String? private var continuation: CheckedContinuation? @@ -144,12 +150,30 @@ final class SystemExtensionRequest: NSObject { manager.submitRequest(request) } } + + private func updateVersion(to version: String) { + self.version = version + } + + private func updateVersionNumberIfMissing() { + guard version == nil, + let versionString = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String else { + return + } + + var extensionVersion = versionString + + if let buildString = Bundle.main.infoDictionary?[kCFBundleVersionKey as String] as? String { + extensionVersion = extensionVersion + "." + buildString + } + } } extension SystemExtensionRequest: OSSystemExtensionRequestDelegate { func request(_ request: OSSystemExtensionRequest, actionForReplacingExtension existing: OSSystemExtensionProperties, withExtension ext: OSSystemExtensionProperties) -> OSSystemExtensionRequest.ReplacementAction { + updateVersion(to: ext.bundleShortVersion + "." + ext.bundleVersion) return .replace } @@ -160,6 +184,7 @@ extension SystemExtensionRequest: OSSystemExtensionRequestDelegate { func request(_ request: OSSystemExtensionRequest, didFinishWithResult result: OSSystemExtensionRequest.Result) { switch result { case .completed: + updateVersionNumberIfMissing() continuation?.resume() case .willCompleteAfterReboot: continuation?.resume(throwing: SystemExtensionRequestError.willActivateAfterReboot) diff --git a/UnitTests/VPNFeedbackForm/VPNFeedbackFormViewModelTests.swift b/UnitTests/VPNFeedbackForm/VPNFeedbackFormViewModelTests.swift index e4794c1972..5ad28ed209 100644 --- a/UnitTests/VPNFeedbackForm/VPNFeedbackFormViewModelTests.swift +++ b/UnitTests/VPNFeedbackForm/VPNFeedbackFormViewModelTests.swift @@ -87,7 +87,8 @@ private class MockVPNMetadataCollector: VPNMetadataCollector { let appInfo = VPNMetadata.AppInfo( appVersion: "1.2.3", - lastVersionRun: "1.2.3", + lastAgentVersionRun: "1.2.3", + lastExtensionVersionRun: "1.2.3", isInternalUser: false, isInApplicationsDirectory: true ) From cb3b645aca00ccf86bcfad8671523eff0645ad50 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Thu, 21 Mar 2024 20:31:09 +1100 Subject: [PATCH 2/4] Main thread issue for NetworkProtection nav bar icon update (#2466) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1201037661562251/1206894457777736/f **Description**: It seems that when the icon publisher fires for the network protection nav bar the app is not running on the main thread. This leads to accessing AppKit down the track and it will probably crash the App in release mode. Screenshot 2024-03-21 at 4 44 19 PM --- .../NetworkProtectionNavBarButtonModel.swift | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift index 057f5b622c..b31c8f007f 100644 --- a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift +++ b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift @@ -100,9 +100,11 @@ final class NetworkProtectionNavBarButtonModel: NSObject, ObservableObject { } private func setupIconSubscription() { - iconPublisherCancellable = iconPublisher.$icon.sink { [weak self] icon in - self?.buttonImage = self?.buttonImageFromWaitlistState(icon: icon) - } + iconPublisherCancellable = iconPublisher.$icon + .receive(on: DispatchQueue.main) + .sink { [weak self] icon in + self?.buttonImage = self?.buttonImageFromWaitlistState(icon: icon) + } } /// Temporary override used for the NetP waitlist beta, as a different asset is used for users who are invited to join the beta but haven't yet accepted. From 3034d50b49125dc0354ebf7371652a61271e2b94 Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Thu, 21 Mar 2024 10:15:15 -0500 Subject: [PATCH 3/4] Add additional parameters to breakage reports (#2416) Task/Issue URL: https://app.asana.com/0/414235014887631/1206842318329792/f Tech Design URL: CC: **Description**: **Steps to test this PR**: 1. See BSK PR --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 10 ++-- .../View/PrivacyDashboardViewController.swift | 54 +++++++++++++------ DuckDuckGo/Tab/Model/Tab.swift | 43 ++++++++++++++- .../DataBrokerProtection/Package.swift | 2 +- .../NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- .../BrokenSiteReportingReferenceTests.swift | 4 ++ .../WebsiteBreakageReportTests.swift | 8 +++ 9 files changed, 102 insertions(+), 25 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 3d58848dbb..49e591a245 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -14109,7 +14109,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 128.0.0; + version = 129.0.0; }; }; AA06B6B52672AF8100F541C5 /* XCRemoteSwiftPackageReference "Sparkle" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index ae1bff9287..142f6cb676 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "6c5ed334b3ea7b4394bec697067654182f75c877", - "version" : "128.0.0" + "revision" : "3a6d8f3f3fe278fb316fe927c4ef005a19d03d71", + "version" : "129.0.0" } }, { @@ -41,8 +41,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/content-scope-scripts", "state" : { - "revision" : "f6241631fc14cc2d0f47950bfdc4d6c30bf90130", - "version" : "5.4.0" + "revision" : "edd96481d49b094c260f9a79e078abdbdd3a83fb", + "version" : "5.6.0" } }, { @@ -165,7 +165,7 @@ { "identity" : "trackerradarkit", "kind" : "remoteSourceControl", - "location" : "https://github.com/duckduckgo/TrackerRadarKit", + "location" : "https://github.com/duckduckgo/TrackerRadarKit.git", "state" : { "revision" : "a6b7ba151d9dc6684484f3785293875ec01cc1ff", "version" : "1.2.2" diff --git a/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift b/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift index 6074ac2343..4a5d5113f3 100644 --- a/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift +++ b/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift @@ -258,11 +258,13 @@ extension PrivacyDashboardViewController: PrivacyDashboardReportBrokenSiteDelega didRequestSubmitBrokenSiteReportWithCategory category: String, description: String) { let source: BrokenSiteReport.Source = privacyDashboardController.initDashboardMode == .report ? .appMenu : .dashboard - do { - let report = try makeBrokenSiteReport(category: category, description: description, source: source) - try brokenSiteReporter.report(report, reportMode: .regular) - } catch { - os_log("Failed to generate or send the broken site report: \(error.localizedDescription)", type: .error) + Task { @MainActor in + do { + let report = try await makeBrokenSiteReport(category: category, description: description, source: source) + try brokenSiteReporter.report(report, reportMode: .regular) + } catch { + os_log("Failed to generate or send the broken site report: \(error.localizedDescription)", type: .error) + } } } @@ -281,13 +283,15 @@ extension PrivacyDashboardViewController: PrivacyDashboardToggleReportDelegate { didRequestSubmitToggleReportWithSource source: BrokenSiteReport.Source, didOpenReportInfo: Bool, toggleReportCounter: Int?) { - do { - let report = try makeBrokenSiteReport(source: source, - didOpenReportInfo: didOpenReportInfo, - toggleReportCounter: toggleReportCounter) - try toggleProtectionsOffReporter.report(report, reportMode: .toggle) - } catch { - os_log("Failed to generate or send the broken site report: %@", type: .error, error.localizedDescription) + Task { @MainActor in + do { + let report = try await makeBrokenSiteReport(source: source, + didOpenReportInfo: didOpenReportInfo, + toggleReportCounter: toggleReportCounter) + try toggleProtectionsOffReporter.report(report, reportMode: .toggle) + } catch { + os_log("Failed to generate or send the broken site report: %@", type: .error, error.localizedDescription) + } } } @@ -301,11 +305,25 @@ extension PrivacyDashboardViewController { case failedToFetchTheCurrentURL } + private func calculateWebVitals(performanceMetrics: PerformanceMetricsSubfeature?, privacyConfig: PrivacyConfiguration) async -> [Double]? { + var webVitalsResult: [Double]? + if privacyConfig.isEnabled(featureKey: .performanceMetrics) { + webVitalsResult = await withCheckedContinuation({ continuation in + guard let performanceMetrics else { continuation.resume(returning: nil); return } + performanceMetrics.notifyHandler { result in + continuation.resume(returning: result) + } + }) + } + + return webVitalsResult + } + private func makeBrokenSiteReport(category: String = "", description: String = "", source: BrokenSiteReport.Source, didOpenReportInfo: Bool = false, - toggleReportCounter: Int? = nil) throws -> BrokenSiteReport { + toggleReportCounter: Int? = nil) async throws -> BrokenSiteReport { // ⚠️ To limit privacy risk, site URL is trimmed to not include query and fragment guard let currentTab = tabViewModel?.tab, @@ -321,12 +339,14 @@ extension PrivacyDashboardViewController { let configuration = ContentBlocking.shared.privacyConfigurationManager.privacyConfig let protectionsState = configuration.isFeature(.contentBlocking, enabledForDomain: currentTab.content.url?.host) + let webVitals = await calculateWebVitals(performanceMetrics: currentTab.performanceMetrics, privacyConfig: configuration) + var errors: [Error]? var statusCodes: [Int]? - if let error = tabViewModel?.tab.lastWebError { + if let error = currentTab.lastWebError { errors = [error] } - if let httpStatusCode = tabViewModel?.tab.lastHttpStatusCode { + if let httpStatusCode = currentTab.lastHttpStatusCode { statusCodes = [httpStatusCode] } @@ -346,6 +366,10 @@ extension PrivacyDashboardViewController { reportFlow: source, errors: errors, httpStatusCodes: statusCodes, + openerContext: currentTab.inferredOpenerContext, + vpnOn: currentTab.tunnelController?.isConnected ?? false, + jsPerformance: webVitals, + userRefreshCount: currentTab.refreshCountSinceLoad, didOpenReportInfo: didOpenReportInfo, toggleReportCounter: toggleReportCounter) return websiteBreakage diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index accb86d699..ad05b12141 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -25,6 +25,7 @@ import Navigation import UserScript import WebKit import History +import PrivacyDashboard #if SUBSCRIPTION import Subscription @@ -344,7 +345,7 @@ protocol NewWindowPolicyDecisionMaker { let pinnedTabsManager: PinnedTabsManager #if NETWORK_PROTECTION - private var tunnelController: NetworkProtectionIPCTunnelController? + private(set) var tunnelController: NetworkProtectionIPCTunnelController? #endif private let webViewConfiguration: WKWebViewConfiguration @@ -779,6 +780,10 @@ protocol NewWindowPolicyDecisionMaker { @Published private(set) var lastWebError: Error? @Published private(set) var lastHttpStatusCode: Int? + @Published private(set) var inferredOpenerContext: BrokenSiteReport.OpenerContext? + @Published private(set) var refreshCountSinceLoad: Int = 0 + private (set) var performanceMetrics: PerformanceMetricsSubfeature? + @Published private(set) var isLoading: Bool = false @Published private(set) var loadingProgress: Double = 0.0 @@ -1028,6 +1033,8 @@ protocol NewWindowPolicyDecisionMaker { return nil } + refreshCountSinceLoad += 1 + self.content = content.forceReload() if webView.url == nil, content.isUrl { // load from cache or interactionStateData when called by lazy loader @@ -1287,6 +1294,9 @@ extension Tab: UserContentControllerDelegate { userScripts.faviconScript.delegate = self userScripts.pageObserverScript.delegate = self userScripts.printingUserScript.delegate = self + + performanceMetrics = PerformanceMetricsSubfeature(targetWebview: webView) + userScripts.contentScopeUserScriptIsolated.registerSubfeature(delegate: performanceMetrics!) } } @@ -1357,11 +1367,32 @@ extension Tab/*: NavigationResponder*/ { // to be moved to Tab+Navigation.swift committedURL = navigation.url } + func resetRefreshCountIfNeeded(action: NavigationAction) { + switch action.navigationType { + case .reload, .other: + break + default: + refreshCountSinceLoad = 0 + } + } + + func setOpenerContextIfNeeded(action: NavigationAction) { + switch action.navigationType { + case .linkActivated, .formSubmitted: + inferredOpenerContext = .navigation + default: + break + } + } + @MainActor func decidePolicy(for navigationAction: NavigationAction, preferences: inout NavigationPreferences) async -> NavigationActionPolicy? { // allow local file navigations if navigationAction.url.isFileURL { return .allow } + resetRefreshCountIfNeeded(action: navigationAction) + setOpenerContextIfNeeded(action: navigationAction) + // when navigating to a URL with basic auth username/password, cache it and redirect to a trimmed URL if let mainFrame = navigationAction.mainFrameTarget, let credential = navigationAction.url.basicAuthCredential { @@ -1416,6 +1447,10 @@ extension Tab/*: NavigationResponder*/ { // to be moved to Tab+Navigation.swift error = nil } + if inferredOpenerContext != .external { + inferredOpenerContext = nil + } + invalidateInteractionStateData() } @@ -1425,6 +1460,12 @@ extension Tab/*: NavigationResponder*/ { // to be moved to Tab+Navigation.swift webViewDidFinishNavigationPublisher.send() statisticsLoader?.refreshRetentionAtb(isSearch: navigation.url.isDuckDuckGoSearch) + Task { @MainActor in + if await webView.isCurrentSiteReferredFromDuckDuckGo { + inferredOpenerContext = .serp + } + } + #if NETWORK_PROTECTION if navigation.url.isDuckDuckGoSearch, tunnelController?.isConnected == true { DailyPixel.fire(pixel: .networkProtectionEnabledOnSearch, frequency: .dailyAndCount) diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 9b47fd9774..d604410732 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "128.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "129.0.0"), .package(path: "../PixelKit"), .package(path: "../SwiftUIExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index afa4ba1c24..6cca70cf1d 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -31,7 +31,7 @@ let package = Package( .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "128.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "129.0.0"), .package(path: "../XPCHelper"), .package(path: "../SwiftUIExtensions"), .package(path: "../LoginItems"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 1d35dab0e0..fea582b151 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "128.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "129.0.0"), .package(path: "../SwiftUIExtensions") ], targets: [ diff --git a/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift b/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift index 7adfb26758..bc52a33041 100644 --- a/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift +++ b/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift @@ -90,6 +90,10 @@ final class BrokenSiteReportingReferenceTests: XCTestCase { reportFlow: .appMenu, errors: errors, httpStatusCodes: test.httpErrorCodes ?? [], + openerContext: nil, + vpnOn: false, + jsPerformance: nil, + userRefreshCount: 0, didOpenReportInfo: false, toggleReportCounter: nil) diff --git a/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift b/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift index 6848c4c481..c5c389feac 100644 --- a/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift +++ b/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift @@ -47,6 +47,10 @@ class WebsiteBreakageReportTests: XCTestCase { reportFlow: .appMenu, errors: nil, httpStatusCodes: nil, + openerContext: nil, + vpnOn: false, + jsPerformance: nil, + userRefreshCount: 0, didOpenReportInfo: false, toggleReportCounter: nil ) @@ -92,6 +96,10 @@ class WebsiteBreakageReportTests: XCTestCase { reportFlow: .appMenu, errors: nil, httpStatusCodes: nil, + openerContext: nil, + vpnOn: false, + jsPerformance: nil, + userRefreshCount: 0, didOpenReportInfo: false, toggleReportCounter: nil ) From e7d089799efc1b4549a0df2887569b7953b869b5 Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Thu, 21 Mar 2024 16:35:56 +0100 Subject: [PATCH 4/4] Bug fixes for refreshing and updating subscription state (#2473) Task/Issue URL: https://app.asana.com/0/1199230911884351/1206896697354395/f **Description**: - when closing DBP tab on subscription account sign out make it on main thread - update cached subscription and entitlements on app launch - hide "View Plans" button on expired subscription state as it is not fully handled - more coherent refresh of subscription settings --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- DuckDuckGo/Application/AppDelegate.swift | 15 +-- .../Tab/View/BrowserTabViewController.swift | 4 +- .../PreferencesSubscriptionModel.swift | 101 ++++++++---------- .../PreferencesSubscriptionView.swift | 5 +- 4 files changed, 53 insertions(+), 72 deletions(-) diff --git a/DuckDuckGo/Application/AppDelegate.swift b/DuckDuckGo/Application/AppDelegate.swift index 1afd63ec5d..fdaaca695a 100644 --- a/DuckDuckGo/Application/AppDelegate.swift +++ b/DuckDuckGo/Application/AppDelegate.swift @@ -261,19 +261,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, FileDownloadManagerDel Task { let accountManager = AccountManager() - do { - try accountManager.migrateAccessTokenToNewStore() - } catch { - if let error = error as? AccountManager.MigrationError { - switch error { - case AccountManager.MigrationError.migrationFailed: - os_log(.default, log: .subscription, "Access token migration failed") - case AccountManager.MigrationError.noMigrationNeeded: - os_log(.default, log: .subscription, "No access token migration needed") - } - } + if let token = accountManager.accessToken { + _ = await SubscriptionService.getSubscription(accessToken: token, cachePolicy: .reloadIgnoringLocalCacheData) + _ = await accountManager.fetchEntitlements(cachePolicy: .reloadIgnoringLocalCacheData) } - await accountManager.checkSubscriptionState() } #endif diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index eccbde153a..ad103788cb 100644 --- a/DuckDuckGo/Tab/View/BrowserTabViewController.swift +++ b/DuckDuckGo/Tab/View/BrowserTabViewController.swift @@ -199,7 +199,9 @@ final class BrowserTabViewController: NSViewController { #if DBP @objc private func onDBPFeatureDisabled(_ notification: Notification) { - tabCollectionViewModel.removeAll(with: .dataBrokerProtection) + Task { @MainActor in + tabCollectionViewModel.removeAll(with: .dataBrokerProtection) + } } @objc diff --git a/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionModel.swift b/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionModel.swift index c2cd52eba6..ff2628ae29 100644 --- a/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionModel.swift +++ b/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionModel.swift @@ -95,35 +95,10 @@ public final class PreferencesSubscriptionModel: ObservableObject { self.isUserAuthenticated = accountManager.isUserAuthenticated - if let token = accountManager.accessToken { + if accountManager.isUserAuthenticated { Task { - let subscriptionResult = await SubscriptionService.getSubscription(accessToken: token, cachePolicy: .returnCacheDataElseLoad) - if case .success(let subscription) = subscriptionResult { - self.updateDescription(for: subscription.expiresOrRenewsAt, status: subscription.status, period: subscription.billingPeriod) - self.subscriptionPlatform = subscription.platform - self.subscriptionStatus = subscription.status - } - - switch await self.accountManager.hasEntitlement(for: .networkProtection, cachePolicy: .returnCacheDataElseLoad) { - case let .success(result): - self.hasAccessToVPN = result - case .failure: - self.hasAccessToVPN = false - } - - switch await self.accountManager.hasEntitlement(for: .dataBrokerProtection, cachePolicy: .returnCacheDataElseLoad) { - case let .success(result): - self.hasAccessToDBP = result - case .failure: - self.hasAccessToDBP = false - } - - switch await self.accountManager.hasEntitlement(for: .identityTheftRestoration, cachePolicy: .returnCacheDataElseLoad) { - case let .success(result): - self.hasAccessToITR = result - case .failure: - self.hasAccessToITR = false - } + await self.updateSubscription(with: .returnCacheDataElseLoad) + await self.updateAllEntitlement(with: .returnCacheDataElseLoad) } } @@ -264,6 +239,8 @@ public final class PreferencesSubscriptionModel: ObservableObject { @MainActor func fetchAndUpdateSubscriptionDetails() { + self.isUserAuthenticated = accountManager.isUserAuthenticated + guard fetchSubscriptionDetailsTask == nil else { return } fetchSubscriptionDetailsTask = Task { [weak self] in @@ -271,43 +248,53 @@ public final class PreferencesSubscriptionModel: ObservableObject { self?.fetchSubscriptionDetailsTask = nil } - guard let token = self?.accountManager.accessToken else { return } + await self?.updateSubscription(with: .reloadIgnoringLocalCacheData) + await self?.updateAllEntitlement(with: .reloadIgnoringLocalCacheData) + } + } - let subscriptionResult = await SubscriptionService.getSubscription(accessToken: token, cachePolicy: .reloadIgnoringLocalCacheData) + @MainActor + private func updateSubscription(with cachePolicy: SubscriptionService.CachePolicy) async { + guard let token = accountManager.accessToken else { + SubscriptionService.signOut() + return + } - if case .success(let subscription) = subscriptionResult { - self?.updateDescription(for: subscription.expiresOrRenewsAt, status: subscription.status, period: subscription.billingPeriod) - self?.subscriptionPlatform = subscription.platform - self?.subscriptionStatus = subscription.status - } else { - self?.accountManager.signOut() - } + switch await SubscriptionService.getSubscription(accessToken: token, cachePolicy: cachePolicy) { + case .success(let subscription): + updateDescription(for: subscription.expiresOrRenewsAt, status: subscription.status, period: subscription.billingPeriod) + subscriptionPlatform = subscription.platform + subscriptionStatus = subscription.status + case .failure: + break + } + } - if let self { - switch await self.accountManager.hasEntitlement(for: .networkProtection) { - case let .success(result): - hasAccessToVPN = result - case .failure: - hasAccessToVPN = false - } + @MainActor + private func updateAllEntitlement(with cachePolicy: AccountManager.CachePolicy) async { + switch await self.accountManager.hasEntitlement(for: .networkProtection, cachePolicy: cachePolicy) { + case let .success(result): + hasAccessToVPN = result + case .failure: + hasAccessToVPN = false + } - switch await self.accountManager.hasEntitlement(for: .dataBrokerProtection) { - case let .success(result): - hasAccessToDBP = result - case .failure: - hasAccessToDBP = false - } + switch await self.accountManager.hasEntitlement(for: .dataBrokerProtection, cachePolicy: cachePolicy) { + case let .success(result): + hasAccessToDBP = result + case .failure: + hasAccessToDBP = false + } - switch await self.accountManager.hasEntitlement(for: .identityTheftRestoration) { - case let .success(result): - hasAccessToITR = result - case .failure: - hasAccessToITR = false - } - } + switch await self.accountManager.hasEntitlement(for: .identityTheftRestoration, cachePolicy: cachePolicy) { + case let .success(result): + hasAccessToITR = result + case .failure: + hasAccessToITR = false } } + @MainActor func updateDescription(for date: Date, status: Subscription.Status, period: Subscription.BillingPeriod) { let formattedDate = dateFormatter.string(from: date) diff --git a/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionView.swift b/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionView.swift index 5424f3920b..52a723b011 100644 --- a/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionView.swift +++ b/LocalPackages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionView.swift @@ -185,8 +185,9 @@ public struct PreferencesSubscriptionView: View { TextMenuItemHeader(model.subscriptionDetails ?? UserText.preferencesSubscriptionInactiveHeader) TextMenuItemCaption(UserText.preferencesSubscriptionExpiredCaption) } buttons: { - Button(UserText.viewPlansButtonTitle) { model.purchaseAction() } - .buttonStyle(DefaultActionButtonStyle(enabled: true)) + // We need to improve re-purchase flow + /* Button(UserText.viewPlansButtonTitle) { model.purchaseAction() } + .buttonStyle(DefaultActionButtonStyle(enabled: true)) */ Menu { Button(UserText.removeFromThisDeviceButton, action: { model.userEventHandler(.removeSubscriptionClick)