Unverified Commit a7e74e30 authored by Tiger Oakes's avatar Tiger Oakes Committed by GitHub
Browse files

Migrate some SessionManager usage to BrowserStore (#10789)

parent 9dd59880
......@@ -13,6 +13,7 @@ import androidx.appcompat.app.AppCompatDelegate
import androidx.core.content.getSystemService
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.async
import kotlinx.coroutines.launch
......@@ -21,6 +22,7 @@ import mozilla.appservices.Megazord
import mozilla.components.browser.session.Session
import mozilla.components.concept.push.PushProcessor
import mozilla.components.feature.addons.update.GlobalAddonDependencyProvider
import mozilla.components.lib.crash.CrashReporter
import mozilla.components.service.glean.Glean
import mozilla.components.service.glean.config.Configuration
import mozilla.components.service.glean.net.ConceptFetchHttpUploader
......@@ -46,14 +48,12 @@ import org.mozilla.fenix.session.PerformanceActivityLifecycleCallbacks
import org.mozilla.fenix.session.VisibilityLifecycleCallback
import org.mozilla.fenix.utils.BrowsersCache
import org.mozilla.fenix.utils.Settings
import mozilla.components.lib.crash.CrashReporter
/**
*The main application class for Fenix. Records data to measure initialization performance.
* Installs [CrashReporter], initializes [Glean] in fenix builds and setup Megazord in the main process.
*/
@SuppressLint("Registered")
@Suppress("TooManyFunctions", "LargeClass")
@Suppress("Registered", "TooManyFunctions", "LargeClass")
open class FenixApplication : LocaleAwareApplication() {
init {
recordOnInit() // DO NOT MOVE ANYTHING ABOVE HERE: the timing of this measurement is critical.
......@@ -66,6 +66,7 @@ open class FenixApplication : LocaleAwareApplication() {
var visibilityLifecycleCallback: VisibilityLifecycleCallback? = null
private set
@ExperimentalCoroutinesApi
override fun onCreate() {
super.onCreate()
......@@ -114,6 +115,7 @@ open class FenixApplication : LocaleAwareApplication() {
Log.addSink(AndroidLogSink())
}
@ExperimentalCoroutinesApi
@CallSuper
open fun setupInMainProcessOnly() {
run {
......@@ -151,7 +153,8 @@ open class FenixApplication : LocaleAwareApplication() {
visibilityLifecycleCallback = VisibilityLifecycleCallback(getSystemService())
registerActivityLifecycleCallbacks(visibilityLifecycleCallback)
components.core.sessionManager.register(NotificationSessionObserver(this))
val privateNotificationObserver = NotificationSessionObserver(this)
privateNotificationObserver.start()
// Storage maintenance disabled, for now, as it was interfering with background migrations.
// See https://github.com/mozilla-mobile/fenix/issues/7227 for context.
......
......@@ -18,6 +18,7 @@ import androidx.navigation.fragment.findNavController
import kotlinx.android.synthetic.main.fragment_installed_add_on_details.view.*
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.AddonManagerException
import mozilla.components.feature.addons.ui.translatedName
......@@ -189,7 +190,7 @@ class InstalledAddonDetailsFragment : Fragment() {
val directions = if (addon.installedState?.openOptionsPageInTab == true) {
val components = it.context.components
val shouldCreatePrivateSession =
components.core.sessionManager.selectedSession?.private
components.core.store.state.selectedTab?.content?.private
?: Settings.instance?.openLinksInAPrivateTab
?: false
......
......@@ -5,38 +5,46 @@
package org.mozilla.fenix.session
import android.content.Context
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map
import mozilla.components.browser.state.selector.privateTabs
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.sessionsOfType
/**
* This observer starts and stops the service to show a notification
* indicating that a private tab is open.
*/
class NotificationSessionObserver(
private val context: Context,
private val notificationService: SessionNotificationService.Companion = SessionNotificationService
) : SessionManager.Observer {
override fun onSessionRemoved(session: Session) {
val privateTabsEmpty = context.components.core.sessionManager.sessionsOfType(private = true).none()
if (privateTabsEmpty) {
notificationService.stop(context)
) {
private var scope: CoroutineScope? = null
private var started = false
@ExperimentalCoroutinesApi
fun start() {
scope = context.components.core.store.flowScoped { flow ->
flow.map { state -> state.privateTabs.isNotEmpty() }
.ifChanged()
.collect { hasPrivateTabs ->
if (hasPrivateTabs) {
notificationService.start(context)
started = true
} else if (started) {
notificationService.stop(context)
started = false
}
}
}
}
override fun onAllSessionsRemoved() {
notificationService.stop(context)
}
override fun onSessionAdded(session: Session) {
// Custom tabs are meant to feel like part of the app that opened them, not Fenix, so we
// don't need to show a 'close tab' notification for them
if (session.private && !session.isCustomTabSession()) {
notificationService.start(context)
}
fun stop() {
scope?.cancel()
}
}
......@@ -13,10 +13,15 @@ import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController
import kotlinx.android.synthetic.main.fragment_delete_browsing_data.*
import kotlinx.android.synthetic.main.fragment_delete_browsing_data.view.*
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event
......@@ -25,23 +30,15 @@ import org.mozilla.fenix.ext.showToolbar
@SuppressWarnings("TooManyFunctions")
class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_data) {
private lateinit var sessionObserver: SessionManager.Observer
private lateinit var controller: DeleteBrowsingDataController
private var scope: CoroutineScope? = null
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
controller = DefaultDeleteBrowsingDataController(requireContext())
sessionObserver = object : SessionManager.Observer {
override fun onSessionAdded(session: Session) = updateTabCount()
override fun onSessionRemoved(session: Session) = updateTabCount()
override fun onSessionSelected(session: Session) = updateTabCount()
override fun onSessionsRestored() = updateTabCount()
override fun onAllSessionsRemoved() = updateTabCount()
}
requireComponents.core.sessionManager.register(sessionObserver, owner = this)
getCheckboxes().forEach {
it.onCheckListener = { _ -> updateDeleteButton() }
}
......@@ -53,11 +50,15 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da
}
}
private fun updateDeleteButton() {
val enabled = getCheckboxes().any { it.isChecked }
@ExperimentalCoroutinesApi
override fun onStart() {
super.onStart()
view?.delete_data?.isEnabled = enabled
view?.delete_data?.alpha = if (enabled) ENABLED_ALPHA else DISABLED_ALPHA
scope = requireComponents.core.store.flowScoped(viewLifecycleOwner) { flow ->
flow.map { state -> state.tabs.size }
.ifChanged()
.collect { openTabs -> updateTabCount(openTabs) }
}
}
override fun onResume() {
......@@ -71,6 +72,13 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da
updateItemCounts()
}
private fun updateDeleteButton() {
val enabled = getCheckboxes().any { it.isChecked }
view?.delete_data?.isEnabled = enabled
view?.delete_data?.alpha = if (enabled) ENABLED_ALPHA else DISABLED_ALPHA
}
private fun askToDelete() {
context?.let {
AlertDialog.Builder(it).apply {
......@@ -162,6 +170,11 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da
progress_bar.visibility = View.GONE
}
override fun onStop() {
super.onStop()
scope?.cancel()
}
private fun updateItemCounts() {
updateTabCount()
updateHistoryCount()
......@@ -170,9 +183,8 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da
updateSitePermissions()
}
private fun updateTabCount() {
private fun updateTabCount(openTabs: Int = requireComponents.core.store.state.tabs.size) {
view?.open_tabs_item?.apply {
val openTabs = requireComponents.core.sessionManager.sessions.size
subtitleView.text = resources.getString(
R.string.preferences_delete_browsing_data_tabs_subtitle,
openTabs
......
......@@ -4,6 +4,7 @@
package org.mozilla.fenix.helpers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import org.mozilla.fenix.FenixApplication
import org.mozilla.fenix.components.TestComponents
......@@ -18,5 +19,6 @@ class FenixRobolectricTestApplication : FenixApplication() {
override fun setupInAllProcesses() = Unit
@ExperimentalCoroutinesApi
override fun setupInMainProcessOnly() = Unit
}
package org.mozilla.fenix.session
import android.content.Context
import io.mockk.Called
import io.mockk.MockKAnnotations
import io.mockk.confirmVerified
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.mockk
import io.mockk.verify
import mozilla.components.support.test.robolectric.testContext
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import mozilla.components.browser.state.action.CustomTabListAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.state.createCustomTab
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import mozilla.components.browser.session.Session
@ExperimentalCoroutinesApi
@RunWith(FenixRobolectricTestRunner::class)
class NotificationSessionObserverTest {
private lateinit var observer: NotificationSessionObserver
private lateinit var store: BrowserStore
@MockK private lateinit var context: Context
@MockK(relaxed = true) private lateinit var notificationService: SessionNotificationService.Companion
@Before
fun before() {
MockKAnnotations.init(this)
observer = NotificationSessionObserver(testContext, notificationService)
store = BrowserStore()
every { context.components.core.store } returns store
observer = NotificationSessionObserver(context, notificationService)
}
@Test
fun `GIVEN session is private and non-custom WHEN it is added THEN notification service should be started`() {
val privateSession = mockSession(true, false)
fun `GIVEN session is private and non-custom WHEN it is added THEN notification service should be started`() = runBlocking {
val privateSession = createTab("https://firefox.com", private = true)
observer.onSessionAdded(privateSession)
verify(exactly = 1) { notificationService.start(any()) }
store.dispatch(TabListAction.AddTabAction(privateSession)).join()
observer.start()
verify(exactly = 1) { notificationService.start(context) }
confirmVerified(notificationService)
}
@Test
fun `GIVEN session is not private WHEN it is added THEN notification service should not be started`() {
val normalSession = mockSession(false, true)
val customSession = mockSession(false, false)
fun `GIVEN session is not private WHEN it is added THEN notification service should not be started`() = runBlocking {
val normalSession = createTab("https://firefox.com")
val customSession = createCustomTab("https://firefox.com")
observer.start()
verify { notificationService wasNot Called }
observer.onSessionAdded(normalSession)
verify(exactly = 0) { notificationService.start(any()) }
store.dispatch(TabListAction.AddTabAction(normalSession)).join()
verify(exactly = 0) { notificationService.start(context) }
observer.onSessionAdded(customSession)
verify(exactly = 0) { notificationService.start(any()) }
store.dispatch(CustomTabListAction.AddCustomTabAction(customSession)).join()
verify(exactly = 0) { notificationService.start(context) }
}
@Test
fun `GIVEN session is custom tab WHEN it is added THEN notification service should not be started`() {
val privateCustomSession = mockSession(true, true)
val customSession = mockSession(false, true)
fun `GIVEN session is custom tab WHEN it is added THEN notification service should not be started`() = runBlocking {
val privateCustomSession = createCustomTab("https://firefox.com").let {
it.copy(content = it.content.copy(private = true))
}
val customSession = createCustomTab("https://firefox.com")
observer.onSessionAdded(privateCustomSession)
verify(exactly = 0) { notificationService.start(any()) }
observer.start()
verify { notificationService wasNot Called }
observer.onSessionAdded(customSession)
verify(exactly = 0) { notificationService.start(any()) }
}
}
store.dispatch(CustomTabListAction.AddCustomTabAction(privateCustomSession)).join()
verify(exactly = 0) { notificationService.start(context) }
private fun mockSession(isPrivate: Boolean, isCustom: Boolean) = mockk<Session> {
every { private } returns isPrivate
every { isCustomTabSession() } returns isCustom
store.dispatch(CustomTabListAction.AddCustomTabAction(customSession)).join()
verify(exactly = 0) { notificationService.start(context) }
}
}
......@@ -130,7 +130,10 @@ allprojects {
tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all {
kotlinOptions.jvmTarget = "1.8"
kotlinOptions.allWarningsAsErrors = true
kotlinOptions.freeCompilerArgs += ["-Xuse-experimental=kotlin.Experimental", "-Xskip-runtime-version-check"]
kotlinOptions.freeCompilerArgs += [
"-Xuse-experimental=kotlin.Experimental",
"-Xskip-runtime-version-check"
]
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment