diff --git a/mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoAutocompleteStorageDelegate.kt b/mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoAutocompleteStorageDelegate.kt index e08caf64dde0..4c7e202d522e 100644 --- a/mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoAutocompleteStorageDelegate.kt +++ b/mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoAutocompleteStorageDelegate.kt @@ -4,9 +4,8 @@ package mozilla.components.browser.engine.gecko.autofill -import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import mozilla.components.browser.engine.gecko.ext.toAddress import mozilla.components.browser.engine.gecko.ext.toAutocompleteAddress @@ -28,17 +27,20 @@ import org.mozilla.geckoview.GeckoResult * retrieving [CreditCard]s from the underlying storage. * @param loginStorageDelegate An instance of [LoginStorageDelegate]. Provides read/write methods for the [Login] * storage. + * @param applicationScope The [CoroutineScope] used for launching storage operations. These callbacks are invoked by + * Gecko and must complete even if a specific component's lifecycle has ended. Callers should pass an + * application-scoped coroutine scope when possible. */ class GeckoAutocompleteStorageDelegate( private val creditCardsAddressesStorageDelegate: CreditCardsAddressesStorageDelegate, private val loginStorageDelegate: LoginStorageDelegate, + private val applicationScope: CoroutineScope, ) : Autocomplete.StorageDelegate { override fun onAddressFetch(): GeckoResult>? { val result = GeckoResult>() - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(IO) { + applicationScope.launch(IO) { val addresses = creditCardsAddressesStorageDelegate.onAddressesFetch().map { it.toAutocompleteAddress() }.toTypedArray() @@ -51,8 +53,7 @@ class GeckoAutocompleteStorageDelegate( override fun onCreditCardFetch(): GeckoResult> { val result = GeckoResult>() - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(IO) { + applicationScope.launch(IO) { val key = creditCardsAddressesStorageDelegate.getOrGenerateKey() val creditCards = @@ -83,15 +84,13 @@ class GeckoAutocompleteStorageDelegate( } override fun onCreditCardSave(creditCard: Autocomplete.CreditCard) { - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(IO) { + applicationScope.launch(IO) { creditCardsAddressesStorageDelegate.onCreditCardSave(creditCard.toCreditCardEntry()) } } override fun onAddressSave(address: Autocomplete.Address) { - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(IO) { + applicationScope.launch(IO) { creditCardsAddressesStorageDelegate.onAddressSave(address.toAddress()) } } @@ -103,8 +102,7 @@ class GeckoAutocompleteStorageDelegate( override fun onLoginFetch(domain: String): GeckoResult> { val result = GeckoResult>() - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(IO) { + applicationScope.launch(IO) { val storedLogins = loginStorageDelegate.onLoginFetch(domain) val logins = storedLogins.await().map { it.toLoginEntry() }.toTypedArray() diff --git a/mobile/android/android-components/components/browser/icons/src/test/java/mozilla/components/browser/icons/extension/IconMessageHandlerTest.kt b/mobile/android/android-components/components/browser/icons/src/test/java/mozilla/components/browser/icons/extension/IconMessageHandlerTest.kt index 7de3a6c618a4..1bade854f524 100644 --- a/mobile/android/android-components/components/browser/icons/src/test/java/mozilla/components/browser/icons/extension/IconMessageHandlerTest.kt +++ b/mobile/android/android-components/components/browser/icons/src/test/java/mozilla/components/browser/icons/extension/IconMessageHandlerTest.kt @@ -7,8 +7,6 @@ package mozilla.components.browser.icons.extension import android.graphics.Bitmap import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlin.test.assertNotNull -import kotlinx.coroutines.DelicateCoroutinesApi -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.async import kotlinx.coroutines.test.runTest import mozilla.components.browser.icons.BrowserIcons @@ -34,13 +32,12 @@ import org.mockito.Mockito.verify @RunWith(AndroidJUnit4::class) class IconMessageHandlerTest { - @OptIn(DelicateCoroutinesApi::class) @Test fun `Complex message (TheVerge) is transformed into IconRequest and loaded`() { runTest { val bitmap: Bitmap = mock() val icon = Icon(bitmap, source = Icon.Source.DOWNLOAD) - val deferredIcon = GlobalScope.async { icon } + val deferredIcon = async { icon } val store: BrowserStore = BrowserStore(BrowserState(tabs = listOf(createTab(url = "https://www.theverge.com/", id = "test-url")))) diff --git a/mobile/android/android-components/components/browser/session-storage/build.gradle b/mobile/android/android-components/components/browser/session-storage/build.gradle index 2ac4bb3ca18a..d72427b524f0 100644 --- a/mobile/android/android-components/components/browser/session-storage/build.gradle +++ b/mobile/android/android-components/components/browser/session-storage/build.gradle @@ -53,6 +53,7 @@ dependencies { androidTestImplementation libs.androidx.test.uiautomator androidTestImplementation libs.kotlin.test androidTestImplementation libs.leakcanary.instrumentation + androidTestImplementation libs.kotlinx.coroutines.test androidTestImplementation libs.okhttp constraints { // Various AndroidX dependencies pull in 1.1.1 transitively; OkHttp 5 requires 1.2.0. diff --git a/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/FullRestoreTest.kt b/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/FullRestoreTest.kt index 89ec2f20fb5d..d522434c1c6d 100644 --- a/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/FullRestoreTest.kt +++ b/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/FullRestoreTest.kt @@ -11,6 +11,7 @@ import java.util.concurrent.TimeoutException import kotlin.test.assertNotNull import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest import mozilla.components.browser.engine.gecko.GeckoEngine import mozilla.components.browser.state.engine.EngineMiddleware import mozilla.components.browser.state.selector.selectedTab @@ -36,7 +37,7 @@ class FullRestoreTest { fun loadAndRestore() { val engine = createEngine() - run { + runTest { // ------------------------------------------------------------------------------------- // Set up // ------------------------------------------------------------------------------------- @@ -58,16 +59,16 @@ class FullRestoreTest { // Save state // ------------------------------------------------------------------------------------- - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) storage.save(store.state) } - run { + runTest { // ------------------------------------------------------------------------------------- // Restore into new classes // ------------------------------------------------------------------------------------- - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) val newStore = createStore(engine) val newUseCases = TabsUseCases(newStore) diff --git a/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/RestoringBrowsingSessionsTest.kt b/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/RestoringBrowsingSessionsTest.kt index 178d084b8795..3fdf6626aef1 100644 --- a/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/RestoringBrowsingSessionsTest.kt +++ b/mobile/android/android-components/components/browser/session-storage/src/androidTest/java/mozilla/components/browser/session/storage/RestoringBrowsingSessionsTest.kt @@ -44,7 +44,7 @@ class RestoringBrowsingSessionsTest { assertTrue(getFileForEngine(context, engine).writeString { json }) - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) val state = storage.restore() assertNotNull(state) @@ -88,7 +88,7 @@ class RestoringBrowsingSessionsTest { assertTrue(getFileForEngine(context, engine).writeString { json }) - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) val state = storage.restore() assertNotNull(state) @@ -169,7 +169,7 @@ class RestoringBrowsingSessionsTest { assertTrue(getFileForEngine(context, engine).writeString { json }) - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) val state = storage.restore() assertNotNull(state) @@ -223,7 +223,7 @@ class RestoringBrowsingSessionsTest { assertTrue(getFileForEngine(context, engine).writeString { json }) - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) val state = storage.restore() assertNotNull(state) @@ -271,7 +271,7 @@ class RestoringBrowsingSessionsTest { assertTrue(getFileForEngine(context, engine).writeString { json }) - val storage = SessionStorage(context, engine) + val storage = SessionStorage(context, engine, applicationScope = this) val state = storage.restore() assertNotNull(state) diff --git a/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/AutoSave.kt b/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/AutoSave.kt index 15115fc1c309..637989a70e51 100644 --- a/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/AutoSave.kt +++ b/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/AutoSave.kt @@ -15,10 +15,9 @@ import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ScheduledFuture import java.util.concurrent.TimeUnit +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -33,10 +32,29 @@ import mozilla.components.lib.state.ext.flow import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.utils.NamedThreadFactory +/** + * Automatically saves the current state of a [BrowserStore] to a provided [Storage] backend. + * + * This class provides configurable triggers for persisting the browser session, such as periodic foreground saving, + * saving when the application moves to the background, or saving when specific session changes occur (e.g., tabs + * added/removed or navigation completed). + * + * To prevent excessive disk I/O, it enforces a [minimumIntervalMs] between save operations. + * + * @property store The [BrowserStore] whose state should be observed and saved. + * @property sessionStorage The [Storage] implementation used to persist the state. + * @property minimumIntervalMs The minimum time in milliseconds that must pass between save operations. + * @property applicationScope The [CoroutineScope] used for performing the save operations on a background thread. This + * scope should outlive individual Activities and survive configuration changes; it is the caller's responsibility to + * cancel it when the application is destroyed. + * @property ioDispatcher The [CoroutineDispatcher] used to execute the save operations. Defaults to [Dispatchers.IO]. + */ class AutoSave( private val store: BrowserStore, private val sessionStorage: Storage, private val minimumIntervalMs: Long, + private val applicationScope: CoroutineScope, + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, ) { interface Storage { /** @@ -50,6 +68,8 @@ class AutoSave( internal val logger = Logger("SessionStorage/AutoSave") internal var saveJob: Job? = null + + @VisibleForTesting internal var monitoringJob: Job? = null private var lastSaveTimestamp: Long = now() /** @@ -83,11 +103,12 @@ class AutoSave( } /** Saves the state automatically when the sessions change, e.g. sessions get added and removed. */ - fun whenSessionsChange(scope: CoroutineScope = CoroutineScope(Dispatchers.IO)): AutoSave { - scope.launch { - val monitoring = StateMonitoring(this@AutoSave) - monitoring.monitor(store.flow()) - } + fun whenSessionsChange(scope: CoroutineScope = applicationScope): AutoSave { + monitoringJob = + scope.launch(ioDispatcher) { + val monitoring = StateMonitoring(this@AutoSave) + monitoring.monitor(store.flow()) + } return this } @@ -112,8 +133,8 @@ class AutoSave( val delayMs = lastSaveTimestamp + minimumIntervalMs - now lastSaveTimestamp = now - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(Dispatchers.IO) { + applicationScope + .launch(ioDispatcher) { if (delaySave && delayMs > 0) { logger.debug("Delaying save (${delayMs}ms)") delay(delayMs) diff --git a/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/SessionStorage.kt b/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/SessionStorage.kt index d343e0ef23ec..f06f0bd6ed84 100644 --- a/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/SessionStorage.kt +++ b/mobile/android/android-components/components/browser/session-storage/src/main/java/mozilla/components/browser/session/storage/SessionStorage.kt @@ -14,6 +14,7 @@ import androidx.core.net.toUri import java.io.File import java.util.Locale import java.util.concurrent.TimeUnit +import kotlinx.coroutines.CoroutineScope import mozilla.components.browser.session.storage.serialize.BrowserStateReader import mozilla.components.browser.session.storage.serialize.BrowserStateWriter import mozilla.components.browser.state.selector.normalTabs @@ -36,6 +37,7 @@ class SessionStorage( private val context: Context, private val engine: Engine, private val crashReporting: CrashReporting? = null, + private val applicationScope: CoroutineScope, ) : AutoSave.Storage { private val logger = Logger("SessionStorage") private val stateWriter = BrowserStateWriter() @@ -110,7 +112,7 @@ class SessionStorage( interval: Long = AutoSave.DEFAULT_INTERVAL_MILLISECONDS, unit: TimeUnit = TimeUnit.MILLISECONDS, ): AutoSave { - return AutoSave(store, this, unit.toMillis(interval)) + return AutoSave(store, this, unit.toMillis(interval), applicationScope) } } diff --git a/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/AutoSaveTest.kt b/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/AutoSaveTest.kt index 69c11489efa1..4bebdbbf66ad 100644 --- a/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/AutoSaveTest.kt +++ b/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/AutoSaveTest.kt @@ -11,7 +11,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ScheduledFuture import java.util.concurrent.TimeUnit -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.runTest @@ -41,7 +40,6 @@ import org.mockito.Mockito.`when` class AutoSaveTest { private val testDispatcher = StandardTestDispatcher() - private val scope = CoroutineScope(testDispatcher) @Test fun `AutoSave - when going to background`() { @@ -60,9 +58,13 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) .whenGoingToBackground(lifecycle) + assertNull(autoSave.saveJob) + verifyNoMoreInteractions(sessionStorage) lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_CREATE) @@ -70,13 +72,15 @@ class AutoSaveTest { lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_RESUME) lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_PAUSE) + testDispatcher.scheduler.advanceUntilIdle() + verifyNoMoreInteractions(sessionStorage) lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_STOP) - autoSave.saveJob!!.join() + testDispatcher.scheduler.advanceUntilIdle() - verify(sessionStorage).save(state) + verify(sessionStorage).save(any()) } } @@ -93,8 +97,10 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) - .whenSessionsChange(scope) + .whenSessionsChange() testDispatcher.scheduler.advanceUntilIdle() @@ -105,9 +111,8 @@ class AutoSaveTest { testDispatcher.scheduler.advanceUntilIdle() - autoSave.saveJob?.join() - verify(sessionStorage).save(any()) + autoSave.monitoringJob?.cancel() } } @@ -133,8 +138,10 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) - .whenSessionsChange(scope) + .whenSessionsChange() testDispatcher.scheduler.advanceUntilIdle() @@ -145,9 +152,8 @@ class AutoSaveTest { testDispatcher.scheduler.advanceUntilIdle() - autoSave.saveJob?.join() - verify(sessionStorage).save(any()) + autoSave.monitoringJob?.cancel() } } @@ -173,8 +179,10 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) - .whenSessionsChange(scope) + .whenSessionsChange() testDispatcher.scheduler.advanceUntilIdle() @@ -185,9 +193,8 @@ class AutoSaveTest { testDispatcher.scheduler.advanceUntilIdle() - autoSave.saveJob?.join() - verify(sessionStorage).save(any()) + autoSave.monitoringJob?.cancel() } } @@ -209,8 +216,10 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) - .whenSessionsChange(scope) + .whenSessionsChange() testDispatcher.scheduler.advanceUntilIdle() @@ -220,9 +229,8 @@ class AutoSaveTest { store.dispatch(TabListAction.RemoveTabAction("firefox")) testDispatcher.scheduler.advanceUntilIdle() - autoSave.saveJob?.join() - verify(sessionStorage).save(any()) + autoSave.monitoringJob?.cancel() } } @@ -248,8 +256,10 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) - .whenSessionsChange(scope) + .whenSessionsChange() testDispatcher.scheduler.advanceUntilIdle() @@ -259,10 +269,10 @@ class AutoSaveTest { store.dispatch(TabListAction.SelectTabAction("mozilla")) testDispatcher.scheduler.advanceUntilIdle() - - autoSave.saveJob?.join() + autoSave.saveJob?.cancel() verify(sessionStorage).save(any()) + autoSave.monitoringJob?.cancel() } } @@ -284,8 +294,10 @@ class AutoSaveTest { store = store, sessionStorage = sessionStorage, minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, ) - .whenSessionsChange(scope) + .whenSessionsChange() store.dispatch( ContentAction.UpdateLoadingStateAction( @@ -308,14 +320,13 @@ class AutoSaveTest { testDispatcher.scheduler.advanceUntilIdle() - autoSave.saveJob?.join() - verify(sessionStorage).save(any()) + autoSave.monitoringJob?.cancel() } } @Test - fun `AutoSave - periodically in foreground`() { + fun `AutoSave - periodically in foreground`() = runTest { val engine: Engine = mock() val scheduler: ScheduledExecutorService = mock() val scheduledFuture = mock(ScheduledFuture::class.java) @@ -337,13 +348,15 @@ class AutoSaveTest { val state = BrowserState() val store = BrowserStore(state) - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) storage.autoSave(store).periodicallyInForeground(300, TimeUnit.SECONDS, scheduler, lifecycle) verifyNoMoreInteractions(scheduler) lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_START) + testDispatcher.scheduler.advanceUntilIdle() + verify(scheduler) .scheduleAtFixedRate( any(), @@ -356,46 +369,54 @@ class AutoSaveTest { lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_STOP) + testDispatcher.scheduler.advanceUntilIdle() + verify(scheduledFuture).cancel(false) } @Test - fun `AutoSave - No new job triggered while save in flight`() { - val sessionStorage: SessionStorage = mock() + fun `AutoSave - No new job triggered while save in flight`() = + runTest(testDispatcher) { + val sessionStorage: SessionStorage = mock() - val state = BrowserState() - val store = BrowserStore(state) - val autoSave = - AutoSave( - store = store, - sessionStorage = sessionStorage, - minimumIntervalMs = 0, - ) + val state = BrowserState() + val store = BrowserStore(state) + val autoSave = + AutoSave( + store = store, + sessionStorage = sessionStorage, + minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, + ) - val runningJob: Job = mock() - doReturn(true).`when`(runningJob).isActive + val runningJob: Job = mock() + doReturn(true).`when`(runningJob).isActive - val saveJob = autoSave.triggerSave() - assertSame(saveJob, saveJob) - } + val saveJob = autoSave.triggerSave() + assertSame(saveJob, saveJob) + } @Test - fun `AutoSave - New job triggered if current job is done`() { - val sessionStorage: SessionStorage = mock() + fun `AutoSave - New job triggered if current job is done`() = + runTest(testDispatcher) { + val sessionStorage: SessionStorage = mock() - val state = BrowserState() - val store = BrowserStore(state) - val autoSave = - AutoSave( - store = store, - sessionStorage = sessionStorage, - minimumIntervalMs = 0, - ) + val state = BrowserState() + val store = BrowserStore(state) + val autoSave = + AutoSave( + store = store, + sessionStorage = sessionStorage, + minimumIntervalMs = 0, + applicationScope = this, + ioDispatcher = testDispatcher, + ) - val completed: Job = mock() - doReturn(false).`when`(completed).isActive + val completed: Job = mock() + doReturn(false).`when`(completed).isActive - val saveJob = autoSave.triggerSave() - assertNotSame(completed, saveJob) - } + val saveJob = autoSave.triggerSave() + assertNotSame(completed, saveJob) + } } diff --git a/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/SessionStorageTest.kt b/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/SessionStorageTest.kt index a9360f650472..76da1b7e9fff 100644 --- a/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/SessionStorageTest.kt +++ b/mobile/android/android-components/components/browser/session-storage/src/test/java/mozilla/components/browser/session/storage/SessionStorageTest.kt @@ -7,6 +7,7 @@ package mozilla.components.browser.session.storage import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlin.test.assertNotNull +import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.ext.getUrl import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.EngineState @@ -31,7 +32,7 @@ import org.mockito.Mockito.verify @RunWith(AndroidJUnit4::class) class SessionStorageTest { @Test - fun `Restored browser state should contain tabs of saved state`() { + fun `Restored browser state should contain tabs of saved state`() = runTest { // Build the state val engineSessionState1 = FakeEngineSessionState("engineState1") @@ -52,7 +53,7 @@ class SessionStorageTest { val engine = FakeEngine() - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val persisted = storage.save(state) assertTrue(persisted) @@ -70,7 +71,7 @@ class SessionStorageTest { } @Test - fun `Predicate is applied when restoring browser state`() { + fun `Predicate is applied when restoring browser state`() = runTest { // Build the state val engineSessionState1 = FakeEngineSessionState("engineState1") @@ -98,7 +99,7 @@ class SessionStorageTest { val engine = FakeEngine() - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val persisted = storage.save(state) assertTrue(persisted) @@ -119,7 +120,7 @@ class SessionStorageTest { } @Test - fun `Tabs with unreadable URI are not restored when restoring browser state`() { + fun `Tabs with unreadable URI are not restored when restoring browser state`() = runTest { // Build the state val engineSessionState1 = FakeEngineSessionState("engineState1") @@ -140,7 +141,7 @@ class SessionStorageTest { val engine = FakeEngine() val context = testContext - val storage = spy(SessionStorage(context, engine)) + val storage = spy(SessionStorage(context, engine, applicationScope = this)) doReturn(true).`when`(storage).isUriReadable(tab1.getUrl()!!.toUri()) doReturn(false).`when`(storage).isUriReadable(tab2.getUrl()!!.toUri()) val persisted = storage.save(state) @@ -160,10 +161,10 @@ class SessionStorageTest { } @Test - fun `Saving empty state`() { + fun `Saving empty state`() = runTest { val engine = FakeEngine() - val storage = spy(SessionStorage(testContext, engine)) + val storage = spy(SessionStorage(testContext, engine, applicationScope = this)) storage.save(BrowserState()) verify(storage).clear() @@ -172,7 +173,7 @@ class SessionStorageTest { } @Test - fun `Should return empty browser state after clearing`() { + fun `Should return empty browser state after clearing`() = runTest { val engine = FakeEngine() val tab1 = createTab("https://www.mozilla.org", id = "tab1") @@ -186,7 +187,7 @@ class SessionStorageTest { // Persist the state - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val persisted = storage.save(state) assertTrue(persisted) @@ -207,7 +208,7 @@ class SessionStorageTest { * test input to make the test pass since such an input does exist on actual devices too. */ @Test - fun deserializeVersion2BrowsingSessionLegacyOrgJson() { + fun deserializeVersion2BrowsingSessionLegacyOrgJson() = runTest { // Do not change this string! (See comment above) val json = """ @@ -219,7 +220,7 @@ class SessionStorageTest { assertTrue(getFileForEngine(testContext, engine).writeString { json }) - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val browsingSession = storage.restore() assertNotNull(browsingSession) @@ -258,7 +259,7 @@ class SessionStorageTest { * test input to make the test pass since such an input does exist on actual devices too. */ @Test - fun deserializeVersion2BrowsingSessionJsonWriter() { + fun deserializeVersion2BrowsingSessionJsonWriter() = runTest { // Do not change this string! (See comment above) val json = """ @@ -270,7 +271,7 @@ class SessionStorageTest { assertTrue(getFileForEngine(testContext, engine).writeString { json }) - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val browsingSession = storage.restore() assertNotNull(browsingSession) @@ -308,7 +309,7 @@ class SessionStorageTest { } @Test - fun `Restored browsing session contains all expected session properties`() { + fun `Restored browsing session contains all expected session properties`() = runTest { val firstTab = createTab( id = "first-tab", @@ -346,7 +347,7 @@ class SessionStorageTest { val engine = FakeEngine() - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val persisted = storage.save(state) assertTrue(persisted) @@ -380,7 +381,7 @@ class SessionStorageTest { } @Test - fun `Saving state with selected tab id for a tab that does not exist`() { + fun `Saving state with selected tab id for a tab that does not exist`() = runTest { val state = BrowserState( tabs = @@ -393,7 +394,7 @@ class SessionStorageTest { val engine = FakeEngine() - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val persisted = storage.save(state) assertTrue(persisted) @@ -411,7 +412,7 @@ class SessionStorageTest { } @Test - fun `WHEN saving state with crash parent tab THEN don't save tab`() { + fun `WHEN saving state with crash parent tab THEN don't save tab`() = runTest { val state = BrowserState( tabs = @@ -424,7 +425,7 @@ class SessionStorageTest { val engine = FakeEngine() - val storage = SessionStorage(testContext, engine) + val storage = SessionStorage(testContext, engine, applicationScope = this) val persisted = storage.save(state) assertTrue(persisted) diff --git a/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt b/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt index 058aac868da6..5381243b4c79 100644 --- a/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt +++ b/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt @@ -9,11 +9,10 @@ import androidx.annotation.VisibleForTesting import androidx.core.content.edit import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ProcessLifecycleOwner -import kotlin.coroutines.CoroutineContext +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.Job import kotlinx.coroutines.launch import mozilla.components.concept.base.crash.Breadcrumb import mozilla.components.concept.base.crash.CrashReporting @@ -48,9 +47,11 @@ internal const val PREF_FXA_SCOPE = "fxa_push_scope" * @param context The application Android context. * @param accountManager The FxaAccountManager. * @param pushFeature The [AutoPushFeature] if that is setup for observing push events. + * @param applicationScope The [CoroutineScope] used to launch long-running asynchronous operations, such as account + * registration, device constellation updates, and processing push events, ensuring they continue even if the UI + * changes. * @param crashReporter Instance of `CrashReporting` to record unexpected caught exceptions. - * @param coroutineScope The scope in which IO work within the feature should be performed on. - * @param uiContext The context on which UI-related operations should be performed. Defaults to [Dispatchers.Main]. + * @param ioDispatcher The dispatcher used for IO work within the feature. * @param owner the lifecycle owner for the observer. Defaults to [ProcessLifecycleOwner]. * @param autoPause whether to stop notifying the observer during onPause lifecycle events. Defaults to false so that * observers are always notified. @@ -59,9 +60,9 @@ class FxaPushSupportFeature( private val context: Context, private val accountManager: FxaAccountManager, private val pushFeature: AutoPushFeature, + private val applicationScope: CoroutineScope, private val crashReporter: CrashReporting? = null, - private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.IO), - private val uiContext: CoroutineContext = Dispatchers.Main, + ioDispatcher: CoroutineDispatcher = Dispatchers.IO, private val owner: LifecycleOwner = ProcessLifecycleOwner.get(), private val autoPause: Boolean = false, ) { @@ -72,29 +73,39 @@ class FxaPushSupportFeature( * This scope is randomly generated and unique to the app install. (why this uuid? Note it is *not* reset on * logout!) */ - private val pushScope = PushScopeProperty(context, coroutineScope) + private val pushScope = PushScopeProperty(context, ioDispatcher) + + private var initializeJob: Job? = null /** Initialize the support feature to launch the appropriate observers. */ - fun initialize() = coroutineScope.launch { - val scopeValue = pushScope.value() + fun initialize() { + synchronized(this) { + if (initializeJob?.isActive == true) return - val autoPushObserver = AutoPushObserver(accountManager, pushFeature, scopeValue, uiContext) + initializeJob = applicationScope.launch { + val scopeValue = pushScope.value() - val accountObserver = - AccountObserver( - context, - pushFeature, - scopeValue, - crashReporter, - owner, - uiContext, - autoPause, - ) + val autoPushObserver = + AutoPushObserver( + accountManager, + pushFeature, + scopeValue, + applicationScope, + ) + val accountObserver = + AccountObserver( + context, + pushFeature, + scopeValue, + crashReporter, + owner, + applicationScope, + autoPause, + ) - coroutineScope.launch(uiContext) { - accountManager.register(accountObserver) - - pushFeature.register(autoPushObserver, owner, autoPause) + accountManager.register(accountObserver) + pushFeature.register(autoPushObserver, owner, autoPause) + } } } @@ -113,14 +124,13 @@ internal class AccountObserver( private val fxaPushScope: String, private val crashReporter: CrashReporting?, private val lifecycleOwner: LifecycleOwner, - private val uiContext: CoroutineContext = Dispatchers.Main, - private val autoPause: Boolean, + private val applicationScope: CoroutineScope, + private val autoPause: Boolean = false, ) : SyncAccountObserver { private val logger = Logger(AccountObserver::class.java.simpleName) private val verificationDelegate = VerificationDelegate(context, push.config.disableRateLimit) - @OptIn(DelicateCoroutinesApi::class) // GlobalScope usage override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { val constellationObserver = ConstellationObserver( @@ -130,13 +140,14 @@ internal class AccountObserver( account = account, verifier = verificationDelegate, crashReporter = crashReporter, + applicationScope = applicationScope, ) // NB: can we just expose registerDeviceObserver on account manager? // registration could happen after onDevicesUpdate has been called, without having to tie this // into the account "auth lifecycle". // See https://github.com/mozilla-mobile/android-components/issues/8766 - GlobalScope.launch(uiContext) { + applicationScope.launch { account.deviceConstellation().registerDeviceObserver(constellationObserver, lifecycleOwner, autoPause) account.deviceConstellation().refreshDevices() } @@ -160,9 +171,9 @@ internal fun pushSubscribe( push: AutoPushFeature, account: OAuthAccount, scope: String, - uiContext: CoroutineContext, crashReporter: CrashReporting?, logContext: String, + applicationScope: CoroutineScope, ) { val logger = Logger("FxaPushSupportFeature") val currentDevice = account.deviceConstellation().state()?.currentDevice @@ -185,7 +196,7 @@ internal fun pushSubscribe( // subscription matches, just to ensure `subscriptionExpired` is reset. if (currentDevice.subscriptionExpired || currentDevice.subscription?.endpoint != subscription.endpoint) { logger.info("Updating account with new subscription info.") - CoroutineScope(uiContext).launch { + applicationScope.launch { account.deviceConstellation().setDevicePushSubscription(subscription.into()) } } @@ -204,7 +215,7 @@ internal class ConstellationObserver( private val account: OAuthAccount, private val verifier: VerificationDelegate = VerificationDelegate(context), private val crashReporter: CrashReporting?, - private val uiContext: CoroutineContext = Dispatchers.Main, + private val applicationScope: CoroutineScope, ) : DeviceConstellationObserver { private val logger = Logger(ConstellationObserver::class.java.simpleName) @@ -235,7 +246,7 @@ internal class ConstellationObserver( // And unconditionally subscribe - if our local DB already has a subscription it will // be returned without hitting the server. If some other problem meant our subscription // was dropped or never made, it will hit the server and deliver a new end-point. - pushSubscribe(push, account, scope, uiContext, crashReporter, "onDevicesUpdate") + pushSubscribe(push, account, scope, crashReporter, "onDevicesUpdate", applicationScope) } } @@ -244,7 +255,7 @@ internal class AutoPushObserver( private val accountManager: FxaAccountManager, private val pushFeature: AutoPushFeature, private val fxaPushScope: String, - private val uiContext: CoroutineContext, + private val applicationScope: CoroutineScope, ) : AutoPushFeature.Observer { private val logger = Logger(AutoPushObserver::class.java.simpleName) @@ -259,7 +270,7 @@ internal class AutoPushObserver( val rawEvent = message ?: return accountManager.withConstellationIfExists { - CoroutineScope(uiContext).launch { + applicationScope.launch { processRawEvent(String(rawEvent)) } } @@ -277,7 +288,7 @@ internal class AutoPushObserver( logger.info("We don't have any account to pass the push subscription to.") return } - pushSubscribe(pushFeature, account, fxaPushScope, uiContext, null, "onSubscriptionChanged") + pushSubscribe(pushFeature, account, fxaPushScope, null, "onSubscriptionChanged", applicationScope) } } diff --git a/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/cache/PushScopeProperty.kt b/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/cache/PushScopeProperty.kt index 53fb0bcf56b5..f0daa1c9102b 100644 --- a/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/cache/PushScopeProperty.kt +++ b/mobile/android/android-components/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/cache/PushScopeProperty.kt @@ -8,7 +8,7 @@ import android.content.Context import android.content.SharedPreferences import androidx.core.content.edit import java.util.UUID -import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import mozilla.components.feature.accounts.push.FxaPushSupportFeature import mozilla.components.feature.accounts.push.PREF_FXA_SCOPE @@ -18,11 +18,11 @@ import mozilla.components.feature.push.PushScope /** An implementation of a [ScopeProperty] that generates and stores a scope in [SharedPreferences]. */ internal class PushScopeProperty( private val context: Context, - private val coroutineScope: CoroutineScope, + private val dispatcher: CoroutineDispatcher, ) : ScopeProperty { override suspend fun value(): PushScope = - withContext(coroutineScope.coroutineContext) { + withContext(dispatcher) { val prefs = preference(context) // Generate a unique scope if one doesn't exist. diff --git a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AccountObserverTest.kt b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AccountObserverTest.kt index 308843778da6..1f596e51befb 100644 --- a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AccountObserverTest.kt +++ b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AccountObserverTest.kt @@ -59,7 +59,7 @@ class AccountObserverTest { pushScope, crashReporter, lifecycleOwner, - coroutineContext, + this, false, ) `when`(lifecycle.currentState).thenReturn(Lifecycle.State.STARTED) @@ -87,7 +87,7 @@ class AccountObserverTest { pushScope, crashReporter, mock(), - coroutineContext, + this, false, ) @@ -114,7 +114,7 @@ class AccountObserverTest { pushScope, crashReporter, mock(), - coroutineContext, + this, false, ) @@ -146,7 +146,7 @@ class AccountObserverTest { pushScope, crashReporter, mock(), - coroutineContext, + this, false, ) @@ -164,7 +164,7 @@ class AccountObserverTest { pushScope, crashReporter, mock(), - coroutineContext, + this, false, ) diff --git a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AutoPushObserverTest.kt b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AutoPushObserverTest.kt index b792c383f469..a5df3e6c4779 100644 --- a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AutoPushObserverTest.kt +++ b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/AutoPushObserverTest.kt @@ -31,7 +31,7 @@ class AutoPushObserverTest { @Test fun `messages are forwarded to account manager`() = runTest { - val observer = AutoPushObserver(manager, mock(), "test", coroutineContext) + val observer = AutoPushObserver(manager, mock(), "test", this) `when`(manager.authenticatedAccount()).thenReturn(account) `when`(account.deviceConstellation()).thenReturn(constellation) @@ -44,7 +44,7 @@ class AutoPushObserverTest { @Test fun `account manager is not invoked if no account is available`() = runTest { - val observer = AutoPushObserver(manager, mock(), "test", coroutineContext) + val observer = AutoPushObserver(manager, mock(), "test", this) observer.onMessageReceived("test", "foobar".toByteArray()) testScheduler.advanceUntilIdle() @@ -55,7 +55,7 @@ class AutoPushObserverTest { @Test fun `messages are not forwarded to account manager if they are for a different scope`() = runTest { - val observer = AutoPushObserver(manager, mock(), "fake", coroutineContext) + val observer = AutoPushObserver(manager, mock(), "fake", this) observer.onMessageReceived("test", "foobar".toByteArray()) testScheduler.advanceUntilIdle() @@ -65,7 +65,7 @@ class AutoPushObserverTest { @Test fun `subscription changes are forwarded to account manager`() = runTest { - val observer = AutoPushObserver(manager, pushFeature, "test", coroutineContext) + val observer = AutoPushObserver(manager, pushFeature, "test", this) whenSubscribe() @@ -85,7 +85,7 @@ class AutoPushObserverTest { @Test fun `do nothing if there is no account manager`() = runTest { - val observer = AutoPushObserver(manager, pushFeature, "test", coroutineContext) + val observer = AutoPushObserver(manager, pushFeature, "test", this) whenSubscribe() @@ -97,7 +97,7 @@ class AutoPushObserverTest { @Test fun `subscription changes are not forwarded to account manager if they are for a different scope`() = runTest { - val observer = AutoPushObserver(manager, mock(), "fake", coroutineContext) + val observer = AutoPushObserver(manager, mock(), "fake", this) `when`(manager.authenticatedAccount()).thenReturn(account) `when`(account.deviceConstellation()).thenReturn(constellation) diff --git a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt index f4d707b63fd7..a7583320eb02 100644 --- a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt +++ b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt @@ -101,8 +101,8 @@ class ConstellationObserverTest { } @Test - fun `notify crash reporter if subscribe error occurs`() { - val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter) + fun `notify crash reporter if subscribe error occurs`() = runTest { + val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter, this) whenSubscribeError() observer.onDevicesUpdate(state) @@ -111,8 +111,8 @@ class ConstellationObserverTest { } @Test - fun `no FCM renewal if verifier is false`() { - val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter) + fun `no FCM renewal if verifier is false`() = runTest { + val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter, this) verifyNoInteractions(push) @@ -128,8 +128,8 @@ class ConstellationObserverTest { } @Test - fun `invoke registration renewal`() { - val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter) + fun `invoke registration renewal`() = runTest { + val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter, this) `when`(device.subscriptionExpired).thenReturn(true) `when`(verifier.allowedToRenew()).thenReturn(true) @@ -180,7 +180,7 @@ class ConstellationObserverTest { account = account, verifier = verifier, crashReporter = crashReporter, - uiContext = coroutineContext, + applicationScope = this, ) } } diff --git a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/FxaPushSupportFeatureTest.kt b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/FxaPushSupportFeatureTest.kt index ec77a0d75f1f..ed083b014677 100644 --- a/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/FxaPushSupportFeatureTest.kt +++ b/mobile/android/android-components/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/FxaPushSupportFeatureTest.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.accounts.push +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import mozilla.components.feature.accounts.push.FxaPushSupportFeature.Companion.PUSH_SCOPE_PREFIX @@ -76,9 +77,8 @@ class FxaPushSupportFeatureTest { context = testContext, accountManager = accountManager, pushFeature = pushFeature, - crashReporter = null, - coroutineScope = this, - uiContext = this.coroutineContext, + applicationScope = this, + ioDispatcher = StandardTestDispatcher(testScheduler), ) } } diff --git a/mobile/android/android-components/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AbstractAutofillService.kt b/mobile/android/android-components/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AbstractAutofillService.kt index f1654f088650..31555de9a7f1 100644 --- a/mobile/android/android-components/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AbstractAutofillService.kt +++ b/mobile/android/android-components/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AbstractAutofillService.kt @@ -12,9 +12,8 @@ import android.service.autofill.FillRequest import android.service.autofill.SaveCallback import android.service.autofill.SaveRequest import android.widget.inline.InlinePresentationSpec -import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import mozilla.components.feature.autofill.handler.FillRequestHandler import mozilla.components.feature.autofill.handler.MAX_LOGINS @@ -23,6 +22,7 @@ import mozilla.components.feature.autofill.structure.toRawStructure /** Service responsible for implementing Android's Autofill framework. */ abstract class AbstractAutofillService : AutofillService() { abstract val configuration: AutofillConfiguration + abstract val applicationScope: CoroutineScope private val fillHandler by lazy { FillRequestHandler(context = this, configuration) } @@ -31,11 +31,7 @@ abstract class AbstractAutofillService : AutofillService() { cancellationSignal: CancellationSignal, callback: FillCallback, ) { - // We are using GlobalScope here instead of a scope bound to the service since the service - // seems to get destroyed before we invoke a method on the callback. So we need a scope that - // lives longer than the service. - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(Dispatchers.IO) { + applicationScope.launch(Dispatchers.IO) { // You may be wondering why we translate the AssistStructure into a RawStructure and then // create a FillResponseBuilder that outputs the FillResponse. This is purely for testing. // Neither AssistStructure nor FillResponse can be created by us and they do not let us diff --git a/mobile/android/android-components/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt b/mobile/android/android-components/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt index 6b5e9257419c..2e60efa0db3e 100644 --- a/mobile/android/android-components/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt +++ b/mobile/android/android-components/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt @@ -18,9 +18,8 @@ import java.net.InetAddress import java.net.MalformedURLException import java.net.URL import java.net.UnknownHostException -import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.externalPackage @@ -42,12 +41,14 @@ import mozilla.components.support.utils.WebURLFinder * @property newTabSearchUseCase A reference to [SearchUseCases.NewTabSearchUseCase] to be used for ACTION_SEND intents * if the provided text is not a URL. * @property isPrivate Whether a processed intent should open a new tab as private + * @property applicationScope A [CoroutineScope] tied to the lifetime of the application process. */ class TabIntentProcessor( private val tabsUseCases: TabsUseCases, private val newTabSearchUseCase: SearchUseCases.NewTabSearchUseCase, private val isPrivate: Boolean = false, private val engine: Engine? = null, + private val applicationScope: CoroutineScope, ) : IntentProcessor { private val logger = Logger("TabIntentProcessor") @@ -85,9 +86,11 @@ class TabIntentProcessor( } } - @OptIn(DelicateCoroutinesApi::class) // GlobalScope usage for DNS warmup in the background + // applicationScope is used here: DNS warmup is a fire-and-forget background task + // with no result and no cleanup needed. It should continue even if the Activity that + // triggered it is destroyed, and the short-lived lookup does not cause a memory leak. private fun warmupNativeDNS(normalizedUrl: String) { - GlobalScope.launch(IO) { + applicationScope.launch(IO) { try { val hostToWarmup = getHostForDnsWarmup(normalizedUrl) if (hostToWarmup != null) { diff --git a/mobile/android/android-components/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt b/mobile/android/android-components/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt index 2a1d2b605f2a..0cca0dfd7ebe 100644 --- a/mobile/android/android-components/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt +++ b/mobile/android/android-components/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt @@ -95,7 +95,7 @@ class TabIntentProcessorTest { @Test fun `open or select tab on ACTION_VIEW intent`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_VIEW) whenever(intent.dataString).thenReturn("http://mozilla.org") @@ -136,7 +136,7 @@ class TabIntentProcessorTest { @Test fun `open or select tab on ACTION_MAIN intent`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_MAIN) whenever(intent.dataString).thenReturn("https://mozilla.org") @@ -168,7 +168,7 @@ class TabIntentProcessorTest { @Test fun `open or select tab on ACTION_NDEF_DISCOVERED intent`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(ACTION_NDEF_DISCOVERED) whenever(intent.dataString).thenReturn("https://mozilla.org") @@ -199,7 +199,7 @@ class TabIntentProcessorTest { @Test fun `open tab on ACTION_SEND intent`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEND) @@ -245,7 +245,7 @@ class TabIntentProcessorTest { @Test fun `open tab and trigger search on ACTION_SEND if text is not a URL`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val searchTerms = "mozilla android" val searchUrl = "https://localhost/?q=mozilla%20android" @@ -265,7 +265,7 @@ class TabIntentProcessorTest { @Test fun `nothing happens on ACTION_SEND if no text is provided`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEND) @@ -277,7 +277,7 @@ class TabIntentProcessorTest { @Test fun `nothing happens on ACTION_SEARCH if text is empty`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEARCH) @@ -289,7 +289,7 @@ class TabIntentProcessorTest { @Test fun `open tab on ACTION_SEARCH intent`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEARCH) @@ -313,7 +313,7 @@ class TabIntentProcessorTest { @Test fun `open tab and trigger search on ACTION_SEARCH intent if text is not a URL`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val searchTerms = "mozilla android" val searchUrl = "https://localhost/?q=mozilla%20android" @@ -333,7 +333,7 @@ class TabIntentProcessorTest { @Test fun `nothing happens on ACTION_WEB_SEARCH if text is empty`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_WEB_SEARCH) @@ -345,7 +345,7 @@ class TabIntentProcessorTest { @Test fun `open tab on ACTION_WEB_SEARCH intent`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_WEB_SEARCH) @@ -368,7 +368,7 @@ class TabIntentProcessorTest { @Test fun `open tab and trigger search on ACTION_WEB_SEARCH intent if text is not a URL`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val searchTerms = "mozilla android" val searchUrl = "https://localhost/?q=mozilla%20android" @@ -387,7 +387,7 @@ class TabIntentProcessorTest { @Test fun `returns external flags when no intent extra for app link launch type extra present`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() val safeIntent = SafeIntent(intent) @@ -398,7 +398,7 @@ class TabIntentProcessorTest { @Test fun `uses app link launch type from intent when the extra is present`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent = Intent().apply { @@ -415,7 +415,7 @@ class TabIntentProcessorTest { @Test fun `uses the default unknown app link launch type when an invalid extra value is present`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent = Intent().apply { @@ -432,7 +432,7 @@ class TabIntentProcessorTest { @Test fun `does not use the default when the extra is present but an invalid integer value`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent = Intent().apply { @@ -451,7 +451,7 @@ class TabIntentProcessorTest { @Test fun `process intent sets app link intent launch type value`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_VIEW) @@ -470,7 +470,7 @@ class TabIntentProcessorTest { @Test fun `process intent sets does not app link intent launch type value when there's no extra`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_VIEW) @@ -486,7 +486,7 @@ class TabIntentProcessorTest { @Test fun `process intent sets default app link intent launch type value when there's no valid value for type`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, applicationScope = scope) val intent: Intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_VIEW) @@ -504,7 +504,13 @@ class TabIntentProcessorTest { fun `getHostForDnsWarmup returns app link host when DoH is disabled`() { val settings = DefaultSettings(dohSettingsMode = Engine.DohSettingsMode.DEFAULT) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") @@ -515,7 +521,13 @@ class TabIntentProcessorTest { fun `getHostForDnsWarmup returns app link host when DoH is OFF`() { val settings = DefaultSettings(dohSettingsMode = Engine.DohSettingsMode.OFF) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") @@ -530,7 +542,13 @@ class TabIntentProcessorTest { dohProviderUrl = "https://cloudflare-dns.com/dns-query", ) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") @@ -545,7 +563,13 @@ class TabIntentProcessorTest { dohProviderUrl = "https://dns.nextdns.io/abc123", ) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") @@ -560,7 +584,13 @@ class TabIntentProcessorTest { dohProviderUrl = "", ) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") @@ -569,7 +599,13 @@ class TabIntentProcessorTest { @Test fun `getHostForDnsWarmup returns app link host when engine is null`() { - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = null) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = null, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") @@ -580,7 +616,13 @@ class TabIntentProcessorTest { fun `getHostForDnsWarmup returns null for malformed app link URL`() { val settings = DefaultSettings(dohSettingsMode = Engine.DohSettingsMode.OFF) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("not a valid url") @@ -595,7 +637,13 @@ class TabIntentProcessorTest { dohProviderUrl = "not a valid url", ) whenever(engine.settings).thenReturn(settings) - val handler = TabIntentProcessor(TabsUseCases(store), searchUseCases.newTabSearch, engine = engine) + val handler = + TabIntentProcessor( + TabsUseCases(store), + searchUseCases.newTabSearch, + engine = engine, + applicationScope = scope, + ) val result = handler.getHostForDnsWarmup("https://mozilla.org/path") diff --git a/mobile/android/android-components/components/feature/prompts/build.gradle b/mobile/android/android-components/components/feature/prompts/build.gradle index eb51bb0b8447..8cf8698dde5a 100644 --- a/mobile/android/android-components/components/feature/prompts/build.gradle +++ b/mobile/android/android-components/components/feature/prompts/build.gradle @@ -57,6 +57,7 @@ dependencies { androidTestImplementation project(':components:support-android-test') androidTestImplementation libs.androidx.test.core androidTestImplementation libs.androidx.test.runner + androidTestImplementation libs.kotlinx.coroutines.test androidTestRuntimeOnly libs.okhttp } diff --git a/mobile/android/android-components/components/feature/prompts/src/androidTest/java/mozilla/components/feature/prompts/file/OnDeviceFilePickerTest.kt b/mobile/android/android-components/components/feature/prompts/src/androidTest/java/mozilla/components/feature/prompts/file/OnDeviceFilePickerTest.kt index 83220e5b9707..62c7ecefa855 100644 --- a/mobile/android/android-components/components/feature/prompts/src/androidTest/java/mozilla/components/feature/prompts/file/OnDeviceFilePickerTest.kt +++ b/mobile/android/android-components/components/feature/prompts/src/androidTest/java/mozilla/components/feature/prompts/file/OnDeviceFilePickerTest.kt @@ -9,6 +9,7 @@ import android.content.Context import android.content.Intent import androidx.core.net.toUri import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.prompt.PromptRequest import mozilla.components.feature.prompts.PromptContainer @@ -36,9 +37,9 @@ class OnDeviceFilePickerTest { } @Test - fun unsafeUrisWillNotBeSelected() { + fun unsafeUrisWillNotBeSelected() = runTest { val promptContainer = PromptContainer.TestPromptContainer(context) - val fileUploadsDirCleaner = FileUploadsDirCleaner { context.cacheDir } + val fileUploadsDirCleaner = FileUploadsDirCleaner(this) { context.cacheDir } val filePicker = FilePicker( container = promptContainer, @@ -68,9 +69,9 @@ class OnDeviceFilePickerTest { } @Test - fun safeUrisWillBeSelected() { + fun safeUrisWillBeSelected() = runTest { val promptContainer = PromptContainer.TestPromptContainer(context) - val fileUploadsDirCleaner = FileUploadsDirCleaner { context.cacheDir } + val fileUploadsDirCleaner = FileUploadsDirCleaner(this) { context.cacheDir } val filePicker = FilePicker( container = promptContainer, @@ -100,9 +101,9 @@ class OnDeviceFilePickerTest { } @Test - fun unsafeUriWillNotBeSelected() { + fun unsafeUriWillNotBeSelected() = runTest { val promptContainer = PromptContainer.TestPromptContainer(context) - val fileUploadsDirCleaner = FileUploadsDirCleaner { context.cacheDir } + val fileUploadsDirCleaner = FileUploadsDirCleaner(this) { context.cacheDir } val filePicker = FilePicker( container = promptContainer, @@ -131,9 +132,9 @@ class OnDeviceFilePickerTest { } @Test - fun safeUriWillBeSelected() { + fun safeUriWillBeSelected() = runTest { val promptContainer = PromptContainer.TestPromptContainer(context) - val fileUploadsDirCleaner = FileUploadsDirCleaner { context.cacheDir } + val fileUploadsDirCleaner = FileUploadsDirCleaner(this) { context.cacheDir } val filePicker = FilePicker( container = promptContainer, diff --git a/mobile/android/android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/file/FileUploadsDirCleaner.kt b/mobile/android/android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/file/FileUploadsDirCleaner.kt index 7a695d374eb5..2e39a655bf30 100644 --- a/mobile/android/android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/file/FileUploadsDirCleaner.kt +++ b/mobile/android/android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/file/FileUploadsDirCleaner.kt @@ -9,18 +9,23 @@ import java.io.File import java.io.IOException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import mozilla.components.concept.engine.prompt.PromptRequest.File.Companion.DEFAULT_UPLOADS_DIR_NAME import mozilla.components.support.base.log.logger.Logger -/** A storage implementation for organizing temporal uploads metadata to be clean up. */ -@OptIn(DelicateCoroutinesApi::class) +/** + * A storage implementation for organizing temporal uploads metadata to be clean up. * + * + * @param scope The [CoroutineScope] used for launching cleanup operations. Callers should pass an application-scoped + * scope so cleanup is not cancelled when the UI component is destroyed. The application-lifetime scope is appropriate + * here since cleanup tasks should outlive individual Activities. + * @param ioDispatcher The dispatcher used for I/O operations. + * @param cacheDirectory Provider for the cache directory used to store temporary uploads. + */ class FileUploadsDirCleaner( - private val scope: CoroutineScope = GlobalScope, + private val scope: CoroutineScope, private val ioDispatcher: CoroutineDispatcher = IO, private val cacheDirectory: () -> File, ) { diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/region/RegionMiddleware.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/region/RegionMiddleware.kt index 7f41777a9dc1..0ed5b0ace0f8 100644 --- a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/region/RegionMiddleware.kt +++ b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/region/RegionMiddleware.kt @@ -7,9 +7,8 @@ package mozilla.components.feature.search.region import android.content.Context import androidx.annotation.VisibleForTesting import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job import kotlinx.coroutines.launch import mozilla.components.browser.state.action.BrowserAction @@ -21,11 +20,21 @@ import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.Store import mozilla.components.service.location.LocationService -/** [Middleware] implementation for updating the [RegionState] using the provided [LocationService]. */ +/** + * [Middleware] implementation for updating the [RegionState] using the provided [LocationService]. * + * + * @param context The [Context] used for internal region management. + * @param locationService The [LocationService] used to determine the device's geographic region. + * @param ioDispatcher The [CoroutineDispatcher] to be used for background operations. + * @param applicationScope The [CoroutineScope] used to fetch the device region. This scope should outlive individual + * Activities. Region detection must remain active for the entire application lifetime, so callers should pass an + * application-scoped scope when possible. + */ class RegionMiddleware( context: Context, locationService: LocationService, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, + private val applicationScope: CoroutineScope, ) : Middleware { @VisibleForTesting internal var regionManager = RegionManager(context, locationService, dispatcher = ioDispatcher) @@ -47,12 +56,11 @@ class RegionMiddleware( next(action) } - @OptIn(DelicateCoroutinesApi::class) private fun determineRegion( store: Store, newDistributionId: String? = null, ) = - GlobalScope.launch(ioDispatcher) { + applicationScope.launch(ioDispatcher) { // Get the region state from the RegionManager. If there's none then dispatch the default // region to be used. val distributionId = newDistributionId ?: store.state.distributionId diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/BundledSearchEnginesStorage.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/BundledSearchEnginesStorage.kt index 7159e4e5e4ee..2c6d8dbec963 100644 --- a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/BundledSearchEnginesStorage.kt +++ b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/BundledSearchEnginesStorage.kt @@ -8,10 +8,8 @@ import android.content.Context import android.content.res.AssetManager import java.util.Locale import kotlin.coroutines.CoroutineContext -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.DelicateCoroutinesApi -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.withContext import mozilla.components.browser.state.search.RegionState import mozilla.components.browser.state.search.SearchEngine @@ -46,7 +44,6 @@ internal class BundledSearchEnginesStorage(private val context: Context) : Searc searchEngineIdentifiers = searchEngineIdentifiers.distinct(), type = SearchEngine.Type.BUNDLED, searchExtraParams = searchExtraParams, - coroutineContext = coroutineContext, ) // Reorder the list of search engines according to the configuration. @@ -90,7 +87,6 @@ internal class BundledSearchEnginesStorage(private val context: Context) : Searc searchEngineIdentifiers = ids.distinct(), type = SearchEngine.Type.BUNDLED_ADDITIONAL, searchExtraParams = searchExtraParams, - coroutineContext = coroutineContext, ) } } @@ -227,28 +223,22 @@ private fun applyOverridesIfNeeded( return searchEngineIdentifiers } -@OptIn(DelicateCoroutinesApi::class) private suspend fun loadSearchEnginesFromList( context: Context, searchEngineIdentifiers: List, type: SearchEngine.Type, searchExtraParams: SearchExtraParams?, - coroutineContext: CoroutineContext, ): List { val assets = context.assets val reader = SearchEngineReader(context, type, searchExtraParams) - val deferredSearchEngines = mutableListOf>() - - searchEngineIdentifiers.forEach { identifier -> - deferredSearchEngines.add( - GlobalScope.async(coroutineContext) { - loadSearchEngine(assets, reader, identifier) + return coroutineScope { + searchEngineIdentifiers + .map { identifier -> + async { loadSearchEngine(assets, reader, identifier) } } - ) + .mapNotNull { it.await() } } - - return deferredSearchEngines.mapNotNull { it.await() } } @Suppress("TooGenericExceptionCaught") diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/region/RegionMiddlewareTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/region/RegionMiddlewareTest.kt index 097f24044936..ac533661fb42 100644 --- a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/region/RegionMiddlewareTest.kt +++ b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/region/RegionMiddlewareTest.kt @@ -46,8 +46,8 @@ class RegionMiddlewareTest { @Test fun `GIVEN a locale is already selected WHEN the locale changes THEN update region on RefreshSearchEngines`() = - runTest { - val middleware = RegionMiddleware(FakeContext(), locationService, testDispatcher) + runTest(testDispatcher) { + val middleware = RegionMiddleware(FakeContext(), locationService, testDispatcher, this) middleware.regionManager = regionManager locationService.region = LocationService.Region("FR", "France") @@ -76,122 +76,124 @@ class RegionMiddlewareTest { } @Test - fun `WHEN the UpdateDistribution action is received THEN the distribution is updated`() = runTest { - val captureActionsMiddleware = CaptureActionsMiddleware() - val middleware = RegionMiddleware(FakeContext(), locationService, testDispatcher) - val regionManager: RegionManager = mock() - middleware.regionManager = regionManager - val store = BrowserStore(middleware = listOf(captureActionsMiddleware)) + fun `WHEN the UpdateDistribution action is received THEN the distribution is updated`() = + runTest(testDispatcher) { + val captureActionsMiddleware = CaptureActionsMiddleware() + val middleware = RegionMiddleware(FakeContext(), locationService, testDispatcher, this) + val regionManager: RegionManager = mock() + middleware.regionManager = regionManager + val store = BrowserStore(middleware = listOf(captureActionsMiddleware)) - // null RegionState - `when`(regionManager.region()).thenReturn(null) + // null RegionState + `when`(regionManager.region()).thenReturn(null) - middleware.invoke( - store, - {}, - UpdateDistribution("testId"), - ) - - testDispatcher.scheduler.advanceUntilIdle() - - captureActionsMiddleware.assertFirstAction(SearchAction.SetRegionAction::class) { action -> - assertEquals(RegionState.Default, action.regionState) - assertEquals("testId", action.distribution) - } - - // non null RegionState - `when`(regionManager.region()).thenReturn(RegionState("US", "US")) - - middleware.invoke( - store, - {}, - UpdateDistribution("testId"), - ) - - testDispatcher.scheduler.advanceUntilIdle() - - captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> - assertEquals(RegionState("US", "US"), action.regionState) - assertEquals("testId", action.distribution) - } - - // region manager update has a new RegionState - `when`(regionManager.region()).thenReturn(null) - `when`(regionManager.update()).thenReturn(RegionState("DE", "DE")) - - middleware.invoke( - store, - {}, - UpdateDistribution("testId"), - ) - - testDispatcher.scheduler.advanceUntilIdle() - - captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> - assertEquals(RegionState("DE", "DE"), action.regionState) - assertEquals("testId", action.distribution) - } - } - - @Test - fun `WHEN the RefreshSearchEngines action is received THEN the distribution is updated`() = runTest { - val captureActionsMiddleware = CaptureActionsMiddleware() - val middleware = RegionMiddleware(FakeContext(), locationService, testDispatcher) - val regionManager: RegionManager = mock() - middleware.regionManager = regionManager - val store = - BrowserStore( - BrowserState(distributionId = "testId"), - middleware = listOf(captureActionsMiddleware), + middleware.invoke( + store, + {}, + UpdateDistribution("testId"), ) - // null RegionState - `when`(regionManager.region()).thenReturn(null) + testDispatcher.scheduler.advanceUntilIdle() - middleware.invoke( - store, - {}, - RefreshSearchEnginesAction, - ) + captureActionsMiddleware.assertFirstAction(SearchAction.SetRegionAction::class) { action -> + assertEquals(RegionState.Default, action.regionState) + assertEquals("testId", action.distribution) + } - testDispatcher.scheduler.advanceUntilIdle() + // non null RegionState + `when`(regionManager.region()).thenReturn(RegionState("US", "US")) - captureActionsMiddleware.assertFirstAction(SearchAction.SetRegionAction::class) { action -> - assertEquals(RegionState.Default, action.regionState) - assertEquals("testId", action.distribution) + middleware.invoke( + store, + {}, + UpdateDistribution("testId"), + ) + + testDispatcher.scheduler.advanceUntilIdle() + + captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> + assertEquals(RegionState("US", "US"), action.regionState) + assertEquals("testId", action.distribution) + } + + // region manager update has a new RegionState + `when`(regionManager.region()).thenReturn(null) + `when`(regionManager.update()).thenReturn(RegionState("DE", "DE")) + + middleware.invoke( + store, + {}, + UpdateDistribution("testId"), + ) + + testDispatcher.scheduler.advanceUntilIdle() + + captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> + assertEquals(RegionState("DE", "DE"), action.regionState) + assertEquals("testId", action.distribution) + } } - // non null RegionState - `when`(regionManager.region()).thenReturn(RegionState("US", "US")) + @Test + fun `WHEN the RefreshSearchEngines action is received THEN the distribution is updated`() = + runTest(testDispatcher) { + val captureActionsMiddleware = CaptureActionsMiddleware() + val middleware = RegionMiddleware(FakeContext(), locationService, testDispatcher, this) + val regionManager: RegionManager = mock() + middleware.regionManager = regionManager + val store = + BrowserStore( + BrowserState(distributionId = "testId"), + middleware = listOf(captureActionsMiddleware), + ) - middleware.invoke( - store, - {}, - RefreshSearchEnginesAction, - ) + // null RegionState + `when`(regionManager.region()).thenReturn(null) - testDispatcher.scheduler.advanceUntilIdle() + middleware.invoke( + store, + {}, + RefreshSearchEnginesAction, + ) - captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> - assertEquals(RegionState("US", "US"), action.regionState) - assertEquals("testId", action.distribution) + testDispatcher.scheduler.advanceUntilIdle() + + captureActionsMiddleware.assertFirstAction(SearchAction.SetRegionAction::class) { action -> + assertEquals(RegionState.Default, action.regionState) + assertEquals("testId", action.distribution) + } + + // non null RegionState + `when`(regionManager.region()).thenReturn(RegionState("US", "US")) + + middleware.invoke( + store, + {}, + RefreshSearchEnginesAction, + ) + + testDispatcher.scheduler.advanceUntilIdle() + + captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> + assertEquals(RegionState("US", "US"), action.regionState) + assertEquals("testId", action.distribution) + } + + // region manager update has a new RegionState + `when`(regionManager.region()).thenReturn(null) + `when`(regionManager.update()).thenReturn(RegionState("DE", "DE")) + + middleware.invoke( + store, + {}, + RefreshSearchEnginesAction, + ) + + testDispatcher.scheduler.advanceUntilIdle() + + captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> + assertEquals(RegionState("DE", "DE"), action.regionState) + assertEquals("testId", action.distribution) + } } - - // region manager update has a new RegionState - `when`(regionManager.region()).thenReturn(null) - `when`(regionManager.update()).thenReturn(RegionState("DE", "DE")) - - middleware.invoke( - store, - {}, - RefreshSearchEnginesAction, - ) - - testDispatcher.scheduler.advanceUntilIdle() - - captureActionsMiddleware.assertLastAction(SearchAction.SetRegionAction::class) { action -> - assertEquals(RegionState("DE", "DE"), action.regionState) - assertEquals("testId", action.distribution) - } - } } diff --git a/mobile/android/android-components/components/tooling/lint/src/test/java/mozilla/components/tooling/lint/ShowSnackbarDetectorTest.kt b/mobile/android/android-components/components/tooling/lint/src/test/java/mozilla/components/tooling/lint/ShowSnackbarDetectorTest.kt index 11b04e95eefb..21bbec485e0d 100644 --- a/mobile/android/android-components/components/tooling/lint/src/test/java/mozilla/components/tooling/lint/ShowSnackbarDetectorTest.kt +++ b/mobile/android/android-components/components/tooling/lint/src/test/java/mozilla/components/tooling/lint/ShowSnackbarDetectorTest.kt @@ -81,7 +81,7 @@ class ShowSnackbarDetectorTest : LintDetectorTest() { val coroutineContext: CoroutineContext } - object GlobalScope : CoroutineScope { + object Scope : CoroutineScope { override val coroutineContext: CoroutineContext = kotlin.coroutines.EmptyCoroutineContext fun launch(block: suspend CoroutineScope.() -> Unit): Job = object : Job {} } @@ -112,12 +112,12 @@ class ShowSnackbarDetectorTest : LintDetectorTest() { import androidx.compose.material3.SnackbarHostState import androidx.compose.runtime.Composable - import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.Scope @Composable fun MyScreen() { val hostState = SnackbarHostState() - GlobalScope.launch { + Scope.launch { hostState.showSnackbar("Hello") // VIOLATION } } @@ -132,12 +132,12 @@ class ShowSnackbarDetectorTest : LintDetectorTest() { import com.example.subclass.MySnackbarHostStateSubclass import androidx.compose.runtime.Composable - import kotlinx.coroutines.GlobalScope + import kotlinx.coroutines.Scope @Composable fun MySubclassScreen() { val hostStateSubclass = MySnackbarHostStateSubclass() - GlobalScope.launch { + Scope.launch { hostStateSubclass.showSnackbar("Hello from subclass") // VIOLATION } } diff --git a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt index 697ada488bad..ce2363b81d0b 100644 --- a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt +++ b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt @@ -12,7 +12,10 @@ import androidx.core.app.NotificationManagerCompat import androidx.core.content.ContextCompat import androidx.core.content.edit import java.util.concurrent.TimeUnit +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.MainScope +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.launch import mozilla.components.browser.domains.autocomplete.ShippedDomainsProvider import mozilla.components.browser.engine.system.SystemEngine @@ -112,6 +115,11 @@ open class DefaultComponents(private val applicationContext: Context) { const val PREF_GLOBAL_PRIVACY_CONTROL = "sample_browser_global_privacy_control" } + /** + * A [CoroutineScope] tied to the lifetime of the application process. + */ + val applicationScope: CoroutineScope = CoroutineScope(Dispatchers.Main + SupervisorJob()) + val preferences: SharedPreferences = applicationContext.getSharedPreferences(SAMPLE_BROWSER_PREFERENCES, Context.MODE_PRIVATE) @@ -161,14 +169,20 @@ open class DefaultComponents(private val applicationContext: Context) { private val lazyHistoryStorage = lazy { PlacesHistoryStorage(applicationContext) } val historyStorage by lazy { lazyHistoryStorage.value } - val sessionStorage by lazy { SessionStorage(applicationContext, engine) } + val sessionStorage by lazy { + SessionStorage( + applicationContext, + engine, + applicationScope = applicationScope, + ) + } val permissionStorage by lazy { OnDiskSitePermissionsStorage(applicationContext) } val thumbnailStorage by lazy { ThumbnailStorage(applicationContext) } val fileUploadsDirCleaner: FileUploadsDirCleaner by lazy { - FileUploadsDirCleaner { applicationContext.cacheDir } + FileUploadsDirCleaner(applicationScope) { applicationContext.cacheDir } } val store by lazy { @@ -179,22 +193,23 @@ open class DefaultComponents(private val applicationContext: Context) { applicationContext = applicationContext, downloadServiceClass = DownloadService::class.java, deleteFileFromStorage = { false }, - downloadFileUtils = DefaultDownloadFileUtils(context = applicationContext), - ), - ReaderViewMiddleware(), - ThumbnailsMiddleware(thumbnailStorage), - UndoMiddleware(), - RegionMiddleware( - applicationContext, - LocationService.default(), - ), - SearchMiddleware(applicationContext), - RecordingDevicesMiddleware(applicationContext, notificationsDelegate), - LastAccessMiddleware(), - PromptMiddleware(), - SessionPrioritizationMiddleware(), - ) + EngineMiddleware.create(engine) - ) + downloadFileUtils = DefaultDownloadFileUtils(context = applicationContext + ), + ), + ReaderViewMiddleware(), + ThumbnailsMiddleware(thumbnailStorage), + UndoMiddleware(), + RegionMiddleware( + applicationContext, + LocationService.default(), + applicationScope = applicationScope, + ), + SearchMiddleware(applicationContext), + RecordingDevicesMiddleware(applicationContext, notificationsDelegate), + LastAccessMiddleware(), + PromptMiddleware(), + SessionPrioritizationMiddleware(), + ) + EngineMiddleware.create(engine)) .apply { WebNotificationFeature( applicationContext, @@ -278,7 +293,11 @@ open class DefaultComponents(private val applicationContext: Context) { // Intent val tabIntentProcessor by lazy { - TabIntentProcessor(tabsUseCases, searchUseCases.newTabSearch) + TabIntentProcessor( + tabsUseCases, + searchUseCases.newTabSearch, + applicationScope = applicationScope, + ) } val externalAppIntentProcessors by lazy { listOf( diff --git a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/SampleApplication.kt b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/SampleApplication.kt index 470cf6a14808..5e609381fd0c 100644 --- a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/SampleApplication.kt +++ b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/SampleApplication.kt @@ -8,9 +8,11 @@ import android.app.Application import java.util.Calendar import java.util.TimeZone import java.util.concurrent.TimeUnit +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.launch import mozilla.components.browser.state.action.SystemAction import mozilla.components.browser.storage.sync.GlobalPlacesDependencyProvider @@ -48,6 +50,8 @@ class SampleApplication : Application() { val components by lazy { Components(this) } + val applicationScope: CoroutineScope = CoroutineScope(Dispatchers.Main + SupervisorJob()) + // Sample-only code with no injectable clock seam and no time-dependent behavior to test. @Suppress("NoSystemCurrentTimeMillis") @OptIn(DelicateCoroutinesApi::class) // Usage of GlobalScope @@ -122,6 +126,11 @@ class SampleApplication : Application() { } } + override fun onTerminate() { + super.onTerminate() + // applicationScope.cancel() - User can add this in commit 2 + } + @DelicateCoroutinesApi private fun restoreBrowserState() = GlobalScope.launch(Dispatchers.Main) { diff --git a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/autofill/AutofillService.kt b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/autofill/AutofillService.kt index e4cecafe6d15..e737ccc84414 100644 --- a/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/autofill/AutofillService.kt +++ b/mobile/android/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/autofill/AutofillService.kt @@ -11,4 +11,5 @@ import org.mozilla.samples.browser.ext.components /** Service responsible for implementing Android's Autofill framework. */ class AutofillService : AbstractAutofillService() { override val configuration: AutofillConfiguration by lazy { components.autofillConfiguration } + override val applicationScope by lazy { components.applicationScope } } diff --git a/mobile/android/android-components/samples/compose-browser/src/main/java/org/mozilla/samples/compose/browser/Components.kt b/mobile/android/android-components/samples/compose-browser/src/main/java/org/mozilla/samples/compose/browser/Components.kt index 7c984d99b47f..41b46d3e10db 100644 --- a/mobile/android/android-components/samples/compose-browser/src/main/java/org/mozilla/samples/compose/browser/Components.kt +++ b/mobile/android/android-components/samples/compose-browser/src/main/java/org/mozilla/samples/compose/browser/Components.kt @@ -7,6 +7,9 @@ package org.mozilla.samples.compose.browser import android.content.Context import androidx.compose.runtime.Composable import androidx.compose.ui.platform.LocalContext +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob import mozilla.appservices.remotesettings.RemoteSettingsServer import mozilla.components.browser.engine.gecko.GeckoEngine import mozilla.components.browser.engine.gecko.fetch.GeckoViewFetchClient @@ -30,6 +33,8 @@ import org.mozilla.samples.compose.browser.app.AppStore class Components(context: Context) { private val runtime by lazy { GeckoRuntime.create(context) } + val applicationScope: CoroutineScope = CoroutineScope(Dispatchers.Main + SupervisorJob()) + val engine: Engine by lazy { GeckoEngine(context, runtime = runtime) } val client: Client by lazy { GeckoViewFetchClient(context, runtime = runtime) } @@ -37,7 +42,7 @@ class Components(context: Context) { BrowserStore( middleware = listOf( - RegionMiddleware(context, locationService), + RegionMiddleware(context, locationService, applicationScope = applicationScope), SearchMiddleware(context), ) + EngineMiddleware.create(engine) ) diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/gecko/CrashPullDelegateTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/gecko/CrashPullDelegateTest.kt index 00838f6fcaeb..c0a75e2625ef 100644 --- a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/gecko/CrashPullDelegateTest.kt +++ b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/gecko/CrashPullDelegateTest.kt @@ -11,6 +11,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.TestScope import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.storage.CreditCardsAddressesStorage import mozilla.components.concept.storage.LoginsStorage @@ -43,7 +44,8 @@ class CrashPullDelegateTest { runBlocking { scope .launch { - val runtime = GeckoProvider.getOrCreateRuntime(context, mockAutofill, mockLogin, mockPolicy) + val runtime = + GeckoProvider.getOrCreateRuntime(context, mockAutofill, mockLogin, mockPolicy, TestScope()) assertNotNull(runtime.crashPullDelegate) runtime.crashPullDelegate?.onCrashPull(arrayOf("1", "2")) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/autofill/AutofillService.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/autofill/AutofillService.kt index bf6bd30ab389..a92ea996a391 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/autofill/AutofillService.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/autofill/AutofillService.kt @@ -11,4 +11,5 @@ import org.mozilla.fenix.ext.components /** Service responsible for implementing Android's Autofill framework. */ class AutofillService : AbstractAutofillService() { override val configuration: AutofillConfiguration by lazy { components.autofillConfiguration } + override val applicationScope by lazy { components.applicationScope } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt index b2f79927f7b7..a70cd4d8472f 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt @@ -10,6 +10,7 @@ import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting.Companion.PRIVATE import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.seconds +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.MainScope import kotlinx.coroutines.launch import mozilla.components.browser.storage.sync.PlacesBookmarksStorage @@ -84,6 +85,7 @@ class BackgroundServices( remoteTabsStorage: Lazy, creditCardsStorage: Lazy, strictMode: StrictModeManager, + private val applicationScope: CoroutineScope, ) { // Allows executing tasks which depend on the account manager, but do not need to eagerly initialize it. val accountManagerAvailableQueue = RunWhenReadyQueue() @@ -225,7 +227,14 @@ class BackgroundServices( // Enable push if it's configured. push.feature?.let { autoPushFeature -> - FxaPushSupportFeature(context, accountManager, autoPushFeature, crashReporter).initialize() + FxaPushSupportFeature( + context, + accountManager, + autoPushFeature, + applicationScope = applicationScope, + crashReporter = crashReporter, + ) + .initialize() } SendTabFeature(accountManager) { device, tabs -> diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt index 8cb1eef3b876..637443191211 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -136,6 +136,7 @@ class Components( core.lazyRemoteTabsStorage, core.lazyAutofillStorage, strictMode, + applicationScope, ) } val services by lazyMonitored { Services(context, core.store, backgroundServices.accountManager) } @@ -178,6 +179,7 @@ class Components( useCases.searchUseCases, core.webAppManifestStorage, core.engine, + applicationScope, ) } @@ -470,7 +472,7 @@ class Components( } val aiFeatureRegistry by lazyMonitored { - AIFeatureRegistry.default(scope = kotlinx.coroutines.MainScope(), context = context).also { + AIFeatureRegistry.default(scope = applicationScope, context = context).also { if (settings.shakeToSummarizeFeatureFlagEnabled) { it.register(PageSummaryFeature(summarizationSettings)) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt index 3ca1573691fd..a1c3d963e182 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -289,7 +289,7 @@ class Core( } val fileUploadsDirCleaner: FileUploadsDirCleaner by lazyMonitored { - FileUploadsDirCleaner { context.cacheDir } + FileUploadsDirCleaner(scope = applicationScope) { context.cacheDir } } val geckoRuntime: GeckoRuntime by lazyMonitored { @@ -298,6 +298,7 @@ class Core( lazyAutofillStorage, lazyPasswordsStorage, trackingProtectionPolicyFactory.createTrackingProtectionPolicy(), + applicationScope, ) } @@ -306,7 +307,7 @@ class Core( } val sessionStorage: SessionStorage by lazyMonitored { - SessionStorage(context, engine, crashReporter) + SessionStorage(context, engine, crashReporter, applicationScope) } private val locationService: LocationService by lazyMonitored { @@ -361,7 +362,7 @@ class Core( TelemetryMiddleware(context, context.components.settings, metrics, crashReporter), ThumbnailsMiddleware(thumbnailStorage), UndoMiddleware(context.components.settings.getUndoDelay()), - RegionMiddleware(context, locationService), + RegionMiddleware(context, locationService, applicationScope = applicationScope), SearchMiddleware( context = context, additionalBundledSearchEngineIds = listOf("reddit", "youtube"), diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/IntentProcessors.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/IntentProcessors.kt index 8935aec0061a..a38a028db40e 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/IntentProcessors.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/IntentProcessors.kt @@ -10,6 +10,7 @@ package org.mozilla.fenix.components import android.content.Context +import kotlinx.coroutines.CoroutineScope import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine import mozilla.components.feature.customtabs.CustomTabIntentProcessor @@ -38,15 +39,28 @@ class IntentProcessors( private val searchUseCases: SearchUseCases, private val manifestStorage: ManifestStorage, private val engine: Engine, + private val applicationScope: CoroutineScope, ) { /** Provides intent processing functionality for ACTION_VIEW and ACTION_SEND intents. */ val intentProcessor by lazyMonitored { - TabIntentProcessor(tabsUseCases, searchUseCases.newTabSearch, isPrivate = false, engine = engine) + TabIntentProcessor( + tabsUseCases, + searchUseCases.newTabSearch, + isPrivate = false, + engine = engine, + applicationScope = applicationScope, + ) } /** Provides intent processing functionality for ACTION_VIEW and ACTION_SEND intents in private tabs. */ val privateIntentProcessor by lazyMonitored { - TabIntentProcessor(tabsUseCases, searchUseCases.newPrivateTabSearch, isPrivate = true, engine = engine) + TabIntentProcessor( + tabsUseCases, + searchUseCases.newPrivateTabSearch, + isPrivate = true, + engine = engine, + applicationScope = applicationScope, + ) } val customTabIntentProcessor by lazyMonitored { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/gecko/GeckoProvider.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/gecko/GeckoProvider.kt index 0d72d61e72a8..1edcf54eebad 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/gecko/GeckoProvider.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/gecko/GeckoProvider.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.gecko import android.content.Context import androidx.annotation.VisibleForTesting +import kotlinx.coroutines.CoroutineScope import mozilla.components.browser.engine.gecko.autofill.GeckoAutocompleteStorageDelegate import mozilla.components.browser.engine.gecko.crash.GeckoCrashPullDelegate import mozilla.components.browser.engine.gecko.ext.toContentBlockingSetting @@ -27,15 +28,27 @@ import org.mozilla.geckoview.GeckoRuntimeSettings object GeckoProvider { private var runtime: GeckoRuntime? = null + /** + * Gets the existing [GeckoRuntime] instance or creates a new one if it hasn't been initialized. This method is + * synchronized to ensure that only a single instance of the runtime is created. + * + * @param context The application context used to initialize the runtime and its settings. + * @param autofillStorage Lazy provider for credit card and address storage. + * @param loginStorage Lazy provider for login and password storage. + * @param trackingProtectionPolicy The policy defining how tracking protection should be configured. + * @param applicationScope The [CoroutineScope] used for background operations within the autocomplete delegate. + * @return The singleton [GeckoRuntime] instance. + */ @Synchronized fun getOrCreateRuntime( context: Context, autofillStorage: Lazy, loginStorage: Lazy, trackingProtectionPolicy: TrackingProtectionPolicy, + applicationScope: CoroutineScope, ): GeckoRuntime { if (runtime == null) { - runtime = createRuntime(context, autofillStorage, loginStorage, trackingProtectionPolicy) + runtime = createRuntime(context, autofillStorage, loginStorage, trackingProtectionPolicy, applicationScope) } return runtime!! @@ -46,6 +59,7 @@ object GeckoProvider { autofillStorage: Lazy, loginStorage: Lazy, policy: TrackingProtectionPolicy, + applicationScope: CoroutineScope, ): GeckoRuntime { val runtimeSettings = createRuntimeSettings(context, policy) @@ -69,6 +83,7 @@ object GeckoProvider { loginStorage = loginStorage, isLoginAutofillEnabled = { context.components.settings.shouldAutofillLogins }, ), + applicationScope = applicationScope, ) geckoRuntime.crashPullDelegate = diff --git a/mobile/android/fenix/config/detekt-baseline.xml b/mobile/android/fenix/config/detekt-baseline.xml index 7896c473a92b..a7c02f6f5e2f 100644 --- a/mobile/android/fenix/config/detekt-baseline.xml +++ b/mobile/android/fenix/config/detekt-baseline.xml @@ -707,7 +707,6 @@ UndocumentedPublicFunction:Fragment.kt$fun Fragment.breadcrumb( message: String, data: Map<String, String> = emptyMap(), ) UndocumentedPublicFunction:Fragment.kt$fun Fragment.getPreferenceKey(@StringRes resourceId: Int): String UndocumentedPublicFunction:FxaServer.kt$FxaServer$fun config(context: Context): ServerConfig - UndocumentedPublicFunction:GeckoProvider.kt$GeckoProvider$@Synchronized fun getOrCreateRuntime( context: Context, autofillStorage: Lazy<CreditCardsAddressesStorage>, loginStorage: Lazy<LoginsStorage>, trackingProtectionPolicy: TrackingProtectionPolicy, ): GeckoRuntime UndocumentedPublicFunction:GroupableRadioButton.kt$GroupableRadioButton$fun addToRadioGroup(radioButton: GroupableRadioButton) UndocumentedPublicFunction:GroupableRadioButton.kt$GroupableRadioButton$fun updateRadioValue(isChecked: Boolean) UndocumentedPublicFunction:GroupableRadioButton.kt$fun Iterable<GroupableRadioButton>.uncheckAll() diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt index c039aa40234d..a29553023c9a 100644 --- a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt +++ b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt @@ -142,7 +142,7 @@ class Components( val settings by lazy { Settings(context) } val fileUploadsDirCleaner: FileUploadsDirCleaner by lazy { - FileUploadsDirCleaner { context.cacheDir } + FileUploadsDirCleaner(applicationScope) { context.cacheDir } } val remoteSettingsSyncScheduler by lazy { @@ -213,7 +213,7 @@ class Components( // We are currently using the default location service. We should consider using // an actual implementation: // https://github.com/mozilla-mobile/focus-android/issues/4781 - RegionMiddleware(context, locationService), + RegionMiddleware(context, locationService, applicationScope = applicationScope), SearchMiddleware( context, migration = SearchMigration(context), diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/session/IntentProcessor.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/session/IntentProcessor.kt index 20e3e7780a3b..f0fab0884691 100644 --- a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/session/IntentProcessor.kt +++ b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/session/IntentProcessor.kt @@ -43,6 +43,7 @@ class IntentProcessor( tabsUseCases, searchUseCases.newTabSearch, isPrivate = true, + applicationScope = context.components.applicationScope, ) private val customTabIntentProcessor =