Merge mozilla-mobile#4129

4129: Improve/simplify sync and device APIs r=Amejia481,jonalmeida a=grigoryk

This patch series aims to improve the API surface which exposes sync and device management.

## First patch:
Refresh device constellation and polls for events during account init (restore, login, signup, recovery, etc). This allows API consumers to worry somewhat less about having to do this manually.
Refresh is performed only for devices that have SEND_TAB capability enabled.

In the future we'll have "system events" that we'd like to poll as soon as possible, which this enables.

## Second patch:
Introduces a `debounce` parameter that can be passed into `syncNowAsync`. When `debounce=true`, sync request will be ignored if sync already ran recently. With this change in place, applications can simply invoke `syncNowAsync(debounce = true)` in `onResume`, without having to worry about running sync twice in a row in case of startup (sync is executed during account manager initialization).

This change lets us trivially fix mozilla-mobile#3284. 

## Third patch:
Simplifies internal implementation of `FxaDeviceConstellation`, removing `PollingDeviceManager` entirely. Public API is now both simpler and provides applications with more control: polling for device events is now separate from refreshing constellation state. Lots of new tests added.



Co-authored-by: Grisha Kruglov <[email protected]>

Author: MickeyMoz

components/concept/sync/src/main/java/mozilla/components/concept/sync/Devices.kt

@@ -48,12 +48,6 @@ interface DeviceConstellation : Observable<DeviceEventsObserver> {
      */
     fun registerDeviceObserver(observer: DeviceConstellationObserver, owner: LifecycleOwner, autoPause: Boolean)
 
-    /**
-     * Get all devices in the constellation.
-     * @return A list of all devices in the constellation, or `null` on failure.
-     */
-    fun fetchAllDevicesAsync(): Deferred<List<Device>?>
-
     /**
      * Set name of the current device.
      * @param name New device name.
@@ -84,29 +78,19 @@ interface DeviceConstellation : Observable<DeviceEventsObserver> {
     fun processRawEventAsync(payload: String): Deferred<Boolean>
 
     /**
-     * Poll for events targeted at the current [Device]. It's expected that if a [DeviceEvent] was
-     * returned after a poll, it will not be returned in consequent polls.
-     * @return A list of [DeviceEvent] instances that are currently pending for this [Device], or `null` on failure.
-     */
-    fun pollForEventsAsync(): Deferred<List<DeviceEvent>?>
-
-    /**
-     * Begin periodically refreshing constellation state, including polling for events.
-     */
-    fun startPeriodicRefresh()
-
-    /**
-     * Stop periodically refreshing constellation state and polling for events.
+     * Refreshes [ConstellationState]. Registered [DeviceConstellationObserver] observers will be notified.
+     *
+     * @return A [Deferred] that will be resolved with a success flag once operation is complete.
      */
-    fun stopPeriodicRefresh()
+    fun refreshDevicesAsync(): Deferred<Boolean>
 
     /**
-     * Refreshes [ConstellationState] and polls for device events.
+     * Polls for any pending [DeviceEvent] events.
+     * In case of new events, registered [DeviceEventsObserver] observers will be notified.
      *
-     * @return A [Deferred] that will be resolved with a success flag once operation is complete. Failure may
-     * indicate a partial success.
+     * @return A [Deferred] that will be resolved with a success flag once operation is complete.
      */
-    fun refreshDeviceStateAsync(): Deferred<Boolean>
+    fun pollForEventsAsync(): Deferred<Boolean>
 }
 
 /**

components/service/firefox-accounts/README.md

@@ -48,7 +48,7 @@ GlobalSyncableStoreProvider.configureStore("bookmarks" to bookmarksStorage)
 val accountManager = FxaAccountManager(
     context = this,
     serverConfig = ServerConfig.release(CLIENT_ID, REDIRECT_URL),
-    deviceConfig =DeviceConfig(
+    deviceConfig = DeviceConfig(
         name = "Sample app",
         type = DeviceType.MOBILE,
         capabilities = setOf(DeviceCapability.SEND_TAB)
@@ -63,6 +63,10 @@ accountManager.register(accountObserver, owner = this, autoPause = true)
 accountManager.registerForSyncEvents(syncObserver, owner = this, autoPause = true)
 
 // Observe incoming device events (e.g. SEND_TAB events from other devices).
+// Note that since the device is configured with a SEND_TAB capability, device constellation will be
+// automatically updated during any account initialization flow (restore, login, sign-up, recovery).
+// It is up to the application to keep it up-to-date beyond that.
+// See `account.deviceConstellation().refreshDeviceStateAsync()`.
 accountManager.registerForDeviceEvents(deviceEventsObserver, owner = this, autoPause = true)
 
 // Now that all of the observers we care about are registered, kick off the account manager.

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt

@@ -90,7 +90,6 @@ class FirefoxAccount internal constructor(
     ) : this(InternalFxAcct(config, persistCallback))
 
     override fun close() {
-        deviceConstellation.stopPeriodicRefresh()
         job.cancel()
         inner.close()
     }

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt

@@ -4,13 +4,13 @@
 
 package mozilla.components.service.fxa
 
+import androidx.annotation.GuardedBy
 import androidx.lifecycle.LifecycleOwner
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.async
 import kotlinx.coroutines.launch
-import mozilla.appservices.fxaclient.AccountEvent
 import mozilla.appservices.fxaclient.FirefoxAccount
 import mozilla.components.concept.sync.ConstellationState
 import mozilla.components.concept.sync.Device
@@ -22,8 +22,6 @@ import mozilla.components.concept.sync.DeviceEventOutgoing
 import mozilla.components.concept.sync.DeviceEventsObserver
 import mozilla.components.concept.sync.DevicePushSubscription
 import mozilla.components.concept.sync.DeviceType
-import mozilla.components.service.fxa.manager.DeviceManagerProvider
-import mozilla.components.service.fxa.manager.PollingDeviceManager
 import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
@@ -39,19 +37,12 @@ class FxaDeviceConstellation(
     private val logger = Logger("FxaDeviceConstellation")
 
     private val deviceObserverRegistry = ObserverRegistry<DeviceConstellationObserver>()
-    private val deviceManager = PollingDeviceManager(this, scope, deviceObserverRegistry)
 
-    init {
-        DeviceManagerProvider.deviceManager = deviceManager
+    @GuardedBy("this")
+    private var constellationState: ConstellationState? = null
 
-        deviceManager.register(object : DeviceEventsObserver {
-            override fun onEvents(events: List<DeviceEvent>) {
-                // TODO notifyObserversOnMainThread
-                CoroutineScope(Dispatchers.Main).launch {
-                    notifyObservers { onEvents(events) }
-                }
-            }
-        })
+    override fun state(): ConstellationState? = synchronized(this) {
+        return constellationState
     }
 
     override fun initDeviceAsync(
@@ -77,33 +68,7 @@ class FxaDeviceConstellation(
     override fun processRawEventAsync(payload: String): Deferred<Boolean> {
         return scope.async {
             handleFxaExceptions(logger, "processing raw events") {
-                val events = account.handlePushMessage(payload).filter {
-                    it is AccountEvent.TabReceived
-                }.map {
-                    (it as AccountEvent.TabReceived).into()
-                }
-                deviceManager.processEvents(events)
-            }
-        }
-    }
-
-    override fun pollForEventsAsync(): Deferred<List<DeviceEvent>?> {
-        // Currently ignoring non-TabReceived events.
-        return scope.async {
-            handleFxaExceptions(logger, "polling for device events", { null }) {
-                account.pollDeviceCommands().filter {
-                    it is AccountEvent.TabReceived
-                }.map {
-                    (it as AccountEvent.TabReceived).into()
-                }
-            }
-        }
-    }
-
-    override fun fetchAllDevicesAsync(): Deferred<List<Device>?> {
-        return scope.async {
-            handleFxaExceptions(logger, "fetching all devices", { null }) {
-                account.getDevices().map { it.into() }
+                processEvents(account.handlePushMessage(payload).map { it.into() })
             }
         }
     }
@@ -122,7 +87,7 @@ class FxaDeviceConstellation(
                 account.setDeviceDisplayName(name)
             }
             // See the latest device (name) changes after changing it.
-            val refreshDevices = deviceManager.refreshDevicesAsync().await()
+            val refreshDevices = refreshDevicesAsync().await()
 
             rename && refreshDevices
         }
@@ -151,26 +116,67 @@ class FxaDeviceConstellation(
         }
     }
 
-    override fun state(): ConstellationState? {
-        return deviceManager.constellationState()
+    override fun pollForEventsAsync(): Deferred<Boolean> {
+        return scope.async {
+            val events = handleFxaExceptions(logger, "polling for device events", { null }) {
+                account.pollDeviceCommands().map { it.into() }
+            }
+
+            if (events == null) {
+                false
+            } else {
+                processEvents(events)
+                true
+            }
+        }
     }
 
-    override fun startPeriodicRefresh() {
-        deviceManager.startPolling()
+    private fun processEvents(events: List<DeviceEvent>) = CoroutineScope(Dispatchers.Main).launch {
+        notifyObservers { onEvents(events) }
     }
 
-    override fun stopPeriodicRefresh() {
-        deviceManager.stopPolling()
+    override fun refreshDevicesAsync(): Deferred<Boolean> = synchronized(this) {
+        return scope.async {
+            logger.info("Refreshing device list...")
+
+            // Attempt to fetch devices, or bail out on failure.
+            val allDevices = fetchAllDevicesAsync().await() ?: return@async false
+
+            // Find the current device.
+            val currentDevice = allDevices.find { it.isCurrentDevice }
+            // Filter out the current devices.
+            val otherDevices = allDevices.filter { !it.isCurrentDevice }
+
+            val newState = ConstellationState(currentDevice, otherDevices)
+            constellationState = newState
+
+            CoroutineScope(Dispatchers.Main).launch {
+                // NB: at this point, 'constellationState' might have changed.
+                // Notify with an immutable, local 'newState' instead.
+                deviceObserverRegistry.notifyObservers { onDevicesUpdate(newState) }
+            }
+
+            // Check if our current device's push subscription needs to be renewed.
+            constellationState?.currentDevice?.let {
+                if (it.subscriptionExpired) {
+                    logger.info("Current device needs push endpoint registration")
+                }
+            }
+
+            logger.info("Refreshed device list; saw ${allDevices.size} device(s).")
+            true
+        }
     }
 
-    override fun refreshDeviceStateAsync(): Deferred<Boolean> {
+    /**
+     * Get all devices in the constellation.
+     * @return A list of all devices in the constellation, or `null` on failure.
+     */
+    private fun fetchAllDevicesAsync(): Deferred<List<Device>?> {
         return scope.async {
-            val refreshedDevices = deviceManager.refreshDevicesAsync().await()
-            val events = pollForEventsAsync().await()?.let {
-                deviceManager.processEvents(it)
-                true
-            } ?: false
-            refreshedDevices && events
+            handleFxaExceptions(logger, "fetching all devices", { null }) {
+                account.getDevices().map { it.into() }
+            }
         }
     }
 }

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Types.kt

@@ -159,6 +159,12 @@ fun mozilla.components.concept.sync.TabData.into(): TabHistoryEntry {
     )
 }
 
+fun AccountEvent.into(): mozilla.components.concept.sync.DeviceEvent {
+    return when (this) {
+        is AccountEvent.TabReceived -> this.into()
+    }
+}
+
 fun AccountEvent.TabReceived.into(): mozilla.components.concept.sync.DeviceEvent.TabReceived {
     return mozilla.components.concept.sync.DeviceEvent.TabReceived(
         from = this.from?.into(),

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt

@@ -18,6 +18,7 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.cancel
 import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.AuthException
+import mozilla.components.concept.sync.DeviceCapability
 import mozilla.components.concept.sync.DeviceEvent
 import mozilla.components.concept.sync.DeviceEventsObserver
 import mozilla.components.concept.sync.OAuthAccount
@@ -321,8 +322,12 @@ open class FxaAccountManager(
      * Request an immediate synchronization, as configured according to [syncConfig].
      *
      * @param startup Boolean flag indicating if sync is being requested in a startup situation.
+     * @param debounce Boolean flag indicating if this sync may be debounced (in case another sync executed recently).
      */
-    fun syncNowAsync(startup: Boolean = false): Deferred<Unit> = CoroutineScope(coroutineContext).async {
+    fun syncNowAsync(
+        startup: Boolean = false,
+        debounce: Boolean = false
+    ): Deferred<Unit> = CoroutineScope(coroutineContext).async {
         // Make sure auth cache is populated before we try to sync.
         maybeUpdateSyncAuthInfoCache()
 
@@ -333,7 +338,7 @@ open class FxaAccountManager(
                         "Sync is not configured. Construct this class with a 'syncConfig' or use 'setSyncConfig'"
                 )
             }
-            syncManager?.now(startup)
+            syncManager?.now(startup, debounce)
         }
         Unit
     }
@@ -562,9 +567,7 @@ open class FxaAccountManager(
                             deviceConfig.name, deviceConfig.type, deviceConfig.capabilities
                         ).await()
 
-                        maybeUpdateSyncAuthInfoCache()
-
-                        notifyObservers { onAuthenticated(account, true) }
+                        postAuthenticated(true)
 
                         Event.FetchProfile
                     }
@@ -583,19 +586,14 @@ open class FxaAccountManager(
                             logger.warn("Failed to ensure device capabilities.")
                         }
 
-                        // We used to perform a periodic device event polling, but for now we do not.
-                        // This cancels any periodic jobs device may still have active.
-                        // See https://github.com/mozilla-mobile/android-components/issues/3433
-                        logger.info("Stopping periodic refresh of the device constellation")
-                        account.deviceConstellation().stopPeriodicRefresh()
-
-                        maybeUpdateSyncAuthInfoCache()
-
-                        notifyObservers { onAuthenticated(account, false) }
+                        postAuthenticated(false)
 
                         Event.FetchProfile
                     }
                     Event.SignedInShareableAccount -> {
+                        // Note that we are not registering an account persistence callback here like
+                        // we do in other `AuthenticatedNoProfile` methods, because it would have been
+                        // already registered while handling `Event.SignInShareableAccount`.
                         logger.info("Registering device constellation observer")
                         account.deviceConstellation().register(deviceEventsIntegration)
 
@@ -605,9 +603,7 @@ open class FxaAccountManager(
                                 deviceConfig.name, deviceConfig.type, deviceConfig.capabilities
                         ).await()
 
-                        maybeUpdateSyncAuthInfoCache()
-
-                        notifyObservers { onAuthenticated(account, true) }
+                        postAuthenticated(true)
 
                         Event.FetchProfile
                     }
@@ -627,9 +623,7 @@ open class FxaAccountManager(
                                 deviceConfig.name, deviceConfig.type, deviceConfig.capabilities
                         ).await()
 
-                        maybeUpdateSyncAuthInfoCache()
-
-                        notifyObservers { onAuthenticated(account, false) }
+                        postAuthenticated(false)
 
                         Event.FetchProfile
                     }
@@ -753,6 +747,21 @@ open class FxaAccountManager(
         return null
     }
 
+    private suspend fun postAuthenticated(newAccount: Boolean) {
+        // Before any sync workers have a chance to access it, make sure our SyncAuthInfo cache is hot.
+        maybeUpdateSyncAuthInfoCache()
+
+        // Notify our internal (sync) and external (app logic) observers.
+        notifyObservers { onAuthenticated(account, newAccount) }
+
+        // If device supports SEND_TAB...
+        if (deviceConfig.capabilities.contains(DeviceCapability.SEND_TAB)) {
+            // ... update constellation state, and poll for any pending device events.
+            account.deviceConstellation().refreshDevicesAsync().await()
+            account.deviceConstellation().pollForEventsAsync().await()
+        }
+    }
+
     @VisibleForTesting
     open fun createAccount(config: ServerConfig): OAuthAccount {
         return FirefoxAccount(config)