Refactor FXIOS-14485 FXIOS-14472 [Swift 6 Migration] Turn on Swift 6 at the Project Level, as well as Client and ClientTest targets by tomerqodo · Pull Request #16 · qodo-benchmark/firefox-ios-combined-coderabbit

tomerqodo · 2026-01-22T03:53:52Z

Benchmark PR from qodo-benchmark#218

Summary by CodeRabbit

Release Notes

Chores
- Updated Swift version to 6.0 and enhanced internal threading mechanisms to improve code stability and maintainability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

@mainactor

…otations: `@escaping @mainactor @Sendable` (matching WKWebView APIs)

coderabbitai · 2026-01-22T03:54:19Z

Walkthrough

This pull request adds comprehensive concurrency safety through MainActor and Sendable annotations across the codebase, upgrades Swift version to 6.0 in build configurations, and refactors closure parameter types to enforce thread-safe execution. One behavior change wraps onboarding action handling in Task blocks for async execution.

Changes

Cohort / File(s)	Summary
Concurrency Annotations on Onboarding & Launcher `BrowserKit/Sources/OnboardingKit/Models/OnboardingFlowViewModel.swift`, `firefox-ios/Client/Coordinators/Launch/LaunchCoordinator.swift`	Added `@MainActor` isolation to public callback APIs (onActionTap, onComplete, onMultipleChoiceActionTap) and closure properties (onCardView, onButtonTap, onMultipleChoiceTap, onDismiss). LaunchCoordinator wraps onboardingService.handleAction calls in Task blocks instead of direct MainActor-bound calls.
Build Configuration & Version Upgrade `firefox-ios/Client.xcodeproj/project.pbxproj`	Updated SWIFT_VERSION from "5.0" to "6.0" across all build configurations for main targets, extensions, and testing configurations.
Concurrency Annotations on UI Components `firefox-ios/Client/Frontend/Autofill/AutofillAccessoryViewButtonItem.swift`, `firefox-ios/Client/Frontend/Browser/DownloadHelper/DownloadToast.swift`, `firefox-ios/Client/Frontend/Browser/TabScrollController/TabProviderAdapter.swift`	Added `@MainActor` and/or `@Sendable` annotations to tappedAction, completion, and onLoadingStateChanged closure parameters. DownloadToast also removed UX struct, inlining buttonSize constant (40) into layout constraints.
Concurrency Annotations on Web Delegates & Context Menu `firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/WebContextMenuActionsProvider.swift`, `firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift`, `firefox-ios/Client/Frontend/Browser/WebEngineIntegration/BrowserWebUIDelegate.swift`	Added `@MainActor` `@Sendable` to closure parameters (addTab, decision handlers, completion handlers for JavaScript alerts/confirms, context menu, media capture, and authentication challenges). BrowserWebUIDelegate removes `@Sendable` from one completionHandler parameter.
Concurrency Annotations on Onboarding Service & Mocks `firefox-ios/Client/Frontend/Onboarding/Protocols/OnboardingService.swift`, `firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockTabProviderProtocol.swift`	Updated handleAction and handleOpenInstructionsPopup completion parameters to be `@Sendable` `@MainActor`. MockTabProviderProtocol aligns onLoadingStateChanged type to match protocol changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop on to the main thread, with @MainActor's might,
Concurrency safety shines, everything stays right!
Swift 6.0 awaits, closures now constrained,
Sendable promises kept, the actor model ordained! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: enabling Swift 6 at the project level and for specific targets, which aligns with the majority of changes across the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧹 Recent nitpick comments

firefox-ios/Client/Coordinators/Launch/LaunchCoordinator.swift (1)
271-281: Clarify: the @MainActor annotation on onActionTap already ensures main‑thread execution.

The onActionTap closure property in OnboardingFlowViewModel is declared as @MainActor, which means Swift enforces that it can only be called on the main thread. The Task {} inside the closure body is already executing in a main-actor context; adding an explicit @MainActor annotation to the task is defensively safe but not required for correctness.

If you prefer to make the execution context explicit for code clarity, the suggested change is a reasonable best practice:
♻️ Optional improvement
-                Task {
+                Task { `@MainActor` in
                     onboardingService.handleAction(
                         action,
                         from: cardName,
                         cards: onboardingModel.cards,
                         with: activityEventHelper,
                         completion: completion
                     )
                 }

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cde2ffd and 5371116.

📒 Files selected for processing (11)

BrowserKit/Sources/OnboardingKit/Models/OnboardingFlowViewModel.swift
firefox-ios/Client.xcodeproj/project.pbxproj
firefox-ios/Client/Coordinators/Launch/LaunchCoordinator.swift
firefox-ios/Client/Frontend/Autofill/AutofillAccessoryViewButtonItem.swift
firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/WebContextMenuActionsProvider.swift
firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift
firefox-ios/Client/Frontend/Browser/BrowserViewController/WebEngineIntegration/BrowserWebUIDelegate.swift
firefox-ios/Client/Frontend/Browser/DownloadHelper/DownloadToast.swift
firefox-ios/Client/Frontend/Browser/TabScrollController/TabProviderAdapter.swift
firefox-ios/Client/Frontend/Onboarding/Protocols/OnboardingService.swift
firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockTabProviderProtocol.swift

🧰 Additional context used

🧬 Code graph analysis (2)

firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/WebContextMenuActionsProvider.swift (3)

firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift (1)

addTab (253-282)

firefox-ios/Client/TabManagement/TabManager.swift (1)

addTab (108-118)

firefox-ios/Client/TabManagement/TabManagerImplementation.swift (3)

addTab (337-343)

addTab (345-356)

addTab (380-389)

firefox-ios/Client/Coordinators/Launch/LaunchCoordinator.swift (1)

firefox-ios/Client/Frontend/Onboarding/Protocols/OnboardingService.swift (1)

handleAction (60-140)

🔇 Additional comments (29)

firefox-ios/Client/Frontend/Autofill/AutofillAccessoryViewButtonItem.swift (2)

61-65: LGTM! Appropriate @MainActor annotation for Swift 6 concurrency.

The @MainActor constraint on tappedAction ensures the tap handler executes on the main thread, which is correct for UI-related actions. This aligns with the Swift 6 migration objective and UIKit's threading requirements.

174-178: Action handler correctly invokes the @MainActor closure.

The tappedAccessoryButton() method can safely call the @MainActor-annotated closure because UIBarButtonItem has implicit @MainActor isolation in modern UIKit, and gesture recognizer callbacks are guaranteed to execute on the main thread.

firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/WebContextMenuActionsProvider.swift (2)

30-31: LGTM.

43-44: LGTM.

firefox-ios/Client/Frontend/Browser/DownloadHelper/DownloadToast.swift (2)

109-113: LGTM.

192-193: LGTM.

firefox-ios/Client/Frontend/Browser/TabScrollController/TabProviderAdapter.swift (2)

14-14: LGTM.

32-35: LGTM.

firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockTabProviderProtocol.swift (1)

15-15: LGTM.

firefox-ios/Client/Frontend/Browser/BrowserViewController/WebEngineIntegration/BrowserWebUIDelegate.swift (1)

38-43: LGTM.

firefox-ios/Client/Frontend/Onboarding/Protocols/OnboardingService.swift (2)

60-67: Swift 6 completion annotation looks consistent.
Please confirm the @Sendable completion’s Result<..., Error> remains Sendable under Swift 6 (e.g., any Error & Sendable if needed).

208-211: Popup completion now MainActor/Sendable — good.
Ensure any non‑main callers hop to the main actor before invoking the callback.

BrowserKit/Sources/OnboardingKit/Models/OnboardingFlowViewModel.swift (4)

12-20: MainActor isolation on action callbacks is appropriate.
Keeps UI mutations on the main actor; ensure call sites remain main‑actor aware.

27-27: onComplete now main‑actor‑bound — good for UI teardown.

30-33: Optional view callbacks now MainActor‑isolated.

38-46: Initializer signature aligns with updated actor‑isolated callbacks.

firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift (11)

76-80: MainActor/Sendable spam callback aligns with Swift 6 concurrency.
Please confirm this matches the current WebKit delegate signature expectations.

94-99: JS alert completion now MainActor/Sendable — LGTM.
Please confirm this matches the latest SDK signature.

109-114: JS confirm completion annotation looks consistent.
Please confirm this matches the latest SDK signature.

125-131: JS text input completion annotation looks consistent.
Please confirm this matches the latest SDK signature.

153-157: Context menu completion now MainActor/Sendable — good.
Please confirm this aligns with current WebKit headers.

182-188: Media capture decision handler annotation looks right.
Please confirm against the latest SDK signature.

313-316: addTab closure now MainActor/Sendable — good for UI safety.
Ensure upstream callers are main‑actor aware.

704-709: Navigation response decision handler annotation looks consistent.
Please confirm this matches the current SDK signature.

1052-1057: Auth challenge completion now MainActor/Sendable — LGTM.
Please confirm this aligns with current WebKit headers.

1233-1235: External alert completion annotated for MainActor/Sendable — good.
Please confirm this matches current UIKit expectations.

1329-1333: Server trust completion now MainActor/Sendable — LGTM.
Please confirm this matches current SDK expectations.

firefox-ios/Client.xcodeproj/project.pbxproj (2)

27951-27951: Swift 6.0 version upgrade looks good across all configurations.

The SWIFT_VERSION = 6.0 change is consistently applied to all build configurations (Client, ClientTests, Firefox, FirefoxBeta, FirefoxStaging, Fennec, Fennec_Enterprise, Fennec_Testing). The upcoming feature flags (ISOLATED_DEFAULT_VALUES, NONFROZEN_ENUM_EXHAUSTIVITY, REGION_BASED_ISOLATION) are already enabled, which aligns well with Swift 6 concurrency requirements.

Also applies to: 27991-27991, 28257-28257, 28466-28466, 28500-28500, 28694-28694, 29779-29779, 29816-29816, 29829-29829, 30103-30103, 30148-30148, 30264-30264, 30430-30430, 30467-30467, 30480-30480, 30972-30972, 31005-31005, 31018-31018

27988-27991: Document the intentional use of mixed SWIFT_STRICT_CONCURRENCY settings during Swift 6 migration.

This codebase currently uses inconsistent concurrency settings: 6 of 30 build configurations (20%) use minimal while 24 use complete. With Swift 6, the minimal setting may suppress concurrency warnings and errors that would otherwise surface, potentially masking data race issues. Recent git history shows ongoing Swift 6 migration work (FXIOS-14344 and related commits), suggesting this mixed approach is intentional for gradual adoption. However, there's no documented migration strategy.

Add a comment to the build configuration file or project documentation explaining:

Which targets/configurations currently use minimal and why (e.g., specific targets undergoing active migration)

The timeline and criteria for migrating remaining configurations to complete

Any blockers or known concurrency issues preventing immediate adoption of complete

This prevents future confusion about whether the inconsistency is accidental or part of a deliberate migration plan.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ih-codes and others added 5 commits January 21, 2026 16:08

Migrate entire project, as well as Client and ClientTests, to Swift 6.

228edeb

Fix lingering strict concurrency errors at the project level.

ab8c14f

Making BVC webView methods consistent in their completion handler ann…

4d0d2af

…otations: `@escaping @mainactor @Sendable` (matching WKWebView APIs)

Fix unit tests.

14c0230

update pr

5371116

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor FXIOS-14485 FXIOS-14472 [Swift 6 Migration] Turn on Swift 6 at the Project Level, as well as Client and ClientTest targets#16

tomerqodo commented Jan 22, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 22, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tomerqodo commented Jan 22, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tomerqodo commented Jan 22, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 22, 2026 •

edited

Loading