For #24130 - Change Shortcuts setting to toggle the display of Top Sites

app/src/main/java/org/mozilla/fenix/FenixApplication.kt

@@ -262,10 +262,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
                         // we can prevent with this.
                         components.core.topSitesStorage.getTopSites(
                             totalSites = components.settings.topSitesMaxLimit,
-                            frecencyConfig = if (components.settings.showTopFrecentSites)
+                            frecencyConfig = FrecencyThresholdOption.SKIP_ONE_TIME_PAGES,
-                                FrecencyThresholdOption.SKIP_ONE_TIME_PAGES
-                            else
-                                null,
                             providerConfig = TopSitesProviderConfig(
                                 showProviderTopSites = components.settings.showContileFeature,
                                 maxThreshold = TOP_SITES_PROVIDER_MAX_THRESHOLD
@@ -758,7 +755,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
             override fun onUpdatesApplied(updated: List<EnrolledExperiment>) {
                 CustomizeHome.jumpBackIn.set(settings.showRecentTabsFeature)
                 CustomizeHome.recentlySaved.set(settings.showRecentBookmarksFeature)
-                CustomizeHome.mostVisitedSites.set(settings.showTopFrecentSites)
+                CustomizeHome.mostVisitedSites.set(settings.showTopSitesFeature)
                 CustomizeHome.recentlyVisited.set(settings.historyMetadataUIFeature)
                 CustomizeHome.pocket.set(settings.showPocketRecommendationsFeature)
             }

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -400,7 +400,7 @@ sealed class Event {
         val context: Context
     ) : Event() {
         private val telemetryAllowMap = mapOf(
-            context.getString(R.string.pref_key_enable_top_frecent_sites) to "most_visited_sites",
+            context.getString(R.string.pref_key_show_top_sites) to "most_visited_sites",
             context.getString(R.string.pref_key_recent_tabs) to "jump_back_in",
             context.getString(R.string.pref_key_recent_bookmarks) to "recently_saved",
             context.getString(R.string.pref_key_history_metadata_feature) to "recently_visited",

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -64,6 +64,7 @@ import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.feature.tab.collections.TabCollection
+import mozilla.components.feature.top.sites.TopSite
 import mozilla.components.feature.top.sites.TopSitesConfig
 import mozilla.components.feature.top.sites.TopSitesFeature
 import mozilla.components.feature.top.sites.TopSitesProviderConfig
@@ -244,7 +245,7 @@ class HomeFragment : Fragment() {
                     collections = components.core.tabCollectionStorage.cachedTabCollections,
                     expandedCollections = emptySet(),
                     mode = currentMode.getCurrentMode(),
-                    topSites = components.core.topSitesStorage.cachedTopSites.sort(),
+                    topSites = getTopSites(components),
                     tip = components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) {
                         FenixTipManager(
                             listOf(
@@ -288,18 +289,20 @@ class HomeFragment : Fragment() {
             }
         }
 
-        topSitesFeature.set(
+        if (requireContext().settings().showTopSitesFeature) {
-            feature = TopSitesFeature(
+            topSitesFeature.set(
-                view = DefaultTopSitesView(
+                feature = TopSitesFeature(
-                    store = homeFragmentStore,
+                    view = DefaultTopSitesView(
-                    settings = components.settings
+                        store = homeFragmentStore,
+                        settings = components.settings
+                    ),
+                    storage = components.core.topSitesStorage,
+                    config = ::getTopSitesConfig
                 ),
-                storage = components.core.topSitesStorage,
+                owner = viewLifecycleOwner,
-                config = ::getTopSitesConfig
+                view = binding.root
-            ),
+            )
-            owner = viewLifecycleOwner,
+        }
-            view = binding.root
-        )
 
         if (requireContext().settings().showRecentTabsFeature) {
             recentTabsListFeature.set(
@@ -434,7 +437,7 @@ class HomeFragment : Fragment() {
         val settings = requireContext().settings()
         return TopSitesConfig(
             totalSites = settings.topSitesMaxLimit,
-            frecencyConfig = if (settings.showTopFrecentSites) FrecencyThresholdOption.SKIP_ONE_TIME_PAGES else null,
+            frecencyConfig = FrecencyThresholdOption.SKIP_ONE_TIME_PAGES,
             providerConfig = TopSitesProviderConfig(
                 showProviderTopSites = settings.showContileFeature,
                 maxThreshold = TOP_SITES_PROVIDER_MAX_THRESHOLD
@@ -704,7 +707,7 @@ class HomeFragment : Fragment() {
             HomeFragmentAction.Change(
                 collections = components.core.tabCollectionStorage.cachedTabCollections,
                 mode = currentMode.getCurrentMode(),
-                topSites = components.core.topSitesStorage.cachedTopSites.sort(),
+                topSites = getTopSites(components),
                 tip = components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) {
                     FenixTipManager(
                         listOf(
@@ -1225,6 +1228,14 @@ class HomeFragment : Fragment() {
             ?.isVisible = tabCount > 0
     }
 
+    private fun getTopSites(components: Components): List<TopSite> {
+        return if (components.settings.showTopSitesFeature) {
+            components.core.topSitesStorage.cachedTopSites.sort()
+        } else {
+            emptyList()
+        }
+    }
+
     private fun getRecentTabs(components: Components): List<RecentTab> {
         return if (components.settings.showRecentTabsFeature) {
             components.core.store.state.asRecentTabs()

app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt

@@ -34,8 +34,8 @@ class HomeSettingsFragment : PreferenceFragmentCompat() {
     }
 
     private fun setupPreferences() {
-        requirePreference<SwitchPreference>(R.string.pref_key_enable_top_frecent_sites).apply {
+        requirePreference<SwitchPreference>(R.string.pref_key_show_top_sites).apply {
-            isChecked = context.settings().showTopFrecentSites
+            isChecked = context.settings().showTopSitesFeature
             onPreferenceChangeListener = CustomizeHomeMetricsUpdater()
         }
 

app/src/main/java/org/mozilla/fenix/utils/Settings.kt

@@ -127,8 +127,11 @@ class Settings(private val appContext: Context) : PreferencesHolder {
     override val preferences: SharedPreferences =
         appContext.getSharedPreferences(FENIX_PREFERENCES, MODE_PRIVATE)
 
-    var showTopFrecentSites by lazyFeatureFlagPreference(
+    /**
-        appContext.getPreferenceKey(R.string.pref_key_enable_top_frecent_sites),
+     * Indicates whether or not top sites should be shown on the home screen.
+     */
+    var showTopSitesFeature by lazyFeatureFlagPreference(
+        appContext.getPreferenceKey(R.string.pref_key_show_top_sites),
         featureFlag = true,
         default = { homescreenSections[HomeScreenSection.TOP_SITES] == true },
     )

app/src/main/res/values/preference_keys.xml

@@ -203,8 +203,8 @@
     <string name="pref_key_top_sites_size" translatable="false">pref_key_top_sites_size</string>
     <!-- Maximum number of top sites to display -->
     <string name="pref_key_top_sites_max_limit" translatable="false">pref_key_top_sites_max_limit</string>
-    <!-- Whether or not the top frecent sites are shown along with pinned top sites -->
+    <!-- Whether or not the top sites section is shown on the home screen -->
-    <string name="pref_key_enable_top_frecent_sites" translatable="false">pref_key_top_frecent_sites</string>
+    <string name="pref_key_show_top_sites" translatable="false">pref_key_show_top_sites</string>
     <!-- Whether or not the sponsored shortcuts are shown along with the top recent sites shortcuts -->
     <string name="pref_key_enable_contile" translatable="false">pref_key_enable_contile</string>
     <!-- Whether or not the Task Continuity enhancements are shown -->

app/src/main/res/xml/home_preferences.xml

@@ -6,11 +6,11 @@
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:app="http://schemas.android.com/apk/res-auto">
     <androidx.preference.SwitchPreference
-        android:key="@string/pref_key_enable_top_frecent_sites"
+        android:key="@string/pref_key_show_top_sites"
         android:title="@string/top_sites_toggle_top_recent_sites_4" />
 
     <androidx.preference.CheckBoxPreference
-        android:dependency="@string/pref_key_enable_top_frecent_sites"
+        android:dependency="@string/pref_key_show_top_sites"
         android:layout="@layout/checkbox_left_sub_preference"
         android:key="@string/pref_key_enable_contile"
         android:title="@string/customize_toggle_contile"

app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt

@@ -137,7 +137,7 @@ class FenixApplicationTest {
         every { settings.signedInFxaAccount } returns true
         every { settings.showRecentTabsFeature } returns true
         every { settings.showRecentBookmarksFeature } returns true
-        every { settings.showTopFrecentSites } returns true
+        every { settings.showTopSitesFeature } returns true
         every { settings.historyMetadataUIFeature } returns true
         every { settings.showPocketRecommendationsFeature } returns true
         every { settings.showPocketRecommendationsFeature } returns true

app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt

@@ -10,7 +10,8 @@ import io.mockk.mockk
 import io.mockk.spyk
 import io.mockk.verify
 import mozilla.components.browser.menu.view.MenuButton
-import org.junit.Assert
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
 import org.junit.Before
 import org.junit.Test
 import org.mozilla.fenix.FenixApplication
@@ -39,23 +40,12 @@ class HomeFragmentTest {
     }
 
     @Test
-    fun `GIVEN showTopFrecentSites is false WHEN getTopSitesConfig is called THEN it returns TopSitesConfig with null frecencyConfig`() {
+    fun `WHEN getTopSitesConfig is called THEN it returns TopSitesConfig with non-null frecencyConfig`() {
-        every { settings.showTopFrecentSites } returns false
         every { settings.topSitesMaxLimit } returns 10
 
         val topSitesConfig = homeFragment.getTopSitesConfig()
 
-        Assert.assertNull(topSitesConfig.frecencyConfig)
+        assertNotNull(topSitesConfig.frecencyConfig)
-    }
-
-    @Test
-    fun `GIVEN showTopFrecentSites is true WHEN getTopSitesConfig is called THEN it returns TopSitesConfig with non-null frecencyConfig`() {
-        every { settings.showTopFrecentSites } returns true
-        every { settings.topSitesMaxLimit } returns 10
-
-        val topSitesConfig = homeFragment.getTopSitesConfig()
-
-        Assert.assertNotNull(topSitesConfig.frecencyConfig)
     }
 
     @Test
@@ -65,7 +55,7 @@ class HomeFragmentTest {
 
         val topSitesConfig = homeFragment.getTopSitesConfig()
 
-        Assert.assertEquals(topSitesMaxLimit, topSitesConfig.totalSites)
+        assertEquals(topSitesMaxLimit, topSitesConfig.totalSites)
     }
 
     @Test