Bug 1820211 - Adds tabKilled event to track when a tab was killed with form data. (#1343)

* Bug 1820211 - Adds `tabKilled` event to track when a tab was killed with form data.

- Also includes if the tab was the active tab and whether the app was in the foreground.

* Bug 1820211 - Adds tests for TelemetryMiddleware and StoreLifecycleObserver

* Bug 1820211 - Renames CheckFormDataAction to UpdateHasFormData
- Removes @property hasFormData comment from sessionState
- Moves checking formdata from TabContentMiddleware to SessionPrioritizationMiddleware

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

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:

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)
 

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)
         }

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),

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()
+    }
 }

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<NativeCodeCrash> = emptyList(),

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)
+        }
     }
 }
 

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)

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)
+    }
+}

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) {

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)
+        }
+    }
+}

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(