From 392ace67d6369d1d110c3b415497aeaae1242aef Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Mon, 10 May 2021 07:46:00 -0400 Subject: [PATCH] Issue #19175: Fix SyncTabs list not updating on changes The main cause for this is that the `LifecycleProvider` needs to be set to `State.RESUMED` to avoid the account manager's internal `ObserverRegistry` from putting the UI observers into the paused state. The rest of the changes is to rely the internal (safe) logic to correctly sync and then update the tabs list. --- .../tabstray/AccessibleNewTabButtonBinding.kt | 5 +- .../tabstray/FloatingActionButtonBinding.kt | 5 +- .../fenix/tabstray/NavigationInteractor.kt | 18 +++++++ .../fenix/tabstray/TabsTrayController.kt | 23 -------- .../fenix/tabstray/TabsTrayFragment.kt | 21 +++----- .../fenix/tabstray/TrayPagerAdapter.kt | 8 +-- .../tabstray/syncedtabs/SyncButtonBinding.kt | 35 ++++++++++++ .../syncedtabs/SyncedTabsInteractor.kt | 34 ------------ .../syncedtabs/SyncedTabsTrayLayout.kt | 36 +++++++------ .../tabstray/syncedtabs/TabClickDelegate.kt | 22 ++++++++ .../viewholders/SyncedTabViewHolder.kt | 7 +-- .../fenix/utils/view/LifecycleViewProvider.kt | 4 +- .../AccessibleNewTabButtonBindingTest.kt | 16 +++--- .../FloatingActionButtonBindingTest.kt | 16 +++--- .../tabstray/NavigationInteractorTest.kt | 34 ++++++++++++ .../syncedtabs/SyncButtonBindingTest.kt | 54 +++++++++++++++++++ .../syncedtabs/TabClickDelegateTest.kt | 36 +++++++++++++ 17 files changed, 252 insertions(+), 122 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBinding.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsInteractor.kt create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegate.kt create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBindingTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegateTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBinding.kt index bf33a4fe2..1efc2e1d8 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBinding.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBinding.kt @@ -16,7 +16,6 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged import org.mozilla.fenix.R import org.mozilla.fenix.tabstray.browser.BrowserTrayInteractor -import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsInteractor import org.mozilla.fenix.utils.Settings /** @@ -29,8 +28,7 @@ class AccessibleNewTabButtonBinding( private val store: TabsTrayStore, private val settings: Settings, private val newTabButton: ImageButton, - private val browserTrayInteractor: BrowserTrayInteractor, - private val syncedTabsInteractor: SyncedTabsInteractor + private val browserTrayInteractor: BrowserTrayInteractor ) : LifecycleAwareFeature { private var scope: CoroutineScope? = null @@ -94,7 +92,6 @@ class AccessibleNewTabButtonBinding( // a sync was requested. if (!syncing) { store.dispatch(TabsTrayAction.SyncNow) - syncedTabsInteractor.onRefresh() } } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/FloatingActionButtonBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/FloatingActionButtonBinding.kt index b3f82ffbc..6b543e55e 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/FloatingActionButtonBinding.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/FloatingActionButtonBinding.kt @@ -15,7 +15,6 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged import org.mozilla.fenix.R import org.mozilla.fenix.tabstray.browser.BrowserTrayInteractor -import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsInteractor import org.mozilla.fenix.utils.Settings /** @@ -28,8 +27,7 @@ class FloatingActionButtonBinding( private val store: TabsTrayStore, private val settings: Settings, private val actionButton: ExtendedFloatingActionButton, - private val browserTrayInteractor: BrowserTrayInteractor, - private val syncedTabsInteractor: SyncedTabsInteractor + private val browserTrayInteractor: BrowserTrayInteractor ) : LifecycleAwareFeature { private var scope: CoroutineScope? = null @@ -98,7 +96,6 @@ class FloatingActionButtonBinding( // a sync was requested. if (!syncing) { store.dispatch(TabsTrayAction.SyncNow) - syncedTabsInteractor.onRefresh() } } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt index ff2e371af..3e57db642 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt @@ -12,9 +12,12 @@ import kotlinx.coroutines.launch import mozilla.components.browser.state.selector.getNormalOrPrivateTabs import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.browser.storage.sync.Tab as SyncTab import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.tabstray.Tab import mozilla.components.service.fxa.manager.FxaAccountManager +import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.collections.CollectionsDialog import org.mozilla.fenix.collections.show import org.mozilla.fenix.components.TabCollectionStorage @@ -75,6 +78,11 @@ interface NavigationInteractor { * Used when adding [Tab]s as bookmarks. */ fun onSaveToBookmarks(tabs: Collection) + + /** + * Called when clicking on a SyncedTab item. + */ + fun onSyncedTabClicked(tab: SyncTab) } /** @@ -83,6 +91,7 @@ interface NavigationInteractor { @Suppress("LongParameterList") class DefaultNavigationInteractor( private val context: Context, + private val activity: HomeActivity, private val browserStore: BrowserStore, private val navController: NavController, private val metrics: MetricController, @@ -193,4 +202,13 @@ class DefaultNavigationInteractor( // TODO show successful snackbar here (regardless of operation succes). } + + override fun onSyncedTabClicked(tab: SyncTab) { + metrics.track(Event.SyncedTabOpened) + activity.openToBrowserAndLoad( + searchTermOrURL = tab.active().url, + newTab = true, + from = BrowserDirection.FromTabTray + ) + } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt index 51b846793..0999ad47e 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -6,10 +6,8 @@ package org.mozilla.fenix.tabstray import androidx.navigation.NavController import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch import mozilla.components.concept.base.profiler.Profiler import mozilla.components.service.fxa.manager.FxaAccountManager -import mozilla.components.service.fxa.sync.SyncReason import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.metrics.Event @@ -23,11 +21,6 @@ interface TabsTrayController { * Called when user clicks the new tab button. */ fun onNewTabTapped(isPrivate: Boolean) - - /** - * Starts user account tab syncing. - * */ - fun onSyncStarted() } class DefaultTabsTrayController( @@ -54,22 +47,6 @@ class DefaultTabsTrayController( sendNewTabEvent(isPrivate) } - override fun onSyncStarted() { - ioScope.launch { - metrics.track(Event.SyncAccountSyncNow) - // Trigger a sync. - accountManager.syncNow(SyncReason.User) - // Poll for device events & update devices. - accountManager.authenticatedAccount() - ?.deviceConstellation()?.run { - refreshDevices() - pollForCommands() - } - }.invokeOnCompletion { - store.dispatch(TabsTrayAction.SyncCompleted) - } - } - private fun sendNewTabEvent(isPrivateModeSelected: Boolean) { val eventToSend = if (isPrivateModeSelected) { Event.NewPrivateTabTapped diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt index f71f16bd8..ef360fcda 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt @@ -53,7 +53,6 @@ import org.mozilla.fenix.tabstray.browser.SelectionBannerBinding import org.mozilla.fenix.tabstray.browser.SelectionBannerBinding.VisibilityModifier import org.mozilla.fenix.tabstray.ext.getTrayPosition import org.mozilla.fenix.tabstray.ext.showWithTheme -import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsInteractor import org.mozilla.fenix.utils.allowUndo import kotlin.math.max @@ -111,6 +110,7 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { val navigationInteractor = DefaultNavigationInteractor( context = requireContext(), + activity = activity, tabsTrayStore = tabsTrayStore, browserStore = requireComponents.core.store, navController = findNavController(), @@ -143,20 +143,13 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { requireComponents.analytics.metrics ) - val syncedTabsTrayInteractor = SyncedTabsInteractor( - requireComponents.analytics.metrics, - requireActivity() as HomeActivity, - this, - controller = tabsTrayController - ) - setupMenu(view, navigationInteractor) setupPager( view.context, tabsTrayStore, this, browserTrayInteractor, - syncedTabsTrayInteractor + navigationInteractor ) setupBackgroundDismissalListener { @@ -216,8 +209,7 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { store = tabsTrayStore, settings = requireComponents.settings, actionButton = new_tab_button, - browserTrayInteractor = browserTrayInteractor, - syncedTabsInteractor = syncedTabsTrayInteractor + browserTrayInteractor = browserTrayInteractor ), owner = this, view = view @@ -228,8 +220,7 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { store = tabsTrayStore, settings = requireComponents.settings, newTabButton = tab_tray_new_tab, - browserTrayInteractor = browserTrayInteractor, - syncedTabsInteractor = syncedTabsTrayInteractor + browserTrayInteractor = browserTrayInteractor ), owner = this, view = view @@ -337,14 +328,14 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { store: TabsTrayStore, trayInteractor: TabsTrayInteractor, browserInteractor: BrowserTrayInteractor, - syncedTabsTrayInteractor: SyncedTabsInteractor + navigationInteractor: NavigationInteractor ) { tabsTray.apply { adapter = TrayPagerAdapter( context, store, browserInteractor, - syncedTabsTrayInteractor, + navigationInteractor, trayInteractor, requireComponents.core.store ) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt index 8c09e439c..7b72b8144 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt @@ -12,10 +12,10 @@ import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.selector.selectedTab import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.feature.syncedtabs.view.SyncedTabsView import org.mozilla.fenix.sync.SyncedTabsAdapter import org.mozilla.fenix.tabstray.browser.BrowserTabsAdapter import org.mozilla.fenix.tabstray.browser.BrowserTrayInteractor +import org.mozilla.fenix.tabstray.syncedtabs.TabClickDelegate import org.mozilla.fenix.tabstray.viewholders.AbstractTrayViewHolder import org.mozilla.fenix.tabstray.viewholders.NormalBrowserTabViewHolder import org.mozilla.fenix.tabstray.viewholders.PrivateBrowserTabViewHolder @@ -25,14 +25,14 @@ class TrayPagerAdapter( private val context: Context, private val store: TabsTrayStore, private val browserInteractor: BrowserTrayInteractor, - private val syncedTabsInteractor: SyncedTabsView.Listener, + private val navInteractor: NavigationInteractor, private val interactor: TabsTrayInteractor, private val browserStore: BrowserStore ) : RecyclerView.Adapter() { private val normalAdapter by lazy { BrowserTabsAdapter(context, browserInteractor, store) } private val privateAdapter by lazy { BrowserTabsAdapter(context, browserInteractor, store) } - private val syncedTabsAdapter by lazy { SyncedTabsAdapter(syncedTabsInteractor) } + private val syncedTabsAdapter by lazy { SyncedTabsAdapter(TabClickDelegate(navInteractor)) } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): AbstractTrayViewHolder { val itemView = LayoutInflater.from(parent.context).inflate(viewType, parent, false) @@ -59,7 +59,7 @@ class TrayPagerAdapter( SyncedTabViewHolder.LAYOUT_ID -> { SyncedTabViewHolder( itemView, - syncedTabsInteractor + store ) } else -> throw IllegalStateException("Unknown viewType.") diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBinding.kt new file mode 100644 index 000000000..e07c7008f --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBinding.kt @@ -0,0 +1,35 @@ +/* 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/. */ + +package org.mozilla.fenix.tabstray.syncedtabs + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.map +import mozilla.components.feature.syncedtabs.view.SyncedTabsView +import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged +import org.mozilla.fenix.components.AbstractBinding +import org.mozilla.fenix.tabstray.TabsTrayState +import org.mozilla.fenix.tabstray.TabsTrayStore + +/** + * An [AbstractBinding] that invokes the [onSyncNow] callback when the [TabsTrayState.syncing] is + * set. + * + * This binding is useful for connecting with [SyncedTabsView.Listener]. + */ +class SyncButtonBinding( + tabsTrayStore: TabsTrayStore, + private val onSyncNow: () -> Unit +) : AbstractBinding(tabsTrayStore) { + override suspend fun onState(flow: Flow) { + flow.map { it.syncing } + .ifChanged() + .collect { syncingNow -> + if (syncingNow) { + onSyncNow() + } + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsInteractor.kt deleted file mode 100644 index 8e619742c..000000000 --- a/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsInteractor.kt +++ /dev/null @@ -1,34 +0,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/. */ - -package org.mozilla.fenix.tabstray.syncedtabs - -import mozilla.components.browser.storage.sync.Tab -import mozilla.components.feature.syncedtabs.view.SyncedTabsView -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.tabstray.TabsTrayController -import org.mozilla.fenix.tabstray.TabsTrayInteractor - -class SyncedTabsInteractor( - private val metrics: MetricController, - private val activity: HomeActivity, - private val trayInteractor: TabsTrayInteractor, - private val controller: TabsTrayController -) : SyncedTabsView.Listener { - override fun onRefresh() { - controller.onSyncStarted() - } - override fun onTabClicked(tab: Tab) { - metrics.track(Event.SyncedTabOpened) - activity.openToBrowserAndLoad( - searchTermOrURL = tab.active().url, - newTab = true, - from = BrowserDirection.FromTabTray - ) - trayInteractor.navigateToBrowser() - } -} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsTrayLayout.kt b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsTrayLayout.kt index 93bc60831..d659f28a9 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsTrayLayout.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsTrayLayout.kt @@ -24,7 +24,9 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.sync.SyncedTabsAdapter import org.mozilla.fenix.sync.ext.toAdapterItem import org.mozilla.fenix.sync.ext.toStringRes +import org.mozilla.fenix.tabstray.TabsTrayAction import org.mozilla.fenix.tabstray.TabsTrayFragment +import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.tabstray.TrayItem import org.mozilla.fenix.utils.view.LifecycleViewProvider @@ -37,6 +39,7 @@ class SyncedTabsTrayLayout @JvmOverloads constructor( private val lifecycleProvider = LifecycleViewProvider(this) private val coroutineScope = CoroutineScope(Dispatchers.Main) + private val syncedTabsFeature by lazy { SyncedTabsFeature( context = context, @@ -52,6 +55,14 @@ class SyncedTabsTrayLayout @JvmOverloads constructor( ) } + private val syncButtonBinding by lazy { + SyncButtonBinding(tabsTrayStore) { + listener?.onRefresh() + } + } + + lateinit var tabsTrayStore: TabsTrayStore + override var listener: SyncedTabsView.Listener? = null override fun displaySyncedTabs(syncedTabs: List) { @@ -79,33 +90,28 @@ class SyncedTabsTrayLayout @JvmOverloads constructor( } } - override fun startLoading() { - // TODO implement with https://github.com/mozilla-mobile/fenix/issues/18515 - // start animating FAB - // interactor.syncingTabs(loading = true) - } - - override fun stopLoading() { - // TODO implement with https://github.com/mozilla-mobile/fenix/issues/18515 - // stop animating FAB - // interactor.syncingTabs(loading = false) - } - override fun onAttachedToWindow() { super.onAttachedToWindow() syncedTabsFeature.start() + syncButtonBinding.start() } override fun onDetachedFromWindow() { super.onDetachedFromWindow() syncedTabsFeature.stop() + syncButtonBinding.stop() + coroutineScope.cancel() } - fun onRefresh() { - // TODO implement with https://github.com/mozilla-mobile/fenix/issues/18515 - // syncedTabsFeature.interactor.onRefresh() + override fun stopLoading() { + tabsTrayStore.dispatch(TabsTrayAction.SyncCompleted) } + + /** + * Do nothing; the UI is handled with FloatingActionButtonBinding. + */ + override fun startLoading() = Unit } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegate.kt b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegate.kt new file mode 100644 index 000000000..53f93ab6b --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegate.kt @@ -0,0 +1,22 @@ +/* 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/. */ + +package org.mozilla.fenix.tabstray.syncedtabs + +import mozilla.components.browser.storage.sync.Tab +import mozilla.components.feature.syncedtabs.view.SyncedTabsView +import org.mozilla.fenix.tabstray.NavigationInteractor + +/** + * A wrapper class that handles tab clicks from a Synced Tabs list. + */ +class TabClickDelegate( + private val interactor: NavigationInteractor +) : SyncedTabsView.Listener { + override fun onTabClicked(tab: Tab) { + interactor.onSyncedTabClicked(tab) + } + + override fun onRefresh() = Unit +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/SyncedTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/SyncedTabViewHolder.kt index 9aec84ff9..066fce9fa 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/SyncedTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/SyncedTabViewHolder.kt @@ -7,12 +7,12 @@ package org.mozilla.fenix.tabstray.viewholders import android.view.View import androidx.recyclerview.widget.RecyclerView import kotlinx.android.synthetic.main.component_sync_tabs_tray_layout.* -import mozilla.components.feature.syncedtabs.view.SyncedTabsView import org.mozilla.fenix.R +import org.mozilla.fenix.tabstray.TabsTrayStore class SyncedTabViewHolder( containerView: View, - private val listener: SyncedTabsView.Listener + private val tabsTrayStore: TabsTrayStore ) : AbstractTrayViewHolder(containerView) { override fun bind( @@ -21,7 +21,8 @@ class SyncedTabViewHolder( ) { synced_tabs_list.layoutManager = layoutManager synced_tabs_list.adapter = adapter - synced_tabs_tray_layout.listener = listener + + synced_tabs_tray_layout.tabsTrayStore = tabsTrayStore } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/utils/view/LifecycleViewProvider.kt b/app/src/main/java/org/mozilla/fenix/utils/view/LifecycleViewProvider.kt index 010f82b6f..b06c46768 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/view/LifecycleViewProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/view/LifecycleViewProvider.kt @@ -14,7 +14,7 @@ import androidx.lifecycle.LifecycleRegistry /** * Provides a [LifecycleOwner] on a given [View] for features that function on lifecycle events. * - * When the [View] is attached to the window, observers will receive the [Lifecycle.Event.ON_START] event. + * When the [View] is attached to the window, observers will receive the [Lifecycle.Event.ON_RESUME] event. * When the [View] is detached to the window, observers will receive the [Lifecycle.Event.ON_STOP] event. * * @param view The [View] that will be observed. @@ -36,7 +36,7 @@ internal class ViewBinding( private val registry: LifecycleRegistry ) : View.OnAttachStateChangeListener { override fun onViewAttachedToWindow(v: View?) { - registry.currentState = State.STARTED + registry.currentState = State.RESUMED } override fun onViewDetachedFromWindow(v: View?) { diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBindingTest.kt index df91bd25c..b6431aa5a 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBindingTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/AccessibleNewTabButtonBindingTest.kt @@ -20,7 +20,6 @@ import org.junit.Rule import org.junit.Test import org.mozilla.fenix.R import org.mozilla.fenix.tabstray.browser.BrowserTrayInteractor -import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsInteractor import org.mozilla.fenix.utils.Settings class AccessibleNewTabButtonBindingTest { @@ -32,7 +31,6 @@ class AccessibleNewTabButtonBindingTest { private val settings: Settings = mockk(relaxed = true) private val newTabButton: ImageButton = mockk(relaxed = true) private val browserTrayInteractor: BrowserTrayInteractor = mockk(relaxed = true) - private val syncedTabsInteractor: SyncedTabsInteractor = mockk(relaxed = true) @Before fun setup() { @@ -44,7 +42,7 @@ class AccessibleNewTabButtonBindingTest { fun `WHEN tab selected page is normal tab THEN new tab button is visible`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.NormalTabs)) val newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns true @@ -57,7 +55,7 @@ class AccessibleNewTabButtonBindingTest { fun `WHEN tab selected page is private tab THEN new tab button is visible`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.PrivateTabs)) val newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns true @@ -70,7 +68,7 @@ class AccessibleNewTabButtonBindingTest { fun `WHEN tab selected page is sync tab THEN new tab button is visible`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.SyncedTabs)) val newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns true @@ -83,7 +81,7 @@ class AccessibleNewTabButtonBindingTest { fun `WHEN accessibility is disabled THEN new tab button is not visible`() { var tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.NormalTabs)) var newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns false @@ -93,7 +91,7 @@ class AccessibleNewTabButtonBindingTest { tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.PrivateTabs)) newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) newTabButtonBinding.start() @@ -102,7 +100,7 @@ class AccessibleNewTabButtonBindingTest { tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.SyncedTabs)) newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) newTabButtonBinding.start() @@ -114,7 +112,7 @@ class AccessibleNewTabButtonBindingTest { fun `WHEN selected page is updated THEN button is updated`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.NormalTabs)) val newTabButtonBinding = AccessibleNewTabButtonBinding( - tabsTrayStore, settings, newTabButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, newTabButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns true diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/FloatingActionButtonBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/FloatingActionButtonBindingTest.kt index 9f6495c82..61533ded8 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/FloatingActionButtonBindingTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/FloatingActionButtonBindingTest.kt @@ -19,7 +19,6 @@ import org.junit.Rule import org.junit.Test import org.mozilla.fenix.R import org.mozilla.fenix.tabstray.browser.BrowserTrayInteractor -import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsInteractor import org.mozilla.fenix.utils.Settings class FloatingActionButtonBindingTest { @@ -31,7 +30,6 @@ class FloatingActionButtonBindingTest { private val settings: Settings = mockk(relaxed = true) private val actionButton: ExtendedFloatingActionButton = mockk(relaxed = true) private val browserTrayInteractor: BrowserTrayInteractor = mockk(relaxed = true) - private val syncedTabsInteractor: SyncedTabsInteractor = mockk(relaxed = true) @Before fun setup() { @@ -43,7 +41,7 @@ class FloatingActionButtonBindingTest { fun `WHEN tab selected page is normal tab THEN shrink and show is called`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.NormalTabs)) val fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns false @@ -59,7 +57,7 @@ class FloatingActionButtonBindingTest { fun `WHEN tab selected page is private tab THEN extend and show is called`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.PrivateTabs)) val fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns false @@ -75,7 +73,7 @@ class FloatingActionButtonBindingTest { fun `WHEN tab selected page is sync tab THEN extend and show is called`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.SyncedTabs)) val fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns false @@ -91,7 +89,7 @@ class FloatingActionButtonBindingTest { fun `WHEN accessibility is enabled THEN show is not called`() { var tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.NormalTabs)) var fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns true @@ -102,7 +100,7 @@ class FloatingActionButtonBindingTest { tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.PrivateTabs)) fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) fabBinding.start() @@ -112,7 +110,7 @@ class FloatingActionButtonBindingTest { tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.SyncedTabs)) fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) fabBinding.start() @@ -125,7 +123,7 @@ class FloatingActionButtonBindingTest { fun `WHEN selected page is updated THEN button is updated`() { val tabsTrayStore = TabsTrayStore(TabsTrayState(selectedPage = Page.NormalTabs)) val fabBinding = FloatingActionButtonBinding( - tabsTrayStore, settings, actionButton, browserTrayInteractor, syncedTabsInteractor + tabsTrayStore, settings, actionButton, browserTrayInteractor ) every { settings.accessibilityServicesEnabled } returns false diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt index 69ae45dad..2eeb75a9a 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt @@ -20,6 +20,8 @@ import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.createTab as createStateTab import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.browser.storage.sync.TabEntry +import mozilla.components.browser.storage.sync.Tab as SyncTab import mozilla.components.concept.tabstray.Tab import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.support.test.rule.MainCoroutineRule @@ -27,6 +29,8 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test +import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.collections.CollectionsDialog import org.mozilla.fenix.collections.show import org.mozilla.fenix.components.TabCollectionStorage @@ -50,6 +54,7 @@ class NavigationInteractorTest { private val context: Context = mockk(relaxed = true) private val collectionStorage: TabCollectionStorage = mockk(relaxed = true) private val accountManager: FxaAccountManager = mockk(relaxed = true) + private val activity: HomeActivity = mockk(relaxed = true) private val testDispatcher = TestCoroutineDispatcher() @@ -65,6 +70,7 @@ class NavigationInteractorTest { tabsTrayStore = TabsTrayStore() navigationInteractor = DefaultNavigationInteractor( context, + activity, store, navController, metrics, @@ -89,6 +95,7 @@ class NavigationInteractorTest { var onShareTabs = false var onSaveToCollections = false var onBookmarkTabs = false + var onSyncedTabsClicked = false class TestNavigationInteractor : NavigationInteractor { @@ -120,6 +127,10 @@ class NavigationInteractorTest { onBookmarkTabs = true } + override fun onSyncedTabClicked(tab: mozilla.components.browser.storage.sync.Tab) { + onSyncedTabsClicked = true + } + override fun onShareTabsOfTypeClicked(private: Boolean) { shareTabsOfTypeClicked = true } @@ -148,6 +159,8 @@ class NavigationInteractorTest { assertTrue(onSaveToCollections) navigationInteractor.onSaveToBookmarks(emptyList()) assertTrue(onBookmarkTabs) + navigationInteractor.onSyncedTabClicked(mockk()) + assertTrue(onSyncedTabsClicked) } @Test @@ -219,6 +232,7 @@ class NavigationInteractorTest { fun `onBookmarkTabs calls navigation on DefaultNavigationInteractor`() = runBlockingTest { navigationInteractor = DefaultNavigationInteractor( context, + activity, store, navController, metrics, @@ -233,4 +247,24 @@ class NavigationInteractorTest { navigationInteractor.onSaveToBookmarks(listOf(createTrayTab())) coVerify(exactly = 1) { bookmarksUseCase.addBookmark(any(), any(), any()) } } + + @Test + fun `onSyncedTabsClicked sets metrics and opens browser`() { + val tab = mockk() + val entry = mockk() + + every { tab.active() }.answers { entry } + every { entry.url }.answers { "https://mozilla.org" } + + navigationInteractor.onSyncedTabClicked(tab) + + verify { metrics.track(Event.SyncedTabOpened) } + verify { + activity.openToBrowserAndLoad( + searchTermOrURL = "https://mozilla.org", + newTab = true, + from = BrowserDirection.FromTabTray + ) + } + } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBindingTest.kt new file mode 100644 index 000000000..38deb7645 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/SyncButtonBindingTest.kt @@ -0,0 +1,54 @@ +/* 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/. */ + +package org.mozilla.fenix.tabstray.syncedtabs + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.rule.MainCoroutineRule +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.mozilla.fenix.tabstray.TabsTrayAction +import org.mozilla.fenix.tabstray.TabsTrayStore + +class SyncButtonBindingTest { + @OptIn(ExperimentalCoroutinesApi::class) + @get:Rule + val coroutinesTestRule = MainCoroutineRule(TestCoroutineDispatcher()) + + @Test + fun `WHEN syncing state is true THEN invoke callback`() { + var invoked = false + val store = TabsTrayStore() + val binding = SyncButtonBinding(store) { invoked = true } + + binding.start() + + store.dispatch(TabsTrayAction.SyncNow) + store.waitUntilIdle() + + assertTrue(invoked) + } + + @Test + fun `WHEN syncing state is false THEN nothing is invoked`() { + var invoked = false + val store = TabsTrayStore() + val binding = SyncButtonBinding(store) { invoked = true } + + binding.start() + + store.waitUntilIdle() + + assertFalse(invoked) + + store.dispatch(TabsTrayAction.SyncCompleted) + store.waitUntilIdle() + + assertFalse(invoked) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegateTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegateTest.kt new file mode 100644 index 000000000..58375c596 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/syncedtabs/TabClickDelegateTest.kt @@ -0,0 +1,36 @@ +/* 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/. */ + +package org.mozilla.fenix.tabstray.syncedtabs + +import io.mockk.Called +import io.mockk.mockk +import io.mockk.verify +import mozilla.components.browser.storage.sync.Tab +import org.junit.Test +import org.mozilla.fenix.tabstray.NavigationInteractor + +class TabClickDelegateTest { + + private val interactor = mockk(relaxed = true) + private val tab = mockk() + + @Test + fun `WHEN tab is clicked THEN invoke the interactor`() { + val delegate = TabClickDelegate(interactor) + + delegate.onTabClicked(tab) + + verify { interactor.onSyncedTabClicked(tab) } + } + + @Test + fun `WHEN refresh is invoked THEN do nothing`() { + val delegate = TabClickDelegate(interactor) + + delegate.onRefresh() + + verify { interactor wasNot Called } + } +}