commit 9dcbdfbe3b6a1100ab636ac24b24fc417ab48547 parent 9857cef5140fec7dc7bb9e913723414d46663691 Author: Mugurell <Mugurell@users.noreply.github.com> Date: Thu, 20 Nov 2025 09:12:31 +0000 Bug 1996643 - part 1 - Remove lazyStore in favor of the new StoreProvider APIs r=android-reviewers,nalexander The new APIs already support the functionality of lazyStore so they bring a great opportunity in simplifying our code. Differential Revision: https://phabricator.services.mozilla.com/D271420 Diffstat:
14 files changed, 99 insertions(+), 131 deletions(-)
diff --git a/mobile/android/android-components/docs/changelog.md b/mobile/android/android-components/docs/changelog.md @@ -16,6 +16,7 @@ permalink: /changelog/ * 🆕 New `verticalScrollPosition` and `verticalScrollDelta` APIs exposing the current scroll position and delta of the webpage [Bug 1990215](https://bugzilla.mozilla.org/show_bug.cgi?id=1990215). * **lib-state** * 🆕 New `fragmentStore`, `activityStore`, `composableStore` and `navBackStackStore` APIs available to build a new Store and persist its State in a ViewModel ensuring that it survives Activity recreations. These APIs supersede the existing ones and avoid the possibility of memory leaks. [Bug 1996676](https://bugzilla.mozilla.org/show_bug.cgi?id=1996676). + * ⚠️ **Breaking change**: The `lazyStore` API was removed in favor of the new `fragmentStore`, `activityStore` and `composableStore` APIs. [Bug 1996676](https://bugzilla.mozilla.org/show_bug.cgi?id=1996676). # 146.0 diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/StoreProvider.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/StoreProvider.kt @@ -4,7 +4,6 @@ package org.mozilla.fenix.components -import androidx.annotation.MainThread import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider @@ -60,31 +59,3 @@ class StoreProviderFactory<T : Store<*, *>>( return StoreProvider(createStore) as VM } } - -/** - * Helper function for lazy creation of a [Store] instance scoped to a [ViewModelStoreOwner]. - * - * @param createStore [Store] factory receiving also the [ViewModel.viewModelScope] associated with this [ViewModel]. - * - * Example: - * ``` - * val store by lazy { scope -> - * MyStore( - * middleware = listOf( - * MyMiddleware( - * settings = requireComponents.settings, - * ... - * scope = scope, - * ), - * ) - * ) - * } - */ -@MainThread -inline fun <reified T : Store<*, *>> ViewModelStoreOwner.lazyStore( - noinline createStore: (CoroutineScope) -> T, -): Lazy<T> { - return lazy(mode = LazyThreadSafetyMode.NONE) { - StoreProvider.get(this, createStore) - } -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/gleandebugtools/GleanDebugToolsFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/gleandebugtools/GleanDebugToolsFragment.kt @@ -26,9 +26,9 @@ import androidx.core.net.toUri import androidx.fragment.app.Fragment import androidx.fragment.compose.content import androidx.navigation.fragment.findNavController +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import mozilla.telemetry.glean.Glean import org.mozilla.fenix.R -import org.mozilla.fenix.components.lazyStore import org.mozilla.fenix.debugsettings.gleandebugtools.ui.GleanDebugToolsScreen import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.theme.FirefoxTheme @@ -39,12 +39,14 @@ import mozilla.components.ui.icons.R as iconsR */ class GleanDebugToolsFragment : Fragment() { - private val store by lazyStore { + private val store by fragmentStore( + GleanDebugToolsState( + logPingsToConsoleEnabled = Glean.getLogPings(), + debugViewTag = Glean.getDebugViewTag() ?: "", + ), + ) { GleanDebugToolsStore( - initialState = GleanDebugToolsState( - logPingsToConsoleEnabled = Glean.getLogPings(), - debugViewTag = Glean.getDebugViewTag() ?: "", - ), + initialState = it, middlewares = listOf( GleanDebugToolsMiddleware( gleanDebugToolsStorage = DefaultGleanDebugToolsStorage(), diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/listscreen/DownloadFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/listscreen/DownloadFragment.kt @@ -10,11 +10,13 @@ import android.view.View import android.view.ViewGroup import androidx.fragment.app.Fragment import androidx.fragment.compose.content +import androidx.lifecycle.viewModelScope import androidx.navigation.fragment.findNavController import mozilla.components.feature.downloads.AbstractFetchDownloadService +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore +import mozilla.components.lib.state.helpers.StoreProvider.Companion.storeProvider import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.appstate.SupportedMenuNotifications -import org.mozilla.fenix.components.lazyStore import org.mozilla.fenix.compose.snackbar.Snackbar import org.mozilla.fenix.compose.snackbar.SnackbarState import org.mozilla.fenix.downloads.getCannotOpenFileErrorMessage @@ -40,11 +42,11 @@ class DownloadFragment : Fragment() { ) } - private val downloadStore by lazyStore { viewModelScope -> + private val downloadStore by fragmentStore(DownloadUIState.INITIAL) { DownloadUIStore( - initialState = DownloadUIState.INITIAL, + initialState = it, middleware = DownloadUIMiddlewareProvider.provideMiddleware( - coroutineScope = viewModelScope, + coroutineScope = storeProvider.viewModelScope, applicationContext = requireContext().applicationContext, ), ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/listscreen/store/DownloadUIStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/listscreen/store/DownloadUIStore.kt @@ -29,7 +29,7 @@ class DownloadUIStore( * The DownloadState Reducer. */ @Suppress("LongMethod") -private fun downloadStateReducer( +fun downloadStateReducer( state: DownloadUIState, action: DownloadUIAction, ): DownloadUIState { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt @@ -24,6 +24,7 @@ import androidx.lifecycle.lifecycleScope import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.navigation.fragment.findNavController import kotlinx.coroutines.launch +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import mozilla.components.service.nimbus.evalJexlSafe import mozilla.components.service.nimbus.messaging.use import mozilla.components.support.base.feature.ViewBoundFeatureWrapper @@ -35,7 +36,6 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.accounts.FenixFxAEntryPoint import org.mozilla.fenix.components.initializeGlean -import org.mozilla.fenix.components.lazyStore import org.mozilla.fenix.components.startMetricsIfEnabled import org.mozilla.fenix.compose.LinkTextState import org.mozilla.fenix.ext.components @@ -50,6 +50,7 @@ import org.mozilla.fenix.nimbus.FxNimbus import org.mozilla.fenix.onboarding.redesign.view.OnboardingScreenRedesign import org.mozilla.fenix.onboarding.store.DefaultOnboardingPreferencesRepository import org.mozilla.fenix.onboarding.store.OnboardingPreferencesMiddleware +import org.mozilla.fenix.onboarding.store.OnboardingState import org.mozilla.fenix.onboarding.store.OnboardingStore import org.mozilla.fenix.onboarding.view.Caption import org.mozilla.fenix.onboarding.view.ManagePrivacyPreferencesDialogFragment @@ -91,8 +92,9 @@ class OnboardingFragment : Fragment() { } private val telemetryRecorder by lazy { OnboardingTelemetryRecorder() } - private val onboardingStore by lazyStore { + private val onboardingStore by fragmentStore(OnboardingState()) { OnboardingStore( + initialState = it, middleware = listOf( OnboardingPreferencesMiddleware( repository = DefaultOnboardingPreferencesRepository( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/store/OnboardingStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/store/OnboardingStore.kt @@ -58,9 +58,12 @@ sealed interface OnboardingAction : Action { * A [Store] that holds the [OnboardingState] for the onboarding pages and reduces [OnboardingAction]s * dispatched to the store. */ -class OnboardingStore(middleware: List<Middleware<OnboardingState, OnboardingAction>> = emptyList()) : +class OnboardingStore( + initialState: OnboardingState = OnboardingState(), + middleware: List<Middleware<OnboardingState, OnboardingAction>> = emptyList(), +) : Store<OnboardingState, OnboardingAction>( - initialState = OnboardingState(), + initialState = initialState, reducer = ::reducer, middleware = middleware, ) { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/view/ManagePrivacyPreferencesDialogFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/view/ManagePrivacyPreferencesDialogFragment.kt @@ -6,10 +6,11 @@ package org.mozilla.fenix.onboarding.view import android.os.Bundle import android.view.LayoutInflater +import android.view.View import android.view.ViewGroup import androidx.fragment.app.DialogFragment import androidx.fragment.compose.content -import org.mozilla.fenix.components.lazyStore +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import org.mozilla.fenix.ext.settings import org.mozilla.fenix.onboarding.ManagePrivacyPreferencesDialog import org.mozilla.fenix.onboarding.store.DefaultPrivacyPreferencesRepository @@ -28,22 +29,6 @@ import org.mozilla.fenix.theme.FirefoxTheme */ class ManagePrivacyPreferencesDialogFragment : DialogFragment() { - private val store by lazyStore { - val repository = DefaultPrivacyPreferencesRepository( - settings = requireContext().settings(), - ) - PrivacyPreferencesStore( - initialState = PrivacyPreferencesState( - crashReportingEnabled = repository.getPreference(PreferenceType.CrashReporting), - usageDataEnabled = repository.getPreference(PreferenceType.UsageData), - ), - middlewares = listOf( - PrivacyPreferencesMiddleware(repository), - PrivacyPreferencesTelemetryMiddleware(), - ), - ) - } - private val crashReportingUrl by lazy { sumoUrlFor(SupportUtils.SumoTopic.CRASH_REPORTS) } private val usageDataUrl by lazy { sumoUrlFor(SupportUtils.SumoTopic.TECHNICAL_AND_INTERACTION_DATA) } @@ -51,21 +36,39 @@ class ManagePrivacyPreferencesDialogFragment : DialogFragment() { inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?, - ) = content { - FirefoxTheme { - ManagePrivacyPreferencesDialog( - store = store, - onDismissRequest = { dismiss() }, - onCrashReportingLinkClick = { - store.dispatch(PrivacyPreferencesAction.CrashReportingLearnMore) - launchSandboxCustomTab(requireContext(), crashReportingUrl) - }, - onUsageDataLinkClick = { - store.dispatch(PrivacyPreferencesAction.UsageDataUserLearnMore) - launchSandboxCustomTab(requireContext(), usageDataUrl) - }, + ): View { + val repository = DefaultPrivacyPreferencesRepository(requireContext().settings()) + val store by fragmentStore( + PrivacyPreferencesState( + crashReportingEnabled = repository.getPreference(PreferenceType.CrashReporting), + usageDataEnabled = repository.getPreference(PreferenceType.UsageData), + ), + ) { + PrivacyPreferencesStore( + initialState = it, + middlewares = listOf( + PrivacyPreferencesMiddleware(repository), + PrivacyPreferencesTelemetryMiddleware(), + ), ) } + + return content { + FirefoxTheme { + ManagePrivacyPreferencesDialog( + store = store, + onDismissRequest = { dismiss() }, + onCrashReportingLinkClick = { + store.dispatch(PrivacyPreferencesAction.CrashReportingLearnMore) + launchSandboxCustomTab(requireContext(), crashReportingUrl) + }, + onUsageDataLinkClick = { + store.dispatch(PrivacyPreferencesAction.UsageDataUserLearnMore) + launchSandboxCustomTab(requireContext(), usageDataUrl) + }, + ) + } + } } private fun sumoUrlFor(topic: SupportUtils.SumoTopic) = diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/CustomReviewPromptBottomSheetFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/CustomReviewPromptBottomSheetFragment.kt @@ -14,12 +14,14 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.fragment.compose.content import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.viewModelScope import androidx.navigation.fragment.findNavController import com.google.android.material.bottomsheet.BottomSheetDialogFragment import kotlinx.coroutines.launch import mozilla.components.lib.state.ext.observeAsState +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore +import mozilla.components.lib.state.helpers.StoreProvider.Companion.storeProvider import org.mozilla.fenix.R -import org.mozilla.fenix.components.lazyStore import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.reviewprompt.CustomReviewPromptAction.LeaveFeedbackButtonClicked import org.mozilla.fenix.reviewprompt.CustomReviewPromptAction.NegativePrePromptButtonClicked @@ -31,11 +33,11 @@ import com.google.android.material.R as materialR /** A bottom sheet fragment for displaying [CustomReviewPrompt]. */ class CustomReviewPromptBottomSheetFragment : BottomSheetDialogFragment() { - private val store by lazyStore { viewModelScope -> + private val store by fragmentStore(CustomReviewPromptState.PrePrompt) { CustomReviewPromptStore( - initialState = CustomReviewPromptState.PrePrompt, + initialState = it, middleware = listOf( - CustomReviewPromptNavigationMiddleware(viewModelScope), + CustomReviewPromptNavigationMiddleware(storeProvider.viewModelScope), CustomReviewPromptTelemetryMiddleware(), ), ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/biometric/ui/SecureScreen.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/biometric/ui/SecureScreen.kt @@ -16,15 +16,15 @@ import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ProcessLifecycleOwner import androidx.lifecycle.compose.LocalLifecycleOwner -import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner import mozilla.components.lib.state.ext.observeAsState -import org.mozilla.fenix.components.lazyStore +import mozilla.components.lib.state.helpers.StoreProvider.Companion.composableStore import org.mozilla.fenix.ext.settings import org.mozilla.fenix.settings.biometric.ui.state.BiometricAuthenticationState import org.mozilla.fenix.settings.biometric.ui.state.SecureScreenAction import org.mozilla.fenix.settings.biometric.ui.state.SecureScreenAction.AuthenticationFlowAction import org.mozilla.fenix.settings.biometric.ui.state.SecureScreenAction.LifecycleAction import org.mozilla.fenix.settings.biometric.ui.state.SecureScreenAction.UnlockScreenAction +import org.mozilla.fenix.settings.biometric.ui.state.SecureScreenState import org.mozilla.fenix.settings.biometric.ui.state.SecureScreenStore import org.mozilla.fenix.settings.logins.ui.BiometricAuthenticationDialog @@ -177,8 +177,6 @@ private fun Window.lock() = addFlags(WindowManager.LayoutParams.FLAG_SECURE) private fun Window.unlock() = clearFlags(WindowManager.LayoutParams.FLAG_SECURE) @Composable -private fun provideStore(): SecureScreenStore { - return LocalViewModelStoreOwner.current?.lazyStore { - SecureScreenStore() - }?.value ?: throw IllegalStateException("Expected LocalViewModelStoreOwner to be provided") -} +private fun provideStore() = composableStore(SecureScreenState.Initial) { + SecureScreenStore(it) +}.value diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/store/TermsOfUsePromptStore.kt @@ -79,9 +79,10 @@ sealed interface TermsOfUsePromptAction : Action { * A [Store] that holds the [TermsOfUsePromptState]. */ class TermsOfUsePromptStore( + initialState: TermsOfUsePromptState = TermsOfUsePromptState, middleware: List<Middleware<TermsOfUsePromptState, TermsOfUsePromptAction>>, ) : Store<TermsOfUsePromptState, TermsOfUsePromptAction>( - initialState = TermsOfUsePromptState, + initialState = initialState, reducer = { _, _ -> TermsOfUsePromptState }, middleware = middleware, ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/ui/TermsOfUseBottomSheetFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/termsofuse/ui/TermsOfUseBottomSheetFragment.kt @@ -13,12 +13,13 @@ import android.view.ViewGroup import androidx.compose.ui.platform.ComposeView import androidx.navigation.fragment.navArgs import com.google.android.material.bottomsheet.BottomSheetDialogFragment -import org.mozilla.fenix.components.lazyStore +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import org.mozilla.fenix.ext.settings import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.termsofuse.store.DefaultTermsOfUsePromptRepository import org.mozilla.fenix.termsofuse.store.TermsOfUsePromptAction import org.mozilla.fenix.termsofuse.store.TermsOfUsePromptPreferencesMiddleware +import org.mozilla.fenix.termsofuse.store.TermsOfUsePromptState import org.mozilla.fenix.termsofuse.store.TermsOfUsePromptStore import org.mozilla.fenix.termsofuse.store.TermsOfUsePromptTelemetryMiddleware import org.mozilla.fenix.theme.FirefoxTheme @@ -31,8 +32,9 @@ class TermsOfUseBottomSheetFragment : BottomSheetDialogFragment() { private val args by navArgs<TermsOfUseBottomSheetFragmentArgs>() - private val termsOfUsePromptStore by lazyStore { + private val termsOfUsePromptStore by fragmentStore(TermsOfUsePromptState) { TermsOfUsePromptStore( + initialState = it, middleware = listOf( TermsOfUsePromptPreferencesMiddleware( repository = DefaultTermsOfUsePromptRepository( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/ui/WebCompatReporterFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/ui/WebCompatReporterFragment.kt @@ -13,13 +13,14 @@ import androidx.fragment.compose.content import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle +import androidx.lifecycle.viewModelScope import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import kotlinx.coroutines.launch +import mozilla.components.lib.state.helpers.StoreProvider.Companion.storeProvider import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.components.lazyStore import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.theme.FirefoxTheme import org.mozilla.fenix.webcompat.WEB_COMPAT_REPORTER_SUMO_URL @@ -36,31 +37,35 @@ class WebCompatReporterFragment : Fragment() { private val args by navArgs<WebCompatReporterFragmentArgs>() - private val webCompatReporterStore by lazyStore { viewModelScope -> - WebCompatReporterStore( - initialState = WebCompatReporterState( - tabUrl = args.tabUrl, - enteredUrl = args.tabUrl, - ), - middleware = WebCompatReporterMiddlewareProvider.provideMiddleware( - browserStore = requireComponents.core.store, - appStore = requireComponents.appStore, - scope = viewModelScope, - nimbusApi = requireComponents.nimbus.sdk, - ), - ) - } + private lateinit var webCompatReporterStore: WebCompatReporterStore override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?, - ): View? = content { - FirefoxTheme { - WebCompatReporter( - store = webCompatReporterStore, + ): View { + webCompatReporterStore = storeProvider.get { restoredState -> + WebCompatReporterStore( + initialState = restoredState ?: WebCompatReporterState( + tabUrl = args.tabUrl, + enteredUrl = args.tabUrl, + ), + middleware = WebCompatReporterMiddlewareProvider.provideMiddleware( + browserStore = requireComponents.core.store, + appStore = requireComponents.appStore, + scope = storeProvider.viewModelScope, + nimbusApi = requireComponents.nimbus.sdk, + ), ) } + + return content { + FirefoxTheme { + WebCompatReporter( + store = webCompatReporterStore, + ) + } + } } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/StoreProviderTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/StoreProviderTest.kt @@ -67,30 +67,6 @@ class StoreProviderTest { } @Test - fun `WHEN store is created lazily THEN createStore is only invoked on access`() { - val fragment = createAddedTestFragment { Fragment() } - - var createCalled = false - val createStore: (CoroutineScope) -> Store<BasicState, Action> = { - createCalled = true - basicStore - } - - val store by fragment.lazyStore(createStore) - // The store is not created yet. - assertFalse(createCalled) - - assertEquals(basicStore, store) - // The store is only created when it's used. - assertTrue(createCalled) - - // The store is not created again. - createCalled = false - fragment.lazyStore(createStore).value - assertFalse(createCalled) - } - - @Test fun `GIVEN different stores are persisted WHEN requesting them THEN get their unique instances`() { val fragment = createAddedTestFragment { Fragment() } var createACalled = false