tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit ce3d2bb176efa1b020acd1146711c3d299094701
parent 96e31a4518043397d30ca4d43f2bfc4e87871a20
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date:   Wed,  5 Nov 2025 15:06:50 +0000

Bug 1996623 - Simplify HomeSettingsFragment and improve testability r=android-reviewers,giorga

This commit refactors `HomeSettingsFragment` to improve its testability and reduce boilerplate code. Key changes include:

- Injecting dependencies like `Settings`, `Components`, and `ContentRecommendationsFeatureHelper` to make the fragment easier to test.
- Introducing a `createMetricPreferenceChangeListener` helper function to remove duplicated logic for handling preference changes and metric reporting.
- Replacing the `SharedPreferenceUpdater` inner class with direct preference updates.
- Updating `HomeSettingsFragmentTest` to use the new dependency injection pattern for more robust and isolated unit tests, removing the need for static mocking of extension functions.

Differential Revision: https://phabricator.services.mozilla.com/D271382

Diffstat:
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt | 190++++++++++++++++++++++++++++++++++++-------------------------------------------
Mmobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/HomeSettingsFragmentTest.kt | 64+++++++++++++++++++++++++++-------------------------------------
2 files changed, 113 insertions(+), 141 deletions(-)

diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt @@ -4,7 +4,10 @@ package org.mozilla.fenix.settings +import android.content.Context import android.os.Bundle +import androidx.annotation.VisibleForTesting +import androidx.core.content.edit import androidx.navigation.findNavController import androidx.navigation.fragment.navArgs import androidx.preference.CheckBoxPreference @@ -13,21 +16,48 @@ import androidx.preference.PreferenceFragmentCompat import androidx.preference.SwitchPreference import org.mozilla.fenix.GleanMetrics.CustomizeHome import org.mozilla.fenix.R -import org.mozilla.fenix.components.appstate.AppAction.ContentRecommendationsAction +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.home.pocket.ContentRecommendationsFeatureHelper +import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.view.addToRadioGroup /** - * Lets the user customize the home screen. + * A [PreferenceFragmentCompat] that displays settings for customizing the Firefox home screen. + * + * User interactions with these preferences are persisted in [Settings] and may trigger + * telemetry events via [CustomizeHome] metrics. */ class HomeSettingsFragment : PreferenceFragmentCompat() { private val args by navArgs<HomeSettingsFragmentArgs>() + @VisibleForTesting + internal var customizeHomeMetrics: CustomizeHome = CustomizeHome + + @VisibleForTesting + internal var contentRecommendationsHelper: ContentRecommendationsFeatureHelper = ContentRecommendationsFeatureHelper + + @VisibleForTesting + internal lateinit var fenixSettings: Settings + + @VisibleForTesting + internal lateinit var fenixComponents: Components + + override fun onAttach(context: Context) { + super.onAttach(context) + if (!::fenixSettings.isInitialized) { + fenixSettings = context.settings() + } + if (!::fenixComponents.isInitialized) { + fenixComponents = context.components + } + } + override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) { setPreferencesFromResource(R.xml.home_preferences, rootKey) setupPreferences() @@ -43,128 +73,63 @@ class HomeSettingsFragment : PreferenceFragmentCompat() { private fun setupPreferences() { requirePreference<SwitchPreference>(R.string.pref_key_show_top_sites).apply { - isChecked = context.settings().showTopSitesFeature - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - CustomizeHome.preferenceToggled.record( - CustomizeHome.PreferenceToggledExtra( - newValue as Boolean, - "most_visited_sites", - ), - ) - - return super.onPreferenceChange(preference, newValue) - } - } + isChecked = fenixSettings.showTopSitesFeature + onPreferenceChangeListener = createMetricPreferenceChangeListener("most_visited_sites") } requirePreference<CheckBoxPreference>(R.string.pref_key_enable_contile).apply { - isChecked = context.settings().showContileFeature - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - CustomizeHome.preferenceToggled.record( - CustomizeHome.PreferenceToggledExtra( - newValue as Boolean, - "contile", - ), - ) - - return super.onPreferenceChange(preference, newValue) - } - } + isChecked = fenixSettings.showContileFeature + onPreferenceChangeListener = createMetricPreferenceChangeListener("contile") } requirePreference<SwitchPreference>(R.string.pref_key_recent_tabs).apply { - isVisible = requireContext().settings().showHomepageRecentTabsSectionToggle - isChecked = context.settings().showRecentTabsFeature - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - CustomizeHome.preferenceToggled.record( - CustomizeHome.PreferenceToggledExtra( - newValue as Boolean, - "jump_back_in", - ), - ) - - return super.onPreferenceChange(preference, newValue) - } - } + isVisible = fenixSettings.showHomepageRecentTabsSectionToggle + isChecked = fenixSettings.showRecentTabsFeature + onPreferenceChangeListener = createMetricPreferenceChangeListener("jump_back_in") } requirePreference<SwitchPreference>(R.string.pref_key_customization_bookmarks).apply { - isVisible = requireContext().settings().showHomepageBookmarksSectionToggle - isChecked = context.settings().showBookmarksHomeFeature - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - CustomizeHome.preferenceToggled.record( - CustomizeHome.PreferenceToggledExtra( - newValue as Boolean, - "bookmarks", - ), - ) - - return super.onPreferenceChange(preference, newValue) - } - } + isVisible = fenixSettings.showHomepageBookmarksSectionToggle + isChecked = fenixSettings.showBookmarksHomeFeature + onPreferenceChangeListener = createMetricPreferenceChangeListener("bookmarks") } requirePreference<SwitchPreference>(R.string.pref_key_pocket_homescreen_recommendations).apply { - isVisible = ContentRecommendationsFeatureHelper.isContentRecommendationsFeatureEnabled(context) - isChecked = context.settings().showPocketRecommendationsFeature - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - CustomizeHome.preferenceToggled.record( - CustomizeHome.PreferenceToggledExtra( - newValue as Boolean, - "pocket", - ), - ) - - return super.onPreferenceChange(preference, newValue) - } - } + isVisible = contentRecommendationsHelper.isContentRecommendationsFeatureEnabled(requireContext()) + isChecked = fenixSettings.showPocketRecommendationsFeature + onPreferenceChangeListener = createMetricPreferenceChangeListener("pocket") } requirePreference<CheckBoxPreference>(R.string.pref_key_pocket_sponsored_stories).apply { - isVisible = ContentRecommendationsFeatureHelper.isPocketSponsoredStoriesFeatureEnabled(context) - isChecked = context.settings().showPocketSponsoredStories - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - when (newValue) { - true -> { - context.components.core.pocketStoriesService.startPeriodicSponsoredContentsRefresh() - } - false -> { - context.components.core.pocketStoriesService.deleteUser() - - context.components.appStore.dispatch( - ContentRecommendationsAction.SponsoredContentsChange( - sponsoredContents = emptyList(), - ), - ) - } + isVisible = contentRecommendationsHelper.isPocketSponsoredStoriesFeatureEnabled(requireContext()) + isChecked = fenixSettings.showPocketSponsoredStories + onPreferenceChangeListener = Preference.OnPreferenceChangeListener { preference, newValue -> + val newBooleanValue = newValue as? Boolean ?: return@OnPreferenceChangeListener false + + when (newBooleanValue) { + true -> { + fenixComponents.core.pocketStoriesService.startPeriodicSponsoredContentsRefresh() + } + false -> { + fenixComponents.core.pocketStoriesService.deleteUser() + + fenixComponents.appStore.dispatch( + AppAction.ContentRecommendationsAction.SponsoredContentsChange( + sponsoredContents = emptyList(), + ), + ) } - - return super.onPreferenceChange(preference, newValue) } + + fenixSettings.preferences.edit { putBoolean(preference.key, newBooleanValue) } + true } } requirePreference<SwitchPreference>(R.string.pref_key_history_metadata_feature).apply { - isVisible = requireContext().settings().showHomepageRecentlyVisitedSectionToggle - isChecked = context.settings().historyMetadataUIFeature - onPreferenceChangeListener = object : SharedPreferenceUpdater() { - override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { - CustomizeHome.preferenceToggled.record( - CustomizeHome.PreferenceToggledExtra( - newValue as Boolean, - "recently_visited", - ), - ) - - return super.onPreferenceChange(preference, newValue) - } - } + isVisible = fenixSettings.showHomepageRecentlyVisitedSectionToggle + isChecked = fenixSettings.historyMetadataUIFeature + onPreferenceChangeListener = createMetricPreferenceChangeListener("recently_visited") } val openingScreenRadioHomepage = @@ -180,7 +145,7 @@ class HomeSettingsFragment : PreferenceFragmentCompat() { directions = HomeSettingsFragmentDirections.actionHomeSettingsFragmentToWallpaperSettingsFragment(), navigateFrom = "HomeSettingsFragment", navigateTo = "ActionHomeSettingsFragmentToWallpaperSettingsFragment", - crashReporter = context.components.analytics.crashReporter, + crashReporter = fenixComponents.analytics.crashReporter, ) true } @@ -192,4 +157,21 @@ class HomeSettingsFragment : PreferenceFragmentCompat() { openingScreenAfterFourHours, ) } + + private fun createMetricPreferenceChangeListener(metricKey: String): Preference.OnPreferenceChangeListener { + return Preference.OnPreferenceChangeListener { preference, newValue -> + val newBooleanValue = newValue as? Boolean ?: return@OnPreferenceChangeListener false + + customizeHomeMetrics.preferenceToggled.record( + CustomizeHome.PreferenceToggledExtra( + newBooleanValue, + metricKey, + ), + ) + + fenixSettings.preferences.edit { putBoolean(preference.key, newBooleanValue) } + + true + } + } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/HomeSettingsFragmentTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/HomeSettingsFragmentTest.kt @@ -4,19 +4,13 @@ package org.mozilla.fenix.settings -import android.content.Context import android.content.SharedPreferences import androidx.fragment.app.FragmentActivity import androidx.preference.CheckBoxPreference import io.mockk.every import io.mockk.mockk -import io.mockk.mockkObject -import io.mockk.mockkStatic -import io.mockk.unmockkStatic import io.mockk.verify import mozilla.components.service.pocket.PocketStoriesService -import mozilla.components.support.test.robolectric.testContext -import org.junit.After import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -24,10 +18,10 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.components.Core import org.mozilla.fenix.components.appstate.AppAction.ContentRecommendationsAction -import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey -import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.pocket.ContentRecommendationsFeatureHelper import org.mozilla.fenix.utils.Settings import org.robolectric.Robolectric @@ -41,11 +35,10 @@ internal class HomeSettingsFragmentTest { private lateinit var appPrefsEditor: SharedPreferences.Editor private lateinit var pocketService: PocketStoriesService private lateinit var appStore: AppStore + private lateinit var contentRecommendationsHelper: ContentRecommendationsFeatureHelper @Before fun setup() { - mockkStatic("org.mozilla.fenix.ext.FragmentKt") - mockkStatic("org.mozilla.fenix.ext.ContextKt") appPrefsEditor = mockk(relaxed = true) appPrefs = mockk(relaxed = true) { every { edit() } returns appPrefsEditor @@ -53,43 +46,27 @@ internal class HomeSettingsFragmentTest { appSettings = mockk(relaxed = true) { every { preferences } returns appPrefs } - every { any<Context>().settings() } returns appSettings appStore = mockk(relaxed = true) pocketService = mockk(relaxed = true) - every { any<Context>().components } returns mockk { - every { appStore } returns this@HomeSettingsFragmentTest.appStore - every { core.pocketStoriesService } returns pocketService - } - - homeSettingsFragment = HomeSettingsFragment() - } - - @After - fun teardown() { - unmockkStatic("org.mozilla.fenix.ext.ContextKt") - unmockkStatic("org.mozilla.fenix.ext.FragmentKt") + contentRecommendationsHelper = mockk(relaxed = true) } @Test fun `GIVEN the Pocket sponsored stories feature is disabled for the app WHEN accessing settings THEN the settings for it are not visible`() { - mockkObject(ContentRecommendationsFeatureHelper) { - every { ContentRecommendationsFeatureHelper.isPocketSponsoredStoriesFeatureEnabled(any()) } returns false + every { contentRecommendationsHelper.isPocketSponsoredStoriesFeatureEnabled(any()) } returns false - activateFragment() + activateFragment() - assertFalse(getSponsoredStoriesPreference().isVisible) - } + assertFalse(getSponsoredStoriesPreference().isVisible) } @Test fun `GIVEN the Pocket sponsored stories feature is enabled for the app WHEN accessing settings THEN the settings for it are visible`() { - mockkObject(ContentRecommendationsFeatureHelper) { - every { ContentRecommendationsFeatureHelper.isPocketSponsoredStoriesFeatureEnabled(any()) } returns true + every { contentRecommendationsHelper.isPocketSponsoredStoriesFeatureEnabled(any()) } returns true - activateFragment() + activateFragment() - assertTrue(getSponsoredStoriesPreference().isVisible) - } + assertTrue(getSponsoredStoriesPreference().isVisible) } @Test @@ -113,12 +90,11 @@ internal class HomeSettingsFragmentTest { @Test fun `GIVEN sponsored stories is disabled WHEN toggling the sponsored setting to enabled THEN start downloading sponsored stories`() { activateFragment() - val result = getSponsoredStoriesPreference().callChangeListener(true) assertTrue(result) verify { - appPrefsEditor.putBoolean(testContext.getString(R.string.pref_key_pocket_sponsored_stories), true) + appPrefsEditor.putBoolean(homeSettingsFragment.getString(R.string.pref_key_pocket_sponsored_stories), true) pocketService.startPeriodicSponsoredContentsRefresh() } } @@ -126,12 +102,11 @@ internal class HomeSettingsFragmentTest { @Test fun `GIVEN sponsored stories is enabled WHEN toggling the sponsored stories setting to disabled THEN delete Pocket profile and remove sponsored contents from showing`() { activateFragment() - val result = getSponsoredStoriesPreference().callChangeListener(false) assertTrue(result) verify { - appPrefsEditor.putBoolean(testContext.getString(R.string.pref_key_pocket_sponsored_stories), false) + appPrefsEditor.putBoolean(homeSettingsFragment.getString(R.string.pref_key_pocket_sponsored_stories), false) pocketService.deleteUser() appStore.dispatch( ContentRecommendationsAction.SponsoredContentsChange( @@ -143,6 +118,21 @@ internal class HomeSettingsFragmentTest { private fun activateFragment() { val activity = Robolectric.buildActivity(FragmentActivity::class.java).create().get() + homeSettingsFragment = HomeSettingsFragment() + + val mockCore: Core = mockk { + every { pocketStoriesService } returns this@HomeSettingsFragmentTest.pocketService + } + val mockComponents: Components = mockk(relaxed = true) { + every { appStore } returns this@HomeSettingsFragmentTest.appStore + every { core } returns mockCore + every { settings } returns this@HomeSettingsFragmentTest.appSettings + } + + homeSettingsFragment.fenixSettings = appSettings + homeSettingsFragment.fenixComponents = mockComponents + homeSettingsFragment.contentRecommendationsHelper = contentRecommendationsHelper + activity.supportFragmentManager.beginTransaction() .add(homeSettingsFragment, "HomeSettingFragmentTest") .commitNow()