Refactor FXIOS-12796 [Swift 6 Migration] Annotate misc. completion handlers with @sendable (#30461)

ih-codes · web-flow · commit 7842184fd104 · 2025-11-07T13:34:13.000-06:00
* Annotate misc. warning completion handlers with @sendable and main actor / threading warnings that come from adding @sendable to closures. * Fix unit tests.
diff --git 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/Frontend/Browser/SearchLoader.swift b/firefox-ios/Client/Frontend/Browser/SearchLoader.swift
@@ -33,7 +33,7 @@ final class SearchLoader: Loader<Cursor<Site>, 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<Cursor<Site>, 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
@@ -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()
@@ -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
@@ -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
@@ -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: @MainActor @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
@@ -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<Data>) -> Void
+        completion: @escaping @Sendable (_ result: Maybe<Data>) -> 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<Data>) -> Void) {
+    func fetchData(endpoint: Endpoint, profile: Profile, completion: @escaping @Sendable (_ result: Maybe<Data>) -> 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
@@ -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
@@ -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
@@ -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()
 
@@ -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/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.
diff --git 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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -375,7 +375,7 @@ extension PasswordDetailViewController {`
`375`	`375`	`}`
`376`	`376`	`}`
`377`	`377`
`378`		`- func onProfileDidFinishSyncing(completion: @escaping () -> Void) {`
	`378`	`+ func onProfileDidFinishSyncing(completion: @escaping @Sendable () -> Void) {`
`379`	`379`	`// Reload details after syncing.`
`380`	`380`	`viewModel.profile.logins.getLogin(id: viewModel.login.id) { [weak self] result in`
`381`	`381`	`DispatchQueue.main.async {`