Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Commit

Permalink
Fixes 24823: remove recent synced tab when logged out
Browse files Browse the repository at this point in the history
  • Loading branch information
@MatthewTighe @mergify
MatthewTighe authored and mergify[bot] committed Jul 14, 2022
1 parent 28c2db0 commit 1d20914
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 38 deletions.
12 changes: 12 additions & 0 deletions app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import mozilla.components.service.fxa.SyncEngine
import mozilla.components.service.fxa.manager.FxaAccountManager
import mozilla.components.service.fxa.manager.SCOPE_SESSION
import mozilla.components.service.fxa.manager.SCOPE_SYNC
import mozilla.components.service.fxa.store.SyncStore
import mozilla.components.service.fxa.store.SyncStoreSupport
import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider
import mozilla.components.service.glean.private.NoExtras
import mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage
Expand Down Expand Up @@ -130,6 +132,12 @@ class BackgroundServices(

val accountAbnormalities = AccountAbnormalities(context, crashReporter, strictMode)

val syncStore by lazyMonitored {
SyncStore()
}

private lateinit var syncStoreSupport: SyncStoreSupport

val accountManager by lazyMonitored {
makeAccountManager(context, serverConfig, deviceConfig, syncConfig, crashReporter)
}
Expand Down Expand Up @@ -183,6 +191,10 @@ class BackgroundServices(

SyncedTabsIntegration(context, accountManager).launch()

syncStoreSupport = SyncStoreSupport(syncStore, lazyOf(accountManager)).also {
it.initialize()
}

MainScope().launch {
accountManager.start()
}
Expand Down
20 changes: 9 additions & 11 deletions app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,6 @@ class HomeFragment : Fragment() {
}
}

private val syncedTabFeature by lazy {
RecentSyncedTabFeature(
store = requireComponents.appStore,
context = requireContext(),
storage = requireComponents.backgroundServices.syncedTabsStorage,
accountManager = requireComponents.backgroundServices.accountManager,
lifecycleOwner = viewLifecycleOwner,
)
}

private var _sessionControlInteractor: SessionControlInteractor? = null
private val sessionControlInteractor: SessionControlInteractor
get() = _sessionControlInteractor!!
Expand Down Expand Up @@ -282,7 +272,15 @@ class HomeFragment : Fragment() {

if (requireContext().settings().enableTaskContinuityEnhancements) {
recentSyncedTabFeature.set(
feature = syncedTabFeature,
feature = RecentSyncedTabFeature(
appStore = requireComponents.appStore,
syncStore = requireComponents.backgroundServices.syncStore,
coroutineScope = viewLifecycleOwner.lifecycleScope,
context = requireContext(),
storage = requireComponents.backgroundServices.syncedTabsStorage,
accountManager = requireComponents.backgroundServices.accountManager,
lifecycleOwner = viewLifecycleOwner,
),
owner = viewLifecycleOwner,
view = binding.root
)
Expand Down
38 changes: 30 additions & 8 deletions app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,20 @@ package org.mozilla.fenix.home.recentsyncedtabs

import android.content.Context
import androidx.lifecycle.LifecycleOwner
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import mozilla.components.browser.storage.sync.SyncedDeviceTabs
import mozilla.components.concept.sync.DeviceType
import mozilla.components.feature.syncedtabs.SyncedTabsFeature
import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage
import mozilla.components.feature.syncedtabs.view.SyncedTabsView
import mozilla.components.lib.state.ext.flow
import mozilla.components.service.fxa.manager.FxaAccountManager
import mozilla.components.service.fxa.store.SyncStatus
import mozilla.components.service.fxa.store.SyncStore
import mozilla.components.support.base.feature.LifecycleAwareFeature
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction
import mozilla.telemetry.glean.GleanTimerId
Expand All @@ -21,15 +28,19 @@ import org.mozilla.fenix.GleanMetrics.RecentSyncedTabs
/**
* Delegate to handle layout updates and dispatch actions related to the recent synced tab.
*
* @property store Store to dispatch actions to when synced tabs are updated or errors encountered.
* @property appStore Store to dispatch actions to when synced tabs are updated or errors encountered.
* @property syncStore Store to observe Sync state from.
* @property coroutineScope The scope to collect Sync state Flow updates in.
* @param accountManager Account manager used to retrieve synced tab state.
* @param context [Context] used for retrieving the sync engine storage state.
* @param storage Storage layer for synced tabs.
* @param lifecycleOwner View lifecycle owner to determine start/stop state for feature.
*/
@Suppress("LongParameterList")
class RecentSyncedTabFeature(
private val store: AppStore,
private val appStore: AppStore,
private val syncStore: SyncStore,
private val coroutineScope: CoroutineScope,
accountManager: FxaAccountManager,
context: Context,
storage: SyncedTabsStorage,
Expand All @@ -54,8 +65,8 @@ class RecentSyncedTabFeature(
override fun startLoading() {
syncStartId?.let { RecentSyncedTabs.recentSyncedTabTimeToLoad.cancel(it) }
syncStartId = RecentSyncedTabs.recentSyncedTabTimeToLoad.start()
if (store.state.recentSyncedTabState == RecentSyncedTabState.None) {
store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Loading))
if (appStore.state.recentSyncedTabState == RecentSyncedTabState.None) {
appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Loading))
}
}

Expand All @@ -74,7 +85,7 @@ class RecentSyncedTabFeature(
)
} ?: return
recordMetrics(syncedTab, lastSyncedTab, syncStartId)
store.dispatch(
appStore.dispatch(
AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(syncedTab))
)
lastSyncedTab = syncedTab
Expand All @@ -87,18 +98,29 @@ class RecentSyncedTabFeature(
*/
override fun stopLoading() {
if (lastSyncedTab == null) {
store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None))
appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None))
}
}

override fun onError(error: SyncedTabsView.ErrorType) {
if (store.state.recentSyncedTabState == RecentSyncedTabState.Loading) {
store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None))
if (appStore.state.recentSyncedTabState == RecentSyncedTabState.Loading) {
appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None))
}
}

override fun start() {
syncedTabsFeature.start()
syncStore.flow()
.ifChanged { state -> state.status }
.onEach { state ->
when (state.status) {
SyncStatus.LoggedOut -> appStore.dispatch(
AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)
)
else -> Unit
}
}
.launchIn(coroutineScope)
}

override fun stop() {
Expand Down
76 changes: 57 additions & 19 deletions app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeatureTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,24 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
import mozilla.components.browser.storage.sync.SyncedDeviceTabs
import mozilla.components.browser.storage.sync.Tab
import mozilla.components.browser.storage.sync.TabEntry
import mozilla.components.concept.sync.Device
import mozilla.components.concept.sync.DeviceType
import mozilla.components.feature.syncedtabs.view.SyncedTabsView
import mozilla.components.service.fxa.manager.FxaAccountManager
import mozilla.components.service.fxa.store.SyncAction
import mozilla.components.service.fxa.store.SyncStatus
import mozilla.components.service.fxa.store.SyncStore
import mozilla.components.service.glean.testing.GleanTestRule
import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
Expand Down Expand Up @@ -68,40 +78,46 @@ class RecentSyncedTabFeatureTest {
subscription = null
)

private val store: AppStore = mockk()
private val accountManager: FxaAccountManager = mockk()
private val appStore: AppStore = mockk()
private val accountManager: FxaAccountManager = mockk(relaxed = true)

private val syncStore = SyncStore()

private lateinit var feature: RecentSyncedTabFeature

@Before
fun setup() {
every { store.dispatch(any()) } returns mockk()
Dispatchers.setMain(StandardTestDispatcher())

every { appStore.dispatch(any()) } returns mockk()

feature = RecentSyncedTabFeature(
store = store,
appStore = appStore,
syncStore = syncStore,
coroutineScope = TestScope(),
accountManager = accountManager,
context = mockk(),
context = mockk(relaxed = true),
storage = mockk(),
lifecycleOwner = mockk(),
)
}

@Test
fun `GIVEN that there is no current state WHEN loading is started THEN loading state is dispatched`() {
every { store.state } returns mockk {
every { appStore.state } returns mockk {
every { recentSyncedTabState } returns RecentSyncedTabState.None
}

feature.startLoading()

verify { store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Loading)) }
verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Loading)) }
}

@Test
fun `WHEN empty synced tabs are displayed THEN no action is dispatched`() {
feature.displaySyncedTabs(listOf())

verify(exactly = 0) { store.dispatch(any()) }
verify(exactly = 0) { appStore.dispatch(any()) }
}

@Test
Expand All @@ -114,7 +130,7 @@ class RecentSyncedTabFeatureTest {
val expectedTab = tab.toRecentSyncedTab(deviceAccessed1)

verify {
store.dispatch(
appStore.dispatch(
AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
)
}
Expand All @@ -134,7 +150,7 @@ class RecentSyncedTabFeatureTest {
val expectedTab = remoteTab.toRecentSyncedTab(deviceAccessed1)

verify {
store.dispatch(
appStore.dispatch(
AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
)
}
Expand All @@ -153,7 +169,7 @@ class RecentSyncedTabFeatureTest {
val expectedTab = remoteTab.toRecentSyncedTab(deviceAccessed1)

verify {
store.dispatch(
appStore.dispatch(
AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
)
}
Expand All @@ -173,7 +189,7 @@ class RecentSyncedTabFeatureTest {
val expectedTab = secondTab.toRecentSyncedTab(deviceAccessed2)

verify {
store.dispatch(
appStore.dispatch(
AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
)
}
Expand All @@ -190,7 +206,7 @@ class RecentSyncedTabFeatureTest {

@Test
fun `GIVEN that tab previously started loading WHEN synced tab displayed THEN load time metric recorded`() {
every { store.state } returns mockk {
every { appStore.state } returns mockk {
every { recentSyncedTabState } returns RecentSyncedTabState.None
}
val tab = SyncedDeviceTabs(deviceAccessed1, listOf(createActiveTab()))
Expand Down Expand Up @@ -226,7 +242,7 @@ class RecentSyncedTabFeatureTest {
fun `GIVEN that no tab is displayed WHEN stopLoading is called THEN none state dispatched`() {
feature.stopLoading()

verify { store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
}

@Test
Expand All @@ -236,28 +252,41 @@ class RecentSyncedTabFeatureTest {
feature.displaySyncedTabs(listOf(tab))
feature.stopLoading()

verify(exactly = 0) { store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
verify(exactly = 0) { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
}

@Test
fun `GIVEN that feature is not loading WHEN error received THEN does not dispatch NONE state`() {
every { store.state } returns mockk {
every { appStore.state } returns mockk {
every { recentSyncedTabState } returns RecentSyncedTabState.None
}
feature.onError(SyncedTabsView.ErrorType.NO_TABS_AVAILABLE)

verify(exactly = 0) { store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
verify(exactly = 0) { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
}

@Test
fun `GIVEN that feature is loading WHEN error received THEN dispatches NONE state`() {
every { store.state } returns mockk {
every { appStore.state } returns mockk {
every { recentSyncedTabState } returns RecentSyncedTabState.Loading
}

feature.onError(SyncedTabsView.ErrorType.MULTIPLE_DEVICES_UNAVAILABLE)

verify { store.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
}

@Test
fun `WHEN LoggedOut is observed THEN tab state is dispatched as none`() = runTest {
every { appStore.state } returns mockk {
every { recentSyncedTabState } returns RecentSyncedTabState.None
}

feature.start()
syncStore.setState(SyncStatus.LoggedOut)
runCurrent()

verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
}

private fun createActiveTab(
Expand All @@ -278,4 +307,13 @@ class RecentSyncedTabFeatureTest {
url = this.active().url,
iconUrl = this.active().iconUrl
)

private fun SyncStore.setState(
status: SyncStatus? = null,
) {
status?.let {
this.dispatch(SyncAction.UpdateSyncStatus(status))
}
this.waitUntilIdle()
}
}

0 comments on commit 1d20914

Please sign in to comment.