commit d62e462ba4d5bdd334ae30962ee5aa59abff9177
parent 3a00af31167ac70ce925d1d2b5b00f990398c7e8
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Wed, 3 Dec 2025 15:56:10 +0000
Bug 2003018 - Refactor Navigator and StoreLink to rely on provided CoroutineScopes r=android-reviewers,giorga
This patch refactors `Navigator` and `StoreLink` to avoid creating their own `MainScope`, instead delegating execution to a provided scope for better lifecycle management.
- In `Navigator`, replace `dispatcherScope` with `LifecycleCoroutineScope`. Use `onEach` and `launchIn` for flow collection.
- In `StoreLink`, update `start()` to accept a `CoroutineScope`. Remove the internal `MainScope` and helper methods, moving the flow observation logic to use `onEach` and `launchIn` within the provided scope.
Differential Revision: https://phabricator.services.mozilla.com/D274905
Diffstat:
5 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/FocusApplication.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/FocusApplication.kt
@@ -77,7 +77,7 @@ open class FocusApplication : LocaleAwareApplication(), Provider {
registerActivityLifecycleCallbacks(visibilityLifeCycleCallback)
registerComponentCallbacks(visibilityLifeCycleCallback)
- storeLink.start()
+ storeLink.start(applicationScope)
initializeWebExtensionSupport()
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/activity/MainActivity.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/activity/MainActivity.kt
@@ -22,6 +22,7 @@ import androidx.core.content.edit
import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen
import androidx.core.view.updateLayoutParams
import androidx.core.view.updatePadding
+import androidx.lifecycle.lifecycleScope
import androidx.preference.PreferenceManager
import mozilla.components.browser.state.selector.privateTabs
import mozilla.components.concept.engine.EngineView
@@ -77,8 +78,8 @@ open class MainActivity : EdgeToEdgeActivity() {
private val onboardingStorage by lazy { OnboardingStorage(this) }
private val navigator by lazy {
Navigator(
- components.appStore.flow(),
- MainActivityNavigation(
+ stateFlow = components.appStore.flow(),
+ navigation = MainActivityNavigation(
supportFragmentManager = supportFragmentManager,
onboardingStorage = onboardingStorage,
isInPictureInPictureMode = { isInPictureInPictureMode },
@@ -86,6 +87,8 @@ open class MainActivity : EdgeToEdgeActivity() {
showStartBrowsingCfr = ::showStartBrowsingCfr,
onEraseAction = ::reactToEraseAction,
),
+ scope = lifecycleScope,
+
)
}
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/Navigator.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/Navigator.kt
@@ -7,11 +7,11 @@ package org.mozilla.focus.navigation
import android.os.Bundle
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
-import kotlinx.coroutines.MainScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChangedBy
+import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.flow.onEach
import mozilla.components.support.base.feature.LifecycleAwareFeature
import org.mozilla.focus.settings.permissions.permissionoptions.SitePermission
import org.mozilla.focus.state.AppState
@@ -27,21 +27,22 @@ import org.mozilla.focus.state.Screen
*
* @param stateFlow A [Flow] that emits the current [AppState].
* @param navigation An implementation of [AppNavigation] that handles the actual screen transitions.
- * @param dispatcherScope The [CoroutineScope] in which the state observation will run. Defaults to [MainScope].
+ * @param scope The [CoroutineScope] in which the state observation will run.
*/
class Navigator(
private val stateFlow: Flow<AppState>,
private val navigation: AppNavigation,
- private val dispatcherScope: CoroutineScope = MainScope(),
+ private val scope: CoroutineScope,
) : LifecycleAwareFeature {
private var navigationJob: Job? = null
override fun start() {
if (navigationJob?.isActive == true) return
- navigationJob = dispatcherScope.launch {
- subscribe()
- }
+ navigationJob = stateFlow.map { state -> state.screen }
+ .distinctUntilChangedBy { screen -> screen.id }
+ .onEach { screen -> navigateTo(screen) }
+ .launchIn(scope)
}
override fun stop() {
@@ -49,12 +50,6 @@ class Navigator(
navigationJob = null
}
- private suspend fun subscribe() {
- stateFlow.map { state -> state.screen }
- .distinctUntilChangedBy { screen -> screen.id }
- .collect { screen -> navigateTo(screen) }
- }
-
private fun navigateTo(screen: Screen) {
when (screen) {
is Screen.Home -> navigation.navigateToHome()
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/StoreLink.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/StoreLink.kt
@@ -4,47 +4,60 @@
package org.mozilla.focus.navigation
-import kotlinx.coroutines.MainScope
-import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterNotNull
+import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.flow.onEach
import mozilla.components.browser.state.selector.privateTabs
-import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.lib.state.ext.flow
import org.mozilla.focus.state.AppAction
import org.mozilla.focus.state.AppStore
/**
- * Helper for subscribing to the [BrowserStore] and updating the [AppStore] for certain state changes.
+ * Helper for linking the [BrowserStore] to the [AppStore].
+ *
+ * This class subscribes to changes in the [BrowserStore] and dispatches corresponding
+ * actions to the [AppStore] to ensure the application state remains synchronized with the
+ * browser state. Specifically, it handles:
+ * - Updating the app state when the selected tab changes.
+ * - Notifying the app state when all private tabs have been closed (NoTabs action).
*/
class StoreLink(
private val appStore: AppStore,
private val browserStore: BrowserStore,
) {
- fun start() {
- MainScope().also { scope ->
- scope.launch { observeSelectionChanges(browserStore.flow()) }
- scope.launch { observeTabsClosed(browserStore.flow()) }
- }
- }
-
- private suspend fun observeSelectionChanges(flow: Flow<BrowserState>) {
- flow.map { state -> state.selectedTabId }
+ /**
+ * Starts observing changes in the [BrowserStore] and syncing them to the [AppStore].
+ *
+ * This function launches two coroutines within the provided [scope]:
+ * 1. Monitors the selected tab ID.
+ * If it changes and is not null, an [AppAction.SelectionChanged] action is dispatched.
+ * 2. Monitors the list of private tabs.
+ * If the list becomes empty, an [AppAction.NoTabs] action is dispatched.
+ *
+ * @param scope The [CoroutineScope] in which the observation flows will be launched.
+ */
+ fun start(scope: CoroutineScope) {
+ browserStore.flow()
+ .map { state -> state.selectedTabId }
.distinctUntilChanged()
.filterNotNull()
- .collect { tabId -> appStore.dispatch(AppAction.SelectionChanged(tabId)) }
- }
+ .onEach { tabId ->
+ appStore.dispatch(AppAction.SelectionChanged(tabId))
+ }
+ .launchIn(scope)
- private suspend fun observeTabsClosed(flow: Flow<BrowserState>) {
- flow.map { state -> state.privateTabs.isEmpty() }
+ browserStore.flow()
+ .map { state -> state.privateTabs.isEmpty() }
.distinctUntilChanged()
.filter { isEmpty -> isEmpty }
- .collect {
+ .onEach {
appStore.dispatch(AppAction.NoTabs)
}
+ .launchIn(scope)
}
}
diff --git a/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/navigation/NavigatorTest.kt b/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/navigation/NavigatorTest.kt
@@ -46,7 +46,7 @@ class NavigatorTest {
navigator = Navigator(
stateFlow = stateFlow,
navigation = navigation,
- dispatcherScope = navigatorTestScope,
+ scope = navigatorTestScope,
)
}
