Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #23431 - Display the order of Contile Top Sites correctly #23656

Merged
merged 2 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions app/src/main/java/org/mozilla/fenix/FenixApplication.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import mozilla.components.feature.addons.update.GlobalAddonDependencyProvider
import mozilla.components.feature.autofill.AutofillUseCases
import mozilla.components.feature.search.ext.buildSearchUrl
import mozilla.components.feature.search.ext.waitForSelectedOrDefaultSearchEngine
import mozilla.components.feature.top.sites.TopSitesProviderConfig
import mozilla.components.lib.crash.CrashReporter
import mozilla.components.service.fxa.manager.SyncEnginesStorage
import mozilla.components.service.glean.Glean
Expand Down Expand Up @@ -82,6 +83,7 @@ import org.mozilla.fenix.session.VisibilityLifecycleCallback
import org.mozilla.fenix.telemetry.TelemetryLifecycleObserver
import org.mozilla.fenix.utils.BrowsersCache
import org.mozilla.fenix.utils.Settings
import org.mozilla.fenix.utils.Settings.Companion.TOP_SITES_PROVIDER_MAX_THRESHOLD
import java.util.concurrent.TimeUnit

/**
Expand Down Expand Up @@ -258,11 +260,14 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
// we can prevent with this.
components.core.topSitesStorage.getTopSites(
totalSites = components.settings.topSitesMaxLimit,
fetchProvidedTopSites = components.settings.showContileFeature,
frecencyConfig = if (components.settings.showTopFrecentSites)
FrecencyThresholdOption.SKIP_ONE_TIME_PAGES
else
null
null,
providerConfig = TopSitesProviderConfig(
showProviderTopSites = components.settings.showContileFeature,
maxThreshold = TOP_SITES_PROVIDER_MAX_THRESHOLD
)
)

// This service uses `historyStorage`, and so we can only touch it when we know
Expand Down
20 changes: 20 additions & 0 deletions app/src/main/java/org/mozilla/fenix/ext/TopSite.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.mozilla.fenix.ext

import mozilla.components.feature.top.sites.TopSite
import org.mozilla.fenix.settings.SupportUtils

/**
* Returns the type name of the [TopSite].
Expand All @@ -15,3 +16,22 @@ fun TopSite.name(): String = when (this) {
is TopSite.Pinned -> "PINNED"
is TopSite.Provided -> "PROVIDED"
}

/**
* Returns a sorted list of [TopSite] with the default Google top site always appearing
* as the first item.
*/
fun List<TopSite>.sort(): List<TopSite> {
val defaultGoogleTopSiteIndex = this.indexOfFirst {
it is TopSite.Default && it.url == SupportUtils.GOOGLE_URL
}

return if (defaultGoogleTopSiteIndex == -1) {
this
} else {
val result = this.toMutableList()
result.removeAt(defaultGoogleTopSiteIndex)
result.add(0, this[defaultGoogleTopSiteIndex])
result
}
}
19 changes: 14 additions & 5 deletions app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import mozilla.components.concept.sync.OAuthAccount
import mozilla.components.feature.tab.collections.TabCollection
import mozilla.components.feature.top.sites.TopSitesConfig
import mozilla.components.feature.top.sites.TopSitesFeature
import mozilla.components.feature.top.sites.TopSitesProviderConfig
import mozilla.components.lib.state.ext.consumeFlow
import mozilla.components.lib.state.ext.consumeFrom
import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
Expand Down Expand Up @@ -102,6 +103,7 @@ import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.runIfFragmentIsAttached
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.ext.sort
import org.mozilla.fenix.home.mozonline.showPrivacyPopWindow
import org.mozilla.fenix.home.pocket.DefaultPocketStoriesController
import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory
Expand All @@ -124,6 +126,7 @@ import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.settings.SupportUtils.SumoTopic.HELP
import org.mozilla.fenix.settings.deletebrowsingdata.deleteAndQuit
import org.mozilla.fenix.theme.ThemeManager
import org.mozilla.fenix.utils.Settings.Companion.TOP_SITES_PROVIDER_MAX_THRESHOLD
import org.mozilla.fenix.utils.ToolbarPopupWindow
import org.mozilla.fenix.utils.allowUndo
import org.mozilla.fenix.wallpapers.WallpaperManager
Expand Down Expand Up @@ -237,7 +240,7 @@ class HomeFragment : Fragment() {
collections = components.core.tabCollectionStorage.cachedTabCollections,
expandedCollections = emptySet(),
mode = currentMode.getCurrentMode(),
topSites = components.core.topSitesStorage.cachedTopSites,
topSites = components.core.topSitesStorage.cachedTopSites.sort(),
tip = components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) {
FenixTipManager(
listOf(
Expand Down Expand Up @@ -282,7 +285,10 @@ class HomeFragment : Fragment() {

topSitesFeature.set(
feature = TopSitesFeature(
view = DefaultTopSitesView(homeFragmentStore),
view = DefaultTopSitesView(
store = homeFragmentStore,
settings = components.settings
),
storage = components.core.topSitesStorage,
config = ::getTopSitesConfig
),
Expand Down Expand Up @@ -421,8 +427,11 @@ class HomeFragment : Fragment() {
val settings = requireContext().settings()
return TopSitesConfig(
totalSites = settings.topSitesMaxLimit,
fetchProvidedTopSites = settings.showContileFeature,
frecencyConfig = if (settings.showTopFrecentSites) FrecencyThresholdOption.SKIP_ONE_TIME_PAGES else null
frecencyConfig = if (settings.showTopFrecentSites) FrecencyThresholdOption.SKIP_ONE_TIME_PAGES else null,
providerConfig = TopSitesProviderConfig(
showProviderTopSites = settings.showContileFeature,
maxThreshold = TOP_SITES_PROVIDER_MAX_THRESHOLD
)
)
}

Expand Down Expand Up @@ -690,7 +699,7 @@ class HomeFragment : Fragment() {
HomeFragmentAction.Change(
collections = components.core.tabCollectionStorage.cachedTabCollections,
mode = currentMode.getCurrentMode(),
topSites = components.core.topSitesStorage.cachedTopSites,
topSites = components.core.topSitesStorage.cachedTopSites.sort(),
tip = components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) {
FenixTipManager(
listOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,25 @@ package org.mozilla.fenix.home.topsites

import mozilla.components.feature.top.sites.TopSite
import mozilla.components.feature.top.sites.view.TopSitesView
import org.mozilla.fenix.ext.sort
import org.mozilla.fenix.home.HomeFragmentAction
import org.mozilla.fenix.home.HomeFragmentStore
import org.mozilla.fenix.utils.Settings

class DefaultTopSitesView(
val store: HomeFragmentStore
val store: HomeFragmentStore,
val settings: Settings
) : TopSitesView {

override fun displayTopSites(topSites: List<TopSite>) {
store.dispatch(HomeFragmentAction.TopSitesChange(topSites))
store.dispatch(
HomeFragmentAction.TopSitesChange(
if (!settings.showContileFeature) {
topSites
} else {
topSites.sort()
}
)
)
}
}
12 changes: 10 additions & 2 deletions app/src/main/java/org/mozilla/fenix/utils/Settings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import androidx.lifecycle.LifecycleOwner
import mozilla.components.feature.sitepermissions.SitePermissionsRules
import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action
import mozilla.components.feature.sitepermissions.SitePermissionsRules.AutoplayAction
import mozilla.components.service.contile.ContileTopSitesProvider
import mozilla.components.support.ktx.android.content.PreferencesHolder
import mozilla.components.support.ktx.android.content.booleanPreference
import mozilla.components.support.ktx.android.content.floatPreference
Expand Down Expand Up @@ -61,7 +62,6 @@ private const val AUTOPLAY_USER_SETTING = "AUTOPLAY_USER_SETTING"
class Settings(private val appContext: Context) : PreferencesHolder {

companion object {
const val topSitesMaxCount = 16
const val FENIX_PREFERENCES = "fenix_preferences"

private const val BLOCKED_INT = 0
Expand All @@ -84,6 +84,14 @@ class Settings(private val appContext: Context) : PreferencesHolder {
*/
const val SEARCH_GROUP_MINIMUM_SITES: Int = 2

// The maximum number of top sites to display.
const val TOP_SITES_MAX_COUNT = 16
/**
* Only fetch top sites from the [ContileTopSitesProvider] when the number of default and
* pinned sites are below this maximum threshold.
*/
const val TOP_SITES_PROVIDER_MAX_THRESHOLD = 8

private fun Action.toInt() = when (this) {
Action.BLOCKED -> BLOCKED_INT
Action.ASK_TO_ALLOW -> ASK_TO_ALLOW_INT
Expand Down Expand Up @@ -1122,7 +1130,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {

val topSitesMaxLimit by intPreference(
appContext.getPreferenceKey(R.string.pref_key_top_sites_max_limit),
default = topSitesMaxCount
default = TOP_SITES_MAX_COUNT
)

var openTabsCount by intPreference(
Expand Down
117 changes: 117 additions & 0 deletions app/src/test/java/org/mozilla/fenix/ext/TopSiteTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package org.mozilla.fenix.ext

import mozilla.components.feature.top.sites.TopSite
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.settings.SupportUtils

@RunWith(FenixRobolectricTestRunner::class)
class TopSiteTest {

val defaultGoogleTopSite = TopSite.Default(
id = 1L,
title = "Google",
url = SupportUtils.GOOGLE_URL,
createdAt = 0
)
val providedSite1 = TopSite.Provided(
id = 3,
title = "Mozilla",
url = "https://mozilla.com",
clickUrl = "https://mozilla.com/click",
imageUrl = "https://test.com/image2.jpg",
impressionUrl = "https://example.com",
createdAt = 3
)
val providedSite2 = TopSite.Provided(
id = 3,
title = "Firefox",
url = "https://firefox.com",
clickUrl = "https://firefox.com/click",
imageUrl = "https://test.com/image2.jpg",
impressionUrl = "https://example.com",
createdAt = 3
)
val pinnedSite1 = TopSite.Pinned(
id = 1L,
title = "DuckDuckGo",
url = "https://duckduckgo.com",
createdAt = 0
)
val pinnedSite2 = TopSite.Pinned(
id = 1L,
title = "Mozilla",
url = "mozilla.org",
createdAt = 0
)
val frecentSite = TopSite.Frecent(
id = 1L,
title = "Mozilla",
url = "mozilla.org",
createdAt = 0
)

@Test
fun `GIVEN the default Google top site is the first item WHEN the list of top sites is sorted THEN the order doesn't change`() {
val topSites = listOf(
defaultGoogleTopSite,
providedSite1,
providedSite2,
pinnedSite1,
pinnedSite2,
frecentSite
)

assertEquals(topSites.sort(), topSites)
}

@Test
fun `GIVEN the default Google top site is after the provided top sites WHEN the list of top sites is sorted THEN the default Google top site should be first`() {
val topSites = listOf(
providedSite1,
providedSite2,
defaultGoogleTopSite,
pinnedSite1,
pinnedSite2,
frecentSite
)
val expected = listOf(
defaultGoogleTopSite,
providedSite1,
providedSite2,
pinnedSite1,
pinnedSite2,
frecentSite
)

assertEquals(topSites.sort(), expected)
}

@Test
fun `GIVEN the default Google top site is the last item WHEN the list of top sites is sorted THEN the default Google top site should be first`() {
val topSites = listOf(
providedSite1,
providedSite2,
pinnedSite1,
pinnedSite2,
frecentSite,
defaultGoogleTopSite
)
val expected = listOf(
defaultGoogleTopSite,
providedSite1,
providedSite2,
pinnedSite1,
pinnedSite2,
frecentSite
)

assertEquals(topSites.sort(), expected)
}
}
2 changes: 1 addition & 1 deletion buildSrc/src/main/java/AndroidComponents.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

object AndroidComponents {
const val VERSION = "99.0.20220209143350"
const val VERSION = "99.0.20220209170927"
}