Skip to content

Refactor FXIOS-14466 FXIOS-14463 [Swift 6 Migration] Migrate ShareTo and BrowserKit Package to Swift 6 #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tomerqodo wants to merge 8 commits into
base: coderabbit_full_base_refactor_fxios-14466_fxios-14463_swift_6_migration_migrate_shareto_and_browserkit_package_to_swift_6_pr10
Choose a base branch
from
Open

Refactor FXIOS-14466 FXIOS-14463 [Swift 6 Migration] Migrate ShareTo and BrowserKit Package to Swift 6 #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
329488e
Migrate entire BrowserKit package to Swift 6.2.
ih-codes Dec 29, 2025
74ff14f
Migrate entire ShareTo package to Swift 6.
ih-codes Dec 29, 2025
edf7e96
Cleanup BrowserKit Package file now that experimental/upcoming featur…
ih-codes Dec 29, 2025
b61fae5
Fix lingering Swift 6 errors in WebEngineTests.
ih-codes Dec 29, 2025
0e926a1
Fix a few more new warnings.
ih-codes Dec 29, 2025
05baecc
Fix the preconcurrency GCDWebServer setup in SiteImageViewTests, as i…
ih-codes Dec 29, 2025
bb3263a
Fix compile issue with Focus from ContentBlockingGenerator strict con…
ih-codes Dec 29, 2025
5a63a2c
update pr
tomerqodo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 1 addition & 51 deletions BrowserKit/Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 5.10
// swift-tools-version: 6.2
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for Swift/Xcode/toolchain settings in repo configs
rg -n "swift-tools-version|SWIFT_VERSION|Xcode|TOOLCHAIN|swift version" -g "*.{yml,yaml,xcconfig,plist,md}" --max-count=50

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 4591

🏁 Script executed:

#!/bin/bash
# Find all Package.swift files
fd -n "Package.swift" --type f

# Look for BrowserKit-specific Swift version settings
rg -n "swift-tools-version|SWIFT_VERSION|swift version" BrowserKit/ --type swift --type yaml --type plist --type xcconfig

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 350

🏁 Script executed:

#!/bin/bash
# Find all Package.swift files
fd "Package.swift" --type f

# Look for BrowserKit-specific Swift version settings
rg -n "swift-tools-version|SWIFT_VERSION|swift version" BrowserKit/ --type swift --type yaml --type xcconfig

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 256

🏁 Script executed:

#!/bin/bash
# Check all Package.swift files for swift-tools-version
echo "=== Root Package.swift ===" 
head -5 Package.swift

echo ""
echo "=== BrowserKit/Package.swift ===" 
head -5 BrowserKit/Package.swift

echo ""
echo "=== MozillaRustComponents/Package.swift ===" 
head -5 MozillaRustComponents/Package.swift

echo ""
echo "=== BlockzillaPackage/Package.swift ===" 
head -5 focus-ios/BlockzillaPackage/Package.swift

# Check for BrowserKit-specific xcconfig or build settings
echo ""
echo "=== BrowserKit Config Files ===" 
fd "xcconfig|config" BrowserKit/ --type f | head -20

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 1797

🏁 Script executed:

#!/bin/bash
# Check if root Package.swift depends on BrowserKit
echo "=== Root Package.swift dependencies ===" 
cat Package.swift

echo ""
echo "=== Check git structure to understand BrowserKit role ===" 
git ls-tree -r HEAD | grep -E "Package\.swift|BrowserKit" | head -20

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 3231

🏁 Script executed:

#!/bin/bash
# Check if firefox-ios project depends on BrowserKit
echo "=== Search for BrowserKit references in firefox-ios ===" 
rg -l "BrowserKit" firefox-ios/ --type swift --type yaml --type xcodeproj | head -10

echo ""
echo "=== Check bitrise.yml for Xcode version specs ===" 
rg -n "xcode.*version|Xcode.*=|XCODE_VERSION" bitrise.yml -A 3 -B 1

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 2765

🏁 Script executed:

#!/bin/bash
# Check if focus-ios or other apps depend on BrowserKit
echo "=== Search for BrowserKit in focus-ios ===" 
rg "BrowserKit" focus-ios/ | head -5

echo ""
echo "=== Check bootstrap.sh and setup scripts ===" 
cat bootstrap.sh 2>/dev/null | head -50

echo ""
echo "=== Check for .xcode-version or version files ===" 
fd -e ".xcode-version" -e ".xcoderc" -e ".swift-version" --type f

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 2232

Update firefox-ios Common.xcconfig and CI to enforce Swift 6.2 alignment with BrowserKit.

BrowserKit now requires Swift 6.2, but firefox-ios/Client/Configuration/Common.xcconfig still sets SWIFT_VERSION = 5.10. Since focus-ios depends on BrowserKit, this version mismatch will cause build failures. Update the build config to SWIFT_VERSION = 6.2 and ensure bitrise.yml pins to Xcode 26.1 (or later compatible version) to align with the new requirement.

🤖 Prompt for AI Agents 
In `@BrowserKit/Package.swift` at line 1, Update the Swift toolchain version in
the iOS build config and CI to match BrowserKit's new requirement: change the
SWIFT_VERSION setting in Common.xcconfig (the SWIFT_VERSION = 5.10 entry) to
SWIFT_VERSION = 6.2, and update the CI configuration (bitrise.yml) to pin the
Xcode stack to 26.1 or a compatible Xcode that supports Swift 6.2 so focus-ios
and BrowserKit use the same Swift version.


import PackageDescription

Expand Down Expand Up @@ -87,24 +87,18 @@ let package = Package(
dependencies: ["Common"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]
),
.target(
name: "ComponentLibrary",
dependencies: ["Common", "SiteImageView"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableUpcomingFeature("InferSendableFromCaptures"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "ComponentLibraryTests",
dependencies: ["ComponentLibrary"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
]
),
.target(
Expand All @@ -114,9 +108,6 @@ let package = Package(
resources: [.process("BundledTopSitesFavicons.xcassets")],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableUpcomingFeature("InferSendableFromCaptures"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "SiteImageViewTests",
Expand All @@ -127,7 +118,6 @@ let package = Package(
.copy("Resources/hackernews.svg")
],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
],
),
.target(
Expand All @@ -137,46 +127,35 @@ let package = Package(
.product(name: "Sentry-Dynamic", package: "sentry-cocoa")],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableUpcomingFeature("InferSendableFromCaptures"),
.enableExperimentalFeature("RegionBasedIsolation")
]
),
.testTarget(
name: "CommonTests",
dependencies: ["Common"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency")
]),
.target(
name: "TabDataStore",
dependencies: ["Common"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "TabDataStoreTests",
dependencies: ["TabDataStore"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
]
),
.target(
name: "Redux",
dependencies: ["Common"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "ReduxTests",
dependencies: ["Redux"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]
),
.target(
Expand All @@ -185,44 +164,34 @@ let package = Package(
.product(name: "GCDWebServers", package: "GCDWebServer")],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "WebEngineTests",
dependencies: ["WebEngine"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
]
),
.target(
name: "ToolbarKit",
dependencies: ["Common"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableUpcomingFeature("InferSendableFromCaptures"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "ToolbarKitTests",
dependencies: ["ToolbarKit"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency")
]),
.target(
name: "MenuKit",
dependencies: ["Common", "ComponentLibrary"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "MenuKitTests",
dependencies: ["MenuKit"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
]
),
.target(
Expand All @@ -234,53 +203,41 @@ let package = Package(
],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]
),
.testTarget(name: "SummarizeKitTests",
dependencies: ["SummarizeKit"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency")
]),
.target(
name: "JWTKit",
dependencies: ["Common", "Shared"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]
),
.testTarget(
name: "JWTKitTests",
dependencies: ["JWTKit"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]
),
.target(
name: "UnifiedSearchKit",
dependencies: ["Common", "ComponentLibrary", "MenuKit"],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.target(
name: "ContentBlockingGenerator",
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "ContentBlockingGeneratorTests",
dependencies: ["ContentBlockingGenerator"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency")
]),
.target(
name: "OnboardingKit",
Expand All @@ -290,8 +247,6 @@ let package = Package(
],
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
],
linkerSettings: [
.linkedFramework("Metal"),
Expand All @@ -301,21 +256,16 @@ let package = Package(
name: "OnboardingKitTests",
dependencies: ["OnboardingKit"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.target(
name: "ActionExtensionKit",
swiftSettings: [
.unsafeFlags(["-enable-testing"]),
.enableExperimentalFeature("StrictConcurrency"),
.enableExperimentalFeature("RegionBasedIsolation")
]),
.testTarget(
name: "ActionExtensionKitTests",
dependencies: ["ActionExtensionKit"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency"),
]),
.executableTarget(
name: "ExecutableContentBlockingGenerator",
Expand Down
7 changes: 2 additions & 5 deletions BrowserKit/Sources/ExecutableContentBlockingGenerator/MainContentBlockerGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import ContentBlockingGenerator

@main
public struct MainContentBlockerGenerator {
static let generator = ContentBlockerGenerator.factory()
// FXIOS-14548 ContentBlockerGenerator is not thread safe
nonisolated(unsafe) static let generator = ContentBlockerGenerator.factory()
Comment on lines +5 to +6
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Swift 6 nonisolated(unsafe) static properties concurrency

💡 Result:

In Swift 6 strict concurrency, a static var is treated like global shared mutable state. Because it can be read/written from any actor/thread, the compiler diagnoses it as not concurrency-safe unless you make its access provably safe (or you explicitly opt out). This is why you see errors like “static property … is not concurrency-safe because it is nonisolated global shared mutable state”. [2]

What nonisolated(unsafe) does (and when to use it)

nonisolated(unsafe) is an unsafe opt-out: it tells the compiler “don’t enforce actor-isolation / data-race checking for this storage; I’m taking responsibility.” It was introduced specifically to handle cases where the compiler can’t “see” your synchronization (locks/queues/C code, etc.), including global/static variables. [1]

Typical use case: the storage is effectively isolated by some external mechanism (a serial queue, a lock, etc.), but the compiler can’t prove it:

import Dispatch

struct Cache {
  static let q = DispatchQueue(label: "cache.q")

  nonisolated(unsafe) static var storage: [String: Int] = [:]

  static func get(_ k: String) -> Int? { q.sync { storage[k] } }
  static func set(_ k: String, _ v: Int) { q.sync { storage[k] = v } }
}

Swift.org’s release notes use the same pattern: guard all accesses externally, then mark the static/global as nonisolated(unsafe) to silence the warning. [1]

Preferred fixes (before nonisolated(unsafe))

The compiler (and Swift team guidance) generally expects one of these first: [2]

  1. Make it immutable: static let (and the value/type should be Sendable).
  2. Isolate it to an actor/global actor (commonly @MainActor), so accesses are serialized and crossing the boundary requires await.
  3. Use nonisolated(unsafe) only if you’ve already made it safe via external synchronization and just need to tell the compiler. [1][2]

Notes / gotchas

  • nonisolated(unsafe) removes compile-time protection; you can still have real races—TSAN/exclusivity can still catch bugs at runtime. [1]
  • If you notice static var inside an actor not being flagged, that has been reported as a compiler bug; don’t assume it’s safe just because it compiles. [3]

Sources: Swift.org Swift 5.10 release notes (introducing nonisolated(unsafe) and showing static/global usage) [1], Swift Forums guidance on global/static mutable state diagnostics and recommended fixes [2], Stack Overflow discussion noting actor static-var checking issue/bug [3].

🏁 Script executed:

# Find and read the MainContentBlockerGenerator.swift file
find . -name "MainContentBlockerGenerator.swift" -type f

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 179

🏁 Script executed:

cat -n BrowserKit/Sources/ExecutableContentBlockingGenerator/MainContentBlockerGenerator.swift

Repository: qodo-benchmark/firefox-ios-combined-coderabbit

Length of output: 596

Avoid nonisolated(unsafe) on a known non-thread-safe singleton.

The ContentBlockerGenerator is explicitly non-thread-safe (per the comment), but nonisolated(unsafe) removes Swift 6 concurrency checks on this static instance, allowing potential unsafe cross-actor access without compile-time protection. Since the generator is only used in main(), move the instantiation locally instead:

Suggested fix 
     public static func main() {
-        generator.generateLists()
+        let generator = ContentBlockerGenerator.factory()
+        generator.generateLists()
     }

Remove the static generator and its nonisolated(unsafe) annotation.

Alternatively, isolate the struct to @MainActor if the generator must be shared across functions.

🤖 Prompt for AI Agents 
In
`@BrowserKit/Sources/ExecutableContentBlockingGenerator/MainContentBlockerGenerator.swift`
around lines 5 - 6, The static nonisolated(unsafe) generator is removing
concurrency protection for the non-thread-safe ContentBlockerGenerator; remove
the static `generator` declaration and instead call
`ContentBlockerGenerator.factory()` locally inside `main()` where it's used, or
if you truly need a shared instance, mark the type or the static as `@MainActor`
to enforce single-threaded access (update uses of `generator` to the new local
variable or actor-isolated static accordingly).


// Static main needs to be used for executable, providing an instance so we can
// call it from a terminal
Expand Down
37 changes: 14 additions & 23 deletions BrowserKit/Tests/SiteImageViewTests/SVGImageProcessorTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Kingfisher
import GCDWebServers

class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
func testDownloadingSVGImage_withKingfisherProcessor_forStandardSVGCase() async {
func testDownloadingSVGImage_withKingfisherProcessor_forStandardSVGCase() {
let assetType: AssetType = .svgCase1
let expectedRasterSize = CGSize(width: 360, height: 360)

Expand All @@ -18,32 +18,26 @@ class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
return
}

guard let mockedURL = try? await startMockImageServer(imageData: imageData, forAssetType: assetType) else {
guard let mockedURL = try? startMockImageServer(imageData: imageData, forAssetType: assetType) else {
XCTFail("Check bundle setup for mock server response data")
return
}

let exp = expectation(description: "Image download and parse")

let siteDownloader = DefaultSiteImageDownloader()
siteDownloader.downloadImage(with: mockedURL, options: [.processor(SVGImageProcessor())]) { result in
switch result {
case .success(let result):
XCTAssertEqual(result.originalData, imageData)
XCTAssertEqual(result.url, mockedURL)
XCTAssertEqual(result.image.size, expectedRasterSize)
exp.fulfill()
case .failure(let error):
XCTFail("Should not have an error: \(error) \(error.errorDescription ?? "")")
exp.fulfill()
}
}

await fulfillment(of: [exp], timeout: 2.0)
}

/// FXIOS-11361: Tests a special SVG which previously caused crashes in older versions of SwiftDraw.
func testDownloadingSVGImage_withKingfisherProcessor_forSpecialSVGCase() async {
func testDownloadingSVGImage_withKingfisherProcessor_forSpecialSVGCase() {
let assetType: AssetType = .svgCase2
let expectedRasterSize = CGSize(width: 360, height: 360)

Expand All @@ -52,7 +46,7 @@ class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
return
}

guard let mockedURL = try? await startMockImageServer(imageData: imageData, forAssetType: assetType) else {
guard let mockedURL = try? startMockImageServer(imageData: imageData, forAssetType: assetType) else {
XCTFail("Check bundle setup for mock server response data")
return
}
Expand All @@ -73,18 +67,18 @@ class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
}
}

await fulfillment(of: [exp], timeout: 2.0)
wait(for: [exp], timeout: 2.0)
}

func testDownloadingICOImage_withKingfisherProcessor() async {
func testDownloadingICOImage_withKingfisherProcessor() {
let assetType: AssetType = .ico

guard let imageData = try? dataFor(type: assetType) else {
XCTFail("Could not load test asset")
return
}

guard let mockedURL = try? await startMockImageServer(imageData: imageData, forAssetType: assetType) else {
guard let mockedURL = try? startMockImageServer(imageData: imageData, forAssetType: assetType) else {
XCTFail("Check bundle setup for mock server response data")
return
}
Expand All @@ -104,16 +98,16 @@ class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
}
}

await fulfillment(of: [exp], timeout: 2.0)
wait(for: [exp], timeout: 2.0)
}

func testDownloadingGarbageData_withKingfisherProcessor() async {
func testDownloadingGarbageData_withKingfisherProcessor() {
guard let garbageData = try? dataFor(type: .badType) else {
XCTFail("Could not load test asset")
return
}

guard let mockedURL = try? await startMockImageServer(imageData: garbageData, forAssetType: .badType) else {
guard let mockedURL = try? startMockImageServer(imageData: garbageData, forAssetType: .badType) else {
XCTFail("Check bundle setup for mock server response data")
return
}
Expand All @@ -131,13 +125,13 @@ class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
}
}

await fulfillment(of: [exp], timeout: 2.0)
wait(for: [exp], timeout: 2.0)
}

func testDownloadingEmptyImage_withKingfisherProcessor() async {
func testDownloadingEmptyImage_withKingfisherProcessor() {
let emptyData = Data()

guard let mockedURL = try? await startMockImageServer(imageData: emptyData, forAssetType: .ico) else {
guard let mockedURL = try? startMockImageServer(imageData: emptyData, forAssetType: .ico) else {
XCTFail("Check bundle setup for mock server response data")
return
}
Expand All @@ -155,7 +149,7 @@ class SVGImageProcessorTests: XCTestCase, @unchecked Sendable {
}
}

await fulfillment(of: [exp], timeout: 2.0)
wait(for: [exp], timeout: 2.0)
}
}

Expand Down Expand Up @@ -211,9 +205,6 @@ extension SVGImageProcessorTests {
throw MockImageServerError.noAssetData
}

// GDCWebServer must be initialized for the first time on the main thread
// So this @MainActor ensures that if this test file is run alone it will not crash
@MainActor
func startMockImageServer(imageData: Data, forAssetType assetType: AssetType) throws -> URL {
let webServer = GCDWebServer()

Expand Down
Loading