[fenix] Set a minimum number of sites a search group should contain.

app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt

@@ -17,6 +17,7 @@ import org.mozilla.fenix.library.history.History
 import org.mozilla.fenix.library.history.HistoryDataSource
 import org.mozilla.fenix.library.history.HistoryItemTimeGroup
 import org.mozilla.fenix.perf.runBlockingIncrement
+import org.mozilla.fenix.utils.Settings.Companion.SEARCH_GROUP_MINIMUM_SITES
 import kotlin.math.abs
 
 private const val BUFFER_TIME = 15000 /* 15 seconds in ms */
@@ -89,6 +90,7 @@ interface PagedHistoryProvider {
 class DefaultPagedHistoryProvider(
     private val historyStorage: PlacesHistoryStorage,
     private val showHistorySearchGroups: Boolean = FeatureFlags.showHistorySearchGroups,
+    private val historyImprovementFeatures: Boolean = FeatureFlags.historyImprovementFeatures,
 ) : PagedHistoryProvider {
 
     val urlSet = Array<MutableSet<String>>(HistoryItemTimeGroup.values().size) { mutableSetOf() }
@@ -136,6 +138,7 @@ class DefaultPagedHistoryProvider(
                 // in the case of a pull to refresh.
                 if (historyGroups == null || offset == 0) {
                     historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE)
+                        .asSequence()
                         .sortedByDescending { it.createdAt }
                         .filter { it.key.searchTerm != null }
                         .groupBy { it.key.searchTerm!! }
@@ -146,6 +149,14 @@ class DefaultPagedHistoryProvider(
                                 items = items.map { it.toHistoryDBMetadata() }
                             )
                         }
+                        .filter {
+                            if (historyImprovementFeatures) {
+                                it.items.size >= SEARCH_GROUP_MINIMUM_SITES
+                            } else {
+                                true
+                            }
+                        }
+                        .toList()
                 }
 
                 history = getHistoryAndSearchGroups(offset, numberOfItems)

app/src/main/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeature.kt

@@ -17,6 +17,8 @@ import mozilla.components.concept.storage.HistoryHighlightWeights
 import mozilla.components.concept.storage.HistoryMetadata
 import mozilla.components.concept.storage.HistoryMetadataStorage
 import mozilla.components.support.base.feature.LifecycleAwareFeature
+import org.mozilla.fenix.FeatureFlags
+import org.mozilla.fenix.FeatureFlags.historyImprovementFeatures
 import org.mozilla.fenix.home.HomeFragment
 import org.mozilla.fenix.home.HomeFragmentAction
 import org.mozilla.fenix.home.HomeFragmentStore
@@ -24,6 +26,7 @@ import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryGrou
 import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryHighlight
 import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItemInternal.HistoryGroupInternal
 import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItemInternal.HistoryHighlightInternal
+import org.mozilla.fenix.utils.Settings.Companion.SEARCH_GROUP_MINIMUM_SITES
 import kotlin.math.max
 
 @VisibleForTesting internal const val MAX_RESULTS_TOTAL = 9
@@ -48,6 +51,7 @@ class RecentVisitsFeature(
     private val historyHighlightsStorage: Lazy<PlacesHistoryStorage>,
     private val scope: CoroutineScope,
     private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
+    private val historyImprovementFeatures: Boolean = FeatureFlags.historyImprovementFeatures,
 ) : LifecycleAwareFeature {
 
     private var job: Job? = null
@@ -208,6 +212,13 @@ class RecentVisitsFeature(
                     groupItems = it.value
                 )
             }
+            .filter {
+                if (historyImprovementFeatures) {
+                    it.groupItems.size >= SEARCH_GROUP_MINIMUM_SITES
+                } else {
+                    true
+                }
+            }
     }
 
     /**

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

@@ -27,6 +27,7 @@ import mozilla.components.support.ktx.android.content.stringPreference
 import org.mozilla.fenix.BuildConfig
 import org.mozilla.fenix.Config
 import org.mozilla.fenix.FeatureFlags
+import org.mozilla.fenix.FeatureFlags.historyImprovementFeatures
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
 import org.mozilla.fenix.components.metrics.MozillaProductDetector
@@ -76,6 +77,12 @@ class Settings(private val appContext: Context) : PreferencesHolder {
         const val ONE_WEEK_MS = 60 * 60 * 24 * 7 * 1000L
         const val ONE_MONTH_MS = (60 * 60 * 24 * 365 * 1000L) / 12
 
+        /**
+         * The minimum number a search groups should contain.
+         * Filtering is applied depending on the [historyImprovementFeatures] flag value.
+         */
+        const val SEARCH_GROUP_MINIMUM_SITES: Int = 2
+
         private fun Action.toInt() = when (this) {
             Action.BLOCKED -> BLOCKED_INT
             Action.ASK_TO_ALLOW -> ASK_TO_ALLOW_INT

app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt

@@ -30,7 +30,8 @@ class PagedHistoryProviderTest {
     fun `getHistory uses getVisitsPaginated`() {
         val provider = DefaultPagedHistoryProvider(
             historyStorage = storage,
-            showHistorySearchGroups = true
+            showHistorySearchGroups = true,
+            false
         )
 
         val visitInfo1 = VisitInfo(
@@ -147,7 +148,8 @@ class PagedHistoryProviderTest {
     fun `history metadata matching lower bound`() {
         val provider = DefaultPagedHistoryProvider(
             historyStorage = storage,
-            showHistorySearchGroups = true
+            showHistorySearchGroups = true,
+            false
         )
         // Oldest history visit on the page is 15 seconds (buffer time) newer than matching
         // metadata record.
@@ -219,7 +221,8 @@ class PagedHistoryProviderTest {
     fun `history metadata matching upper bound`() {
         val provider = DefaultPagedHistoryProvider(
             historyStorage = storage,
-            showHistorySearchGroups = true
+            showHistorySearchGroups = true,
+            false
         )
         // Newest history visit on the page is 15 seconds (buffer time) older than matching
         // metadata record.
@@ -291,7 +294,8 @@ class PagedHistoryProviderTest {
     fun `redirects are filtered out from history metadata groups`() {
         val provider = DefaultPagedHistoryProvider(
             historyStorage = storage,
-            showHistorySearchGroups = true
+            showHistorySearchGroups = true,
+            false
         )
 
         val visitInfo1 = VisitInfo(

app/src/test/java/org/mozilla/fenix/home/recentvisits/RecentVisitsFeatureTest.kt

@@ -384,7 +384,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN a list of history highlights and groups WHEN updateState is called THEN emit RecentHistoryChange`() {
-        val feature = spyk(RecentVisitsFeature(homeStore, mockk(), mockk(), mockk(), mockk()))
+        val feature = spyk(RecentVisitsFeature(homeStore, mockk(), mockk(), mockk(), mockk(), false))
         val expected = List<RecentHistoryHighlight>(1) { mockk() }
         every { feature.getCombinedHistory(any(), any()) } returns expected
 
@@ -398,7 +398,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN highlights visits exist in search groups WHEN getCombined is called THEN remove the highlights already in groups`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(4)
         val directVisits = getDirectVisitsHistoryMetadataItems(4)
         val directDupeVisits = getSearchFromHistoryMetadataItems(2).map {
@@ -422,7 +422,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN fewer than needed highlights and search groups WHEN getCombined is called THEN the result is sorted by date`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(4)
         val directVisits = getDirectVisitsHistoryMetadataItems(4)
         val expected = directVisits.reversed().toRecentHistoryHighlights()
@@ -441,7 +441,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN more highlights are newer than search groups WHEN getCombined is called THEN then return an even split then sorted by date`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(5)
         val directVisits = getDirectVisitsHistoryMetadataItems(14)
         val expected = directVisits.takeLast(5).reversed().toRecentHistoryHighlights() +
@@ -457,7 +457,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN more search groups are newer than highlights WHEN getCombined is called THEN then return an even split then sorted by date`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(14)
         val directVisits = getDirectVisitsHistoryMetadataItems(5)
         val expected = visitsFromSearch.takeLast(4).toIndividualRecentHistoryGroups() +
@@ -473,7 +473,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN all highlights have metadata WHEN getHistoryHighlights is called THEN return a list of highlights with an inferred last access time`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
 
@@ -490,7 +490,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN not all highlights have metadata WHEN getHistoryHighlights is called THEN set 0 for the highlights with not found last access time`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
         val highlightsWithUnknownAccessTime = directVisits.toHistoryHighlightsInternal().take(5).map {
@@ -511,7 +511,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN multiple metadata records for the same highlight WHEN getHistoryHighlights is called THEN set the latest access time from multiple available`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
         val newerDirectVisits = directVisits.mapIndexed { index, item ->
@@ -533,7 +533,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN multiple metadata entries only for direct accessed pages WHEN getHistorySearchGroups is called THEN return an empty list`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
 
         val result = feature.getHistorySearchGroups(directVisits)
@@ -543,7 +543,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN multiple metadata entries WHEN getHistorySearchGroups is called THEN group all entries by their search term`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
 
@@ -556,7 +556,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN multiple metadata entries for the same url WHEN getHistorySearchGroups is called THEN entries are deduped`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val newerVisitsFromSearch = visitsFromSearch.map { it.copy(updatedAt = it.updatedAt * 2) }
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
@@ -575,7 +575,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN highlights and search groups WHEN getSortedHistory is called THEN sort descending all items based on the last access time`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val directVisits = getDirectVisitsHistoryMetadataItems(10)
         val expected = directVisits.reversed().toRecentHistoryHighlights()
@@ -594,7 +594,7 @@ class RecentVisitsFeatureTest {
 
     @Test
     fun `GIVEN highlights don't have a valid title WHEN getSortedHistory is called THEN the url is set as title`() {
-        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk())
+        val feature = RecentVisitsFeature(mockk(), mockk(), mockk(), mockk(), mockk(), false)
         val visitsFromSearch = getSearchFromHistoryMetadataItems(10)
         val directVisits = getDirectVisitsHistoryMetadataItems(10).mapIndexed { index, item ->
             when (index % 3) {
@@ -646,6 +646,7 @@ class RecentVisitsFeatureTest {
             lazy { historyHightlightsStorage },
             CoroutineScope(testDispatcher),
             testDispatcher,
+            false
         )
 
         assertEquals(emptyList<RecentHistoryGroup>(), homeStore.state.recentHistory)