Skip to content

Commit 594a499

Browse files
authored
Refactor FXIOS-13532 [Swift 6 Migration] NotificationManager strict concurrency warnings (#29412)
* NotificationManager swift migration * Fix mock * Adjust notification manager telemetry to remove from telemetry wrapper * Adjust to be async instead of completion handlers * Swiftlint * Fixing the unit test - created a task to come back to this * Oups
1 parent 3cb71d1 commit 594a499

16 files changed

+199
-213
lines changed

firefox-ios/Client.xcodeproj/project.pbxproj

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,7 @@
877877
8A1A93592B757C7C0069C190 /* landscape.json in Resources */ = {isa = PBXBuildFile; fileRef = 8A1A93532B757C7B0069C190 /* landscape.json */; };
878878
8A1A935A2B757C7C0069C190 /* portrait.json in Resources */ = {isa = PBXBuildFile; fileRef = 8A1A93542B757C7B0069C190 /* portrait.json */; };
879879
8A1A935B2B757C7C0069C190 /* wave.json in Resources */ = {isa = PBXBuildFile; fileRef = 8A1A93552B757C7B0069C190 /* wave.json */; };
880+
8A1AB6BB2E7B8D5C00167FF5 /* NotificationManagerTelemetry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A1AB6BA2E7B8D5C00167FF5 /* NotificationManagerTelemetry.swift */; };
880881
8A1C0B8F2E723EDF0030DCC8 /* AppLaunchUtilTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A1C0B8E2E723EDC0030DCC8 /* AppLaunchUtilTests.swift */; };
881882
8A1CBB952BE017D3008BE4D4 /* MicrosurveyPromptAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A1CBB942BE017D3008BE4D4 /* MicrosurveyPromptAction.swift */; };
882883
8A1CBB972BE0182C008BE4D4 /* MicrosurveyPromptMiddleware.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A1CBB962BE0182C008BE4D4 /* MicrosurveyPromptMiddleware.swift */; };
@@ -1237,6 +1238,7 @@
12371238
8AD40FD127BADCBA00672675 /* ToolbarButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AD40FD027BADCBA00672675 /* ToolbarButton.swift */; };
12381239
8AD40FD327BB068F00672675 /* MainMenuActionHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AD40FD227BB068F00672675 /* MainMenuActionHelper.swift */; };
12391240
8AD40FD527BB1C1000672675 /* LockButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AD40FD427BB1C1000672675 /* LockButton.swift */; };
1241+
8ADA19A22E7DB86400E95E5E /* NotificationManagerTelemetryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADA19A12E7DB86400E95E5E /* NotificationManagerTelemetryTests.swift */; };
12401242
8ADAA6D42CD175FB00E3D3E8 /* StickersCatalog.xcstickers in Resources */ = {isa = PBXBuildFile; fileRef = 8ADAA6D32CD175FB00E3D3E8 /* StickersCatalog.xcstickers */; };
12411243
8ADAE41E2A33A0E2007BF926 /* ShowIntroductionSetting.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADAE41D2A33A0E2007BF926 /* ShowIntroductionSetting.swift */; };
12421244
8ADAE4202A33A0FD007BF926 /* SendFeedbackSetting.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADAE41F2A33A0FD007BF926 /* SendFeedbackSetting.swift */; };
@@ -8605,6 +8607,7 @@
86058607
8A1A93532B757C7B0069C190 /* landscape.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = landscape.json; sourceTree = "<group>"; };
86068608
8A1A93542B757C7B0069C190 /* portrait.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = portrait.json; sourceTree = "<group>"; };
86078609
8A1A93552B757C7B0069C190 /* wave.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = wave.json; sourceTree = "<group>"; };
8610+
8A1AB6BA2E7B8D5C00167FF5 /* NotificationManagerTelemetry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationManagerTelemetry.swift; sourceTree = "<group>"; };
86088611
8A1C0B8E2E723EDC0030DCC8 /* AppLaunchUtilTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppLaunchUtilTests.swift; sourceTree = "<group>"; };
86098612
8A1CBB942BE017D3008BE4D4 /* MicrosurveyPromptAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MicrosurveyPromptAction.swift; sourceTree = "<group>"; };
86108613
8A1CBB962BE0182C008BE4D4 /* MicrosurveyPromptMiddleware.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MicrosurveyPromptMiddleware.swift; sourceTree = "<group>"; };
@@ -8948,6 +8951,7 @@
89488951
8AD40FD227BB068F00672675 /* MainMenuActionHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MainMenuActionHelper.swift; sourceTree = "<group>"; };
89498952
8AD40FD427BB1C1000672675 /* LockButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LockButton.swift; sourceTree = "<group>"; };
89508953
8AD440CBBF202B15E74E186D /* ur */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ur; path = ur.lproj/Search.strings; sourceTree = "<group>"; };
8954+
8ADA19A12E7DB86400E95E5E /* NotificationManagerTelemetryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationManagerTelemetryTests.swift; sourceTree = "<group>"; };
89518955
8ADAA6D32CD175FB00E3D3E8 /* StickersCatalog.xcstickers */ = {isa = PBXFileReference; lastKnownFileType = folder.stickers; path = StickersCatalog.xcstickers; sourceTree = "<group>"; };
89528956
8ADAE41D2A33A0E2007BF926 /* ShowIntroductionSetting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShowIntroductionSetting.swift; sourceTree = "<group>"; };
89538957
8ADAE41F2A33A0FD007BF926 /* SendFeedbackSetting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SendFeedbackSetting.swift; sourceTree = "<group>"; };
@@ -13526,6 +13530,7 @@
1352613530
8AC225632B6D3F9600CDA7FD /* Telemetry */ = {
1352713531
isa = PBXGroup;
1352813532
children = (
13533+
8ADA19A12E7DB86400E95E5E /* NotificationManagerTelemetryTests.swift */,
1352913534
C7F051572D9F098200EC52C0 /* ContextMenuTelemetryTests.swift */,
1353013535
8A87E98B2E70792F006F0239 /* FxSuggestTelemetryTests.swift */,
1353113536
C7F0514F2D95EA5C00EC52C0 /* HistoryDeletionUtilityTelemetryTests.swift */,
@@ -15592,6 +15597,7 @@
1559215597
E69DB0B91E97E301008A67E6 /* Telemetry */ = {
1559315598
isa = PBXGroup;
1559415599
children = (
15600+
8A1AB6BA2E7B8D5C00167FF5 /* NotificationManagerTelemetry.swift */,
1559515601
8A732A812DE8AFCA0099F575 /* FxSuggestTelemetry.swift */,
1559615602
8A0DF2C12D6D1C670066EC00 /* AutoplaySettingTelemetry.swift */,
1559715603
43175DB726B87D2C00C41C31 /* AdsTelemetryHelper.swift */,
@@ -18478,6 +18484,7 @@
1847818484
63F7A9AA2C7529ED005846F5 /* NativeErrorPageModel.swift in Sources */,
1847918485
8A6E63CF2D4A7FB70040D355 /* SectionHeaderConfiguration.swift in Sources */,
1848018486
63F7A9AC2C752BB0005846F5 /* NativeErrorPageViewController.swift in Sources */,
18487+
8A1AB6BB2E7B8D5C00167FF5 /* NotificationManagerTelemetry.swift in Sources */,
1848118488
0E65573E2D81CD1A00D7F017 /* DownloadQueue.swift in Sources */,
1848218489
C88E7A572A0553360072E638 /* OnboardingButtonInfoModel.swift in Sources */,
1848318490
AB9A0C012C6CBC9100BFA22A /* TrackingProtectionDetailsViewController.swift in Sources */,
@@ -19227,6 +19234,7 @@
1922719234
212985E72A72B39D00546684 /* ThemeSettingsControllerTests.swift in Sources */,
1922819235
8A9E46BD2A6599E5003327D4 /* MockStatusBarScrollDelegate.swift in Sources */,
1922919236
216424D42E56175100A3AF40 /* MockTabScrollHandlerDelegate.swift in Sources */,
19237+
8ADA19A22E7DB86400E95E5E /* NotificationManagerTelemetryTests.swift in Sources */,
1923019238
8A1A66072CDD168A00DD883C /* MockTopSiteHistoryManager.swift in Sources */,
1923119239
8A5189C92C1B614E00CDB668 /* SearchViewModelTests.swift in Sources */,
1923219240
E1463D0629830E4F0074E16E /* MockUserNotificationCenter.swift in Sources */,

firefox-ios/Client/Application/BackgroundNotificationSurfaceUtility.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class BackgroundNotificationSurfaceUtility: BackgroundUtilityProtocol {
4949
let hasPermission = await notificationManager.hasPermission()
5050

5151
if hasPermission, surfaceManager.shouldShowSurface {
52-
surfaceManager.showNotificationSurface()
52+
await surfaceManager.showNotificationSurface()
5353
}
5454
}
5555

firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,9 +1053,11 @@ class BrowserViewController: UIViewController,
10531053
subscribeToRedux()
10541054
enqueueTabRestoration()
10551055

1056+
// FXIOS-13551 - testWillNavigateAway calls into viewDidLoad during unit tests, creates a leak
1057+
guard !AppConstants.isRunningUnitTest else { return }
10561058
Task(priority: .background) { [weak self] in
10571059
// App startup telemetry accesses RustLogins to queryLogins, shouldn't be on the app startup critical path
1058-
self?.trackStartupTelemetry()
1060+
await self?.trackStartupTelemetry()
10591061
}
10601062
}
10611063

@@ -5123,11 +5125,11 @@ extension BrowserViewController: DevicePickerViewControllerDelegate, Instruction
51235125
}
51245126

51255127
extension BrowserViewController {
5126-
func trackStartupTelemetry() {
5128+
func trackStartupTelemetry() async {
51275129
let toolbarLocation: SearchBarPosition = self.isBottomSearchBar ? .bottom : .top
51285130
SearchBarSettingsViewModel.recordLocationTelemetry(for: toolbarLocation)
51295131
trackAccessibility()
5130-
trackNotificationPermission()
5132+
await trackNotificationPermission()
51315133
appStartupTelemetry.sendStartupTelemetry()
51325134
creditCardInitialSetupTelemetry()
51335135
}
@@ -5178,7 +5180,7 @@ extension BrowserViewController {
51785180
}
51795181
}
51805182

5181-
func trackNotificationPermission() {
5182-
NotificationManager().getNotificationSettings(sendTelemetry: true) { _ in }
5183+
func trackNotificationPermission() async {
5184+
_ = await NotificationManager().getNotificationSettings(sendTelemetry: true)
51835185
}
51845186
}

firefox-ios/Client/Frontend/NotificationSurface/NotificationSurfaceManager.swift

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,17 @@ class NotificationSurfaceManager: NotificationSurfaceDelegate {
5050
// MARK: - Functionality
5151
/// Checks whether a message exists, and is not expired, and schedules
5252
/// a notification to be presented.
53-
func showNotificationSurface() {
53+
func showNotificationSurface() async {
5454
guard let message = message, !message.isExpired else { return }
5555

5656
let notificationId = Constant.notificationBaseId + ".\(message.id)"
5757

5858
// Check if message is already getting displayed
59-
notificationManager.findDeliveredNotificationForId(id: notificationId) { [weak self] notification in
60-
// Don't schedule the notification again if it was already delivered
61-
guard notification == nil else { return }
59+
let notification = await notificationManager.findDeliveredNotificationForId(id: notificationId)
60+
// Don't schedule the notification again if it was already delivered
61+
guard notification == nil else { return }
6262

63-
self?.scheduleNotification(message: message, notificationId: notificationId)
64-
}
63+
scheduleNotification(message: message, notificationId: notificationId)
6564
}
6665

6766
// MARK: NotificationSurfaceDelegate

firefox-ios/Client/NotificationManager.swift

Lines changed: 34 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@ import UserNotifications
88
import Shared
99

1010
protocol NotificationManagerProtocol {
11-
func requestAuthorization(completion: @escaping (Bool, Error?) -> Void)
12-
func requestAuthorization(completion: @escaping (Result<Bool, Error>) -> Void)
11+
func requestAuthorization(completion: @escaping @Sendable (Bool, Error?) -> Void)
12+
func requestAuthorization(completion: @escaping @Sendable (Result<Bool, Error>) -> Void)
1313
func requestAuthorization() async throws -> Bool
14-
func getNotificationSettings(sendTelemetry: Bool, completion: @escaping (UNNotificationSettings) -> Void)
1514
func getNotificationSettings(sendTelemetry: Bool) async -> UNNotificationSettings
16-
func hasPermission(completion: @escaping (Bool) -> Void)
1715
func hasPermission() async -> Bool
1816
func schedule(title: String,
1917
body: String,
@@ -29,37 +27,33 @@ protocol NotificationManagerProtocol {
2927
categoryIdentifier: String,
3028
interval: TimeInterval,
3129
repeats: Bool)
32-
func findDeliveredNotifications(completion: @escaping ([UNNotification]) -> Void)
33-
func findDeliveredNotificationForId(id: String, completion: @escaping (UNNotification?) -> Void)
30+
func findDeliveredNotifications() async -> [UNNotification]
31+
func findDeliveredNotificationForId(id: String) async -> UNNotification?
3432
func removeAllPendingNotifications()
3533
func removePendingNotificationsWithId(ids: [String])
3634
}
3735

3836
class NotificationManager: NotificationManagerProtocol {
37+
private let telemetry: NotificationManagerTelemetry
3938
private var center: UserNotificationCenterProtocol
4039

41-
init(center: UserNotificationCenterProtocol = UNUserNotificationCenter.current()) {
40+
init(center: UserNotificationCenterProtocol = UNUserNotificationCenter.current(),
41+
telemetry: NotificationManagerTelemetry = NotificationManagerTelemetry()) {
4242
self.center = center
43+
self.telemetry = telemetry
4344
}
4445

4546
// Requests the user’s authorization to allow local and remote notifications and sends Telemetry
46-
func requestAuthorization(completion: @escaping (Bool, Error?) -> Void) {
47+
func requestAuthorization(completion: @escaping @Sendable (Bool, Error?) -> Void) {
4748
center.requestAuthorization(options: [.alert, .badge, .sound]) { (granted, error) in
4849
completion(granted, error)
4950

50-
guard !AppConstants.isRunningUnitTest else { return }
51-
52-
let extras = [TelemetryWrapper.EventExtraKey.notificationPermissionIsGranted.rawValue:
53-
granted]
54-
TelemetryWrapper.recordEvent(category: .prompt,
55-
method: .tap,
56-
object: .notificationPermission,
57-
extras: extras)
51+
self.telemetry.sendNotificationPermissionPrompt(isPermissionGranted: granted)
5852
}
5953
}
6054

6155
@available(*, renamed: "requestAuthorization()")
62-
func requestAuthorization(completion: @escaping (Result<Bool, Error>) -> Void) {
56+
func requestAuthorization(completion: @escaping @Sendable (Result<Bool, Error>) -> Void) {
6357
self.requestAuthorization { granted, error in
6458
if let error = error {
6559
completion(.failure(error))
@@ -78,56 +72,29 @@ class NotificationManager: NotificationManagerProtocol {
7872
}
7973

8074
// Retrieves the authorization and feature-related notification settings and sends Telemetry
81-
func getNotificationSettings(sendTelemetry: Bool = false,
82-
completion: @escaping (UNNotificationSettings) -> Void) {
83-
center.getNotificationSettings { settings in
84-
completion(settings)
85-
86-
guard sendTelemetry else { return }
87-
self.sendTelemetry(settings: settings)
88-
}
89-
}
90-
9175
func getNotificationSettings(sendTelemetry: Bool = false) async -> UNNotificationSettings {
92-
return await withCheckedContinuation { continuation in
93-
getNotificationSettings(sendTelemetry: sendTelemetry) { result in
94-
continuation.resume(returning: result)
95-
}
96-
}
97-
}
76+
let settings = await center.notificationSettings()
9877

99-
// Determines if the user has allowed notifications
100-
func hasPermission(completion: @escaping (Bool) -> Void) {
101-
getNotificationSettings { settings in
102-
var hasPermission = false
103-
switch settings.authorizationStatus {
104-
case .authorized, .ephemeral, .provisional:
105-
hasPermission = true
106-
case .notDetermined, .denied:
107-
fallthrough
108-
@unknown default:
109-
hasPermission = false
110-
}
111-
completion(hasPermission)
78+
if sendTelemetry {
79+
telemetry.sendNotificationPermission(settings: settings)
11280
}
81+
82+
return settings
11383
}
11484

11585
// Determines if the user has allowed notifications
11686
func hasPermission() async -> Bool {
117-
// FXIOS-11895 Temporary test for reverting continuation workaround we put in place for iOS 18.0 (beta?) users
118-
if ContinuationsChecker.shouldUseCheckedContinuation {
119-
await withCheckedContinuation { continuation in
120-
hasPermission { hasPermission in
121-
continuation.resume(returning: hasPermission)
122-
}
123-
}
124-
} else {
125-
await withUnsafeContinuation { continuation in
126-
hasPermission { hasPermission in
127-
continuation.resume(returning: hasPermission)
128-
}
129-
}
87+
let settings = await getNotificationSettings()
88+
var hasPermission = false
89+
switch settings.authorizationStatus {
90+
case .authorized, .ephemeral, .provisional:
91+
hasPermission = true
92+
case .notDetermined, .denied:
93+
fallthrough
94+
@unknown default:
95+
hasPermission = false
13096
}
97+
return hasPermission
13198
}
13299

133100
// Scheduling push notification based on the Date trigger (Ex 25 December at 10:00PM)
@@ -169,21 +136,17 @@ class NotificationManager: NotificationManagerProtocol {
169136
}
170137

171138
// Fetches all delivered notifications that are still present in Notification Center.
172-
func findDeliveredNotifications(completion: @escaping ([UNNotification]) -> Void) {
173-
center.getDeliveredNotifications { notificationList in
174-
completion(notificationList)
175-
}
139+
func findDeliveredNotifications() async -> [UNNotification] {
140+
return await center.deliveredNotifications()
176141
}
177142

178143
// Fetches all delivered notifications that are still present in Notification Center by id
179-
func findDeliveredNotificationForId(id: String,
180-
completion: @escaping (UNNotification?) -> Void) {
181-
findDeliveredNotifications { notificationList in
182-
let notification = notificationList.first(where: { notification -> Bool in
183-
notification.request.identifier == id
184-
})
185-
completion(notification)
186-
}
144+
func findDeliveredNotificationForId(id: String) async -> UNNotification? {
145+
let notificationList: [UNNotification] = await findDeliveredNotifications()
146+
let notification = notificationList.first(where: { notification -> Bool in
147+
notification.request.identifier == id
148+
})
149+
return notification
187150
}
188151

189152
// Remove all pending notifications
@@ -221,34 +184,4 @@ class NotificationManager: NotificationManagerProtocol {
221184
trigger: trigger)
222185
center.add(request, withCompletionHandler: nil)
223186
}
224-
225-
private func sendTelemetry(settings: UNNotificationSettings) {
226-
guard !AppConstants.isRunningUnitTest else { return }
227-
228-
var authorizationStatus = ""
229-
switch settings.authorizationStatus {
230-
case .authorized: authorizationStatus = "authorized"
231-
case .denied: authorizationStatus = "denied"
232-
case .ephemeral: authorizationStatus = "ephemeral"
233-
case .provisional: authorizationStatus = "provisional"
234-
case .notDetermined: authorizationStatus = "notDetermined"
235-
@unknown default: authorizationStatus = "notDetermined"
236-
}
237-
238-
var alertSetting = ""
239-
switch settings.alertSetting {
240-
case .enabled: alertSetting = "enabled"
241-
case .disabled: alertSetting = "disabled"
242-
case .notSupported: alertSetting = "notSupported"
243-
@unknown default: alertSetting = "notSupported"
244-
}
245-
246-
let extras = [TelemetryWrapper.EventExtraKey.notificationPermissionStatus.rawValue: authorizationStatus,
247-
TelemetryWrapper.EventExtraKey.notificationPermissionAlertSetting.rawValue: alertSetting]
248-
249-
TelemetryWrapper.recordEvent(category: .action,
250-
method: .view,
251-
object: .notificationPermission,
252-
extras: extras)
253-
}
254187
}

0 commit comments

Comments
 (0)