From 93b13ac977b34546229582ac05ead5cb943cddde Mon Sep 17 00:00:00 2001 From: mcarare Date: Tue, 24 Nov 2020 13:35:26 +0200 Subject: [PATCH] For #15369: Add synced tabs usage metrics. --- .../java/org/mozilla/fenix/components/metrics/Event.kt | 2 ++ .../fenix/components/metrics/GleanMetricsService.kt | 4 ++++ .../main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt | 8 +++++++- .../org/mozilla/fenix/tabtray/SyncedTabsController.kt | 6 ++++-- .../main/java/org/mozilla/fenix/tabtray/TabTrayView.kt | 3 ++- .../java/org/mozilla/fenix/sync/ListenerDelegateTest.kt | 7 ++++++- .../org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt | 4 +++- 7 files changed, 28 insertions(+), 6 deletions(-) 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 5f051f6d6..ed988beae 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 @@ -187,6 +187,8 @@ sealed class Event { object TabSettingsOpened : Event() + object SyncedTabOpened : Event() + object RecentlyClosedTabsOpened : Event() // Interaction events with extras 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 80cfc5a58..9d0aa8fd9 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 @@ -674,6 +674,10 @@ private val Event.wrapper: EventWrapper<*>? { ProgressiveWebApp.backgroundKeys.valueOf(it) } ) + is Event.SyncedTabOpened -> EventWrapper( + { Events.syncedTabOpened.record(it) } + ) + is Event.RecentlyClosedTabsOpened -> EventWrapper( { Events.recentlyClosedTabsOpened.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt b/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt index 2d3b5c0a4..56255910c 100644 --- a/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt +++ b/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt @@ -21,6 +21,9 @@ import mozilla.components.browser.storage.sync.Tab import mozilla.components.feature.syncedtabs.view.SyncedTabsView import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.components import org.mozilla.fenix.sync.ext.toAdapterItem import org.mozilla.fenix.sync.ext.toStringRes import java.lang.IllegalStateException @@ -32,8 +35,9 @@ class SyncedTabsLayout @JvmOverloads constructor( ) : FrameLayout(context, attrs, defStyleAttr), SyncedTabsView { override var listener: SyncedTabsView.Listener? = null + private val metrics = context.components.analytics.metrics - private val adapter = SyncedTabsAdapter(ListenerDelegate { listener }) + private val adapter = SyncedTabsAdapter(ListenerDelegate(metrics) { listener }) private val coroutineScope = CoroutineScope(Dispatchers.Main) init { @@ -110,6 +114,7 @@ class SyncedTabsLayout @JvmOverloads constructor( * when we get a null reference, we never get a new binding to the non-null listener. */ class ListenerDelegate( + private val metrics: MetricController, private val listener: (() -> SyncedTabsView.Listener?) ) : SyncedTabsView.Listener { override fun onRefresh() { @@ -118,5 +123,6 @@ class ListenerDelegate( override fun onTabClicked(tab: Tab) { listener.invoke()?.onTabClicked(tab) + metrics.track(Event.SyncedTabOpened) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt b/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt index c41579095..b982ca21a 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/SyncedTabsController.kt @@ -21,6 +21,7 @@ import mozilla.components.browser.storage.sync.SyncedDeviceTabs import mozilla.components.feature.syncedtabs.view.SyncedTabsView import mozilla.components.lib.state.ext.flowScoped import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged +import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.sync.ListenerDelegate import org.mozilla.fenix.sync.SyncedTabsAdapter import org.mozilla.fenix.sync.ext.toAdapterList @@ -34,11 +35,12 @@ class SyncedTabsController( private val view: View, store: TabTrayDialogFragmentStore, private val concatAdapter: ConcatAdapter, - coroutineContext: CoroutineContext = Dispatchers.Main + coroutineContext: CoroutineContext = Dispatchers.Main, + metrics: MetricController ) : SyncedTabsView { override var listener: SyncedTabsView.Listener? = null - val adapter = SyncedTabsAdapter(ListenerDelegate { listener }) + val adapter = SyncedTabsAdapter(ListenerDelegate(metrics) { listener }) private val scope: CoroutineScope = CoroutineScope(coroutineContext) diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt index 59aa40fe7..5d82b2775 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -97,9 +97,10 @@ class TabTrayView( private var tabsTouchHelper: TabsTouchHelper private val collectionsButtonAdapter = SaveToCollectionsButtonAdapter(interactor, isPrivate) + private val metrics = container.context.components.analytics.metrics private val syncedTabsController = - SyncedTabsController(lifecycleOwner, view, store, concatAdapter) + SyncedTabsController(lifecycleOwner, view, store, concatAdapter, metrics = metrics) private val syncedTabsFeature = ViewBoundFeatureWrapper() private var hasLoaded = false diff --git a/app/src/test/java/org/mozilla/fenix/sync/ListenerDelegateTest.kt b/app/src/test/java/org/mozilla/fenix/sync/ListenerDelegateTest.kt index f1863bd60..c21e87833 100644 --- a/app/src/test/java/org/mozilla/fenix/sync/ListenerDelegateTest.kt +++ b/app/src/test/java/org/mozilla/fenix/sync/ListenerDelegateTest.kt @@ -8,12 +8,16 @@ import io.mockk.mockk import io.mockk.verify import mozilla.components.feature.syncedtabs.view.SyncedTabsView import org.junit.Test +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController class ListenerDelegateTest { @Test fun `delegate invokes nullable listener`() { val listener: SyncedTabsView.Listener? = mockk(relaxed = true) - val delegate = ListenerDelegate { listener } + val metrics: MetricController = mockk(relaxed = true) + + val delegate = ListenerDelegate(metrics) { listener } delegate.onRefresh() @@ -22,5 +26,6 @@ class ListenerDelegateTest { delegate.onTabClicked(mockk()) verify { listener?.onTabClicked(any()) } + verify { metrics.track(Event.SyncedTabOpened) } } } diff --git a/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt index 962611317..01e9c598f 100644 --- a/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabtray/SyncedTabsControllerTest.kt @@ -25,6 +25,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R +import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.sync.SyncedTabsViewHolder import org.mozilla.fenix.tabtray.TabTrayDialogFragmentAction.EnterMultiSelectMode @@ -65,8 +66,9 @@ class SyncedTabsControllerTest { ) view = LayoutInflater.from(testContext).inflate(R.layout.about_list_item, null) + val metrics: MetricController = mockk() controller = - SyncedTabsController(lifecycleOwner, view, store, concatAdapter, coroutineContext) + SyncedTabsController(lifecycleOwner, view, store, concatAdapter, coroutineContext, metrics) } @After