Bug 1990613 - AC: Refactor multiple components to replace GlobalScope and coroutineContext with an explicit, application-provided CoroutineScope. r=android-reviewers,pollymce
Key changes: - Refactored `TabIntentProcessor`, `RegionMiddleware`, `FileUploadsDirCleaner`, `AbstractAutofillService`, `GeckoAutocompleteStorageDelegate`, and `AutoSave` to accept a `CoroutineScope` (typically named `applicationScope`) in their constructors. - Replaced `GlobalScope.launch` and `GlobalScope.async` with operations on the provided internal scopes to ensure structured concurrency and better lifecycle management. - Updated `BundledSearchEnginesStorage` to use `coroutineScope` and `async` for loading search engines, removing the dependency on an external `coroutineContext`. - Refactored `FxaPushSupportFeature` and its internal observers (`AccountObserver`, `ConstellationObserver`, `AutoPushObserver`) to utilize a centralized `applicationScope` instead of mixing global and UI contexts. - Updated Fenix and sample browser `AutofillService` implementations to provide the `applicationScope` from the main components container. - Updated associated unit tests to pass the appropriate test scope or `runTest` scope to the refactored classes. - Removed several `@OptIn(DelicateCoroutinesApi::class)` annotations previously required for `GlobalScope` usage. Differential Revision: https://phabricator.services.mozilla.com/D297659
This commit is contained in:
committed by
mcarare@mozilla.com
parent
046369af65
commit
c043f6bb6b
+10
-12
@@ -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<Array<Autocomplete.Address>>? {
|
||||
val result = GeckoResult<Array<Autocomplete.Address>>()
|
||||
|
||||
@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<Array<Autocomplete.CreditCard>> {
|
||||
val result = GeckoResult<Array<Autocomplete.CreditCard>>()
|
||||
|
||||
@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<Array<Autocomplete.LoginEntry>> {
|
||||
val result = GeckoResult<Array<Autocomplete.LoginEntry>>()
|
||||
|
||||
@OptIn(DelicateCoroutinesApi::class)
|
||||
GlobalScope.launch(IO) {
|
||||
applicationScope.launch(IO) {
|
||||
val storedLogins = loginStorageDelegate.onLoginFetch(domain)
|
||||
|
||||
val logins = storedLogins.await().map { it.toLoginEntry() }.toTypedArray()
|
||||
|
||||
+1
-4
@@ -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"))))
|
||||
|
||||
@@ -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.
|
||||
|
||||
+5
-4
@@ -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)
|
||||
|
||||
+5
-5
@@ -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)
|
||||
|
||||
+30
-9
@@ -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)
|
||||
|
||||
+3
-1
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+75
-54
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
+21
-20
@@ -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)
|
||||
|
||||
|
||||
+47
-36
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+3
-3
@@ -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.
|
||||
|
||||
+5
-5
@@ -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,
|
||||
)
|
||||
|
||||
|
||||
+6
-6
@@ -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)
|
||||
|
||||
+7
-7
@@ -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,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+3
-3
@@ -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),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+3
-7
@@ -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
|
||||
|
||||
+7
-4
@@ -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) {
|
||||
|
||||
+75
-27
@@ -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")
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
+9
-8
@@ -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,
|
||||
|
||||
+10
-5
@@ -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,
|
||||
) {
|
||||
|
||||
+13
-5
@@ -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<BrowserState, BrowserAction> {
|
||||
@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<BrowserState, BrowserAction>,
|
||||
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
|
||||
|
||||
+6
-16
@@ -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<String>,
|
||||
type: SearchEngine.Type,
|
||||
searchExtraParams: SearchExtraParams?,
|
||||
coroutineContext: CoroutineContext,
|
||||
): List<SearchEngine> {
|
||||
val assets = context.assets
|
||||
val reader = SearchEngineReader(context, type, searchExtraParams)
|
||||
|
||||
val deferredSearchEngines = mutableListOf<Deferred<SearchEngine?>>()
|
||||
|
||||
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")
|
||||
|
||||
+109
-107
@@ -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<BrowserState, BrowserAction>()
|
||||
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<BrowserState, BrowserAction>()
|
||||
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<BrowserState, BrowserAction>()
|
||||
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<BrowserState, BrowserAction>()
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+5
-5
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
+38
-19
@@ -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(
|
||||
|
||||
+9
@@ -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) {
|
||||
|
||||
+1
@@ -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 }
|
||||
}
|
||||
|
||||
+6
-1
@@ -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)
|
||||
)
|
||||
|
||||
+3
-1
@@ -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"))
|
||||
}
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
|
||||
+10
-1
@@ -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<RemoteTabsStorage>,
|
||||
creditCardsStorage: Lazy<AutofillCreditCardsAddressesStorage>,
|
||||
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 ->
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
@@ -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"),
|
||||
|
||||
+16
-2
@@ -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 {
|
||||
|
||||
@@ -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<CreditCardsAddressesStorage>,
|
||||
loginStorage: Lazy<LoginsStorage>,
|
||||
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<CreditCardsAddressesStorage>,
|
||||
loginStorage: Lazy<LoginsStorage>,
|
||||
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 =
|
||||
|
||||
@@ -707,7 +707,6 @@
|
||||
<ID>UndocumentedPublicFunction:Fragment.kt$fun Fragment.breadcrumb( message: String, data: Map<String, String> = emptyMap(), )</ID>
|
||||
<ID>UndocumentedPublicFunction:Fragment.kt$fun Fragment.getPreferenceKey(@StringRes resourceId: Int): String</ID>
|
||||
<ID>UndocumentedPublicFunction:FxaServer.kt$FxaServer$fun config(context: Context): ServerConfig</ID>
|
||||
<ID>UndocumentedPublicFunction:GeckoProvider.kt$GeckoProvider$@Synchronized fun getOrCreateRuntime( context: Context, autofillStorage: Lazy<CreditCardsAddressesStorage>, loginStorage: Lazy<LoginsStorage>, trackingProtectionPolicy: TrackingProtectionPolicy, ): GeckoRuntime</ID>
|
||||
<ID>UndocumentedPublicFunction:GroupableRadioButton.kt$GroupableRadioButton$fun addToRadioGroup(radioButton: GroupableRadioButton)</ID>
|
||||
<ID>UndocumentedPublicFunction:GroupableRadioButton.kt$GroupableRadioButton$fun updateRadioValue(isChecked: Boolean)</ID>
|
||||
<ID>UndocumentedPublicFunction:GroupableRadioButton.kt$fun Iterable<GroupableRadioButton>.uncheckAll()</ID>
|
||||
|
||||
@@ -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),
|
||||
|
||||
+1
@@ -43,6 +43,7 @@ class IntentProcessor(
|
||||
tabsUseCases,
|
||||
searchUseCases.newTabSearch,
|
||||
isPrivate = true,
|
||||
applicationScope = context.components.applicationScope,
|
||||
)
|
||||
|
||||
private val customTabIntentProcessor =
|
||||
|
||||
Reference in New Issue
Block a user