From 01811f2dacd30c5758d4152fc3a8be706ef4452e Mon Sep 17 00:00:00 2001 From: ih-codes Date: Thu, 6 Nov 2025 15:54:24 -0600 Subject: [PATCH 1/4] Annotate misc. warning completion handlers with @Sendable and main actor / threading warnings that come from adding @Sendable to closures. --- .../Frontend/Browser/SearchLoader.swift | 4 +-- .../Bookmarks/BookmarksViewController.swift | 23 ++++++------ .../Legacy/BookmarksPanelViewModel.swift | 36 +++++++++++-------- .../Onboarding/Models/UpdateViewModel.swift | 2 +- .../BreachAlertsClient.swift | 8 ++--- .../PasswordManagerViewModel.swift | 4 +-- .../PasswordDetailViewController.swift | 2 +- .../Document/TemporaryDocument.swift | 2 +- firefox-ios/Providers/RustSyncManager.swift | 7 ++-- .../FirefoxAccountSignInViewController.swift | 2 +- 10 files changed, 50 insertions(+), 40 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/SearchLoader.swift b/firefox-ios/Client/Frontend/Browser/SearchLoader.swift index 917deedc6998f..79ba256066284 100644 --- a/firefox-ios/Client/Frontend/Browser/SearchLoader.swift +++ b/firefox-ios/Client/Frontend/Browser/SearchLoader.swift @@ -33,7 +33,7 @@ final class SearchLoader: Loader, SearchViewModel>, FeatureFlaggabl fileprivate func getBookmarksAsSites( matchingSearchQuery query: String, limit: UInt, - completionHandler: @escaping (([Site]) -> Void) + completionHandler: @escaping @Sendable (([Site]) -> Void) ) { profile.places.searchBookmarks(query: query, limit: limit).upon { result in guard let bookmarkItems = result.successValue else { @@ -49,7 +49,7 @@ final class SearchLoader: Loader, SearchViewModel>, FeatureFlaggabl private func getHistoryAsSites( matchingSearchQuery query: String, limit: Int, - completionHandler: @escaping (([Site]) -> Void) + completionHandler: @escaping @Sendable (([Site]) -> Void) ) { profile.places.interruptReader() profile.places.queryAutocomplete(matchingSearchQuery: query, limit: limit).upon { result in diff --git a/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift b/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift index e23032ac456e1..c9d8d11bb0bee 100644 --- a/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift +++ b/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift @@ -668,16 +668,19 @@ final class BookmarksViewController: SiteTableViewController, extension BookmarksViewController: LibraryPanelContextMenu { func presentContextMenu(for indexPath: IndexPath) { - viewModel.getSiteDetails(for: indexPath) { [weak self] site in - guard let self else { return } - if let site { - presentContextMenu(for: site, with: indexPath, completionHandler: { - return self.contextMenu(for: site, with: indexPath) - }) - } else if let bookmarkNode = viewModel.bookmarkNodes[safe: indexPath.row], - bookmarkNode.type == .folder, - isCurrentFolderEditable(at: indexPath) { - presentContextMenu(for: bookmarkNode, indexPath: indexPath) + viewModel.getSiteDetails(for: indexPath) { site in + ensureMainThread { [weak self] in + guard let self else { return } + + if let site { + self.presentContextMenu(for: site, with: indexPath, completionHandler: { + return self.contextMenu(for: site, with: indexPath) + }) + } else if let bookmarkNode = self.viewModel.bookmarkNodes[safe: indexPath.row], + bookmarkNode.type == .folder, + self.isCurrentFolderEditable(at: indexPath) { + self.presentContextMenu(for: bookmarkNode, indexPath: indexPath) + } } } } diff --git a/firefox-ios/Client/Frontend/Library/Bookmarks/Legacy/BookmarksPanelViewModel.swift b/firefox-ios/Client/Frontend/Library/Bookmarks/Legacy/BookmarksPanelViewModel.swift index 61e50b4ccefc1..878b9d6d83b6b 100644 --- a/firefox-ios/Client/Frontend/Library/Bookmarks/Legacy/BookmarksPanelViewModel.swift +++ b/firefox-ios/Client/Frontend/Library/Bookmarks/Legacy/BookmarksPanelViewModel.swift @@ -58,7 +58,7 @@ final class BookmarksPanelViewModel { return true } - func reloadData(completion: @escaping () -> Void) { + func reloadData(completion: @escaping @Sendable () -> Void) { // Can be called while app backgrounded and the db closed, don't try to reload the data source in this case if profile.isShutdown { completion() @@ -97,7 +97,7 @@ final class BookmarksPanelViewModel { bookmarkNodes.insert(bookmarkNode, at: destinationIndexPath.row) } - func getSiteDetails(for indexPath: IndexPath, completion: @escaping (Site?) -> Void) { + func getSiteDetails(for indexPath: IndexPath, completion: @escaping @Sendable (Site?) -> Void) { guard let bookmarkNode = bookmarkNodes[safe: indexPath.row], let bookmarkItem = bookmarkNode as? BookmarkItemData else { @@ -117,7 +117,7 @@ final class BookmarksPanelViewModel { func createPinUnpinAction( for site: Site, isPinned: Bool, - successHandler: @escaping (String) -> Void + successHandler: @MainActor @escaping @Sendable (String) -> Void ) -> PhotonRowActions { return SingleActionViewModel( title: isPinned ? .Bookmarks.Menu.RemoveFromShortcutsTitle : .AddToShortcutsActionTitle, @@ -129,14 +129,16 @@ final class BookmarksPanelViewModel { : profile.pinnedSites.addPinnedTopSite(site) action.uponQueue(.main) { result in - if result.isSuccess { - let message: String = isPinned - ? .LegacyAppMenu.RemovePinFromShortcutsConfirmMessage - : .LegacyAppMenu.AddPinToShortcutsConfirmMessage - successHandler(message) - } else { - let logMessage = isPinned ? "Could not remove pinned site" : "Could not add pinne site" - logger.log(logMessage, level: .debug, category: .library) + MainActor.assumeIsolated { + if result.isSuccess { + let message: String = isPinned + ? .LegacyAppMenu.RemovePinFromShortcutsConfirmMessage + : .LegacyAppMenu.AddPinToShortcutsConfirmMessage + successHandler(message) + } else { + let logMessage = isPinned ? "Could not remove pinned site" : "Could not add pinne site" + logger.log(logMessage, level: .debug, category: .library) + } } } } @@ -157,7 +159,7 @@ final class BookmarksPanelViewModel { return max(index - LocalDesktopFolder.numberOfRowsTaken, 0) } - private func setupMobileFolderData(completion: @escaping () -> Void) { + private func setupMobileFolderData(completion: @escaping @Sendable () -> Void) { bookmarksHandler .getBookmarksTree(rootGUID: BookmarkRoots.MobileFolderGUID, recursive: false) .uponQueue(.main) { result in @@ -190,7 +192,7 @@ final class BookmarksPanelViewModel { } /// Subfolder data case happens when we select a folder created by a user - private func setupSubfolderData(completion: @escaping () -> Void) { + private func setupSubfolderData(completion: @escaping @Sendable () -> Void) { bookmarksHandler.getBookmarksTree(rootGUID: bookmarkFolderGUID, recursive: false).uponQueue(.main) { result in guard let folder = result.successValue as? BookmarkFolderData else { @@ -217,7 +219,7 @@ final class BookmarksPanelViewModel { // Create a local "Desktop bookmarks" folder only if there exists a bookmark in one of it's nested // subfolders - private func createDesktopBookmarksFolder(completion: @escaping () -> Void) { + private func createDesktopBookmarksFolder(completion: @escaping @Sendable () -> Void) { bookmarksHandler.countBookmarksInTrees(folderGuids: BookmarkRoots.DesktopRoots.map { $0 }) { result in switch result { case .success(let bookmarkCount): @@ -237,7 +239,11 @@ final class BookmarksPanelViewModel { } } - private func checkIfPinnedURL(_ url: String, queue: DispatchQueue = .main, completion: @escaping (Bool) -> Void ) { + private func checkIfPinnedURL( + _ url: String, + queue: DispatchQueue = .main, + completion: @escaping @Sendable (Bool) -> Void + ) { profile.pinnedSites.isPinnedTopSite(url) .uponQueue(queue) { result in completion(result.successValue ?? false) diff --git a/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift b/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift index dcc9455c9801f..99df988133189 100644 --- a/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift +++ b/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift @@ -73,7 +73,7 @@ class UpdateViewModel: OnboardingViewModelProtocol, // Function added to wait for AccountManager initialization to get // if the user is Sign in with Sync Account to decide which cards to show - func hasSyncableAccount(completion: @escaping () -> Void) { + func hasSyncableAccount(completion: @escaping @Sendable () -> Void) { hasSyncableAccount = profile.hasAccount() ensureMainThread { completion() diff --git a/firefox-ios/Client/Frontend/PasswordManagement/BreachAlertsClient.swift b/firefox-ios/Client/Frontend/PasswordManagement/BreachAlertsClient.swift index baeffa30a5221..387b0fd705cd2 100644 --- a/firefox-ios/Client/Frontend/PasswordManagement/BreachAlertsClient.swift +++ b/firefox-ios/Client/Frontend/PasswordManagement/BreachAlertsClient.swift @@ -14,12 +14,12 @@ protocol BreachAlertsClientProtocol { func fetchEtag( endpoint: BreachAlertsClient.Endpoint, profile: Profile, - completion: @escaping (_ etag: String?) -> Void + completion: @escaping @Sendable (_ etag: String?) -> Void ) func fetchData( endpoint: BreachAlertsClient.Endpoint, profile: Profile, - completion: @escaping (_ result: Maybe) -> Void + completion: @escaping @Sendable (_ result: Maybe) -> Void ) } @@ -33,7 +33,7 @@ class BreachAlertsClient: BreachAlertsClientProtocol { static let etagDateKey = "BreachAlertsDataDate" /// Makes a header-only request to an endpoint and hands off the endpoint's etag to a completion handler. - func fetchEtag(endpoint: Endpoint, profile: Profile, completion: @escaping (_ etag: String?) -> Void) { + func fetchEtag(endpoint: Endpoint, profile: Profile, completion: @escaping @Sendable (_ etag: String?) -> Void) { guard let url = URL(string: endpoint.rawValue) else { return } var request = URLRequest(url: url) request.httpMethod = "HEAD" @@ -58,7 +58,7 @@ class BreachAlertsClient: BreachAlertsClientProtocol { } /// Makes a network request to an endpoint and hands off the result to a completion handler. - func fetchData(endpoint: Endpoint, profile: Profile, completion: @escaping (_ result: Maybe) -> Void) { + func fetchData(endpoint: Endpoint, profile: Profile, completion: @escaping @Sendable (_ result: Maybe) -> Void) { guard let url = URL(string: endpoint.rawValue) else { return } dataTask?.cancel() diff --git a/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift b/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift index 5478ea0b3b2f5..3738a997e5651 100644 --- a/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift +++ b/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift @@ -78,7 +78,7 @@ final class PasswordManagerViewModel { /// Searches SQLite database for logins that match query. /// Wraps the SQLiteLogins method to allow us to cancel it from our end. - func queryLogins(_ query: String, completion: @escaping ([Login]) -> Void) { + func queryLogins(_ query: String, completion: @escaping @Sendable ([Login]) -> Void) { loginProvider.searchLoginsWithQuery(query) { result in ensureMainThread { switch result { @@ -155,7 +155,7 @@ final class PasswordManagerViewModel { } } - public func save(loginRecord: LoginEntry, completion: @escaping ((String?) -> Void)) { + public func save(loginRecord: LoginEntry, completion: @escaping @Sendable ((String?) -> Void)) { loginProvider.addLogin(login: loginRecord, completionHandler: { result in switch result { case .success(let encryptedLogin): diff --git a/firefox-ios/Client/Frontend/Settings/PasswordDetailViewController.swift b/firefox-ios/Client/Frontend/Settings/PasswordDetailViewController.swift index d218a6db89c8d..70e94d452e527 100644 --- a/firefox-ios/Client/Frontend/Settings/PasswordDetailViewController.swift +++ b/firefox-ios/Client/Frontend/Settings/PasswordDetailViewController.swift @@ -375,7 +375,7 @@ extension PasswordDetailViewController { } } - func onProfileDidFinishSyncing(completion: @escaping () -> Void) { + func onProfileDidFinishSyncing(completion: @escaping @Sendable () -> Void) { // Reload details after syncing. viewModel.profile.logins.getLogin(id: viewModel.login.id) { [weak self] result in DispatchQueue.main.async { diff --git a/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift b/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift index dbb41af64b217..4b329b5864a36 100644 --- a/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift +++ b/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift @@ -152,7 +152,7 @@ final class DefaultTemporaryDocument: NSObject, return tempFileURL } - func download(_ completion: @escaping (URL?) -> Void) { + func download(_ completion: @escaping @Sendable (URL?) -> Void) { if let tempFile = queryTempFile() { ensureMainThread { completion(tempFile) diff --git a/firefox-ios/Providers/RustSyncManager.swift b/firefox-ios/Providers/RustSyncManager.swift index 2599f282d6336..e408293339bbb 100644 --- a/firefox-ios/Providers/RustSyncManager.swift +++ b/firefox-ios/Providers/RustSyncManager.swift @@ -356,7 +356,7 @@ public class RustSyncManager: NSObject, SyncManager { public let description = "Failed to get token server endpoint url." } - func shouldSyncLogins(completion: @escaping (Bool) -> Void) { + func shouldSyncLogins(completion: @escaping @Sendable (Bool) -> Void) { if !(self.prefs.boolForKey(PrefsKeys.LoginsHaveBeenVerified) ?? false) { // We should only sync logins when the verification step has completed successfully. // Otherwise logins could exist in the database that can't be decrypted and would @@ -378,7 +378,8 @@ public class RustSyncManager: NSObject, SyncManager { creditCardKey: String?, completion: @escaping @Sendable (([String], [String: String])) -> Void) { var localEncryptionKeys: [String: String] = [:] - var rustEngines: [String] = [] + // FIXME: FXIOS-14052 Unprotected properties should not be mutated on background threads + nonisolated(unsafe) var rustEngines: [String] = [] var registeredPlaces = false var registeredAutofill = false @@ -468,7 +469,7 @@ public class RustSyncManager: NSObject, SyncManager { } } - private func doSync(params: SyncParams, completion: @escaping (SyncResult) -> Void) { + private func doSync(params: SyncParams, completion: @escaping @Sendable (SyncResult) -> Void) { beginSyncing() syncManagerAPI.sync(params: params) { syncResult in // Save the persisted state diff --git a/firefox-ios/RustFxA/FirefoxAccountSignInViewController.swift b/firefox-ios/RustFxA/FirefoxAccountSignInViewController.swift index aafd029180477..24ae1c4925caa 100644 --- a/firefox-ios/RustFxA/FirefoxAccountSignInViewController.swift +++ b/firefox-ios/RustFxA/FirefoxAccountSignInViewController.swift @@ -280,7 +280,7 @@ class FirefoxAccountSignInViewController: UIViewController, Themeable { navigationController?.pushViewController(fxaWebVC, animated: true) } - private func showFxAWebViewController(_ url: URL, completion: @escaping (URL) -> Void) { + private func showFxAWebViewController(_ url: URL, completion: @escaping @Sendable (URL) -> Void) { if let accountManager = profile.rustFxA.accountManager { let entrypoint = self.deepLinkParams.entrypoint.rawValue accountManager.getManageAccountURL(entrypoint: "ios_settings_\(entrypoint)") { [weak self] result in From 62620f20b26a01aec6a7288293ee1c7a68686427 Mon Sep 17 00:00:00 2001 From: ih-codes Date: Thu, 6 Nov 2025 16:03:36 -0600 Subject: [PATCH 2/4] Fix unit tests. --- .../Client/Frontend/Onboarding/Models/UpdateViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift b/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift index 99df988133189..9b3ff9a5be165 100644 --- a/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift +++ b/firefox-ios/Client/Frontend/Onboarding/Models/UpdateViewModel.swift @@ -73,7 +73,7 @@ class UpdateViewModel: OnboardingViewModelProtocol, // Function added to wait for AccountManager initialization to get // if the user is Sign in with Sync Account to decide which cards to show - func hasSyncableAccount(completion: @escaping @Sendable () -> Void) { + func hasSyncableAccount(completion: @MainActor @escaping @Sendable () -> Void) { hasSyncableAccount = profile.hasAccount() ensureMainThread { completion() From 9745080e18eb314869493bf6dd8e2a1934958ad2 Mon Sep 17 00:00:00 2001 From: ih-codes Date: Thu, 6 Nov 2025 16:24:11 -0600 Subject: [PATCH 3/4] Fix a few more warnings. --- .../Frontend/Library/Bookmarks/BookmarksViewController.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift b/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift index c9d8d11bb0bee..cc880cffa86a6 100644 --- a/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift +++ b/firefox-ios/Client/Frontend/Library/Bookmarks/BookmarksViewController.swift @@ -181,8 +181,8 @@ final class BookmarksViewController: SiteTableViewController, // MARK: - Data override func reloadData() { - viewModel.reloadData { [weak self] in - ensureMainThread { + viewModel.reloadData { + ensureMainThread { [weak self] in self?.tableView.reloadData() if self?.viewModel.shouldFlashRow ?? false { self?.flashRow() From f4b3c5a73b8a1a587f854e9c692f145c9f86e9b0 Mon Sep 17 00:00:00 2001 From: ih-codes Date: Thu, 6 Nov 2025 16:29:56 -0600 Subject: [PATCH 4/4] Aaaaand more warnings! --- .../Extensions/BrowserViewController+WebViewDelegates.swift | 3 ++- .../Client/TabManagement/Document/TemporaryDocument.swift | 2 +- firefox-ios/Client/TabManagement/Tab.swift | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift index 6272694470d2c..4fccabfad2d54 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift @@ -744,7 +744,8 @@ extension BrowserViewController: WKNavigationDelegate { let request = request { // Certain files are too large to download before the preview presents, so block until we have something to show let group = DispatchGroup() - var url: URL? + // FIXME: FXIOS-14054 Should not mutate local properties in concurrent code + nonisolated(unsafe) var url: URL? group.enter() let temporaryDocument = DefaultTemporaryDocument(preflightResponse: response, request: request) temporaryDocument.download { docURL in diff --git a/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift b/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift index 4b329b5864a36..93b0f38591076 100644 --- a/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift +++ b/firefox-ios/Client/TabManagement/Document/TemporaryDocument.swift @@ -17,7 +17,7 @@ protocol TemporaryDocument: Sendable { func download() async -> URL? - func download(_ completion: @escaping (URL?) -> Void) + func download(_ completion: @escaping @Sendable (URL?) -> Void) func cancelDownload() diff --git a/firefox-ios/Client/TabManagement/Tab.swift b/firefox-ios/Client/TabManagement/Tab.swift index 0d98d195ce82c..f48f89b252b5f 100644 --- a/firefox-ios/Client/TabManagement/Tab.swift +++ b/firefox-ios/Client/TabManagement/Tab.swift @@ -956,8 +956,8 @@ class Tab: NSObject, ThemeApplicable, FeatureFlaggable, ShareTab { func enqueueDocument(_ document: TemporaryDocument) { temporaryDocument = document - temporaryDocument?.download { [weak self] url in - ensureMainThread { + temporaryDocument?.download { url in + ensureMainThread { [weak self] in guard let url else { return } // Prevent the WebView to load a new item so it doesn't add a new entry to the back and forward list.