Integrate appservices SyncManager

[grigoryk]

┆Issue is synchronized with this Jira Task

[liuche]

@grigoryk we're looking at this for Fenix Q3 - can you give some context on what this is blocked by? It seems like a series of bugs linking to each other :)

[grigoryk]

Landing this is blocked on parts of this "epic": mozilla/application-services#1316 (it's tracked in jira, but i've asked that updates are mentioned in that issue).

Rust SyncManager RFC describes an API that is very close to what will land above, allowing us to start integration work in a-c.

From another direction, there is the WebExtension work. See #3826 and #3817.

Project board that tracks this work overall: https://github.com/orgs/mozilla-mobile/projects/37

[liuche]

Thanks! This is really helpful.

[grigoryk]

Some implementation details.

Our current sync implementation directly calls sync on configured SyncableStore objects, passing in SyncAuthInfo. Our sync worker implementation is thus as follows: obtain cached auth info, and iterate over configured stores calling sync for each one.

Straw-man API as described in the rfc introduces a SyncManager API, and a concept of an Engine, which is an abstraction over one or more sync1.5 collections, and possibly other things.

enum Engine {
  History, // The "History" and "Forms" stores.
  Bookmarks, // The "Bookmark" store.
  Passwords,
}

SyncManager will have a sync method, that takes SyncParams.

fn sync(&self, params: SyncParams) -> Result<SyncResult>;

// Instead of massive param and result lists, we use structures.
// This structure is passed to each and every sync.
struct SyncParams {
  // The engines to Sync. None means "sync all"
  engines: Option<Vec<Engine>>,
  // Why this sync is being done.
  reason: SyncReason,

  // Any state changes which should be done as part of this sync.
  engine_state_changes: Vec<EngineState>,

  // An opaque state "blob". This should be persisted by the app so it is
  // reused next sync.
  persisted_state: Option<String>,
}

struct SyncResult {
  // The general health.
  service_status: ServiceStatus,

  // The result for each engine.
  engine_results: HashMap<Engine, Result<()>>,

  // The list of declined engines, or None if we failed to get that far.
  declined_engines: Option<Vec<Engine>>,

  // When we are allowed to sync again. If > now() then there's some kind
  // of back-off. Note that it's not strictly necessary for the app to
  // enforce this (ie, it can keep asking us to sync, but we might decline).
  // But we might not too - eg, we might try a user-initiated sync.
  next_sync_allowed_at: Timestamp,

  // New opaque state which should be persisted by the embedding app and supplied
  // the next time Sync is called.
  persisted_state: String,

  // Telemetry. Nailing this down is tbd.
  telemetry: Option<JSONValue>,
}

sync methods exposed on individual stores can be treated as deprecated, and are likely to be removed at some point. This doesn't bode well for our concept SyncableStore that storage layers can implement. We don't have to remove this right away.

However, this means we should be able to remove the GlobalSyncableStoreProvider singleton.

New concepts:

• persisted state resulting from each sync, that need to be passed along to the next sync
• list of enabled engines as a sync parameter
• list of disabled engines as a result coming out of sync (recall that this is a global list; other clients might have changed it)
  • this needs to be propagated upward to the application UI

So, start working on this by figuring out the following items:

• how to persist sync state. SharedPreferences is probably fine. Sync worker should own read/write of this state
• how to persist list of enabled/disabled engines (again, SharedPreferences should be fine)
  • how to expose that list to the application
  • how to allow application to monitor changes to that list. Likely as part of existing SyncStatus observers
• FxaAccountManager API for changing the list of enabled engines, and engines that are allowed to be synced. We already have a SyncConfig that is a good fit here.
• how to treat telemetry. Currently this is done at the Connection level, beneath PlacesStorage. - see assembleHistoryPing. We won't be going through storage instances directly while syncing anymore, and so that logic should be moved out someplace where sync worker can access it

[grigoryk]

We've done a first pass at this locally against a straw-man API, and now the a-s PR (mozilla/application-services#1447) is in a state where we can start testing this end-to-end.

[sblatz]

@Amejia481 Is this still blocked? From @grigoryk's comment on the Fenix ticket, it seems like this is ready to be implemented in AC?

[Amejia481]

Thanks for keeping an eye on it, two days ago it has some pending work from AS, but now it has finally landed mozilla/application-services#1447 (comment)