Refactor FXIOS-12796 [Swift 6 Migration] Annotate misc. completion handlers with @Sendable #30461
Conversation
…tor / threading warnings that come from adding @sendable to closures.
| } else { | ||
| let logMessage = isPinned ? "Could not remove pinned site" : "Could not add pinne site" | ||
| logger.log(logMessage, level: .debug, category: .library) | ||
| MainActor.assumeIsolated { |
There was a problem hiding this comment.
Safe here since this is executed upon the .main queue above.
| isCurrentFolderEditable(at: indexPath) { | ||
| presentContextMenu(for: bookmarkNode, indexPath: indexPath) | ||
| viewModel.getSiteDetails(for: indexPath) { site in | ||
| ensureMainThread { [weak self] in |
There was a problem hiding this comment.
By marking some closures as @Sendable, the call sites had errors because the compiler was able to detect that we were (potentially) hopping threads, so we add ensureMainThread here for safety. 👀
[weak self] gets moved a level lower so we don't pass the non-Sendable self from the closure thread to the main thread.
| var localEncryptionKeys: [String: String] = [:] | ||
| var rustEngines: [String] = [] | ||
| // FIXME: FXIOS-14052 Unprotected properties should not be mutated on background threads | ||
| nonisolated(unsafe) var rustEngines: [String] = [] |
There was a problem hiding this comment.
This is suppressing an error for how this var gets (mis)used below.
🥇 Perfect PR sizeSmaller PRs are easier to review. Thanks for making life easy for reviewers! ✨ 🙌 Friday high-fiveThanks for pushing us across the finish line this week! 🙌 💬 Description craftsmanGreat PR description! Reviewers salute you 🫡 ❌ Per-file test coverage gateThe following changed file(s) are below 35.0% coverage:
Client.app: Coverage: 37.18
CredentialProvider.appex: Coverage: 27.97
NotificationService.appex: Coverage: 33.69
ShareTo.appex: Coverage: 29.1
Generated by 🚫 Danger Swift against f4b3c5a |
|
🚀 PR merged to |
📜 Tickets
Jira ticket
Github issue
💡 Description
Annotate misc. warning completion handlers with
@Sendableand main actor / threading warnings that come from adding@Sendableto closures.Sample of the type of errors this work resolves:
cc @Cramsden @lmarceau @dataports | Swift 6 Migration
📝 Checklist