diff --git a/app/metrics.yaml b/app/metrics.yaml index d1becb7263..3529f4f81c 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -7752,6 +7752,31 @@ engine_tab: metadata: tags: - Performance + tab_killed: + type: event + description: | + A tab was killed by the engine to free memory. + extra_keys: + foreground_tab: + description: | + Whether or not the tab was the currently active tab. + type: boolean + app_foreground: + description: | + Whether or not the app was in the foreground when the tab was killed. + type: boolean + had_form_data: + description: | + Whether or not the tab had unsubmitted form data when it was killed. + type: boolean + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1820211 + data_reviews: + - https://github.com/mozilla-mobile/firefox-android/pull/1343#issuecomment-1478535296 + notification_emails: + - android-probes@mozilla.com + - kbrosnan@mozilla.com + expires: never synced_tabs: synced_tabs_suggestion_clicked: diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 69e3dba6d0..6ad73e2898 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -51,6 +51,7 @@ import mozilla.components.service.glean.net.ConceptFetchHttpUploader import mozilla.components.support.base.facts.register import mozilla.components.support.base.log.Log import mozilla.components.support.base.log.logger.Logger +import mozilla.components.support.ktx.android.arch.lifecycle.addObservers import mozilla.components.support.ktx.android.content.isMainProcess import mozilla.components.support.ktx.android.content.runOnlyInMainProcess import mozilla.components.support.locale.LocaleAwareApplication @@ -85,6 +86,7 @@ import org.mozilla.fenix.ext.isKnownSearchDomain import org.mozilla.fenix.ext.isNotificationChannelEnabled import org.mozilla.fenix.ext.setCustomEndpointIfAvailable import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.lifecycle.StoreLifecycleObserver import org.mozilla.fenix.nimbus.FxNimbus import org.mozilla.fenix.onboarding.FenixOnboarding import org.mozilla.fenix.onboarding.MARKETING_CHANNEL_ID @@ -278,7 +280,13 @@ open class FenixApplication : LocaleAwareApplication(), Provider { components.startupActivityLog.registerInAppOnCreate(this) initVisualCompletenessQueueAndQueueTasks() - ProcessLifecycleOwner.get().lifecycle.addObserver(TelemetryLifecycleObserver(components.core.store)) + ProcessLifecycleOwner.get().lifecycle.addObservers( + TelemetryLifecycleObserver(components.core.store), + StoreLifecycleObserver( + appStore = components.appStore, + browserStore = components.core.store, + ), + ) components.analytics.metricsStorage.tryRegisterAsUsageRecorder(this) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index b38fd22577..16afc18da7 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -85,7 +85,6 @@ import org.mozilla.fenix.addons.AddonPermissionsDetailsFragmentDirections import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.browser.browsingmode.DefaultBrowsingModeManager -import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.metrics.BreadcrumbsRecorder import org.mozilla.fenix.databinding.ActivityHomeBinding import org.mozilla.fenix.exceptions.trackingprotection.TrackingProtectionExceptionsFragmentDirections @@ -439,11 +438,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { Events.defaultBrowserChanged.record(NoExtras()) } - // We attempt to send metrics onResume so that the start of new user sessions is not - // missed. Previously, this was done in FenixApplication::onCreate, but it was decided - // that we should not rely on the application being killed between user sessions. - components.appStore.dispatch(AppAction.ResumedMetricsAction) - ReEngagementNotificationWorker.setReEngagementNotificationIfNeeded(applicationContext) MessageNotificationWorker.setMessageNotificationWorker(applicationContext) } diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 32269b5463..fee96e2cb0 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -241,7 +241,7 @@ class Core( RecentlyClosedMiddleware(recentlyClosedTabsStorage, RECENTLY_CLOSED_MAX), DownloadMiddleware(context, DownloadService::class.java), ReaderViewMiddleware(), - TelemetryMiddleware(context.settings(), metrics, crashReporter), + TelemetryMiddleware(context, context.settings(), metrics, crashReporter), ThumbnailsMiddleware(thumbnailStorage), UndoMiddleware(context.getUndoDelay()), RegionMiddleware(context, locationService), diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index a3850a9d74..c1bd2c6d4b 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -194,7 +194,18 @@ sealed class AppAction : Action { } /** - * Indicates that the app has been resumed and metrics that relate to that should be sent. + * [AppAction] implementations related to the application lifecycle. */ - object ResumedMetricsAction : AppAction() + sealed class AppLifecycleAction : AppAction() { + + /** + * The application has received an ON_RESUME event. + */ + object ResumeAction : AppLifecycleAction() + + /** + * The application has received an ON_PAUSE event. + */ + object PauseAction : AppLifecycleAction() + } } diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt index 37c8fc754a..465dbdbc7e 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt @@ -51,6 +51,7 @@ import org.mozilla.fenix.wallpapers.WallpaperState * Also serves as an in memory cache of all stories mapped by category allowing for quick stories filtering. */ data class AppState( + val isForeground: Boolean = true, val inactiveTabsExpanded: Boolean = false, val firstFrameDrawn: Boolean = false, val nonFatalCrashes: List = emptyList(), diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index 8dc93b9878..d51e7ae1c6 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -130,7 +130,8 @@ internal object AppStoreReducer { ) } is AppAction.PocketStoriesCategoriesChange -> { - val updatedCategoriesState = state.copy(pocketStoriesCategories = action.storiesCategories) + val updatedCategoriesState = + state.copy(pocketStoriesCategories = action.storiesCategories) // Whenever categories change stories to be displayed needs to also be changed. updatedCategoriesState.copy( pocketStories = updatedCategoriesState.getFilteredStories(), @@ -220,7 +221,12 @@ internal object AppStoreReducer { val wallpaperState = state.wallpaperState.copy(availableWallpapers = wallpapers) state.copy(wallpaperState = wallpaperState) } - is AppAction.ResumedMetricsAction -> state + is AppAction.AppLifecycleAction.ResumeAction -> { + state.copy(isForeground = true) + } + is AppAction.AppLifecycleAction.PauseAction -> { + state.copy(isForeground = false) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt index 1e42ed2a34..4b35527f33 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsMiddleware.kt @@ -25,7 +25,7 @@ class MetricsMiddleware( } private fun handleAction(action: AppAction) = when (action) { - is AppAction.ResumedMetricsAction -> { + is AppAction.AppLifecycleAction.ResumeAction -> { metrics.track(Event.GrowthData.SetAsDefault) metrics.track(Event.GrowthData.FirstAppOpenForDay) metrics.track(Event.GrowthData.FirstWeekSeriesActivity) diff --git a/app/src/main/java/org/mozilla/fenix/lifecycle/StoreLifecycleObserver.kt b/app/src/main/java/org/mozilla/fenix/lifecycle/StoreLifecycleObserver.kt new file mode 100644 index 0000000000..212384aa1b --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/lifecycle/StoreLifecycleObserver.kt @@ -0,0 +1,33 @@ +/* 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.lifecycle + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleObserver +import androidx.lifecycle.LifecycleOwner +import mozilla.components.browser.state.action.AppLifecycleAction +import mozilla.components.browser.state.store.BrowserStore +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction + +/** + * [LifecycleObserver] to dispatch app lifecycle actions to the [AppStore] and [BrowserStore]. + */ +class StoreLifecycleObserver( + private val appStore: AppStore, + private val browserStore: BrowserStore, +) : DefaultLifecycleObserver { + override fun onPause(owner: LifecycleOwner) { + super.onPause(owner) + appStore.dispatch(AppAction.AppLifecycleAction.PauseAction) + browserStore.dispatch(AppLifecycleAction.PauseAction) + } + + override fun onResume(owner: LifecycleOwner) { + super.onResume(owner) + appStore.dispatch(AppAction.AppLifecycleAction.ResumeAction) + browserStore.dispatch(AppLifecycleAction.ResumeAction) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt index 822f0ade6f..56524b6a6d 100644 --- a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.telemetry +import android.content.Context import mozilla.components.browser.state.action.BrowserAction import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.DownloadAction @@ -26,6 +27,7 @@ import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.components import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics @@ -36,6 +38,7 @@ import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics * @property metrics [MetricController] to pass events that have been mapped from actions */ class TelemetryMiddleware( + private val context: Context, private val settings: Settings, private val metrics: MetricController, private val crashReporting: CrashReporting? = null, @@ -50,6 +53,7 @@ class TelemetryMiddleware( action: BrowserAction, ) { // Pre process actions + when (action) { is ContentAction.UpdateLoadingStateAction -> { context.state.findTab(action.sessionId)?.let { tab -> @@ -120,6 +124,13 @@ class TelemetryMiddleware( // Increment the counter of killed foreground/background tabs val tabKillLabel = if (isSelected) { "foreground" } else { "background" } EngineMetrics.kills[tabKillLabel].add() + EngineMetrics.tabKilled.record( + EngineMetrics.TabKilledExtra( + foregroundTab = isSelected, + appForeground = context.components.appStore.state.isForeground, + hadFormData = tab.content.hasFormData, + ), + ) // Record the age of the engine session of the killed foreground/background tab. if (isSelected && age != null) { diff --git a/app/src/test/java/org/mozilla/fenix/lifecycle/StoreLifecycleObserverTest.kt b/app/src/test/java/org/mozilla/fenix/lifecycle/StoreLifecycleObserverTest.kt new file mode 100644 index 0000000000..4b14a1d506 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/lifecycle/StoreLifecycleObserverTest.kt @@ -0,0 +1,52 @@ +/* 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.lifecycle + +import androidx.test.core.app.ApplicationProvider +import io.mockk.mockk +import io.mockk.verifySequence +import mozilla.components.browser.state.action.AppLifecycleAction +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.service.glean.testing.GleanTestRule +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class StoreLifecycleObserverTest { + @get:Rule + val gleanRule = GleanTestRule(ApplicationProvider.getApplicationContext()) + + @Test + fun `WHEN onPause is called THEN dispatch PauseAction`() { + val appStore: AppStore = mockk(relaxed = true) + val browserStore: BrowserStore = mockk(relaxed = true) + val observer = StoreLifecycleObserver(appStore, browserStore) + + observer.onPause(mockk()) + + verifySequence { + appStore.dispatch(AppAction.AppLifecycleAction.PauseAction) + browserStore.dispatch(AppLifecycleAction.PauseAction) + } + } + + @Test + fun `WHEN onResume is called THEN dispatch ResumeAction`() { + val appStore: AppStore = mockk(relaxed = true) + val browserStore: BrowserStore = mockk(relaxed = true) + val observer = StoreLifecycleObserver(appStore, browserStore) + + observer.onResume(mockk()) + + verifySequence { + appStore.dispatch(AppAction.AppLifecycleAction.ResumeAction) + browserStore.dispatch(AppLifecycleAction.ResumeAction) + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt index 6404a4c869..bae0577cc2 100644 --- a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.telemetry import androidx.test.core.app.ApplicationProvider +import io.mockk.every import io.mockk.mockk import io.mockk.verify import mozilla.components.browser.state.action.ContentAction @@ -33,8 +34,11 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Metrics +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.components import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings import org.robolectric.shadows.ShadowLooper @@ -44,6 +48,7 @@ import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics class TelemetryMiddlewareTest { private lateinit var store: BrowserStore + private lateinit var appStore: AppStore private lateinit var settings: Settings private lateinit var telemetryMiddleware: TelemetryMiddleware @@ -59,13 +64,14 @@ class TelemetryMiddlewareTest { @Before fun setUp() { Clock.delegate = clock - settings = Settings(testContext) - telemetryMiddleware = TelemetryMiddleware(settings, metrics) + telemetryMiddleware = TelemetryMiddleware(testContext, settings, metrics) store = BrowserStore( middleware = listOf(telemetryMiddleware) + EngineMiddleware.create(engine = mockk()), initialState = BrowserState(), ) + appStore = AppStore() + every { testContext.components.appStore } returns appStore } @After @@ -232,6 +238,7 @@ class TelemetryMiddlewareTest { TabListAction.RestoreAction( listOf( RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")), + RecoverableTab(null, TabState(url = "https://developer.mozilla.org", id = "foreground_form_data", hasFormData = true)), RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket")), RecoverableTab(null, TabState(url = "https://theverge.com", id = "background_verge")), ), @@ -241,13 +248,18 @@ class TelemetryMiddlewareTest { ).joinBlocking() assertNull(EngineMetrics.kills["foreground"].testGetValue()) + assertNull(EngineMetrics.kills["foreground-has-form-data"].testGetValue()) assertNull(EngineMetrics.kills["background"].testGetValue()) store.dispatch( EngineAction.KillEngineSessionAction("foreground"), ).joinBlocking() - assertNotNull(EngineMetrics.kills["foreground"].testGetValue()) + store.dispatch( + EngineAction.KillEngineSessionAction("foreground_form_data"), + ).joinBlocking() + + assertEquals(1, EngineMetrics.kills["foreground"].testGetValue()) } @Test @@ -282,6 +294,48 @@ class TelemetryMiddlewareTest { assertEquals(2, EngineMetrics.kills["background"].testGetValue()) } + @Test + fun `WHEN tabs gets killed THEN middleware sends an event`() { + store.dispatch( + TabListAction.RestoreAction( + listOf( + RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")), + RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket", hasFormData = true)), + ), + selectedTabId = "foreground", + restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING, + ), + ).joinBlocking() + + assertNull(EngineMetrics.tabKilled.testGetValue()) + + store.dispatch( + EngineAction.KillEngineSessionAction("background_pocket"), + ).joinBlocking() + + assertEquals(1, EngineMetrics.tabKilled.testGetValue()?.size) + EngineMetrics.tabKilled.testGetValue()?.get(0)?.extra?.also { + assertEquals("false", it["foreground_tab"]) + assertEquals("true", it["had_form_data"]) + assertEquals("true", it["app_foreground"]) + } + + appStore.dispatch( + AppAction.AppLifecycleAction.PauseAction, + ).joinBlocking() + + store.dispatch( + EngineAction.KillEngineSessionAction("foreground"), + ).joinBlocking() + + assertEquals(2, EngineMetrics.tabKilled.testGetValue()?.size) + EngineMetrics.tabKilled.testGetValue()?.get(1)?.extra?.also { + assertEquals("true", it["foreground_tab"]) + assertEquals("false", it["had_form_data"]) + assertEquals("false", it["app_foreground"]) + } + } + @Test fun `WHEN foreground tab gets killed THEN middleware records foreground age`() { store.dispatch(