Skip to content

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

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_combined_20260121_augment_sentry_coderabbit_1_base_refactor_fxios-14466_fxios-14463_swift_6_migration_migrate_shareto_and_browserkit_package_to_swift_6_pr220
Choose a base branch
from
Open

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

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
54f6cc2
Migrate entire BrowserKit package to Swift 6.2.
ih-codes Dec 29, 2025
2f8c750
Migrate entire ShareTo package to Swift 6.
ih-codes Dec 29, 2025
1ad6509
Cleanup BrowserKit Package file now that experimental/upcoming featur…
ih-codes Dec 29, 2025
61460d5
Fix lingering Swift 6 errors in WebEngineTests.
ih-codes Dec 29, 2025
84998a2
Fix a few more new warnings.
ih-codes Dec 29, 2025
ca8483c
Fix the preconcurrency GCDWebServer setup in SiteImageViewTests, as i…
ih-codes Dec 29, 2025
b554fc0
Fix compile issue with Focus from ContentBlockingGenerator strict con…
ih-codes Dec 29, 2025
5439cb2
update pr
tomerqodo Jan 21, 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 22, 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:

# Locate Swift/Xcode version pins and CI references
fd -a '\.xcode-version|\.tool-versions|\.swift-version'
rg -n "xcode|swift" -g '*.yml' -g '*.yaml' -g '*.json'

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

Length of output: 50402

🏁 Script executed:

# Check if BrowserKit/Package.swift exists and its content
find . -path "*/BrowserKit/Package.swift" -o -path "*/Package.swift" | head -20

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

Length of output: 214

🏁 Script executed:

# Look for all Package.swift files to understand the project structure
fd -t f "Package.swift" | head -30

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

Length of output: 206

🏁 Script executed:

# Check the actual Xcode version requirements in bitrise.yml more carefully
rg -A 5 -B 5 "swift.tools.version|6\.2" bitrise.yml

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

Length of output: 4087

🏁 Script executed:

# Read BrowserKit/Package.swift to confirm swift-tools-version
cat -n BrowserKit/Package.swift | head -20

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

Length of output: 677

🏁 Script executed:

# Search for swift-tools-version in all Package.swift files
rg "swift-tools-version" --type swift

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

Length of output: 567

🏁 Script executed:

# Check if there's any documentation or configuration about Swift version requirements
rg -i "swift.*version|toolchain" --type md --type txt -g "README*" -g "CONTRIBUTING*" -g ".github*" | head -30

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

Length of output: 445

🏁 Script executed:

# Check GitHub workflows to see if they specify Swift/Xcode versions
fd -t f ".github" -o "workflows" | xargs ls -la 2>/dev/null | head -20

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

Length of output: 1456

🏁 Script executed:

# Look for any Swift version pinning or documentation files
fd -t f "\.(yml|yaml|txt|md)$" -x rg -l "6\.2|swift.*6" {}

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

Length of output: 103

🏁 Script executed:

# Check if there's a root README with version information
cat README.md | head -100

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

Length of output: 3135

🏁 Script executed:

# Look for version documentation or requirements files
fd -t f "\.(md|txt)$" | xargs grep -l "Xcode\|Swift" | head -10

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

Length of output: 3179

🏁 Script executed:

# Check the .github directory for workflow files
find .github -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | xargs cat 2>/dev/null | grep -i "xcode\|swift" | head -20

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

Length of output: 1678

Update BrowserKit/Package.swift swift-tools-version to match project requirements

The swift-tools-version: 6.2 in BrowserKit/Package.swift is incompatible with the project's documented toolchain (Xcode 26.1, Swift 5.10 per README.md). This will cause build failures in CI workflows using Xcode 26.1. Align BrowserKit's swift-tools-version with the other package manifests (Package.swift: 5.7, MozillaRustComponents: 5.10) or update the project-wide toolchain requirements if Swift 6.2 is intended.

🤖 Prompt for AI Agents 
In `@BrowserKit/Package.swift` at line 1, Replace the incorrect
swift-tools-version header in BrowserKit/Package.swift (the line "//
swift-tools-version: 6.2") with the project's supported toolchain version to
match the rest of the repo (use "// swift-tools-version: 5.10" to align with
Xcode 26.1 / Swift 5.10); update only that top-of-file directive so the package
manifest matches other manifests (e.g., MozillaRustComponents) and CI will use
the correct Swift toolchain.


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()

// 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)
}
Comment on lines 26 to 37
Copy link

@coderabbitai coderabbitai bot Jan 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Await the async download in this test.

This test no longer waits for the async callback, so it can pass without executing assertions (or be flaky). Add an expectation and wait like the other tests.

✅ Proposed fix 
-        let siteDownloader = DefaultSiteImageDownloader()
-        siteDownloader.downloadImage(with: mockedURL, options: [.processor(SVGImageProcessor())]) { result in
+        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()
             }
         }
+        wait(for: [exp], timeout: 2.0)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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)
}
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()
}
}
wait(for: [exp], timeout: 2.0)
}
🤖 Prompt for AI Agents 
In `@BrowserKit/Tests/SiteImageViewTests/SVGImageProcessorTests.swift` around
lines 26 - 37, The test invokes
DefaultSiteImageDownloader.downloadImage(with:options:){...} asynchronously but
never waits for the callback; add an XCTestExpectation (e.g., let exp =
expectation(description: "download") ), fulfill it inside both the .success and
.failure branches, and call wait(for: [exp], timeout: ...) after initiating the
download so the assertions under SVGImageProcessor are actually executed; update
the test that uses SVGImageProcessor() to create and wait on the expectation.


/// 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