Merge #7114

7114: Close #6963: Sync account when SyncedTabsFeature is started r=grigoryk a=jonalmeida

Instead of syncing tabs on start, we refresh the devices and sync the account. The allows our mechanism to sync tabs once that is complete to be triggered where we know we're in the most current state we can be in.
Co-authored-by: Jonathan Almeida <jalmeida@mozilla.com>

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/SyncedTabsFeature.kt

@@ -33,6 +33,7 @@ import kotlin.coroutines.CoroutineContext
  * @param presenter See [SyncedTabsPresenter].
  * @param interactor See [SyncedTabsInteractor].
  */
+@Suppress("LongParameterList")
 class SyncedTabsFeature(
     storage: SyncedTabsStorage,
     accountManager: FxaAccountManager,
@@ -40,7 +41,7 @@ class SyncedTabsFeature(
     lifecycleOwner: LifecycleOwner,
     coroutineContext: CoroutineContext = Dispatchers.IO,
     onTabClicked: (Tab) -> Unit,
-    controller: SyncedTabsController = DefaultController(
+    private val controller: SyncedTabsController = DefaultController(
         storage,
         accountManager,
         view,
@@ -53,9 +54,8 @@ class SyncedTabsFeature(
         lifecycleOwner
     ),
     private val interactor: SyncedTabsInteractor = DefaultInteractor(
-        accountManager,
+        controller,
         view,
-        coroutineContext,
         onTabClicked
     )
 ) : LifecycleAwareFeature {

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/controller/DefaultController.kt

@@ -12,19 +12,23 @@ import mozilla.components.feature.syncedtabs.view.SyncedTabsView
 import mozilla.components.feature.syncedtabs.view.SyncedTabsView.ErrorType
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.service.fxa.manager.ext.withConstellation
+import mozilla.components.service.fxa.sync.SyncReason
 import kotlin.coroutines.CoroutineContext
 
 internal class DefaultController(
     override val provider: SyncedTabsProvider,
     override val accountManager: FxaAccountManager,
     override val view: SyncedTabsView,
-    private val coroutineContext: CoroutineContext
+    coroutineContext: CoroutineContext
 ) : SyncedTabsController {
 
-    override fun syncTabs() {
-        view.startLoading()
+    private val scope = CoroutineScope(coroutineContext)
 
-        val scope = CoroutineScope(coroutineContext)
+    /**
+     * See [SyncedTabsController.refreshSyncedTabs]
+     */
+    override fun refreshSyncedTabs() {
+        view.startLoading()
 
         scope.launch {
             accountManager.withConstellation {
@@ -45,4 +49,16 @@ internal class DefaultController(
             }
         }
     }
+
+    /**
+     * See [SyncedTabsController.syncAccount]
+     */
+    override fun syncAccount() {
+        scope.launch {
+            accountManager.withConstellation {
+                refreshDevicesAsync().await()
+            }
+            accountManager.syncNowAsync(SyncReason.User)
+        }
+    }
 }

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/controller/SyncedTabsController.kt

@@ -21,5 +21,10 @@ interface SyncedTabsController {
      * Requests for remote tabs and notifies the [SyncedTabsView] when available with [SyncedTabsView.displaySyncedTabs]
      * otherwise notifies the appropriate error to [SyncedTabsView.onError].
      */
-    fun syncTabs()
+    fun refreshSyncedTabs()
+
+    /**
+     * Requests for the account on the [FxaAccountManager] to perform a sync.
+     */
+    fun syncAccount()
 }

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/interactor/DefaultInteractor.kt

@@ -4,19 +4,13 @@
 
 package mozilla.components.feature.syncedtabs.interactor
 
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
 import mozilla.components.browser.storage.sync.Tab
+import mozilla.components.feature.syncedtabs.controller.SyncedTabsController
 import mozilla.components.feature.syncedtabs.view.SyncedTabsView
-import mozilla.components.service.fxa.manager.FxaAccountManager
-import mozilla.components.service.fxa.manager.ext.withConstellation
-import mozilla.components.service.fxa.sync.SyncReason
-import kotlin.coroutines.CoroutineContext
 
 internal class DefaultInteractor(
-    override val accountManager: FxaAccountManager,
+    override val controller: SyncedTabsController,
     override val view: SyncedTabsView,
-    private val coroutineContext: CoroutineContext,
     override val tabClicked: (Tab) -> Unit
 ) : SyncedTabsInteractor {
 
@@ -33,11 +27,6 @@ internal class DefaultInteractor(
     }
 
     override fun onRefresh() {
-        CoroutineScope(coroutineContext).launch {
-            accountManager.withConstellation {
-                refreshDevicesAsync()
-            }
-            accountManager.syncNowAsync(SyncReason.User, true)
-        }
+        controller.syncAccount()
     }
 }

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/interactor/SyncedTabsInteractor.kt

@@ -5,15 +5,15 @@
 package mozilla.components.feature.syncedtabs.interactor
 
 import mozilla.components.browser.storage.sync.Tab
+import mozilla.components.feature.syncedtabs.controller.SyncedTabsController
 import mozilla.components.feature.syncedtabs.view.SyncedTabsView
-import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 
 /**
  * An interactor that handles events from [SyncedTabsView.Listener].
  */
 interface SyncedTabsInteractor : SyncedTabsView.Listener, LifecycleAwareFeature {
-    val accountManager: FxaAccountManager
+    val controller: SyncedTabsController
     val view: SyncedTabsView
     val tabClicked: (Tab) -> Unit
 }

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenter.kt

@@ -72,7 +72,7 @@ internal class DefaultPresenter(
             return
         }
 
-        controller.syncTabs()
+        controller.syncAccount()
     }
 
     override fun stop() {
@@ -89,7 +89,7 @@ internal class DefaultPresenter(
         }
 
         override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
-            controller.syncTabs()
+            controller.refreshSyncedTabs()
         }
 
         override fun onAuthenticationProblems() {
@@ -103,7 +103,7 @@ internal class DefaultPresenter(
     ) : SyncStatusObserver {
 
         override fun onIdle() {
-            controller.syncTabs()
+            controller.refreshSyncedTabs()
         }
 
         override fun onError(error: Exception?) {

components/feature/syncedtabs/src/main/java/mozilla/components/feature/syncedtabs/storage/SyncedTabsStorage.kt

@@ -60,7 +60,7 @@ class SyncedTabsStorage(
     }
 
     /**
-     * Get the list of synced tabs.
+     * See [SyncedTabsProvider.getSyncedTabs].
      */
     override suspend fun getSyncedTabs(): List<SyncedDeviceTabs> {
         val otherDevices = syncClients() ?: return emptyList()

components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/controller/DefaultControllerTest.kt

-/*
- * This Source Code Form is subject to the terms of the Mozilla Public
- *  License, v. 2.0. If a copy of the MPL was not distributed with this
- *  file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -11,6 +5,7 @@
 package mozilla.components.feature.syncedtabs.controller
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.test.TestCoroutineDispatcher
 import kotlinx.coroutines.test.runBlockingTest
@@ -23,6 +18,7 @@ import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage
 import mozilla.components.feature.syncedtabs.view.SyncedTabsView
 import mozilla.components.feature.syncedtabs.view.SyncedTabsView.ErrorType
 import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.service.fxa.sync.SyncReason
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.rule.MainCoroutineRule
 import org.junit.After
@@ -64,7 +60,7 @@ class DefaultControllerTest {
             coroutineContext
         )
 
-        controller.syncTabs()
+        controller.refreshSyncedTabs()
 
         verify(view).startLoading()
         verify(view).stopLoading()
@@ -91,8 +87,51 @@ class DefaultControllerTest {
 
         `when`(storage.getSyncedTabs()).thenReturn(emptyList())
 
-        controller.syncTabs()
+        controller.refreshSyncedTabs()
 
         verify(view).onError(ErrorType.MULTIPLE_DEVICES_UNAVAILABLE)
     }
+
+    @Test
+    fun `display synced tabs`() = runBlockingTest {
+        val controller = DefaultController(
+            storage,
+            accountManager,
+            view,
+            coroutineContext
+        )
+        val account: OAuthAccount = mock()
+        val constellation: DeviceConstellation = mock()
+        val state: ConstellationState = mock()
+
+        `when`(accountManager.authenticatedAccount()).thenReturn(account)
+        `when`(account.deviceConstellation()).thenReturn(constellation)
+        `when`(constellation.state()).thenReturn(state)
+        `when`(state.otherDevices).thenReturn(listOf(mock()))
+
+        `when`(storage.getSyncedTabs()).thenReturn(emptyList())
+
+        controller.refreshSyncedTabs()
+    }
+
+    @Test
+    fun `syncAccount refreshes devices and syncs`() = runBlockingTest {
+        val controller = DefaultController(
+            storage,
+            accountManager,
+            view,
+            coroutineContext
+        )
+        val account: OAuthAccount = mock()
+        val constellation: DeviceConstellation = mock()
+
+        `when`(accountManager.authenticatedAccount()).thenReturn(account)
+        `when`(account.deviceConstellation()).thenReturn(constellation)
+        `when`(constellation.refreshDevicesAsync()).thenReturn(CompletableDeferred(true))
+
+        controller.syncAccount()
+
+        verify(constellation).refreshDevicesAsync()
+        verify(accountManager).syncNowAsync(SyncReason.User, false)
+    }
 }

components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/interactor/DefaultInteractorTest.kt

-/*
- * This Source Code Form is subject to the terms of the Mozilla Public
- *  License, v. 2.0. If a copy of the MPL was not distributed with this
- *  file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -12,32 +6,27 @@ package mozilla.components.feature.syncedtabs.interactor
 
 import kotlinx.coroutines.test.runBlockingTest
 import mozilla.components.browser.storage.sync.SyncedDeviceTabs
-import mozilla.components.concept.sync.DeviceConstellation
-import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.feature.syncedtabs.controller.SyncedTabsController
 import mozilla.components.feature.syncedtabs.view.SyncedTabsView
-import mozilla.components.service.fxa.manager.FxaAccountManager
-import mozilla.components.service.fxa.sync.SyncReason
 import mozilla.components.support.test.mock
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
-import org.mockito.Mockito.`when`
 import org.mockito.Mockito.verify
 
 class DefaultInteractorTest {
 
-    private val accountManager: FxaAccountManager = mock()
     private val view: SyncedTabsView = mock()
+    private val controller: SyncedTabsController = mock()
 
     @Test
     fun start() = runBlockingTest {
         val view =
             TestSyncedTabsView()
         val feature = DefaultInteractor(
-            accountManager,
-            view,
-            coroutineContext
+            controller,
+            view
         ) {}
 
         assertNull(view.listener)
@@ -52,9 +41,8 @@ class DefaultInteractorTest {
         val view =
             TestSyncedTabsView()
         val feature = DefaultInteractor(
-            accountManager,
-            view,
-            coroutineContext
+            controller,
+            view
         ) {}
 
         assertNull(view.listener)
@@ -72,9 +60,8 @@ class DefaultInteractorTest {
     fun `onTabClicked invokes callback`() = runBlockingTest {
         var invoked = false
         val feature = DefaultInteractor(
-            accountManager,
-            view,
-            coroutineContext
+            controller,
+            view
         ) {
             invoked = true
         }
@@ -87,33 +74,25 @@ class DefaultInteractorTest {
     @Test
     fun `onRefresh does not update devices when there is no constellation`() = runBlockingTest {
         val feature = DefaultInteractor(
-            accountManager,
-            view,
-            coroutineContext
+            controller,
+            view
         ) {}
 
         feature.onRefresh()
 
-        verify(accountManager).syncNowAsync(SyncReason.User, true)
+        verify(controller).syncAccount()
     }
 
     @Test
     fun `onRefresh updates devices when there is a constellation`() = runBlockingTest {
         val feature = DefaultInteractor(
-            accountManager,
-            view,
-            coroutineContext
+            controller,
+            view
         ) {}
-        val account: OAuthAccount = mock()
-        val constellation: DeviceConstellation = mock()
-
-        `when`(accountManager.authenticatedAccount()).thenReturn(account)
-        `when`(account.deviceConstellation()).thenReturn(constellation)
 
         feature.onRefresh()
 
-        verify(constellation).refreshDevicesAsync()
-        verify(accountManager).syncNowAsync(SyncReason.User, true)
+        verify(controller).syncAccount()
     }
 
     private class TestSyncedTabsView : SyncedTabsView {

components/feature/syncedtabs/src/test/java/mozilla/components/feature/syncedtabs/presenter/DefaultPresenterTest.kt

@@ -102,7 +102,7 @@ class DefaultPresenterTest {
 
         presenter.start()
 
-        verify(controller).syncTabs()
+        verify(controller).syncAccount()
     }
 
     @Test
@@ -130,7 +130,7 @@ class DefaultPresenterTest {
 
         presenter.accountObserver.onAuthenticated(mock(), mock())
 
-        verify(controller).syncTabs()
+        verify(controller).refreshSyncedTabs()
     }
 
     @Test
@@ -158,7 +158,7 @@ class DefaultPresenterTest {
 
         presenter.eventObserver.onIdle()
 
-        verify(controller).syncTabs()
+        verify(controller).refreshSyncedTabs()
     }
 
     @Test

docs/changelog.md

@@ -56,6 +56,9 @@ permalink: /changelog/
     menu also contains an access entry for Add-ons Manager, for which `onAddonsManagerTapped` needs to be passed in the
     constructor.
 
+* **feature-syncedtabs**
+  * When the SyncedTabsFeature is started it syncs the devices and account first.
+
 # 42.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v41.0.0...42.0.0)