1479929 - Evaluate disabling locally enabled engines that aren't mentioned in the remote `meta/global`

Status: NEW

(Firefox :: Sync, enhancement, P3)

Description

[Lina Butler (ex-Mozilla)]

Attachment: locally-enabled-remotely-unmentioned.diff

Currently, if we have an engine that's enabled locally, and download a `m/g` record that doesn't mention the engine at all (in `engines` or `declined`), we'll disable that engine, but not decline it. See the attached test.

Desktop (https://searchfox.org/mozilla-central/rev/033d45ca70ff32acf04286244644d19308c359d5/services/sync/modules/stages/enginesync.js#326-333), Android (https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java#93-96,101-105), and iOS (https://github.com/mozilla-mobile/firefox-ios/blob/4074c99f59cf1634b0a57b3d9bfe25c5c45d1075/Sync/SyncStateMachine.swift#L700-L703) all do this.

A consequence of this behavior is, if we have a client that doesn't support a particular engine (for example, iOS and Android don't support `addresses` or `creditcards`), and that client is the first to sync after a node reassignment, it'll disable that engine on all other clients that *do* support it. Ditto if you connect a new client that supports new engines. Since we don't show CWTS after a node reassignment, or when connecting a new client, folks won't know the engine is disabled until they open Sync prefs.

iOS hard-codes collection names that it doesn't support into https://github.com/mozilla-mobile/firefox-ios/blob/4074c99f59cf1634b0a57b3d9bfe25c5c45d1075/Sync/SyncStateMachine.swift#L34-L38, which works, but it's not clear to me that the current behavior is correct...especially now that `declined` exists on all clients (for explicitly unchecked engines; IIUC, it didn't when bug 969669 was filed), and we should expect clients to support different data types now and in the future.

Comment 1

[Lina Butler (ex-Mozilla)]

In the new Sync Manager proposal for a-s, Mark suggested adding an enabled list to the meta/global payload. This would allow clients that don't support a particular engine or data type (for example, Fenix doesn't support passwords, Lockwise only syncs passwords) to round-trip engine selections after a node reassignment.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Yeah, I think we should make the same change to desktop as we end up making in the rust components. Even though desktop currently supports all known engines (well, to some degree, addresses and credit-cards are, err, odd) we might as well ensure that it does correctly round-trip unknown engines to handle a possible future.

(While it doesn't change things much, I was thinking the new attribute be called "accepted", but the intent is the same)

Do we need followups for iOS and Fennec too? We probably need not implement everything there, but we do need to ensure they don't break the scheme

Comment 3

[Lina Butler (ex-Mozilla)]

We'll still want this for CWTS, but I'm not actively working on this, and we'll likely revisit this next year.