commit 1aac42db3a1bd617c4864aee4be43ce395231ea7 parent 24cc01672db4982ab737dee40244e2b4969b5ba6 Author: Mugurell <Mugurell@users.noreply.github.com> Date: Mon, 17 Nov 2025 17:52:51 +0000 Bug 1996643 - part 3 - Migrate from the environment idiom within BrowserScreenStore r=android-reviewers,matt-tighe,nalexander Differential Revision: https://phabricator.services.mozilla.com/D272156 Diffstat:
8 files changed, 28 insertions(+), 189 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -125,6 +125,7 @@ import mozilla.components.feature.webauthn.WebAuthnFeature import mozilla.components.lib.state.ext.consumeFlow import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.lib.state.ext.flowScoped +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import mozilla.components.service.sync.autofill.DefaultCreditCardValidationDelegate import mozilla.components.service.sync.logins.DefaultLoginValidationDelegate import mozilla.components.service.sync.logins.LoginsApiException @@ -163,6 +164,8 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.permissions.FenixSitePermissionLearnMoreUrlProvider import org.mozilla.fenix.browser.readermode.DefaultReaderModeController import org.mozilla.fenix.browser.readermode.ReaderModeController +import org.mozilla.fenix.browser.store.BrowserScreenMiddleware +import org.mozilla.fenix.browser.store.BrowserScreenState import org.mozilla.fenix.browser.store.BrowserScreenStore import org.mozilla.fenix.browser.tabstrip.TabStrip import org.mozilla.fenix.components.Components @@ -361,7 +364,7 @@ abstract class BaseBrowserFragment : @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) internal var webAppToolbarShouldBeVisible = true - protected lateinit var browserScreenStore: BrowserScreenStore + protected val browserScreenStore by buildBrowserScreenStore() private val homeViewModel: HomeScreenViewModel by activityViewModels() private var downloadDialog: AlertDialog? = null @@ -442,8 +445,6 @@ abstract class BaseBrowserFragment : // DO NOT ADD ANYTHING ABOVE THIS getProfilerTime CALL! val profilerStartTime = requireComponents.core.engine.profiler?.getProfilerTime() - browserScreenStore = buildBrowserScreenStore() - initializeUI(view) appLinksFeature.set( @@ -1491,11 +1492,17 @@ abstract class BaseBrowserFragment : awesomeBarComposable = it } - private fun buildBrowserScreenStore() = BrowserScreenStoreBuilder.build( - context = requireContext(), - lifecycleOwner = this, - fragmentManager = childFragmentManager, - ) + private fun buildBrowserScreenStore() = fragmentStore(BrowserScreenState()) { + BrowserScreenStore( + middleware = listOf( + BrowserScreenMiddleware( + uiContext = requireContext(), + crashReporter = requireContext().components.analytics.crashReporter, + fragmentManager = childFragmentManager, + ), + ), + ) + } private fun buildToolbarStore( activity: HomeActivity, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserScreenStoreBuilder.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserScreenStoreBuilder.kt @@ -1,61 +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.browser - -import android.content.Context -import androidx.fragment.app.Fragment -import androidx.fragment.app.FragmentManager -import androidx.lifecycle.DefaultLifecycleObserver -import androidx.lifecycle.LifecycleOwner -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentCleared -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentRehydrated -import org.mozilla.fenix.browser.store.BrowserScreenMiddleware -import org.mozilla.fenix.browser.store.BrowserScreenStore -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment -import org.mozilla.fenix.components.StoreProvider -import org.mozilla.fenix.ext.components - -/** - * Delegate for building the [BrowserScreenStore]. - */ -object BrowserScreenStoreBuilder { - - /** - * Builds the [BrowserScreenStore]. - * - * @param context [Context] needed for various Android interactions. - * @param lifecycleOwner [Fragment] which will have it's lifecycle observed for configuring - * lifecycle related operation. - * @param fragmentManager [FragmentManager] used for managing child fragments. - */ - fun build( - context: Context, - lifecycleOwner: Fragment, - fragmentManager: FragmentManager, - ) = StoreProvider.get(lifecycleOwner) { - BrowserScreenStore( - middleware = listOf( - BrowserScreenMiddleware(context.components.analytics.crashReporter), - ), - ) - }.also { - it.dispatch( - EnvironmentRehydrated( - Environment( - context = context, - viewLifecycleOwner = lifecycleOwner, - fragmentManager = fragmentManager, - ), - ), - ) - lifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(EnvironmentCleared) - } - }, - ) - } -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/store/BrowserScreenAction.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/store/BrowserScreenAction.kt @@ -7,25 +7,12 @@ package org.mozilla.fenix.browser.store import mozilla.components.lib.state.Action import org.mozilla.fenix.browser.PageTranslationStatus import org.mozilla.fenix.browser.ReaderModeStatus -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment /** * Actions related to the browser screen. */ sealed class BrowserScreenAction : Action { /** - * Signals a new valid [Environment] has been set. - * - * @property environment The new [Environment]. - */ - data class EnvironmentRehydrated(val environment: Environment) : BrowserScreenAction() - - /** - * Signals the current [Environment] is not valid anymore. - */ - data object EnvironmentCleared : BrowserScreenAction() - - /** * [Action] for when the last private tab is about to be closed. * * @property tabId Id of the tab that was just closed. diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/store/BrowserScreenMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/store/BrowserScreenMiddleware.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.browser.store import android.content.Context import android.view.Gravity import androidx.annotation.VisibleForTesting +import androidx.fragment.app.FragmentManager import mozilla.components.concept.base.crash.Breadcrumb import mozilla.components.feature.downloads.ui.DownloadCancelDialogFragment import mozilla.components.lib.crash.CrashReporter @@ -16,22 +17,21 @@ import mozilla.components.lib.state.Store import org.mozilla.fenix.R import org.mozilla.fenix.browser.store.BrowserScreenAction.CancelPrivateDownloadsOnPrivateTabsClosedAccepted import org.mozilla.fenix.browser.store.BrowserScreenAction.ClosingLastPrivateTab -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentCleared -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentRehydrated -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment import org.mozilla.fenix.ext.pixelSizeFor import org.mozilla.fenix.theme.ThemeManager /** * [Middleware] responsible for handling actions related to the browser screen. * + * @param uiContext [Context] used for various system interactions. * @param crashReporter [CrashReporter] for recording crashes. + * @param fragmentManager [FragmentManager] to use for showing other fragments. */ class BrowserScreenMiddleware( + private val uiContext: Context, private val crashReporter: CrashReporter, + private val fragmentManager: FragmentManager, ) : Middleware<BrowserScreenState, BrowserScreenAction> { - @VisibleForTesting - internal var environment: Environment? = null override fun invoke( context: MiddlewareContext<BrowserScreenState, BrowserScreenAction>, @@ -39,18 +39,6 @@ class BrowserScreenMiddleware( action: BrowserScreenAction, ) { when (action) { - is EnvironmentRehydrated -> { - next(action) - - environment = action.environment - } - - is EnvironmentCleared -> { - next(action) - - environment = null - } - is ClosingLastPrivateTab -> { next(action) @@ -70,14 +58,12 @@ class BrowserScreenMiddleware( downloadCount: Int, tabId: String?, ) { - val environment = environment ?: return - crashReporter.recordCrashBreadcrumb( Breadcrumb("DownloadCancelDialogFragment shown in browser screen"), ) - val dialog = createDownloadCancelDialog(environment.context, store, downloadCount, tabId) + val dialog = createDownloadCancelDialog(uiContext, store, downloadCount, tabId) - dialog.show(environment.fragmentManager, CANCEL_PRIVATE_DOWNLOADS_DIALOG_FRAGMENT_TAG) + dialog.show(fragmentManager, CANCEL_PRIVATE_DOWNLOADS_DIALOG_FRAGMENT_TAG) } /** diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/store/BrowserScreenStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/store/BrowserScreenStore.kt @@ -4,16 +4,11 @@ package org.mozilla.fenix.browser.store -import android.content.Context -import androidx.fragment.app.FragmentManager -import androidx.lifecycle.LifecycleOwner import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.Store import org.mozilla.fenix.browser.store.BrowserScreenAction.CancelPrivateDownloadsOnPrivateTabsClosedAccepted import org.mozilla.fenix.browser.store.BrowserScreenAction.ClosingLastPrivateTab import org.mozilla.fenix.browser.store.BrowserScreenAction.CustomTabColorsUpdated -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentCleared -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentRehydrated import org.mozilla.fenix.browser.store.BrowserScreenAction.PageTranslationStatusUpdated import org.mozilla.fenix.browser.store.BrowserScreenAction.ReaderModeStatusUpdated @@ -30,23 +25,7 @@ class BrowserScreenStore( initialState = initialState, reducer = ::reduce, middleware = middleware, -) { - /** - * The current environment of the browser screen allowing access to various - * other application features that this integrates with. - * - * This is Activity/Fragment lifecycle dependent and should be handled carefully to avoid memory leaks. - * - * @property context [Context] used for various system interactions. - * @property viewLifecycleOwner [LifecycleOwner] depending on which lifecycle related operations will be scheduled. - * @property fragmentManager [FragmentManager] to use for showing other fragments. - */ - data class Environment( - val context: Context, - val viewLifecycleOwner: LifecycleOwner, - val fragmentManager: FragmentManager, - ) -} +) private fun reduce(state: BrowserScreenState, action: BrowserScreenAction): BrowserScreenState = when (action) { is ClosingLastPrivateTab -> state.copy( @@ -66,12 +45,4 @@ private fun reduce(state: BrowserScreenState, action: BrowserScreenAction): Brow is CustomTabColorsUpdated -> state.copy( customTabColors = action.customTabColors, ) - - is EnvironmentRehydrated, - is EnvironmentCleared, - -> { - // no-op - // Expected to be handled in middlewares set by integrators. - state - } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/store/BrowserScreenMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/store/BrowserScreenMiddlewareTest.kt @@ -4,10 +4,7 @@ package org.mozilla.fenix.browser.store -import android.content.Context import androidx.fragment.app.FragmentManager -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleOwner import androidx.test.ext.junit.runners.AndroidJUnit4 import io.mockk.every import io.mockk.mockk @@ -18,27 +15,19 @@ import mozilla.components.lib.crash.CrashReporter import mozilla.components.lib.state.Middleware import mozilla.components.support.test.middleware.CaptureActionsMiddleware import mozilla.components.support.test.robolectric.testContext -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.browser.store.BrowserScreenAction.CancelPrivateDownloadsOnPrivateTabsClosedAccepted -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentCleared -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentRehydrated import org.mozilla.fenix.browser.store.BrowserScreenMiddleware.Companion.CANCEL_PRIVATE_DOWNLOADS_DIALOG_FRAGMENT_TAG -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner @RunWith(AndroidJUnit4::class) class BrowserScreenMiddlewareTest { - - private val lifecycleOwner = TestLifecycleOwner(Lifecycle.State.RESUMED) private val fragmentManager: FragmentManager = mockk(relaxed = true) private val crashReporter: CrashReporter = mockk(relaxed = true) @Test fun `WHEN the last private tab is closing THEN record a breadcrumb and show a warning dialog`() { - val middleware = spyk(BrowserScreenMiddleware(crashReporter)) + val middleware = spyk(BrowserScreenMiddleware(testContext, crashReporter, fragmentManager)) val store = buildStore(listOf(middleware)) val warningDialog: DownloadCancelDialogFragment = mockk(relaxed = true) @@ -60,7 +49,7 @@ class BrowserScreenMiddlewareTest { @Test fun `GIVEN a warning dialog for closing private tabs is shown WHEN the warning is accepted THEN inform about this`() { - val middleware = spyk(BrowserScreenMiddleware(crashReporter)) + val middleware = spyk(BrowserScreenMiddleware(testContext, crashReporter, fragmentManager)) val captureActionsMiddleware = CaptureActionsMiddleware<BrowserScreenState, BrowserScreenAction>() val store = buildStore(listOf(middleware, captureActionsMiddleware)) val warningDialog: DownloadCancelDialogFragment = mockk(relaxed = true) @@ -74,28 +63,9 @@ class BrowserScreenMiddlewareTest { captureActionsMiddleware.assertLastAction(CancelPrivateDownloadsOnPrivateTabsClosedAccepted::class) {} } - @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val middleware = BrowserScreenMiddleware(crashReporter) - val store = buildStore(listOf(middleware)) - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } - private fun buildStore( middlewares: List<Middleware<BrowserScreenState, BrowserScreenAction>> = emptyList(), - context: Context = testContext, - viewLifecycleOwner: LifecycleOwner = lifecycleOwner, - fragmentManager: FragmentManager = this.fragmentManager, ) = BrowserScreenStore( middleware = middlewares, - ).also { - it.dispatch( - EnvironmentRehydrated(Environment(context, viewLifecycleOwner, fragmentManager)), - ) - } + ) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/store/BrowserScreenStoreKtTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/store/BrowserScreenStoreKtTest.kt @@ -4,14 +4,10 @@ package org.mozilla.fenix.browser.store -import android.content.Context import androidx.fragment.app.FragmentManager -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleOwner import io.mockk.mockk import mozilla.components.concept.engine.translate.Language import mozilla.components.lib.state.Middleware -import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -23,18 +19,14 @@ import org.mozilla.fenix.browser.PageTranslationStatus import org.mozilla.fenix.browser.ReaderModeStatus import org.mozilla.fenix.browser.store.BrowserScreenAction.CancelPrivateDownloadsOnPrivateTabsClosedAccepted import org.mozilla.fenix.browser.store.BrowserScreenAction.ClosingLastPrivateTab -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentRehydrated import org.mozilla.fenix.browser.store.BrowserScreenAction.PageTranslationStatusUpdated import org.mozilla.fenix.browser.store.BrowserScreenAction.ReaderModeStatusUpdated -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class BrowserScreenStoreKtTest { @get:Rule val mainCoroutineRule = MainCoroutineRule() - private val lifecycleOwner = TestLifecycleOwner(Lifecycle.State.RESUMED) private val fragmentManager: FragmentManager = mockk() @Test @@ -86,15 +78,8 @@ class BrowserScreenStoreKtTest { private fun buildStore( initialState: BrowserScreenState = BrowserScreenState(), middlewares: List<Middleware<BrowserScreenState, BrowserScreenAction>> = emptyList(), - context: Context = testContext, - viewLifecycleOwner: LifecycleOwner = lifecycleOwner, - fragmentManager: FragmentManager = this.fragmentManager, ) = BrowserScreenStore( initialState = initialState, middleware = middlewares, - ).also { - it.dispatch( - EnvironmentRehydrated(Environment(context, viewLifecycleOwner, fragmentManager)), - ) - } + ) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/store/BrowserScreenStoreTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/store/BrowserScreenStoreTest.kt @@ -6,8 +6,6 @@ package org.mozilla.fenix.browser.store import androidx.lifecycle.Lifecycle import androidx.test.ext.junit.runners.AndroidJUnit4 -import io.mockk.mockk -import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue @@ -16,8 +14,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.browser.store.BrowserScreenAction.CancelPrivateDownloadsOnPrivateTabsClosedAccepted import org.mozilla.fenix.browser.store.BrowserScreenAction.ClosingLastPrivateTab -import org.mozilla.fenix.browser.store.BrowserScreenAction.EnvironmentRehydrated -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner @RunWith(AndroidJUnit4::class) @@ -50,7 +46,5 @@ class BrowserScreenStoreTest { initialState = BrowserScreenState( cancelPrivateDownloadsAccepted = cancelPrivateDownloadsAccepted, ), - ).also { - it.dispatch(EnvironmentRehydrated(Environment(testContext, lifecycleOwner, mockk()))) - } + ) }