Skip to content

Conversation

@ih-codes
Copy link
Collaborator

@ih-codes ih-codes commented Sep 18, 2025

📜 Tickets

Jira ticket
Github issue

💡 Description

Fix main actor isolation warnings in MozillaRustComponents.

See that #29425 description for how I was able to surface the warnings in MozillaRustComponents...

cc @Cramsden @lmarceau @dataports | Swift 6 Migration

📝 Checklist

  • I filled in the ticket numbers and a description of my work
  • I updated the PR name to follow our PR naming guidelines
  • I ensured unit tests pass and wrote tests for new code
  • If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
  • If adding telemetry, I read the data stewardship requirements and will request a data review
  • If needed, I updated documentation and added comments to complex code
  • If needed, I added a backport comment (example @Mergifyio backport release/v150.0)

@ih-codes ih-codes requested a review from dataports September 18, 2025 20:18
@ih-codes ih-codes requested a review from a team as a code owner September 18, 2025 20:18
@ih-codes ih-codes force-pushed the ih/FXIOS-13502-MozillaRustComponents-fix-actor-isolation-warnings branch 2 times, most recently from bab4bc7 to b199df6 Compare September 19, 2025 19:57
@ih-codes
Copy link
Collaborator Author

ih-codes commented Sep 19, 2025

I just force pushed a completely different fix. After quibbling over it for a while, it seems best we make a janky workaround at the Rust Comp level instead of trying to fix actual errors (not warnings) in the Client generated code. This is such a trivial use case as well, that while I don't love this workaround, I'm hoping it's good enough "for now" with a FIXME ticket there.

…orkaround for setting accessibility identifiers to prevent larger impacts and errors in Client generated code.
@ih-codes ih-codes force-pushed the ih/FXIOS-13502-MozillaRustComponents-fix-actor-isolation-warnings branch from b199df6 to dc6c911 Compare September 19, 2025 19:59
@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Sep 19, 2025

🧹 Tidy commit

Just 1 file(s) touched. Thanks for keeping it clean and review-friendly!

🌱 Tiny but mighty

Only 25 line(s) changed. Fast to review, faster to land! 🚀

Generated by 🚫 Danger Swift against dc6c911

@ih-codes
Copy link
Collaborator Author

ih-codes commented Sep 22, 2025

After chatting with @Cramsden and @lmarceau, I'm going to see if I can just remove these lines and instead have the call sites set the accessibilityIdentifier to the name.

Copy link
Contributor

@dataports dataports left a comment

Choose a reason for hiding this comment

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

Not sure from the recent comments if this is ready, but LGTM?

@ih-codes
Copy link
Collaborator Author

ih-codes commented Sep 23, 2025

Not sure from the recent comments if this is ready, but LGTM?

I'm chatting about this in my 1:1 later with @Cramsden ! But it might be staying this way after all to prevent wider-reaching changes. 😆

@ih-codes ih-codes changed the base branch from main to epic-branch/swift-migration September 24, 2025 18:13
@ih-codes
Copy link
Collaborator Author

After some discussion, we're just going to do this and leave the FIXMEs for later improvements. I've also changed the merge target to the epic branch.

@ih-codes ih-codes merged commit d7b28ce into epic-branch/swift-migration Sep 24, 2025
10 checks passed
@ih-codes ih-codes deleted the ih/FXIOS-13502-MozillaRustComponents-fix-actor-isolation-warnings branch September 24, 2025 19:13
ih-codes added a commit that referenced this pull request Sep 24, 2025
…nings in MozillaRustComponents (#29426)

"Fix" main actor isolation warnings in MozillaRustComponents with a workaround for setting accessibility identifiers to prevent larger impacts and errors in Client generated code.
ih-codes added a commit that referenced this pull request Sep 25, 2025
…nings in MozillaRustComponents (#29426)

"Fix" main actor isolation warnings in MozillaRustComponents with a workaround for setting accessibility identifiers to prevent larger impacts and errors in Client generated code.
ih-codes added a commit that referenced this pull request Sep 26, 2025
… from last week (#5) (#29657)

* Refactor FXIOS-13511 [Swift 6] Remove Equatable from AppState and other redux states (#29495)

* Remove unneeded equatable conformance on redux states

* Fix tests

* Fix test

* Refactor FXIOS-13532 [Swift 6 Migration] Fix strict concurrency errors related to Notifications  (#29514)

Fun stuff

* Refactor FXIOS-13502 [Swift 6 Migration] Fix main actor isolation warnings in MozillaRustComponents (#29426)

"Fix" main actor isolation warnings in MozillaRustComponents with a workaround for setting accessibility identifiers to prevent larger impacts and errors in Client generated code.

* Refactor FXIOS-13511 [Swift 6] Move Tab to MainActor and resolve strict concurrency warnings (#29497)

* Refactor FXIOS-13511 [Swift 6] Remove Equatable from AppState and other redux states (#29495)

* Remove unneeded equatable conformance on redux states

* Fix tests

* Fix test

* Refactor FXIOS-13532 [Swift 6 Migration] Fix strict concurrency errors related to Notifications  (#29514)

Fun stuff

* first pass of isolating tab to the main actor

* Clean up tests and push commented out code

* fix additional warnings

* Clean up remaining warnings

* fix tests

* add missing ticket numbers

* resolve introduced warning

---------

Co-authored-by: lmarceau <lmarceau@mozilla.com>

* Refactor FXIOS-13532 #29411 [Swift 6 Migration] AppDelegate + PushNotification (#29590)

* Fix for Constellation update

* Fix UNUserNotificationCenterDelegate methods

* Fix tests

* Refactor FXIOS-13235 [Swift 6 Migration] Enable strict concurrency checking in SummarizerKit (#29575)

* Enable strict concurrency in SummarizerKit.

* Fix closure `Sendable` warnings.

* Fix some other warnings. Suppress others with FIXME tickets.

* Refactor FXIOS-13463 #29256 [Swift 6 Migration] WebEngine @mainactor (#29393)

* Policy decider

* Make main actor

* @mainactor stuff

* Fix the tests

* Adjust Client

* Add ticket number

* Adjust following comments

* Refactor FXIOS-12796 [Swift 6 Migration] Fix more under-specified protocols (#29650)

* Fix under-specified protocols: AddressToolbarDelegate, EmptyPrivateTabView, InsetUpdatable, PhotonActionSheetContainerCellDelegate, NavigationController.

* Fix unit tests.

* Refactor FXIOS-13502 [Swift 6 Migration] Fix or suppress `Sendable` and misc. warnings in MozillaRustComponents (round 1) (#29425)

* Fix or suppress Sendable warnings in MozillaRustComponents.

* Janky workaround for main actor isolation warning.

* Update some Rust Components Sendable callbacks with @mainactor as well so the Client can reason better about main actor isolation.

* Refactor FXIOS-13511 [Swift 6] Additional Strict Concurrency Clean up for Tab and Friends (#29628)

* Additional tab clean up

* Isolate UserScriptManager to main actor

* fix tests

* turn off strict concurrency

* more main actor isolation

* resolve introduced warnings

---------

Co-authored-by: Carson Ramsden <carsonramsden@gmail.com>
Co-authored-by: lmarceau <lmarceau@mozilla.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants