commit 548c3a27b1a7037396eab56fc317626f747a0746
parent 25e0aa97a04da4290de5d02a3244cc3cb9d49864
Author: Segun Famisa <sfamisa@mozilla.com>
Date: Mon, 13 Oct 2025 09:50:13 +0000
Bug 1987602 - For app link startups, don't refresh top sites until visual completeness is ready r=android-reviewers,boek
Differential Revision: https://phabricator.services.mozilla.com/D266408
Diffstat:
3 files changed, 134 insertions(+), 7 deletions(-)
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
@@ -145,6 +145,7 @@ import org.mozilla.fenix.nimbus.FxNimbus
import org.mozilla.fenix.onboarding.ReEngagementNotificationWorker
import org.mozilla.fenix.pbmlock.DefaultPrivateBrowsingLockStorage
import org.mozilla.fenix.pbmlock.PrivateBrowsingLockFeature
+import org.mozilla.fenix.perf.DefaultStartupPathProvider
import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks
import org.mozilla.fenix.perf.MarkersFragmentLifecycleCallbacks
import org.mozilla.fenix.perf.Performance
@@ -553,6 +554,8 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
TopSitesRefresher(
settings = settings(),
topSitesProvider = components.core.marsTopSitesProvider,
+ startupPathProvider = startupPathProvider,
+ visualCompletenessQueue = components.performance.visualCompletenessQueue,
),
downloadSnackbar,
privateBrowsingLockFeature,
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/TopSitesRefresher.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/TopSitesRefresher.kt
@@ -13,6 +13,8 @@ import kotlinx.coroutines.cancel
import kotlinx.coroutines.launch
import mozilla.components.feature.top.sites.TopSitesProvider
import mozilla.components.support.base.log.logger.Logger
+import mozilla.components.support.utils.RunWhenReadyQueue
+import org.mozilla.fenix.perf.StartupPathProvider
import org.mozilla.fenix.utils.Settings
/**
@@ -20,12 +22,17 @@ import org.mozilla.fenix.utils.Settings
*
* @param settings Fenix [Settings]
* @param topSitesProvider [TopSitesProvider] to refresh top sites
+ * @param visualCompletenessQueue [RunWhenReadyQueue] for visual completeness
+ * @param startupPathProvider [StartupPathProvider] that tells us whether the app was started
+ * for "app link" (aka [StartupPathProvider.StartupPath.VIEW]) or "home" (aka [StartupPathProvider.StartupPath.MAIN])
* @param dispatcher [CoroutineDispatcher] to use launch the refresh job.
* Default value is [Dispatchers.IO]. It is helpful to improve testability
*/
class TopSitesRefresher(
private val settings: Settings,
private val topSitesProvider: TopSitesProvider,
+ private val visualCompletenessQueue: RunWhenReadyQueue,
+ private val startupPathProvider: StartupPathProvider,
private val dispatcher: CoroutineDispatcher = Dispatchers.IO,
) : DefaultLifecycleObserver {
@@ -33,18 +40,35 @@ class TopSitesRefresher(
private val scope = CoroutineScope(dispatcher)
override fun onResume(owner: LifecycleOwner) {
+ if (!settings.showContileFeature) return
+
+ if (isAppLinkStartup()) {
+ // for app link startups
+ // do not refresh top sites until visual completeness is ready
+ // see bug https://bugzilla.mozilla.org/show_bug.cgi?id=1987602
+ visualCompletenessQueue.runIfReadyOrQueue {
+ refreshTopSites()
+ }
+ } else {
+ refreshTopSites()
+ }
+ }
+
+ override fun onPause(owner: LifecycleOwner) {
+ scope.cancel()
+ }
+
+ private fun refreshTopSites() {
scope.launch(dispatcher) {
runCatching {
- if (settings.showContileFeature) {
- topSitesProvider.refreshTopSitesIfCacheExpired()
- }
+ topSitesProvider.refreshTopSitesIfCacheExpired()
}.onFailure { exception ->
logger.error("Failed to refresh contile top sites", exception)
}
}
}
- override fun onPause(owner: LifecycleOwner) {
- scope.cancel()
+ private fun isAppLinkStartup(): Boolean {
+ return startupPathProvider.startupPathForActivity == StartupPathProvider.StartupPath.VIEW
}
}
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/TopSitesRefresherTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/TopSitesRefresherTest.kt
@@ -4,6 +4,8 @@
package org.mozilla.fenix.home
+import android.content.Intent
+import androidx.lifecycle.Lifecycle
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -13,24 +15,37 @@ import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import mozilla.components.feature.top.sites.TopSite
import mozilla.components.feature.top.sites.TopSitesProvider
+import mozilla.components.support.test.rule.MainCoroutineRule
+import mozilla.components.support.utils.RunWhenReadyQueue
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
+import org.junit.Rule
import org.junit.Test
import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner
+import org.mozilla.fenix.perf.StartupPathProvider
import org.mozilla.fenix.utils.Settings
/**
* Class to test the [TopSitesRefresher]
*/
+@OptIn(ExperimentalCoroutinesApi::class) // advanceUntilIdle
class TopSitesRefresherTest {
+ private val testScheduler = TestCoroutineScheduler()
+ val testDispatcher = StandardTestDispatcher(testScheduler)
+
+ @get:Rule
+ val coroutinesRule = MainCoroutineRule(testDispatcher = testDispatcher)
+
private val lifecycleOwner = TestLifecycleOwner()
private val topSitesProvider = FakeTopSitesProvider()
private val settings: Settings = mockk(relaxed = true)
- private val testScheduler = TestCoroutineScheduler()
+ private val visualCompletenessQueue = RunWhenReadyQueue()
+ private val startupPathProvider =
+ FakeStartupPathProvider(expectedPath = StartupPathProvider.StartupPath.NOT_SET)
private lateinit var topSitesRefresher: TopSitesRefresher
@Before
@@ -38,7 +53,9 @@ class TopSitesRefresherTest {
topSitesRefresher = TopSitesRefresher(
settings = settings,
topSitesProvider = topSitesProvider,
- dispatcher = StandardTestDispatcher(testScheduler),
+ visualCompletenessQueue = visualCompletenessQueue,
+ startupPathProvider = startupPathProvider,
+ dispatcher = testDispatcher,
)
lifecycleOwner.registerObserver(
@@ -70,6 +87,79 @@ class TopSitesRefresherTest {
assertFalse(topSitesProvider.cacheRefreshed)
}
+ @Test
+ fun `GIVEN app link startup WHEN lifecycle resumes AND visual completeness is ready THEN top sites are refreshed`() =
+ runTest(context = testScheduler) {
+ // given we want to show top sites
+ every { settings.showContileFeature } returns true
+
+ // given that this is an app link startup
+ startupPathProvider.expectedPath = StartupPathProvider.StartupPath.VIEW
+
+ // given that the visual completeness queue is read
+ visualCompletenessQueue.ready()
+
+ // When we resume
+ lifecycleOwner.onResume()
+ advanceUntilIdle()
+
+ // Then validate that cache is refreshed
+ assertTrue(
+ "App link startup with visual completeness should refresh cache",
+ topSitesProvider.cacheRefreshed,
+ )
+ }
+
+ @Test
+ fun `GIVEN app link startup WHEN lifecycle resumes AND visual completeness is NOT ready THEN top sites are NOT refreshed`() =
+ runTest(context = testScheduler) {
+ // given we want to show top sites
+ every { settings.showContileFeature } returns true
+
+ // given that this is an app link startup
+ startupPathProvider.expectedPath = StartupPathProvider.StartupPath.VIEW
+
+ // When we resume
+ lifecycleOwner.onResume()
+ advanceUntilIdle()
+
+ // Then validate that cache is not refreshed
+ assertFalse(
+ "App link startup should not refresh cache before visual completeness",
+ topSitesProvider.cacheRefreshed,
+ )
+ }
+
+ @Test
+ fun `GIVEN app link startup WHEN lifecycle resumes THEN top sites are NOT refreshed only after visual completeness is ready`() =
+ runTest(context = testScheduler) {
+ // given we want to show top sites
+ every { settings.showContileFeature } returns true
+
+ // given that this is an app link startup
+ startupPathProvider.expectedPath = StartupPathProvider.StartupPath.VIEW
+
+ // When we resume
+ lifecycleOwner.onResume()
+ advanceUntilIdle()
+
+ // Then validate that cache is not refreshed
+ assertFalse(
+ "App link startup should not refresh cache BEFORE visual completeness",
+ topSitesProvider.cacheRefreshed,
+ )
+
+ // When visual completeness queue is ready
+ visualCompletenessQueue.ready()
+ advanceUntilIdle()
+
+ // Then cache should be refreshed
+ assertTrue(
+ "App link startup should refresh cache AFTER visual completeness",
+ topSitesProvider.cacheRefreshed,
+ )
+ }
+
private class FakeTopSitesProvider : TopSitesProvider {
var expectedTopSites: List<TopSite> = emptyList()
@@ -81,4 +171,14 @@ class TopSitesRefresherTest {
cacheRefreshed = true
}
}
+
+ private class FakeStartupPathProvider(
+ var expectedPath: StartupPathProvider.StartupPath,
+ ) : StartupPathProvider {
+ override val startupPathForActivity: StartupPathProvider.StartupPath
+ get() = expectedPath
+
+ override fun attachOnActivityOnCreate(lifecycle: Lifecycle, intent: Intent?) = Unit
+ override fun onIntentReceived(intent: Intent?) = Unit
+ }
}
