commit bdecb87aef9e0d05c26cb581c2f312f5b5158df3 parent c0aad7c261285e6536f3ab572a58f3ab103925b0 Author: Gabriel Luong <gabriel.luong@gmail.com> Date: Sat, 1 Nov 2025 00:30:34 +0000 Bug 1796263 - Replace hard-coded default top sites to be read from JSON file r=android-reviewers,jonalmeida,skhan - Implements `DefaultTopSitesBinding` that will read the default top sites list from a JSON and add the default top sites to storage based on the region. - The JSON schema follows the desktop schema for default top sites: https://firefox.settings.services.mozilla.com/v1/buckets/main/collections/top-sites/records - Future intentions would be to fetch a similar mobile compatible JSON from Remote Settings. Differential Revision: https://phabricator.services.mozilla.com/D267522 Diffstat:
17 files changed, 523 insertions(+), 51 deletions(-)
diff --git a/mobile/android/android-components/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt b/mobile/android/android-components/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt @@ -57,6 +57,13 @@ class DefaultTopSitesStorage( } } + override fun addTopSites(topSites: List<Pair<String, String>>, isDefault: Boolean) { + scope.launch { + pinnedSitesStorage.addAllPinnedSites(topSites = topSites, isDefault = isDefault) + notifyObservers { onStorageUpdated() } + } + } + override fun removeTopSite(topSite: TopSite) { scope.launch { if (topSite is TopSite.Default || topSite is TopSite.Pinned) { diff --git a/mobile/android/android-components/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt b/mobile/android/android-components/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt @@ -22,6 +22,15 @@ interface TopSitesStorage : Observable<TopSitesStorage.Observer> { fun addTopSite(title: String, url: String, isDefault: Boolean = false) /** + * Adds a list of top sites. + * + * @param topSites A list containing a title to url pair of top sites to be added. + * @param isDefault Whether or not the pinned site added should be a default pinned site. This + * is used to identify pinned sites that are added by the application. + */ + fun addTopSites(topSites: List<Pair<String, String>>, isDefault: Boolean = false) + + /** * Removes the given [TopSite]. * * @param topSite The top site. diff --git a/mobile/android/android-components/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt b/mobile/android/android-components/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt @@ -6,7 +6,9 @@ package mozilla.components.feature.top.sites.presenter import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import mozilla.components.feature.top.sites.TopSitesConfig import mozilla.components.feature.top.sites.TopSitesStorage import mozilla.components.feature.top.sites.view.TopSitesView @@ -29,6 +31,7 @@ internal class DefaultTopSitesPresenter( ) : TopSitesPresenter, TopSitesStorage.Observer { private val scope = CoroutineScope(coroutineContext) + private var job: Job? = null override fun start() { onStorageUpdated() @@ -38,19 +41,24 @@ internal class DefaultTopSitesPresenter( override fun stop() { storage.unregister(this) + job?.cancel() } override fun onStorageUpdated() { val innerConfig = config.invoke() - scope.launch { + // Cancel any existing job before starting a new one since onStorageUpdated can be called + // multiple times in quick succession. Ensure only the latest job is running. + job?.cancel() + + job = scope.launch { val topSites = storage.getTopSites( totalSites = innerConfig.totalSites, frecencyConfig = innerConfig.frecencyConfig, providerConfig = innerConfig.providerConfig, ) - scope.launch(Dispatchers.Main) { + withContext(Dispatchers.Main) { view.displayTopSites(topSites) } } diff --git a/mobile/android/android-components/components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt b/mobile/android/android-components/components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt @@ -68,6 +68,26 @@ class DefaultTopSitesStorageTest { } @Test + fun `GIVEN a list of top sites WHEN add top sites is invoked THEN add top sites to storage`() = runTest { + val defaultTopSitesStorage = DefaultTopSitesStorage( + pinnedSitesStorage = pinnedSitesStorage, + historyStorage = historyStorage, + defaultTopSites = listOf(), + coroutineContext = coroutineContext, + ) + val topSites = listOf( + Pair("Mozilla", "https://mozilla.com"), + Pair("Firefox", "https://firefox.com"), + ) + val isDefault = false + + defaultTopSitesStorage.addTopSites(topSites = topSites, isDefault = isDefault) + testScheduler.advanceUntilIdle() + + verify(pinnedSitesStorage).addAllPinnedSites(topSites = topSites, isDefault = isDefault) + } + + @Test fun removeTopSite() = runTest { val defaultTopSitesStorage = DefaultTopSitesStorage( pinnedSitesStorage = pinnedSitesStorage, diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt @@ -33,7 +33,7 @@ import org.mozilla.fenix.helpers.HomeActivityTestRule * * Say no to main thread IO! 🙅 */ -private const val EXPECTED_SUPPRESSION_COUNT = 13 +private const val EXPECTED_SUPPRESSION_COUNT = 12 /** * The number of times we call the `runBlocking` coroutine method on the main thread during this diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsAddonsTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsAddonsTest.kt @@ -129,7 +129,7 @@ class SettingsAddonsTest : TestSetup() { }.goToHomescreen(activityTestRule) { }.openTopSiteTabWithTitle( activityTestRule, - getStringResource(R.string.default_top_site_wikipedia), + "Wikipedia", ) { }.openThreeDotMenu { }.openSettings { diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsDeleteBrowsingDataOnQuitTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsDeleteBrowsingDataOnQuitTest.kt @@ -243,8 +243,6 @@ class SettingsDeleteBrowsingDataOnQuitTest : TestSetup() { @Ignore("Failing, see https://bugzilla.mozilla.org/show_bug.cgi?id=1987355") @Test fun deleteCachedFilesOnQuitTest() { - val wikipedia = getStringResource(R.string.default_top_site_wikipedia) - homeScreen { }.openThreeDotMenu { }.openSettings { @@ -253,8 +251,8 @@ class SettingsDeleteBrowsingDataOnQuitTest : TestSetup() { exitMenu() } homeScreen { - verifyExistingTopSitesTabs(composeTestRule, wikipedia) - }.openTopSiteTabWithTitle(composeTestRule, wikipedia) { + verifyExistingTopSitesTabs(composeTestRule, "Wikipedia") + }.openTopSiteTabWithTitle(composeTestRule, "Wikipedia") { verifyUrl("wikipedia.org") }.goToHomescreen(composeTestRule) { }.openThreeDotMenu { diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsDeleteBrowsingDataTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsDeleteBrowsingDataTest.kt @@ -237,11 +237,9 @@ class SettingsDeleteBrowsingDataTest : TestSetup() { @SmokeTest @Test fun deleteCachedFilesTest() { - val wikipedia = getStringResource(R.string.default_top_site_wikipedia) - homeScreen { - verifyExistingTopSitesTabs(composeTestRule, wikipedia) - }.openTopSiteTabWithTitle(composeTestRule, wikipedia) { + verifyExistingTopSitesTabs(composeTestRule, "Wikipedia") + }.openTopSiteTabWithTitle(composeTestRule, "Wikipedia") { verifyUrl("wikipedia.org") }.openTabDrawer(composeTestRule) { }.openNewTab { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -136,6 +136,7 @@ import org.mozilla.fenix.home.intent.OpenSpecificTabIntentProcessor import org.mozilla.fenix.home.intent.ReEngagementIntentProcessor import org.mozilla.fenix.home.intent.SpeechProcessingIntentProcessor import org.mozilla.fenix.home.intent.StartSearchIntentProcessor +import org.mozilla.fenix.home.topsites.DefaultTopSitesBinding import org.mozilla.fenix.messaging.FenixMessageSurfaceId import org.mozilla.fenix.messaging.MessageNotificationWorker import org.mozilla.fenix.nimbus.FxNimbus @@ -220,6 +221,16 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { ) } + private val defaultTopSitesBinding by lazy { + DefaultTopSitesBinding( + browserStore = components.core.store, + topSitesStorage = components.core.topSitesStorage, + settings = settings(), + resources = resources, + crashReporter = components.analytics.crashReporter, + ) + } + private val aboutHomeBinding by lazy { AboutHomeBinding( browserStore = components.core.store, @@ -526,6 +537,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { serviceWorkerSupport, aboutHomeBinding, crashReporterBinding, + defaultTopSitesBinding, TopSitesRefresher( settings = settings(), topSitesProvider = components.core.marsTopSitesProvider, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -6,7 +6,6 @@ package org.mozilla.fenix.components import android.content.Context import android.content.res.Configuration -import android.os.StrictMode import androidx.core.content.ContextCompat import androidx.datastore.preferences.preferencesDataStore import kotlinx.coroutines.CoroutineScope @@ -127,7 +126,6 @@ import org.mozilla.fenix.media.MediaSessionService import org.mozilla.fenix.nimbus.FxNimbus import org.mozilla.fenix.perf.StrictModeManager import org.mozilla.fenix.perf.lazyMonitored -import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.settings.advanced.getSelectedLocale import org.mozilla.fenix.share.DefaultSentFromFirefoxManager import org.mozilla.fenix.share.DefaultSentFromStorage @@ -620,33 +618,10 @@ class Core( } val topSitesStorage by lazyMonitored { - val defaultTopSites = mutableListOf<Pair<String, String>>() - - strictMode.allowViolation(StrictMode::allowThreadDiskReads) { - if (!context.settings().defaultTopSitesAdded) { - defaultTopSites.add( - Pair( - context.getString(R.string.default_top_site_google), - SupportUtils.GOOGLE_URL, - ), - ) - - defaultTopSites.add( - Pair( - context.getString(R.string.default_top_site_wikipedia), - SupportUtils.WIKIPEDIA_URL, - ), - ) - - context.settings().defaultTopSitesAdded = true - } - } - DefaultTopSitesStorage( pinnedSitesStorage = pinnedSiteStorage, historyStorage = historyStorage, topSitesProvider = marsTopSitesProvider, - defaultTopSites = defaultTopSites, ) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBinding.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBinding.kt @@ -0,0 +1,136 @@ +/* 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.home.topsites + +import android.content.res.Resources +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.withContext +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import mozilla.components.browser.state.search.RegionState +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.base.crash.Breadcrumb +import mozilla.components.feature.top.sites.DefaultTopSitesStorage +import mozilla.components.lib.crash.CrashReporter +import mozilla.components.lib.state.helpers.AbstractBinding +import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl +import org.mozilla.fenix.Config +import org.mozilla.fenix.R +import org.mozilla.fenix.utils.Settings + +/** + * A binding for observing [RegionState] and adding default top sites that are included in the + * application. + * + * @param browserStore The [BrowserStore] to observe state changes. + * @param topSitesStorage An instance of the [DefaultTopSitesStorage] used add to default top sites. + * @param settings [Settings] used for accessing the application preferences. + * @param resources [Resources] used for accessing application resources. + * @param crashReporter [CrashReporter] used for recording caught exceptions. + * @param isReleased Whether or not the build is in a release channel. + */ +class DefaultTopSitesBinding( + browserStore: BrowserStore, + private val topSitesStorage: DefaultTopSitesStorage, + private val settings: Settings, + private val resources: Resources, + private val crashReporter: CrashReporter, + private val isReleased: Boolean = Config.channel.isReleased, +) : AbstractBinding<BrowserState>(browserStore) { + + override suspend fun onState(flow: Flow<BrowserState>) { + if (settings.defaultTopSitesAdded) return + + flow + .mapNotNull { it.search.region } + .distinctUntilChanged() + .collect { regionState -> + if (isReleased && regionState == RegionState.Default) { + return@collect + } + + val defaultTopSites = getTopSites(region = regionState.current) + + if (defaultTopSites.isNotEmpty()) { + topSitesStorage.addTopSites(topSites = defaultTopSites, isDefault = true) + settings.defaultTopSitesAdded = true + } + } + } + + internal suspend fun getTopSites(region: String): List<Pair<String, String>> = withContext(Dispatchers.IO) { + try { + val json = Json { ignoreUnknownKeys = true } + val jsonString = resources.openRawResource(R.raw.initial_shortcuts).bufferedReader() + .use { it.readText() } + + json.decodeFromString<DefaultTopSitesList>(jsonString).data.filter { item -> + val includedInRegions = + item.includeRegions.isEmpty() || region in item.includeRegions + val notExcludedInRegions = + item.excludeRegions.isEmpty() || region !in item.excludeRegions + + includedInRegions && notExcludedInRegions + }.map { + Pair( + it.title?.takeIf(String::isNotBlank) ?: it.url.tryGetHostFromUrl(), + it.url, + ) + } + } catch (e: SerializationException) { + crashReporter.recordCrashBreadcrumb( + Breadcrumb( + message = "DefaultShortcutsProvider - Failed to parse initial_shortcuts.json", + ), + ) + crashReporter.submitCaughtException(e) + listOf() + } catch (e: IllegalArgumentException) { + crashReporter.recordCrashBreadcrumb( + Breadcrumb( + message = "DefaultShortcutsProvider - Failed to parse initial_shortcuts.json", + ), + ) + crashReporter.submitCaughtException(e) + listOf() + } + } +} + +@Serializable +private data class DefaultTopSiteItem( + val url: String, + val title: String? = null, + val order: Int, + val schema: Long, + @SerialName("exclude_locales") + val excludeLocales: List<String> = emptyList(), + @SerialName("exclude_regions") + val excludeRegions: List<String> = emptyList(), + @SerialName("include_locales") + val includeLocales: List<String> = emptyList(), + @SerialName("include_regions") + val includeRegions: List<String> = emptyList(), + @SerialName("exclude_experiments") + val excludeExperiments: List<String> = emptyList(), + @SerialName("include_experiments") + val includeExperiments: List<String> = emptyList(), + val id: String, + @SerialName("last_modified") + val lastModified: Long, + @SerialName("search_shortcut") + val searchShortcut: Boolean = false, +) + +@Serializable +private data class DefaultTopSitesList( + val data: List<DefaultTopSiteItem>, +) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/SupportUtils.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/SupportUtils.kt @@ -22,7 +22,6 @@ import java.util.Locale object SupportUtils { const val RATE_APP_URL = "market://details?id=" + BuildConfig.APPLICATION_ID - const val WIKIPEDIA_URL = "https://www.wikipedia.org/" const val FENIX_PLAY_STORE_URL = "https://play.google.com/store/apps/details?id=${BuildConfig.APPLICATION_ID}" const val GOOGLE_URL = "https://www.google.com/" const val GOOGLE_US_URL = "https://www.google.com/webhp?client=firefox-b-1-m&channel=ts" diff --git a/mobile/android/fenix/app/src/main/res/raw/initial_shortcuts.json b/mobile/android/fenix/app/src/main/res/raw/initial_shortcuts.json @@ -0,0 +1,40 @@ +{ + "data": [ + { + "url": "https://www.google.com/", + "order": 0, + "title": "Google", + "schema": 1, + "exclude_locales": [], + "exclude_regions": [ + "CN", + "RU", + "TR", + "KZ", + "BY" + ], + "include_locales": [], + "include_regions": [], + "exclude_experiments": [], + "include_experiments": [], + "id": "", + "last_modified": 1 + }, + { + "url": "https://www.wikipedia.org/", + "order": 1, + "title": "Wikipedia", + "schema": 1, + "exclude_locales": [], + "exclude_regions": [ + "CN" + ], + "include_locales": [], + "include_regions": [], + "exclude_experiments": [], + "include_experiments": [], + "id": "", + "last_modified": 1 + } + ] +} diff --git a/mobile/android/fenix/app/src/main/res/values/static_strings.xml b/mobile/android/fenix/app/src/main/res/values/static_strings.xml @@ -16,10 +16,6 @@ <!-- GeckoView abbreviation used in AboutFragment --> <string name="gecko_view_abbreviation" translatable="false">GV</string> - <!-- Default title for pinned Wikipedia top site that links to Wikipedia home page --> - <string name="default_top_site_wikipedia" translatable="false">Wikipedia</string> - <!-- Default title for pinned Google top site that links to Google home page --> - <string name="default_top_site_google" translatable="false">Google</string> <!-- Application Services abbreviation used in AboutFragment --> <string name="app_services_abbreviation" translatable="false">AS</string> @@ -151,8 +147,6 @@ <!-- Profiler settings --> <string name="preferences_start_profiler">Start Profiler</string> <string name="profiler_stop">Stop Profiler</string> - - <string name="profiler_active_notification" translatable="false">Profiler active</string> <string name="profiler_notification_text" translatable="false">Profiling is active. Tap to stop profiler</string> <string name="profiler_settings_title">Profiler Settings</string> @@ -167,23 +161,17 @@ <string name="profiler_filter_networking_explain">Preset for investigating networking bugs in Firefox</string> <string name="profiler_filter_debug">Debug</string> <string name="profiler_filter_debug_explain">Preset for debugging. High overhead, do not use for performance work but use for focusing on understanding browser behavior.</string> - <string name="profiler_start_dialog_started">Profiler started</string> - <string name="profiler_start_cancel">Cancel</string> - <string name="profiler_gathering">Gathering the profile</string> <string name="profiler_stopping">Stopping the profiler</string> <string name="profiler_no_info">No information was gathered</string> - <string name="profiler_waiting_start">Waiting for Profiler to start</string> - <string name="profiler_url_warning">⚠️ Warning: Profile Upload</string> <string name="profiler_url_warning_explained">Performance profiles may contain sensitive information such as websites visited and screenshots of them. If you continue, your profile will be uploaded and made accessible to anyone with the link. Do you wish to continue?</string> <string name="profiler_as_url">Upload profile</string> <string name="profiler_error">Something went wrong with the profiler</string> <string name="profiler_io_error">Something went wrong contacting the Profiler server.</string> - <string name="profiler_uploaded_url_to_clipboard">URL copied to clipboard successfully</string> <!-- Debug drawer "contextual feature recommendation" (CFR) tools --> @@ -243,7 +231,6 @@ <!-- The send pings toast message. The first parameter is the type of ping --> <string name="glean_debug_tools_send_ping_toast_message">Sent %1$s ping</string> - <!-- The title of the debug drawer tab to help debug the crash reporter--> <string name="crash_debug_tools_title">Crash Debug Tools</string> <!-- Button text to trigger a app parent process crash --> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBindingTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/topsites/DefaultTopSitesBindingTest.kt @@ -0,0 +1,236 @@ +/* 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.home.topsites + +import android.content.res.Resources +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.SerializationException +import mozilla.components.browser.state.action.SearchAction +import mozilla.components.browser.state.search.RegionState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.feature.top.sites.DefaultTopSitesStorage +import mozilla.components.lib.crash.CrashReporter +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.rule.runTestOnMain +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.utils.Settings +import java.io.ByteArrayInputStream + +@RunWith(AndroidJUnit4::class) +class DefaultTopSitesBindingTest { + @get:Rule + val coroutineRule = MainCoroutineRule() + + private lateinit var browserStore: BrowserStore + private lateinit var topSitesStorage: DefaultTopSitesStorage + private lateinit var settings: Settings + private lateinit var resources: Resources + private lateinit var crashReporter: CrashReporter + + @Before + fun setUp() { + topSitesStorage = mockk(relaxed = true) + settings = mockk(relaxed = true) + resources = mockk(relaxed = true) + crashReporter = mockk(relaxed = true) + + browserStore = BrowserStore() + + every { resources.openRawResource(R.raw.initial_shortcuts) } answers { + this.javaClass.classLoader!!.getResourceAsStream("raw/test_initial_shortcuts.json")!! + } + } + + @Test + fun `GIVEN build is in the release channel WHEN region is set to the default THEN do nothing`() = runTestOnMain { + every { settings.defaultTopSitesAdded } returns false + + val binding = createBinding() + binding.start() + + browserStore.dispatch( + SearchAction.SetRegionAction(RegionState.Default), + ) + browserStore.waitUntilIdle() + + verify(exactly = 0) { + topSitesStorage.addTopSites(topSites = any(), isDefault = any()) + settings.defaultTopSitesAdded = any() + } + } + + @Test + fun `GIVEN debug build WHEN region is set to the default THEN add the default sites to storage`() = runTestOnMain { + every { settings.defaultTopSitesAdded } returns false + + val binding = createBinding(isReleased = false) + binding.start() + + browserStore.dispatch( + SearchAction.SetRegionAction(RegionState.Default), + ) + browserStore.waitUntilIdle() + + val topSites = binding.getTopSites(region = "XX") + + verify { + topSitesStorage.addTopSites(topSites = topSites, isDefault = true) + settings.defaultTopSitesAdded = true + } + } + + @Test + fun `WHEN region is set to a non-default value THEN add the sites for that region to storage`() = runTestOnMain { + every { settings.defaultTopSitesAdded } returns false + + val region = "CA" + val binding = createBinding() + binding.start() + + browserStore.dispatch( + SearchAction.SetRegionAction(RegionState(home = region, current = region)), + ) + browserStore.waitUntilIdle() + + val topSites = binding.getTopSites(region = region) + + verify { + topSitesStorage.addTopSites(topSites = topSites, isDefault = true) + settings.defaultTopSitesAdded = true + } + } + + @Test + fun `GIVEN default top sites have already been added WHEN region is set to a non-default value THEN do nothing`() = runTestOnMain { + every { settings.defaultTopSitesAdded } returns true + + val region = "CA" + val binding = createBinding() + binding.start() + + browserStore.dispatch( + SearchAction.SetRegionAction(RegionState(home = region, current = region)), + ) + browserStore.waitUntilIdle() + + verify(exactly = 0) { + topSitesStorage.addTopSites(topSites = any(), isDefault = any()) + settings.defaultTopSitesAdded = any() + } + } + + @Test + fun `GIVEN region is in an included region WHEN getTopSites is called THEN the sites for that region are returned`() = runTest { + val binding = createBinding() + val topSites = binding.getTopSites(region = "US") + + assertEquals(5, topSites.size) + assertEquals("US Region Site", topSites[0].first) + assertEquals("https://www.example1.com/", topSites[0].second) + assertEquals("CA Excluded Region Site", topSites[1].first) + assertEquals("https://www.example2.com/", topSites[1].second) + assertEquals("All Region Site", topSites[2].first) + assertEquals("https://www.example3.com/", topSites[2].second) + assertEquals("www.example4.com", topSites[3].first) + assertEquals("https://www.example4.com/", topSites[3].second) + assertEquals("www.example5.com", topSites[4].first) + assertEquals("https://www.example5.com/", topSites[4].second) + } + + @Test + fun `GIVEN region is in an excluded region WHEN getTopSites is called THEN the sites for that region are not returned`() = runTest { + val binding = createBinding() + val topSites = binding.getTopSites(region = "CA") + + assertEquals(3, topSites.size) + assertEquals("All Region Site", topSites[0].first) + assertEquals("https://www.example3.com/", topSites[0].second) + assertEquals("www.example4.com", topSites[1].first) + assertEquals("https://www.example4.com/", topSites[1].second) + assertEquals("www.example5.com", topSites[2].first) + assertEquals("https://www.example5.com/", topSites[2].second) + } + + @Test + fun `GIVEN the default region region WHEN getTopSites is called THEN the sites for that region are returned`() = runTest { + val binding = createBinding() + val topSites = binding.getTopSites(region = "XX") + + assertEquals(4, topSites.size) + assertEquals("CA Excluded Region Site", topSites[0].first) + assertEquals("https://www.example2.com/", topSites[0].second) + assertEquals("All Region Site", topSites[1].first) + assertEquals("https://www.example3.com/", topSites[1].second) + assertEquals("www.example4.com", topSites[2].first) + assertEquals("https://www.example4.com/", topSites[2].second) + assertEquals("www.example5.com", topSites[3].first) + assertEquals("https://www.example5.com/", topSites[3].second) + } + + @Test + fun `GIVEN invalid json WHEN getTopSites is called THEN return empty list and report crash`() = runTest { + val malformedJson = + "{\"data\": [{\"url\": \"https://example.com\", \"title\": \"Valid\"}, {\"id\": \"invalid\"}]}" + every { resources.openRawResource(R.raw.initial_shortcuts) } returns ByteArrayInputStream( + malformedJson.toByteArray(), + ) + + val binding = createBinding() + val topSites = binding.getTopSites(region = "XX") + + assertTrue(topSites.isEmpty()) + verify { + crashReporter.recordCrashBreadcrumb(any()) + crashReporter.submitCaughtException(any<SerializationException>()) + } + verify(exactly = 0) { + topSitesStorage.addTopSites(topSites = any(), isDefault = any()) + settings.defaultTopSitesAdded = any() + } + } + + @Test + fun `GIVEN malformed json WHEN getTopSites is called THEN return empty list and report crash for illegal argument`() = runTest { + val malformedJson = "this is not a valid json" + every { resources.openRawResource(R.raw.initial_shortcuts) } returns ByteArrayInputStream( + malformedJson.toByteArray(), + ) + + val binding = createBinding() + val topSites = binding.getTopSites(region = "XX") + + assertTrue(topSites.isEmpty()) + verify { + crashReporter.recordCrashBreadcrumb(any()) + crashReporter.submitCaughtException(any<SerializationException>()) + } + verify(exactly = 0) { + topSitesStorage.addTopSites(topSites = any(), isDefault = any()) + settings.defaultTopSitesAdded = any() + } + } + + private fun createBinding( + isReleased: Boolean = true, + ) = DefaultTopSitesBinding( + browserStore = browserStore, + topSitesStorage = topSitesStorage, + settings = settings, + resources = resources, + crashReporter = crashReporter, + isReleased = isReleased, + ) +} diff --git a/mobile/android/fenix/app/src/test/resources/raw/test_initial_shortcuts.json b/mobile/android/fenix/app/src/test/resources/raw/test_initial_shortcuts.json @@ -0,0 +1,45 @@ +{ + "data": [ + { + "id": "1", + "url": "https://www.example1.com/", + "title": "US Region Site", + "order": 0, + "schema": 1, + "include_regions": ["US"], + "last_modified": 1 + }, + { + "id": "2", + "url": "https://www.example2.com/", + "title": "CA Excluded Region Site", + "order": 1, + "schema": 1, + "exclude_regions": ["CA"], + "last_modified": 1 + }, + { + "id": "3", + "url": "https://www.example3.com/", + "title": "All Region Site", + "order": 2, + "schema": 1, + "last_modified": 1 + }, + { + "id": "4", + "url": "https://www.example4.com/", + "title": "", + "order": 3, + "schema": 1, + "last_modified": 1 + }, + { + "id": "5", + "url": "https://www.example5.com/", + "order": 4, + "schema": 1, + "last_modified": 1 + } + ] +} diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/topsites/DefaultTopSitesStorage.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/topsites/DefaultTopSitesStorage.kt @@ -38,6 +38,8 @@ class DefaultTopSitesStorage( } } + override fun addTopSites(topSites: List<Pair<String, String>>, isDefault: Boolean) = Unit + override fun removeTopSite(topSite: TopSite) { scope.launch { pinnedSitesStorage.removePinnedSite(topSite)