Add SyncReason for engine requesting a sync

[jonalmeida]

Based on: #7114 (comment)

Features/engines may want to request a sync before performing an action. We don't have a good SyncReason to indicate the difference between a user-initiated action and a programmatic one.

Similarly, we do not know which sync engines are requesting a sync the most. This would help with telemetry on usage patterns.

cc: @grigoryk

┆Issue is synchronized with this Jira Task

[grigoryk]

Copying from the original issue, perhaps something like syncNowAsync(reason = SyncReason.User, engines = [Tabs], updateConstellation = true) would be a nice API?

[jonalmeida]

It would be nice if there was some kind of sync request builder. A data class of attributes even would make this a nicer way to programmatically build up a request of things until we want to "submit" it to the account manager.

[grigoryk]

🤷 SyncParams data class could be okay, yeah. Personally I'd either start with that or by simply expanding the function signature a bit; if things get cumbersome again we can move up to a builder, but let's avoid extra complexity if we don't need it.

[jonalmeida]

A builder might not be the best solution to the problem I'd like to solve, which the complexity of a all the configurations in one call. Being able to execute transactions on the account manager and letting it figure out how to handle the rest would make it easier as a consumer, somewhat similar to our lib-state:

accountManager.apply {
  dispatch(AccountAction.RefreshDevices)
  dispatch(AccountAction.UpdateEngine(Engine.SyncTabs)
  dispatch(AccountAction.SyncNow(/*all the params you want*/))
}

Having the account manager as a state object then would make it easier to observe the account changes, or (in the Sync Tabs case) only when the one engine data store changes and not have to consider all the other state changes that we need to get to the one.

This is falling more into the territory of #6879 thought and is not a small fix that you're probably looking for. 🙂

[grigoryk]

I had a feeling you'll go this way :) The dispatch style API (or a builder, as well) really starts to pay off if we have consumers that care about different permutations of a broad set of these values.

In practice our usages are:

• request a sync (for different reasons, e.g. enable/disable engine via separate API then ask for a sync due to engine change)
• request a sync and refresh devices

Where do you see these requirements evolving, beyond this issue itself? Combined with the cases above, we'll cover quite a lot of ground already as far as product features go.

The "sync" operation itself is the common denominator here, in practice we'd likely always want to sync in reaction to outside events. E.g. if user disables a history engine, we need to sync to propagate that change to other clients. One exception currently is "refresh devices while sharing a tab".

In case of synced tabs what we really want is to know what we have both:

• synced tabs from all of the clients
• latest set of clients

If our synced tabs API also included client info alongside the tabs (and not just the client ID), we could do away with a single API call, but as things stand they need to be resolved together.

It'll be nice if the public API here was aware of this - if you ask for a sync and request a client refresh, we consider that to be complete (e.g. "idle" callback is called) once both sync is complete and client list is refreshed.

Now that I've typed this, I think it's a good direction to follow - consider the end-to-end use cases we care about, not just entry points into the API, and make the common ones easy and without foot guns.

[jonalmeida]

Now that I've typed this, I think it's a good direction to follow - consider the end-to-end use cases we care about, not just entry points into the API, and make the common ones easy and without foot guns.

As long as this is a part of our consideration, I'm fine with any direction we go. :) As a consumer of the API, the ergonomics and extensibility are what I focus on.

I'll continue the other parts of my discussion in the other ticket as they come by.

[jonalmeida]

Moved to bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1794208

Change performed by the Move to Bugzilla add-on.