From d4a6facd81eb18dc35131c8defcfbc07cff7a26f Mon Sep 17 00:00:00 2001 From: Mugurell Date: Thu, 11 Nov 2021 19:13:21 +0200 Subject: [PATCH] For #22155 - Store inactive tabs count telemetry when user opens tabs tray Setting this value in FenixApplication.onCreate was buggy because of a race with restoring BrowserState. Setting it here would ensure a better granularity of the events and so to more accurate reporting. --- app/metrics.yaml | 4 +-- .../org/mozilla/fenix/FenixApplication.kt | 3 -- .../mozilla/fenix/components/metrics/Event.kt | 1 + .../components/metrics/GleanMetricsService.kt | 3 ++ .../fenix/tabstray/browser/TabSorter.kt | 2 ++ .../org/mozilla/fenix/FenixApplicationTest.kt | 9 +---- .../fenix/tabstray/browser/TabSorterTest.kt | 35 +++++++++++++++++++ 7 files changed, 44 insertions(+), 13 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index e63e9a239..12e397db6 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -1255,9 +1255,9 @@ metrics: expires: "2022-02-01" inactive_tabs_count: type: quantity - lifetime: application description: | - How many inactive tabs does the user have. + How many inactive tabs does the user have, checked when the user opens + the tabs tray. Value will be 0 if the feature is disabled. send_in_pings: - metrics diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index cc59a0349..04eb6ba8e 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -80,7 +80,6 @@ import org.mozilla.fenix.components.Core import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.toolbar.ToolbarPosition -import org.mozilla.fenix.ext.actualInactiveTabs import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks import org.mozilla.fenix.utils.Settings @@ -628,8 +627,6 @@ open class FenixApplication : LocaleAwareApplication(), Provider { tabViewSetting.set(settings.getTabViewPingString()) closeTabSetting.set(settings.getTabTimeoutPingString()) - inactiveTabsCount.set(browserStore.state.actualInactiveTabs(settings).size.toLong()) - val installSourcePackage = if (SDK_INT >= Build.VERSION_CODES.R) { packageManager.getInstallSourceInfo(packageName).installingPackageName } else { diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index 2c9cf5de1..c265484ca 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -224,6 +224,7 @@ sealed class Event { override val extras: Map get() = mapOf(Preferences.turnOffInactiveTabsSurveyKeys.feedback to feedback.lowercase(Locale.ROOT)) } + data class InactiveTabsCountUpdate(val count: Int) : Event() object ProgressiveWebAppOpenFromHomescreenTap : Event() object ProgressiveWebAppInstallAsShortcut : Event() diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 67ae117ca..ff3da8eb7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -652,6 +652,9 @@ private val Event.wrapper: EventWrapper<*>? { Preferences.turnOffInactiveTabsSurvey.record(it) }, { Preferences.turnOffInactiveTabsSurveyKeys.valueOf(it) } ) + is Event.InactiveTabsCountUpdate -> EventWrapper( + { Metrics.inactiveTabsCount.set(this.count.toLong()) }, + ) is Event.TabsTrayInactiveTabsCFRGotoSettings -> EventWrapper( { TabsTray.inactiveTabsCfrSettings.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt index 7f3a11ca6..7a932d987 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt @@ -57,6 +57,8 @@ class TabSorter( if (shouldReportMetrics) { shouldReportMetrics = false + metrics.track(Event.InactiveTabsCountUpdate(inactiveTabs.size)) + if (settings.inactiveTabsAreEnabled) { metrics.track(Event.TabsTrayHasInactiveTabs(inactiveTabs.size)) } diff --git a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt index 42842810b..c5853086a 100644 --- a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt +++ b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt @@ -11,7 +11,6 @@ import io.mockk.Runs import io.mockk.every import io.mockk.just import io.mockk.mockk -import io.mockk.mockkStatic import io.mockk.spyk import io.mockk.verify import mozilla.components.browser.state.store.BrowserStore @@ -32,7 +31,6 @@ import org.mozilla.fenix.GleanMetrics.Preferences import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.toolbar.ToolbarPosition -import org.mozilla.fenix.ext.actualInactiveTabs import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.BrowsersCache import org.mozilla.fenix.utils.Settings @@ -143,13 +141,8 @@ class FenixApplicationTest { every { settings.searchTermTabGroupsAreEnabled } returns true every { application.reportHomeScreenMetrics(settings) } just Runs every { settings.inactiveTabsAreEnabled } returns true - mockkStatic("org.mozilla.fenix.ext.BrowserStateKt") { - every { browserStore.state.actualInactiveTabs(any()) } returns listOf(mockk(), mockk()) - application.setStartupMetrics(browserStore, settings, browsersCache, mozillaProductDetector) - - assertEquals(2, Metrics.inactiveTabsCount.testGetValue()) - } + application.setStartupMetrics(browserStore, settings, browsersCache, mozillaProductDetector) // Verify that browser defaults metrics are set. assertEquals("Mozilla", Metrics.distributionId.testGetValue()) diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt index 133cb8073..603cd0953 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt @@ -6,11 +6,13 @@ package org.mozilla.fenix.tabstray.browser import io.mockk.every import io.mockk.mockk +import io.mockk.verify import mozilla.components.browser.state.state.createTab import mozilla.components.support.test.libstate.ext.waitUntilIdle import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.utils.Settings @@ -284,4 +286,37 @@ class TabSorterTest { assertEquals(tabsTrayStore.state.searchTermGroups.size, 1) assertEquals(tabsTrayStore.state.normalTabs.size, 0) } + + @Test + fun `GIVEN the inactive tabs feature is enabled WHEN the tray is opened THEN we report the number of inactive tabs`() { + val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + + tabSorter.updateTabs( + listOf( + createTab(url = "url", id = "tab1", createdAt = inactiveTimestamp), + createTab(url = "url", id = "tab2", createdAt = inactiveTimestamp), + ), + selectedTabId = "tab2" + ) + tabsTrayStore.waitUntilIdle() + + verify { metrics.track(Event.InactiveTabsCountUpdate(2)) } + } + + @Test + fun `GIVEN the inactive tabs feature is disabled WHEN the tray is opened THEN we report 0 for the number of inactive tabs`() { + val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + every { settings.inactiveTabsAreEnabled }.answers { false } + + tabSorter.updateTabs( + listOf( + createTab(url = "url", id = "tab1", createdAt = inactiveTimestamp), + createTab(url = "url", id = "tab2", createdAt = inactiveTimestamp), + ), + selectedTabId = "tab2" + ) + tabsTrayStore.waitUntilIdle() + + verify { metrics.track(Event.InactiveTabsCountUpdate(0)) } + } }