Enforce IO thread inside of components (#14704)

* Automatically run PermissionStorage on IO thread

* Run TabCollectionStorage on dedicated scope + IO

* Update findSitePermissionsBy calls
pull/90/head
Tiger Oakes 4 years ago committed by GitHub
parent 7aa6514499
commit 113241e8ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -962,11 +962,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
private fun showQuickSettingsDialog() { private fun showQuickSettingsDialog() {
val session = getSessionById() ?: return val session = getSessionById() ?: return
viewLifecycleOwner.lifecycleScope.launch(Main) { viewLifecycleOwner.lifecycleScope.launch(Main) {
val sitePermissions: SitePermissions? = withContext(IO) { val sitePermissions: SitePermissions? = session.url.toUri().host?.let { host ->
session.url.toUri().host?.let { host -> val storage = requireComponents.core.permissionStorage
val storage = requireContext().components.core.permissionStorage storage.findSitePermissionsBy(host)
storage.findSitePermissionsBy(host)
}
} }
view?.let { view?.let {

@ -69,7 +69,7 @@ fun List<Tab>.toSessionBundle(sessionManager: SessionManager): List<Session> {
* @param metrics Controller that handles telemetry events. * @param metrics Controller that handles telemetry events.
* @param tabCollectionStorage Storage used to save tab collections to disk. * @param tabCollectionStorage Storage used to save tab collections to disk.
* @param sessionManager Used to query and serialize tabs. * @param sessionManager Used to query and serialize tabs.
* @param ioScope Coroutine scope that launches on the IO thread. * @param scope Coroutine scope to launch coroutines.
*/ */
class DefaultCollectionCreationController( class DefaultCollectionCreationController(
private val store: CollectionCreationStore, private val store: CollectionCreationStore,
@ -77,7 +77,7 @@ class DefaultCollectionCreationController(
private val metrics: MetricController, private val metrics: MetricController,
private val tabCollectionStorage: TabCollectionStorage, private val tabCollectionStorage: TabCollectionStorage,
private val sessionManager: SessionManager, private val sessionManager: SessionManager,
private val ioScope: CoroutineScope private val scope: CoroutineScope
) : CollectionCreationController { ) : CollectionCreationController {
companion object { companion object {
@ -89,7 +89,7 @@ class DefaultCollectionCreationController(
dismiss() dismiss()
val sessionBundle = tabs.toSessionBundle(sessionManager) val sessionBundle = tabs.toSessionBundle(sessionManager)
ioScope.launch { scope.launch {
tabCollectionStorage.createCollection(name, sessionBundle) tabCollectionStorage.createCollection(name, sessionBundle)
} }
@ -100,7 +100,7 @@ class DefaultCollectionCreationController(
override fun renameCollection(collection: TabCollection, name: String) { override fun renameCollection(collection: TabCollection, name: String) {
dismiss() dismiss()
ioScope.launch { scope.launch {
tabCollectionStorage.renameCollection(collection, name) tabCollectionStorage.renameCollection(collection, name)
} }
metrics.track(Event.CollectionRenamed) metrics.track(Event.CollectionRenamed)
@ -130,7 +130,7 @@ class DefaultCollectionCreationController(
override fun selectCollection(collection: TabCollection, tabs: List<Tab>) { override fun selectCollection(collection: TabCollection, tabs: List<Tab>) {
dismiss() dismiss()
val sessionBundle = tabs.toList().toSessionBundle(sessionManager) val sessionBundle = tabs.toList().toSessionBundle(sessionManager)
ioScope.launch { scope.launch {
tabCollectionStorage tabCollectionStorage
.addTabsToCollection(collection, sessionBundle) .addTabsToCollection(collection, sessionBundle)
} }

@ -14,9 +14,7 @@ import androidx.fragment.app.DialogFragment
import androidx.lifecycle.lifecycleScope import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.navArgs import androidx.navigation.fragment.navArgs
import kotlinx.android.synthetic.main.fragment_create_collection.view.* import kotlinx.android.synthetic.main.fragment_create_collection.view.*
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.plus
import mozilla.components.browser.state.selector.findTab import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.TabSessionState
@ -80,7 +78,7 @@ class CollectionCreationFragment : DialogFragment() {
requireComponents.analytics.metrics, requireComponents.analytics.metrics,
requireComponents.core.tabCollectionStorage, requireComponents.core.tabCollectionStorage,
requireComponents.core.sessionManager, requireComponents.core.sessionManager,
ioScope = lifecycleScope + Dispatchers.IO scope = lifecycleScope
) )
) )
collectionCreationView = CollectionCreationView( collectionCreationView = CollectionCreationView(

@ -6,6 +6,8 @@ package org.mozilla.fenix.components
import android.content.Context import android.content.Context
import androidx.paging.DataSource import androidx.paging.DataSource
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.sitepermissions.SitePermissions
import mozilla.components.feature.sitepermissions.SitePermissions.Status import mozilla.components.feature.sitepermissions.SitePermissions.Status
import mozilla.components.feature.sitepermissions.SitePermissionsStorage import mozilla.components.feature.sitepermissions.SitePermissionsStorage
@ -38,11 +40,11 @@ class PermissionStorage(private val context: Context) {
return sitePermissions return sitePermissions
} }
fun findSitePermissionsBy(origin: String): SitePermissions? { suspend fun findSitePermissionsBy(origin: String): SitePermissions? = withContext(Dispatchers.IO) {
return permissionsStorage.findSitePermissionsBy(origin) permissionsStorage.findSitePermissionsBy(origin)
} }
fun updateSitePermissions(sitePermissions: SitePermissions) { suspend fun updateSitePermissions(sitePermissions: SitePermissions) = withContext(Dispatchers.IO) {
permissionsStorage.update(sitePermissions) permissionsStorage.update(sitePermissions)
} }
@ -50,11 +52,11 @@ class PermissionStorage(private val context: Context) {
return permissionsStorage.getSitePermissionsPaged() return permissionsStorage.getSitePermissionsPaged()
} }
fun deleteSitePermissions(sitePermissions: SitePermissions) { suspend fun deleteSitePermissions(sitePermissions: SitePermissions) = withContext(Dispatchers.IO) {
permissionsStorage.remove(sitePermissions) permissionsStorage.remove(sitePermissions)
} }
fun deleteAllSitePermissions() { suspend fun deleteAllSitePermissions() = withContext(Dispatchers.IO) {
permissionsStorage.removeAll() permissionsStorage.removeAll()
} }
} }

@ -9,6 +9,9 @@ import android.os.StrictMode
import androidx.lifecycle.LiveData import androidx.lifecycle.LiveData
import androidx.lifecycle.asLiveData import androidx.lifecycle.asLiveData
import androidx.paging.DataSource import androidx.paging.DataSource
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import mozilla.components.browser.session.Session import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager import mozilla.components.browser.session.SessionManager
import mozilla.components.feature.tab.collections.Tab import mozilla.components.feature.tab.collections.Tab
@ -49,6 +52,7 @@ class TabCollectionStorage(
fun onCollectionRenamed(tabCollection: TabCollection, title: String) = Unit fun onCollectionRenamed(tabCollection: TabCollection, title: String) = Unit
} }
private val ioScope = CoroutineScope(Dispatchers.IO)
var cachedTabCollections = listOf<TabCollection>() var cachedTabCollections = listOf<TabCollection>()
private val collectionStorage by lazy { private val collectionStorage by lazy {
@ -57,15 +61,15 @@ class TabCollectionStorage(
} }
} }
fun createCollection(title: String, sessions: List<Session>) { suspend fun createCollection(title: String, sessions: List<Session>) = ioScope.launch {
collectionStorage.createCollection(title, sessions) collectionStorage.createCollection(title, sessions)
notifyObservers { onCollectionCreated(title, sessions) } notifyObservers { onCollectionCreated(title, sessions) }
} }.join()
fun addTabsToCollection(tabCollection: TabCollection, sessions: List<Session>) { suspend fun addTabsToCollection(tabCollection: TabCollection, sessions: List<Session>) = ioScope.launch {
collectionStorage.addTabsToCollection(tabCollection, sessions) collectionStorage.addTabsToCollection(tabCollection, sessions)
notifyObservers { onTabsAdded(tabCollection, sessions) } notifyObservers { onTabsAdded(tabCollection, sessions) }
} }.join()
fun getTabCollectionsCount(): Int { fun getTabCollectionsCount(): Int {
return collectionStorage.getTabCollectionsCount() return collectionStorage.getTabCollectionsCount()
@ -79,18 +83,18 @@ class TabCollectionStorage(
return collectionStorage.getCollectionsPaged() return collectionStorage.getCollectionsPaged()
} }
fun removeCollection(tabCollection: TabCollection) { suspend fun removeCollection(tabCollection: TabCollection) = ioScope.launch {
collectionStorage.removeCollection(tabCollection) collectionStorage.removeCollection(tabCollection)
} }.join()
fun removeTabFromCollection(tabCollection: TabCollection, tab: Tab) { suspend fun removeTabFromCollection(tabCollection: TabCollection, tab: Tab) = ioScope.launch {
collectionStorage.removeTabFromCollection(tabCollection, tab) collectionStorage.removeTabFromCollection(tabCollection, tab)
} }.join()
fun renameCollection(tabCollection: TabCollection, title: String) { suspend fun renameCollection(tabCollection: TabCollection, title: String) = ioScope.launch {
collectionStorage.renameCollection(tabCollection, title) collectionStorage.renameCollection(tabCollection, title)
notifyObservers { onCollectionRenamed(tabCollection, title) } notifyObservers { onCollectionRenamed(tabCollection, title) }
} }.join()
} }
fun TabCollection.description(context: Context): String { fun TabCollection.description(context: Context): String {

@ -240,7 +240,7 @@ class DefaultSessionControlController(
handleSwipedItemDeletionCancel handleSwipedItemDeletionCancel
) )
} else { } else {
viewLifecycleScope.launch(Dispatchers.IO) { viewLifecycleScope.launch {
tabCollectionStorage.removeTabFromCollection(collection, tab) tabCollectionStorage.removeTabFromCollection(collection, tab)
} }
} }

@ -18,6 +18,7 @@ import kotlinx.coroutines.withContext
import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.sitepermissions.SitePermissions
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.ext.showToolbar
import org.mozilla.fenix.settings.PhoneFeature import org.mozilla.fenix.settings.PhoneFeature
import org.mozilla.fenix.settings.PhoneFeature.CAMERA import org.mozilla.fenix.settings.PhoneFeature.CAMERA
@ -44,13 +45,10 @@ class SitePermissionsDetailsExceptionsFragment : PreferenceFragmentCompat() {
override fun onResume() { override fun onResume() {
super.onResume() super.onResume()
showToolbar(sitePermissions.origin) showToolbar(sitePermissions.origin)
viewLifecycleOwner.lifecycleScope.launch(IO) { viewLifecycleOwner.lifecycleScope.launch(Main) {
val context = requireContext()
sitePermissions = sitePermissions =
requireNotNull(context.components.core.permissionStorage.findSitePermissionsBy(sitePermissions.origin)) requireNotNull(requireComponents.core.permissionStorage.findSitePermissionsBy(sitePermissions.origin))
withContext(Main) { bindCategoryPhoneFeatures()
bindCategoryPhoneFeatures()
}
} }
} }

@ -24,7 +24,6 @@ import androidx.paging.PagedListAdapter
import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView
import kotlinx.coroutines.Dispatchers.IO
import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.Dispatchers.Main
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.sitepermissions.SitePermissions
@ -108,13 +107,12 @@ class SitePermissionsExceptionsFragment :
} }
private fun deleteAllSitePermissions() { private fun deleteAllSitePermissions() {
viewLifecycleOwner.lifecycleScope.launch(IO) { viewLifecycleOwner.lifecycleScope.launch(Main) {
requireContext().components.core.permissionStorage.deleteAllSitePermissions() requireContext().components.core.permissionStorage.deleteAllSitePermissions()
launch(Main) {
showEmptyListMessage() showEmptyListMessage()
// Reload the selected session. // Reload the selected session.
requireContext().components.useCases.sessionUseCases.reload() requireContext().components.useCases.sessionUseCases.reload()
}
} }
} }

@ -18,7 +18,6 @@ import androidx.appcompat.app.AlertDialog
import androidx.fragment.app.Fragment import androidx.fragment.app.Fragment
import androidx.lifecycle.lifecycleScope import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.navArgs import androidx.navigation.fragment.navArgs
import kotlinx.coroutines.Dispatchers.IO
import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.Dispatchers.Main
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.sitepermissions.SitePermissions
@ -142,11 +141,9 @@ class SitePermissionsManageExceptionsPhoneFeatureFragment : Fragment() {
private fun updatedSitePermissions(status: SitePermissions.Status) { private fun updatedSitePermissions(status: SitePermissions.Status) {
val updatedSitePermissions = args.sitePermissions.update(args.phoneFeature, status) val updatedSitePermissions = args.sitePermissions.update(args.phoneFeature, status)
viewLifecycleOwner.lifecycleScope.launch(IO) { viewLifecycleOwner.lifecycleScope.launch(Main) {
requireComponents.core.permissionStorage.updateSitePermissions(updatedSitePermissions) requireComponents.core.permissionStorage.updateSitePermissions(updatedSitePermissions)
launch(Main) { requireComponents.tryReloadTabBy(updatedSitePermissions.origin)
requireComponents.tryReloadTabBy(updatedSitePermissions.origin)
}
} }
} }
} }

@ -370,7 +370,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
val selectedCollection = val selectedCollection =
(list.adapter as CollectionsAdapter).getSelectedCollection() (list.adapter as CollectionsAdapter).getSelectedCollection()
val collection = tabCollectionStorage.cachedTabCollections[selectedCollection] val collection = tabCollectionStorage.cachedTabCollections[selectedCollection]
viewLifecycleOwner.lifecycleScope.launch(Dispatchers.IO) { viewLifecycleOwner.lifecycleScope.launch(Main) {
tabCollectionStorage.addTabsToCollection(collection, sessionList) tabCollectionStorage.addTabsToCollection(collection, sessionList)
it.metrics.track( it.metrics.track(
Event.CollectionTabsAdded( Event.CollectionTabsAdded(
@ -378,10 +378,8 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler
sessionList.size sessionList.size
) )
) )
launch(Main) { tabTrayDialogStore.dispatch(TabTrayDialogFragmentAction.ExitMultiSelectMode)
tabTrayDialogStore.dispatch(TabTrayDialogFragmentAction.ExitMultiSelectMode) dialog.dismiss()
dialog.dismiss()
}
} }
}.setNegativeButton(android.R.string.cancel) { dialog, _ -> }.setNegativeButton(android.R.string.cancel) { dialog, _ ->
tabTrayDialogStore.dispatch(TabTrayDialogFragmentAction.ExitMultiSelectMode) tabTrayDialogStore.dispatch(TabTrayDialogFragmentAction.ExitMultiSelectMode)

@ -1,6 +1,7 @@
package org.mozilla.fenix.collections package org.mozilla.fenix.collections
import io.mockk.MockKAnnotations import io.mockk.MockKAnnotations
import io.mockk.coVerify
import io.mockk.every import io.mockk.every
import io.mockk.impl.annotations.MockK import io.mockk.impl.annotations.MockK
import io.mockk.mockk import io.mockk.mockk
@ -69,7 +70,7 @@ class DefaultCollectionCreationControllerTest {
controller.saveCollectionName(tabs, "name") controller.saveCollectionName(tabs, "name")
verify { dismiss() } verify { dismiss() }
verify { tabCollectionStorage.createCollection("name", listOf(session)) } coVerify { tabCollectionStorage.createCollection("name", listOf(session)) }
verify { metrics.track(Event.CollectionSaved(2, 1)) } verify { metrics.track(Event.CollectionSaved(2, 1)) }
} }
@ -117,9 +118,9 @@ class DefaultCollectionCreationControllerTest {
verifyAll { verifyAll {
dismiss() dismiss()
tabCollectionStorage.renameCollection(collection, "name")
metrics.track(Event.CollectionRenamed) metrics.track(Event.CollectionRenamed)
} }
coVerify { tabCollectionStorage.renameCollection(collection, "name") }
} }
@Test @Test
@ -162,7 +163,7 @@ class DefaultCollectionCreationControllerTest {
controller.selectCollection(collection, tabs) controller.selectCollection(collection, tabs)
verify { dismiss() } verify { dismiss() }
verify { tabCollectionStorage.addTabsToCollection(collection, listOf(session)) } coVerify { tabCollectionStorage.addTabsToCollection(collection, listOf(session)) }
verify { metrics.track(Event.CollectionTabsAdded(2, 1)) } verify { metrics.track(Event.CollectionTabsAdded(2, 1)) }
} }

@ -97,7 +97,7 @@ class DefaultDeleteBrowsingDataControllerTest {
fun deleteSitePermissions() = runBlockingTest { fun deleteSitePermissions() = runBlockingTest {
controller.deleteSitePermissions() controller.deleteSitePermissions()
verify { coVerify {
engine.clearData(Engine.BrowsingData.select(Engine.BrowsingData.ALL_SITE_SETTINGS)) engine.clearData(Engine.BrowsingData.select(Engine.BrowsingData.ALL_SITE_SETTINGS))
permissionStorage.deleteAllSitePermissions() permissionStorage.deleteAllSitePermissions()
} }

@ -9,7 +9,6 @@ package org.mozilla.fenix.settings.deletebrowsingdata
import io.mockk.coVerify import io.mockk.coVerify
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import io.mockk.verify
import io.mockk.verifyOrder import io.mockk.verifyOrder
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.TestCoroutineDispatcher
@ -72,7 +71,7 @@ class DeleteAndQuitTest {
activity.finish() activity.finish()
} }
verify(exactly = 0) { coVerify(exactly = 0) {
engine.clearData( engine.clearData(
Engine.BrowsingData.select( Engine.BrowsingData.select(
Engine.BrowsingData.COOKIES Engine.BrowsingData.COOKIES
@ -100,7 +99,7 @@ class DeleteAndQuitTest {
deleteAndQuit(activity, this, snackbar) deleteAndQuit(activity, this, snackbar)
verify(exactly = 1) { coVerify(exactly = 1) {
snackbar.show() snackbar.show()
engine.clearData(Engine.BrowsingData.allCaches()) engine.clearData(Engine.BrowsingData.allCaches())

@ -7,12 +7,12 @@ package org.mozilla.fenix.settings.quicksettings
import androidx.navigation.NavController import androidx.navigation.NavController
import androidx.navigation.NavDirections import androidx.navigation.NavDirections
import io.mockk.Runs import io.mockk.Runs
import io.mockk.coVerifyOrder
import io.mockk.every import io.mockk.every
import io.mockk.just import io.mockk.just
import io.mockk.mockk import io.mockk.mockk
import io.mockk.spyk import io.mockk.spyk
import io.mockk.verify import io.mockk.verify
import io.mockk.verifyOrder
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineScope import kotlinx.coroutines.test.TestCoroutineScope
import kotlinx.coroutines.test.runBlockingTest import kotlinx.coroutines.test.runBlockingTest
@ -179,7 +179,7 @@ class DefaultQuickSettingsControllerTest {
controller.handlePermissionsChange(testPermissions) controller.handlePermissionsChange(testPermissions)
advanceUntilIdle() advanceUntilIdle()
verifyOrder { coVerifyOrder {
permissionStorage.updateSitePermissions(testPermissions) permissionStorage.updateSitePermissions(testPermissions)
reload(browserSession) reload(browserSession)
} }

Loading…
Cancel
Save