Refactor SYNC-4917 Migrate to use app-services FxA state machine by skhamis · Pull Request #32716 · mozilla-mobile/firefox-ios

skhamis · 2026-03-26T18:57:58Z

📜 Tickets

💡 Description

This migrates the FxA auth lifecycle on iOS over to the Rust-native state machine, which Android already uses. Previously iOS was manually tracking auth states, calling initDevice/ensureCapabilities on login, and reading the local device ID from DeviceConstellation — which required waiting on an async server round-trip. Now we just call processEvent(_:) into Rust and let it handle the state transitions and device registration internally.

What changed

State machine: Auth lifecycle is now driven through processEvent(_:) / FxaEvent. No longer manual calls initDevice or ensureCapabilities -> a-s handles those as part of completeOAuthFlow.
Device ID fix: Swapped out deviceConstellation().state().localDevice for getCurrentDeviceId(), which is available immediately after login. This fixes a silent sync failure (DeviceIdError) that happened when DeviceConstellation.refreshState() hadn't finished yet.
Post-login tabs fix: AccountSyncHandler now listens for .accountAuthenticated to call setLocalTabs() on login. Before this, syncEverything() would fire on login but the Rust TabsStore had no local tabs yet, so tabs wouldn't show up on other clients until the user actually add/removed/switched tabs.

📝 Checklist

[x ] 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 adding or modifying strings, I read the guidelines and will request a string review from l10n
If needed, I updated documentation and added comments to complex code

mhammond

this looks like a nice improvement. I wonder if claude can help identify if this patch leaves behind dead code that can be removed?

firefox-ios/Providers/RustSyncManager.swift

bendk

This looks really nice to me.

The one thing I think needs to be figured out is the threading model for sending events and processing the side effects. I think it might be just mean making fxaFsmQueue serialized and documenting the correct usage.

An actor could be a great way of handling this, since it's mostly about making sure the state isn't being mutated by 2 threads at once. However, that would probably be too big of a change.

I don't know enough about the firefox-ios side of things to review that side of the integration code. I just focused on checking if the Rust parts seem correct. For that I guess we need an ios engineer and/or some good testing.

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

firefox-ios/Providers/RustSyncManager.swift

mhammond

I've only a couple of nit about your comments - sorry about that 😅 . LGTM, nice work! I'm happy when Ben and iOS are happy :)

firefox-ios/Providers/RustSyncManager.swift

bendk

I agree with Mark's comment suggestions, other than that this is looking right to me. Thanks for the serial comment on the dispatch queue, that makes me feel better that this is all going to run correctly.

mobiletest-ci-bot · 2026-04-07T23:28:45Z

	Messages
📖	Project coverage: 41.11%

💪 Quality guardian

2 tests files modified. You're a champion of test coverage! 🚀

🏔️ Summit Climber

This PR is a big climb with 828 lines changed!
Thanks for taking on the heavy lifting 💪.
Reviewers: a quick overview or walkthrough will make the ascent smoother.

💬 Description craftsman

Great PR description! Reviewers salute you 🫡

✅ New file code coverage

No new file detected so code coverage gate wasn't ran.

Client.app: Coverage: 39.41

File	Coverage
AccountSyncHandler.swift	88.59%	✅
RustSyncManager.swift	45.25%	⚠️

CredentialProvider.appex: Coverage: 31.36

File	Coverage
RustSyncManager.swift	45.25%	⚠️

NotificationService.appex: Coverage: 36.46

File	Coverage
RustSyncManager.swift	45.25%	⚠️

ShareTo.appex: Coverage: 28.48

File	Coverage
RustSyncManager.swift	45.25%	⚠️

Generated by 🚫 Danger Swift against e263ace

firefox-ios/Client/AccountSyncHandler.swift

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

lmarceau · 2026-04-08T01:01:55Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

        return try acct.gatherTelemetry()
    }

+    // Serial by default (no .concurrent attribute). All FSM state mutations and


What does FSM means + we should move this variable declaration at the top of the file. This is where we normally search and expect variables

lmarceau · 2026-04-08T01:13:14Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

+                DispatchQueue.main.async {
+                    NotificationCenter.default.post(name: .accountLoggedOut, object: nil)
                }


Posting the notification on the main queue doesn't guarantee the actual notification to be received in the observer on that queue since this responsibility falls onto the observer itself, not sure if this was the intent here?

I checked, and we're actually calling ensureMainThread where we are listening to that notification, which does guarantee that code will be run on the main thread.

So I would personally remove this DispatchQueue, unless the intent was something else?

lmarceau · 2026-04-08T01:13:35Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

-                        FxALog.info("Ensuring device capabilities...")
-                        requireConstellation().ensureCapabilities(capabilities: deviceConfig.capabilities)
+        case .authIssues:
+            DispatchQueue.main.async {


Same comment about queue/notification, but not sure where this notification is observed.

lmarceau · 2026-04-08T01:13:45Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

+    private func refreshProfileAsync(ignoreCache: Bool) {
+        DispatchQueue.global().async {
+            do {
+                let p = try self.requireAccount().getProfile(ignoreCache: ignoreCache)


Suggested change

let p = try self.requireAccount().getProfile(ignoreCache: ignoreCache)

let profile = try self.requireAccount().getProfile(ignoreCache: ignoreCache)

lmarceau · 2026-04-08T01:15:49Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

+                    guard self.state == .connected || self.state == .authIssues else { return }
+                    self.profile = p
+                    DispatchQueue.main.async {
+                        NotificationCenter.default.post(


Same comment about queue/notification

lmarceau · 2026-04-08T01:16:08Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/FxAClient/FxAccountManager.swift

+        // starts in .uninitialized. Without this call,
+        // subsequent events like .beginOAuthFlow would be rejected by the FSM.
+        account = createAccount()
+        let dc = DeviceConfig(


Suggested change

let dc = DeviceConfig(

let deviceConfig = DeviceConfig(

skhamis requested review from bendk, lougeniaC64 and mhammond March 26, 2026 18:57

mhammond reviewed Mar 30, 2026

View reviewed changes

firefox-ios/Providers/RustSyncManager.swift Outdated Show resolved Hide resolved

bendk suggested changes Mar 30, 2026

View reviewed changes

skhamis marked this pull request as ready for review April 7, 2026 04:19

skhamis requested a review from a team as a code owner April 7, 2026 04:19

skhamis requested a review from mattreaganmozilla April 7, 2026 04:19

mhammond reviewed Apr 7, 2026

View reviewed changes

firefox-ios/Providers/RustSyncManager.swift Outdated Show resolved Hide resolved

firefox-ios/Providers/RustSyncManager.swift Outdated Show resolved Hide resolved

bendk approved these changes Apr 7, 2026

View reviewed changes

Refactor SYNC-4917 Migrate to use app-services FxA state machine

4e7bd82

skhamis force-pushed the fxa-state-machine branch from 0de9847 to 4e7bd82 Compare April 7, 2026 19:15

mattreaganmozilla requested a review from issammani April 7, 2026 19:39

lmarceau reviewed Apr 8, 2026

View reviewed changes

changes from review feedback

e263ace

	let p = try self.requireAccount().getProfile(ignoreCache: ignoreCache)
	let profile = try self.requireAccount().getProfile(ignoreCache: ignoreCache)

Conversation

skhamis commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Tickets

💡 Description

📝 Checklist

Uh oh!

mhammond left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bendk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhammond left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bendk left a comment

Choose a reason for hiding this comment

Uh oh!

mobiletest-ci-bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💪 Quality guardian

🏔️ Summit Climber

💬 Description craftsman

✅ New file code coverage

Client.app: Coverage: 39.41

CredentialProvider.appex: Coverage: 31.36

NotificationService.appex: Coverage: 36.46

ShareTo.appex: Coverage: 28.48

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lmarceau Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lmarceau Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lmarceau Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lmarceau Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lmarceau Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lmarceau Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

skhamis commented Mar 26, 2026 •

edited

Loading

mobiletest-ci-bot commented Apr 7, 2026 •

edited

Loading