diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 966935420..411692612 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -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. diff --git a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt index 6ba5dbd10..57f709748 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt @@ -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 diff --git a/app/src/main/java/org/mozilla/fenix/session/NotificationSessionObserver.kt b/app/src/main/java/org/mozilla/fenix/session/NotificationSessionObserver.kt index 2df2cdb88..9a3dbfd72 100644 --- a/app/src/main/java/org/mozilla/fenix/session/NotificationSessionObserver.kt +++ b/app/src/main/java/org/mozilla/fenix/session/NotificationSessionObserver.kt @@ -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() + private var scope: CoroutineScope? = null + private var started = false - if (privateTabsEmpty) { - notificationService.stop(context) + @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() } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt index c06b6918f..abf17ed3d 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt @@ -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 diff --git a/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt b/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt index b333bd675..9e35eef29 100644 --- a/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt +++ b/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt @@ -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 } diff --git a/app/src/test/java/org/mozilla/fenix/session/NotificationSessionObserverTest.kt b/app/src/test/java/org/mozilla/fenix/session/NotificationSessionObserverTest.kt index 1fa988b72..77d9e2bee 100644 --- a/app/src/test/java/org/mozilla/fenix/session/NotificationSessionObserverTest.kt +++ b/app/src/test/java/org/mozilla/fenix/session/NotificationSessionObserverTest.kt @@ -1,63 +1,82 @@ 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.onSessionAdded(normalSession) - verify(exactly = 0) { notificationService.start(any()) } + observer.start() + verify { notificationService wasNot Called } - observer.onSessionAdded(customSession) - verify(exactly = 0) { notificationService.start(any()) } + store.dispatch(TabListAction.AddTabAction(normalSession)).join() + verify(exactly = 0) { notificationService.start(context) } + + 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) } + + store.dispatch(CustomTabListAction.AddCustomTabAction(customSession)).join() + verify(exactly = 0) { notificationService.start(context) } } } - -private fun mockSession(isPrivate: Boolean, isCustom: Boolean) = mockk { - every { private } returns isPrivate - every { isCustomTabSession() } returns isCustom -} diff --git a/build.gradle b/build.gradle index e34a56126..609fb38a9 100644 --- a/build.gradle +++ b/build.gradle @@ -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" + ] } }