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

Commit

Permalink
For #23431 - Display the order of Contile Top Sites correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
gabrielluong committed Feb 9, 2022
1 parent 80c9ad9 commit 45676d1
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 12 deletions.
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.20220208145842"
const val VERSION = "99.0.20220209170927"
}

0 comments on commit 45676d1

Please sign in to comment.