From ecd79c4d6ed1527793e0a32101b9fc7d772b74be Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Mon, 2 Nov 2020 11:06:17 -0500 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/12062: Switch to consumeFlow and add tests --- .../fenix/browser/BaseBrowserFragment.kt | 60 ++++++++++--------- .../fenix/browser/BrowserFragmentTest.kt | 32 ++++++++++ 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index d6d9d6fd5c..07c7b1b3b9 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -21,6 +21,7 @@ import androidx.core.view.isVisible import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels import androidx.lifecycle.lifecycleScope +import androidx.navigation.NavController import androidx.navigation.fragment.findNavController import androidx.preference.PreferenceManager import com.google.android.material.snackbar.Snackbar @@ -88,7 +89,6 @@ import org.mozilla.fenix.OnBackLongPressedListener import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.readermode.DefaultReaderModeController -import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.FindInPageIntegration import org.mozilla.fenix.components.StoreProvider @@ -134,7 +134,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, private lateinit var browserFragmentStore: BrowserFragmentStore private lateinit var browserAnimator: BrowserAnimator - private lateinit var components: Components private var _browserInteractor: BrowserToolbarViewInteractor? = null protected val browserInteractor: BrowserToolbarViewInteractor @@ -196,28 +195,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, val view = inflater.inflate(R.layout.fragment_browser, container, false) val activity = activity as HomeActivity - components = requireComponents - - if (customTabSessionId == null) { - // Once tab restoration is complete, if there are no tabs to show in the browser, go home - components.core.store.flowScoped(viewLifecycleOwner) { flow -> - flow.map { state -> state.restoreComplete } - .ifChanged() - .collect { restored -> - if (restored) { - val tabs = - components.core.store.state.getNormalOrPrivateTabs( - activity.browsingModeManager.mode.isPrivate - ) - if (tabs.isEmpty()) findNavController().popBackStack( - R.id.homeFragment, - false - ) - } - } - } - } - activity.themeManager.applyStatusBarTheme(activity) browserFragmentStore = StoreProvider.get(this) { @@ -231,6 +208,14 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, final override fun onViewCreated(view: View, savedInstanceState: Bundle?) { browserInitialized = initializeUI(view) != null + + if (customTabSessionId == null) { + // We currently only need this observer to navigate to home + // in case all tabs have been removed on startup. No need to + // this if we have a known session to display. + observeRestoreComplete(requireComponents.core.store, findNavController()) + } + observeTabSelection(requireComponents.core.store) requireContext().accessibilityManager.addAccessibilityStateChangeListener(this) } @@ -821,6 +806,27 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, sitePermissionWifiIntegration.get()?.maybeAddWifiConnectedListener() } + @VisibleForTesting + internal fun observeRestoreComplete(store: BrowserStore, navController: NavController) { + val activity = activity as HomeActivity + consumeFlow(store) { flow -> + flow.map { state -> state.restoreComplete } + .ifChanged() + .collect { restored -> + if (restored) { + // Once tab restoration is complete, if there are no tabs to show in the browser, go home + val tabs = + store.state.getNormalOrPrivateTabs( + activity.browsingModeManager.mode.isPrivate + ) + if (tabs.isEmpty() || store.state.selectedTabId == null) { + navController.popBackStack(R.id.homeFragment, false) + } + } + } + } + } + @VisibleForTesting internal fun observeTabSelection(store: BrowserStore) { consumeFlow(store) { flow -> @@ -995,13 +1001,13 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, * Returns the layout [android.view.Gravity] for the quick settings and ETP dialog. */ protected fun getAppropriateLayoutGravity(): Int = - components.settings.toolbarPosition.androidGravity + requireComponents.settings.toolbarPosition.androidGravity /** * Updates the site permissions rules based on user settings. */ private fun assignSitePermissionsRules() { - val rules = components.settings.getSitePermissionsCustomSettingsRules() + val rules = requireComponents.settings.getSitePermissionsCustomSettingsRules() sitePermissionsFeature.withFeature { it.sitePermissionsRules = rules @@ -1046,7 +1052,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, * Returns the current session. */ protected fun getSessionById(): Session? { - val sessionManager = components.core.sessionManager + val sessionManager = requireComponents.core.sessionManager val localCustomTabId = customTabSessionId return if (localCustomTabId != null) { sessionManager.findSessionById(localCustomTabId) diff --git a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt index c8621734bd..29ddf88da0 100644 --- a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt @@ -9,6 +9,7 @@ import android.view.View import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry +import androidx.navigation.NavController import io.mockk.every import io.mockk.mockk import io.mockk.spyk @@ -16,7 +17,9 @@ import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineDispatcher import mozilla.components.browser.state.action.ContentAction +import mozilla.components.browser.state.action.RestoreCompleteAction import mozilla.components.browser.state.action.TabListAction +import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.LoadRequestState import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.createTab @@ -30,6 +33,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.FenixApplication import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.R import org.mozilla.fenix.components.toolbar.BrowserToolbarView import org.mozilla.fenix.ext.application import org.mozilla.fenix.ext.components @@ -47,6 +51,7 @@ class BrowserFragmentTest { private lateinit var fenixApplication: FenixApplication private lateinit var context: Context private lateinit var lifecycleOwner: MockedLifecycleOwner + private lateinit var navController: NavController private val testDispatcher = TestCoroutineDispatcher() @@ -62,6 +67,7 @@ class BrowserFragmentTest { homeActivity = mockk(relaxed = true) view = mockk(relaxed = true) lifecycleOwner = MockedLifecycleOwner(Lifecycle.State.STARTED) + navController = mockk(relaxed = true) browserFragment = spyk(BrowserFragment()) every { browserFragment.view } returns view @@ -173,6 +179,32 @@ class BrowserFragmentTest { verify(exactly = 1) { toolbar.expand() } } + @Test + fun `GIVEN tabs are restored WHEN there are no tabs THEN navigate to home`() { + browserFragment.observeRestoreComplete(store, navController) + store.dispatch(RestoreCompleteAction).joinBlocking() + + verify(exactly = 1) { navController.popBackStack(R.id.homeFragment, false) } + } + + @Test + fun `GIVEN tabs are restored WHEN there are tabs THEN do not navigate`() { + addAndSelectTab(testTab) + browserFragment.observeRestoreComplete(store, navController) + store.dispatch(RestoreCompleteAction).joinBlocking() + + verify(exactly = 0) { navController.popBackStack(R.id.homeFragment, false) } + } + + @Test + fun `GIVEN tabs are restored WHEN there is no selected tab THEN navigate to home`() { + val store = BrowserStore(initialState = BrowserState(tabs = listOf(testTab))) + browserFragment.observeRestoreComplete(store, navController) + store.dispatch(RestoreCompleteAction).joinBlocking() + + verify(exactly = 1) { navController.popBackStack(R.id.homeFragment, false) } + } + private fun addAndSelectTab(tab: TabSessionState) { store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() store.dispatch(TabListAction.SelectTabAction(tab.id)).joinBlocking()