2058441 - Support migrate addresses to the Rust store

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, task, P3)

Comment 1

[Dimi Lee [:dimi]]

Attachment: Bug 2058441 - P1. Vendor application-services 1677375cb with the address metadata APIs r=joschmidt

Groundwork for mirroring desktop addresses into the Application Services
autofill store. No desktop behaviour changes here -- nothing calls any of it
yet.

Storage APIs for importing records that already have metadata:

• add_address_with_meta / add_many_addresses_with_meta take a caller-supplied
  guid, timestamps and change counter, as per the logins *_with_meta functions.
  The bulk variant isolates per-record failures so one bad record cannot abort
  a migration.
• update_address_with_meta and add_many_address_tombstones have no logins
  equivalent. The latter takes the caller's time_deleted rather than stamping
  now, so a deletion still pending upload is not lost and its record is not
  resurrected from the server.
• AddressMeta carries sync_change_counter, which internal_address_from_meta was
  hardcoding to 0. update_internal_address could previously only increment the
  counter or leave it, so it now takes a CounterUpdate of Increment, Leave or
  Set(i64) - otherwise the insert and update paths disagree about it.
• Negative timestamps are clamped to 0, since Timestamp is a u64 and a negative
  value would wrap to a huge one and win every comparison in Metadata::merge.

This is a change to the vendored application-services component and should be
upstreamed before this lands; it is carried locally only so the rest of the
stack can be developed and tested. It stacks on Bug 2056496 P1, whose
Store::shutdown() also renumbers the shared UniFFI function table -- the
regenerated bindings and scaffolding here include both.

Desktop cannot drive address sync from this store yet: that needs a bridged
sync engine, which will land with the sync work rather than here.

Co-Authored-By: Claude Opus 5 (1M context) <[e-mail]>

Comment 2

[Dimi Lee [:dimi]]

Attachment: WIP: Bug 2058441 - P2. Add the Rust address storage adapter

Presents the subset of the AutofillRecords address interface that desktop
consumers use, over the Application Services autofill Store, so it can later
back FormAutofillStorage.getAddresses(). Nothing constructs it yet, so this
commit changes no behaviour.

Scope is Tier-1 CRUD for addresses only. Reads reconstruct the computed fields
(country-name, address-line*, -name, tel-) and drop empty-string and hidden
keys, matching AutofillRecords._cloneAndCleanUp, so a record read from Rust is
identical to the same record read from JSON -- consumers compare stored records
against freshly normalised input, and any divergence would silently break
deduping.

The *WithMeta writes preserve a caller-supplied guid and timestamps, which the
mirror needs to keep the two stores in parity: update()/notifyUsed() let the
store self-assign now() and drift.

The Tier-2 sync surface (changeGUID, reconcile, pullSyncChanges, ...) throws
NS_ERROR_NOT_IMPLEMENTED. The JSON backend owns sync, and once the Rust-backed
sync engine is selected those per-record methods are not used at all.

The adapter takes the shared store as a promise from
RustAutofillStore.ensureOpen() rather than opening its own, matching how
PassportStorage consumes it (Bug 2056496 P2). There is one autofill.sqlite per
profile and RustAutofillStore owns its lifecycle and shutdown blocker; opening
a second connection to the same file is exactly the contention that handler
exists to avoid.

Depends on D314678

Comment 3

[Dimi Lee [:dimi]]

Attachment: WIP: Bug 2058441 - P3. Mirror address writes into the Rust store and migrate

Turns the Rust address store on as a write-only mirror. Reads are untouched --
every read still comes from the JSON store, which stays authoritative for
guids, normalisation, sync metadata and unknown-field roundtrip. The point of
this phase is to populate Rust and keep it current so telemetry can show
whether that works on real profiles, before a later phase reads from it.

Two prefs, both off by default:

• storage.rust.enabled -- intent. Creates the Rust storage, turns on
  dual-write, and runs the one-time migration.
• storage.rust.migrationVersion -- which migration generation this profile has
  completed. An int rather than a boolean so a later phase can bump the
  expected value and force one fresh re-migration over this phase's data.

The two temporary pieces are kept separate from the storage that survives them,
so deprecating JSON later is a deletion rather than a refactor:

• AddressMirror (112 lines) replays each write from the authoritative store into
  the follower and reports failures. It holds no storage logic of its own, and
  which store leads is a constructor argument -- when Rust becomes
  authoritative the pair swaps and the module is unchanged.
• AddressStorageMigrator owns the one-shot bulk copy: wipe, import, verify.
  Mirroring is a per-write concern and migrating is a single operation; they
  have different lifetimes and no shared state.

Everything either of them would need long-term already lives in
RustAutofillAddressesAdapter, which is a peer of the JSON collection.

Dual-write: getAddresses() returns a Proxy over the JSON collection that
replays every mutation into Rust after the JSON write lands. Reads are not
routed at all, so there is no event ordering to coordinate -- JSON is the only
store anything reads from or notifies for. A Rust write failure poisons the
mirror and is reported, but never fails the caller's write.

Mirror writes use the *WithMeta APIs rather than update()/notifyUsed(): those
let the Rust store self-assign now() for timeLastModified/timeLastUsed and
drift out of parity with the record JSON just wrote.

Migration copies the records that predate the mirror, verifying that every
record migrated AND that Rust holds as many records as JSON before recording
the generation. It carries sync change counters and tombstones so a profile
does not look freshly-created to Sync later, and reads _data directly because
_cloneAndCleanUp strips the hidden _sync and excludes tombstones. On failure
the version pref is left alone so the next launch retries.

Reconciliations are serialised onto _mirrorStatePromise rather than
overwriting it. Pref observers fire synchronously, so a change arriving while
one is still suspended on store setup would otherwise run two concurrently and
build two adapters over the same sqlite file.

FormAutofillSync.storage is resolved on every access instead of being cached.
It previously latched whatever collection it saw first, which in this phase
means sync could pin itself to the unmirrored JSON collection and every record
arriving from sync would bypass the mirror -- precisely the divergence this
phase's telemetry exists to catch. The sourceSync remove in applyIncoming() is
now awaited for the same reason.

Telemetry: migrate_to_rust (one event per migration attempt, with counts,
duration and whether verification passed) and rust_write_failure (per failed
Rust write, with a normalised PII-free message).

NOTE the metrics' bugs/data_reviews point at the tracking bug as placeholders
and must be replaced with a real data review before this rides to release.

Depends on D314679

Comment 4

[Dimi Lee [:dimi]]

Attachment: Bug 2058441 - P2. Test the address migration r=joschmidt

xpcshell coverage for the previous three commits, plus a browser suite that
runs the address tests with the mirror on.

xpcshell:

• store smoke: addresses round-trip through the a-s Store from JS at all.
• adapter: Tier-1 CRUD, guid/timestamp preservation through the *WithMeta
  writes, bulk import with a duplicate-guid record isolated while the good one
  persists.
• bridged engine: the sync-metadata plumbing (last sync, sync id association,
  rotation, reset) and a full round trip -- stage an incoming remote record,
  apply it, confirm the local-only record is staged for upload and the remote
  one is persisted and not echoed back. This is where the bridge is actually
  verified; the equivalent Rust unit test cannot link NSS under plain
  cargo test.
• mirror: every mutation is replayed into Rust, and reads keep coming from JSON.
• parity: a record read from Rust is identical to the same record read from
  JSON, across a corpus including unicode, CJK, RTL, SQL metacharacters, emoji,
  multiline values and very long values. Consumers compare stored records
  against freshly normalised input, so any divergence would silently break
  deduping.
• migration: populates Rust, records the generation, runs once per generation,
  handles an empty store, and refuses to record the generation on a partial
  failure or when verification catches Rust holding fewer records than JSON.
  Also covers tombstones, sync change counters, the wipe of divergent rows, and
  that rapid pref toggling builds exactly one mirror.
• sync routing: FormAutofillStore.storage resolves the collection live, so a
  record arriving from sync reaches the mirror instead of bypassing it.

browser: mirror/ runs the address tests with storage.rust.enabled set. Its
head.js asserts, before every record deletion and at the end of every task,
that the JSON store and the Rust mirror hold exactly the same addresses -- so
every one of these tests doubles as a parity check over whatever operations it
happens to perform. 6245 assertions, and the address/ suite still passes
unchanged with the mirror off.

Addresses only. Credit cards are not mirrored, so their tests prove nothing
about this phase, and the open Rust store races the NSS token reset some of
them perform -- two of them time out under the mirror. Bug 2041273 disabled the
Rust mirror in test_logins_decrypt_failure.js for the same interaction.

Two things the shared store changes about testing. The adapter takes a
Promise<Store>, so tests that want isolation pass their own Store.init(temp
path) rather than going through RustAutofillStore -- production keeps the
one-per-profile rule, tests do not have to. And tests that go through
FormAutofillStorage do share the one profile store, so
test_migration_carries_sync_metadata scopes its outgoing-records assertion to
its own guids: earlier cases in the file leave tombstones behind, and
migrate()'s wipe only clears addresses_data.

That last point is worth following up on separately -- a re-migration inherits
stale tombstones, which matters once a later phase re-migrates over this
phase's data.

Depends on D314680

Comment 5

[Dimi Lee [:dimi]]

Attachment: Bug 2058441 - P2. Add the Rust address storage adapter r=NeilDeakin

Presents the AutofillRecords address interface over the Application Services
autofill Store, so it can later back FormAutofillStorage.getAddresses(). Nothing
constructs it yet.

Reads rebuild the computed fields and drop empty and hidden keys, so a record
read from Rust matches the same record read from JSON. Consumers compare stored
records against freshly normalised input, and a divergence would break deduping.

The *WithMeta writes keep a caller-supplied guid, timestamps and change counter,
for the migration and the mirror, which place records under an identity another
store assigned. Sync needs no per-record surface: bridgedEngine() reconciles
inside Rust.

shared() opens the one autofill.db per profile through RustAutofillStore, which
owns its shutdown blocker (Bug 2056496 P2). It caches the promise, and resolves
null rather than rejecting, because callers reach it from an initialize() that
FormAutofillParent leaves uncaught.

Comment 6

[Dimi Lee [:dimi]]

Attachment: Bug 2058441 - P1. Support migrating addresses to the Rust store and serving them from it r=joschmidt

Copies the profile's addresses into the Rust store once, then replays every later
write into it, so telemetry can show whether that works on real profiles before a
later phase makes Rust the only store.

Four prefs, inert by default: rust.enabled (intent, read at startup),
rust.migrationVersion (the generation copied, an int so a later phase can force a
fresh one), rust.migrationAttempts (gives up after 10, as logins does), and
rust.active (which store serves reads and fires storage-changed, observed so it
can be flipped mid-session).

getAddresses() returns AddressMirror's collection, a Proxy over the JSON one that
replays each mutation into Rust after the JSON write lands. Writes always go to
JSON first, so deactivating loses nothing, and a Rust failure poisons the mirror
and is reported but never fails the caller's write. Each write is replayed
through the API a Phase 3 caller would use, so a divergence is reported rather
than repaired. reconcile is the exception: it can update, replace, fork or drop,
and which is not knowable here, so Rust is brought to JSON's resulting state.

Reads go to whichever store is active, except sync's rawData and includeDeleted,
which stay with sync on JSON this phase, and the synchronous isEmpty(). Only the
active store notifies, as logins does with its isActive flag.

The mirror is built only once the migration reports every record copied:
constructing it applies the active pref, so a partial copy would serve reads and
show the user fewer addresses than they have. FormAutofillSync.storage is
resolved per access rather than cached, which pinned sync to the unmirrored
collection and bypassed the mirror for every incoming record.

Telemetry: rust_write, rust_read and migrate_to_rust record successes as well as
failures, since a failure count has no denominator otherwise, and carry an
error_code, whether the mirror was already poisoned, and a metric_version so
schema generations are never averaged together.

NOTE the metrics' bugs/data_reviews are placeholders and need a real data review
before this rides to release.

Depends on D315621

Comment 7

[Dimi Lee [:dimi]]

Attachment: WIP: Bug 2058441 - P5. Serve v2 Remote Settings routes from the quicksuggest mock server r=#urlbar

Fallout from the application-services bump in P1, and not otherwise related to
addresses. Remote Settings made v2 routes the default upstream, so for a custom
server URL the Rust client now joins v2 where it used to join v1. The pref
still holds a v1 URL, so the JS client keeps asking for /v1 while the Rust one
asks for /v2, and RemoteSettingsServer only registers /v1 routes.

Register each versioned route under both prefixes rather than moving it, so the
two clients can disagree about the version and both be served. That is already
how the attachment routes work.

Depends on D314681

Comment 8

[Phabricator Automation]

Comment on attachment 9624605 [details]
WIP: Bug 2058441 - P5. Serve v2 Remote Settings routes from the quicksuggest mock server r=#urlbar

Revision D317378 was moved to bug 2062539. Setting attachment 9624605 [details] to obsolete.

Comment 9

[Phabricator Automation]

Comment on attachment 9617388 [details]
Bug 2058441 - P1. Vendor application-services 1677375cb with the address metadata APIs r=joschmidt

Revision D314678 was moved to bug 2062539. Setting attachment 9617388 [details] to obsolete.

Comment 10

[Phabricator Automation]

Comment on attachment 9620425 [details]
Bug 2058441 - P2. Add the Rust address storage adapter r=NeilDeakin

Revision D315621 was moved to bug 2062540. Setting attachment 9620425 [details] to obsolete.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cf238de472f5
https://hg.mozilla.org/integration/autoland/rev/6e822b59e0de
P1. Support migrating addresses to the Rust store and serving them from it r=joschmidt,credential-management-reviewers,niklas
https://github.com/mozilla-firefox/firefox/commit/0b96f67b14ca
https://hg.mozilla.org/integration/autoland/rev/68b313991c56
P2. Test the address migration r=joschmidt,credential-management-reviewers

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6e822b59e0de
https://hg.mozilla.org/mozilla-central/rev/68b313991c56