For #24599 - Remove Event.wrapper for Tab telemetry

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -28,8 +28,6 @@ sealed class Event {
     object AddBookmark : Event()
     object HistoryHighlightOpened : Event()
     object HistorySearchGroupOpened : Event()
-    object TabMediaPlay : Event()
-    object TabMediaPause : Event()
     object MediaPlayState : Event()
     object MediaPauseState : Event()
     object MediaStopState : Event()

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

@@ -34,7 +34,6 @@ import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
 import org.mozilla.fenix.GleanMetrics.SearchTerms
 import org.mozilla.fenix.GleanMetrics.StartOnHome
 import org.mozilla.fenix.GleanMetrics.SyncedTabs
-import org.mozilla.fenix.GleanMetrics.Tab
 import org.mozilla.fenix.GleanMetrics.Tabs
 import org.mozilla.fenix.GleanMetrics.TopSites
 import org.mozilla.fenix.GleanMetrics.Wallpapers
@@ -114,12 +113,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.SetDefaultBrowserToolbarMenuClicked -> EventWrapper<NoExtraKeys>(
             { ExperimentsDefaultBrowser.toolbarMenuClicked.record(it) }
         )
-        is Event.TabMediaPlay -> EventWrapper<NoExtraKeys>(
-            { Tab.mediaPlay.record(it) }
-        )
-        is Event.TabMediaPause -> EventWrapper<NoExtraKeys>(
-            { Tab.mediaPause.record(it) }
-        )
         is Event.MediaPlayState -> EventWrapper<NoExtraKeys>(
             { MediaState.play.record(it) }
         )

app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt

@@ -32,9 +32,8 @@ import mozilla.components.concept.engine.mediasession.MediaSession
 import mozilla.telemetry.glean.private.NoExtras
 import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.GleanMetrics.Collections
+import org.mozilla.fenix.GleanMetrics.Tab
 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.ext.increaseTapArea
 import org.mozilla.fenix.ext.removeAndDisable
@@ -55,7 +54,6 @@ import org.mozilla.fenix.tabstray.ext.isSelect
  * @param trayStore [TabsTrayStore] containing the complete state of tabs tray and methods to update that.
  * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting.
  * @param store [BrowserStore] containing the complete state of the browser and methods to update that.
- * @param metrics [MetricController] used for handling telemetry events.
  */
 @Suppress("LongParameterList")
 abstract class AbstractBrowserTabViewHolder(
@@ -66,7 +64,6 @@ abstract class AbstractBrowserTabViewHolder(
     @VisibleForTesting
     internal val featureName: String,
     private val store: BrowserStore = itemView.context.components.core.store,
-    private val metrics: MetricController = itemView.context.components.analytics.metrics
 ) : TabViewHolder(itemView) {
 
     private val faviconView: ImageView? =
@@ -194,12 +191,12 @@ abstract class AbstractBrowserTabViewHolder(
             setOnClickListener {
                 when (sessionState?.mediaSessionState?.playbackState) {
                     MediaSession.PlaybackState.PLAYING -> {
-                        metrics.track(Event.TabMediaPause)
+                        Tab.mediaPause.record(NoExtras())
                         sessionState.mediaSessionState?.controller?.pause()
                     }
 
                     MediaSession.PlaybackState.PAUSED -> {
-                        metrics.track(Event.TabMediaPlay)
+                        Tab.mediaPlay.record(NoExtras())
                         sessionState.mediaSessionState?.controller?.play()
                     }
                     else -> throw AssertionError(

app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt

@@ -166,41 +166,6 @@ class MetricControllerTest {
         }
     }
 
-    @Test
-    fun `tracking events should be sent to matching service`() {
-        val controller = ReleaseMetricController(
-            listOf(dataService1, marketingService1),
-            isDataTelemetryEnabled = { true },
-            isMarketingDataTelemetryEnabled = { true },
-            mockk()
-        )
-        every { dataService1.shouldTrack(Event.TabMediaPause) } returns false
-        every { marketingService1.shouldTrack(Event.TabMediaPause) } returns true
-
-        controller.start(MetricServiceType.Marketing)
-        controller.track(Event.TabMediaPause)
-        verify { marketingService1.track(Event.TabMediaPause) }
-    }
-
-    @Test
-    fun `tracking events should be sent to enabled service`() {
-        var enabled = true
-        val controller = ReleaseMetricController(
-            listOf(dataService1, marketingService1),
-            isDataTelemetryEnabled = { enabled },
-            isMarketingDataTelemetryEnabled = { true },
-            mockk()
-        )
-        every { dataService1.shouldTrack(Event.TabMediaPause) } returns true
-        every { marketingService1.shouldTrack(Event.TabMediaPause) } returns true
-
-        controller.start(MetricServiceType.Marketing)
-        enabled = false
-
-        controller.track(Event.TabMediaPause)
-        verify { marketingService1.track(Event.TabMediaPause) }
-    }
-
     @Test
     fun `topsites fact should set value in SharedPreference`() {
         val enabled = true

app/src/test/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolderTest.kt

@@ -6,9 +6,12 @@ package org.mozilla.fenix.tabstray.browser
 
 import android.view.LayoutInflater
 import android.view.View
+import android.widget.ImageButton
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
+import mozilla.components.browser.state.state.BrowserState
+import mozilla.components.browser.state.state.MediaSessionState
 import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.base.images.ImageLoader
@@ -17,16 +20,25 @@ import org.junit.Assert.assertTrue
 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.selection.SelectionHolder
 import org.mozilla.fenix.tabstray.TabsTrayStore
 import mozilla.components.browser.state.state.createTab
+import mozilla.components.concept.engine.mediasession.MediaSession
 import mozilla.components.lib.publicsuffixlist.PublicSuffixList
+import mozilla.telemetry.glean.testing.GleanTestRule
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Rule
+import org.mozilla.fenix.GleanMetrics.Tab
 import org.mozilla.fenix.ext.components
 
 @RunWith(FenixRobolectricTestRunner::class)
 class AbstractBrowserTabViewHolderTest {
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
+
     val store = TabsTrayStore()
     val browserStore = BrowserStore()
     val interactor = mockk<BrowserTrayInteractor>(relaxed = true)
@@ -41,7 +53,6 @@ class AbstractBrowserTabViewHolderTest {
             store,
             null,
             browserStore,
-            mockk(relaxed = true),
             interactor
         )
 
@@ -63,7 +74,6 @@ class AbstractBrowserTabViewHolderTest {
             store,
             selectionHolder,
             browserStore,
-            mockk(relaxed = true),
             interactor
         )
 
@@ -75,6 +85,80 @@ class AbstractBrowserTabViewHolderTest {
         assertTrue(selectionHolder.invoked)
     }
 
+    @Test
+    fun `WHEN the current media state is paused AND playPause button is clicked THEN the media is played AND the right metric is recorded`() {
+        every { testContext.components.publicSuffixList } returns PublicSuffixList(testContext)
+        val view = LayoutInflater.from(testContext).inflate(R.layout.tab_tray_item, null)
+        val mediaSessionController = mockk<MediaSession.Controller>(relaxed = true)
+        val mediaTab = createTab(
+            url = "url",
+            mediaSessionState = MediaSessionState(
+                mediaSessionController,
+                playbackState = MediaSession.PlaybackState.PAUSED
+            )
+        )
+        val mediaBrowserStore = BrowserStore(
+            initialState =
+            BrowserState(listOf(mediaTab))
+        )
+        val holder = TestTabTrayViewHolder(
+            view,
+            mockk(relaxed = true),
+            store,
+            TestSelectionHolder(emptySet()),
+            mediaBrowserStore,
+            interactor
+        )
+        assertFalse(Tab.mediaPlay.testHasValue())
+
+        holder.bind(mediaTab, false, mockk(), mockk())
+
+        holder.itemView.findViewById<ImageButton>(R.id.play_pause_button).performClick()
+
+        assertTrue(Tab.mediaPlay.testHasValue())
+        assertEquals(1, Tab.mediaPlay.testGetValue().size)
+        assertNull(Tab.mediaPlay.testGetValue().single().extra)
+
+        verify { mediaSessionController.play() }
+    }
+
+    @Test
+    fun `WHEN the current media state is playing AND playPause button is clicked THEN the media is paused AND the right metric is recorded`() {
+        every { testContext.components.publicSuffixList } returns PublicSuffixList(testContext)
+        val view = LayoutInflater.from(testContext).inflate(R.layout.tab_tray_item, null)
+        val mediaSessionController = mockk<MediaSession.Controller>(relaxed = true)
+        val mediaTab = createTab(
+            url = "url",
+            mediaSessionState = MediaSessionState(
+                mediaSessionController,
+                playbackState = MediaSession.PlaybackState.PLAYING
+            )
+        )
+        val mediaBrowserStore = BrowserStore(
+            initialState =
+            BrowserState(listOf(mediaTab))
+        )
+        val holder = TestTabTrayViewHolder(
+            view,
+            mockk(relaxed = true),
+            store,
+            TestSelectionHolder(emptySet()),
+            mediaBrowserStore,
+            interactor
+        )
+        assertFalse(Tab.mediaPause.testHasValue())
+
+        holder.bind(mediaTab, false, mockk(), mockk())
+
+        holder.itemView.findViewById<ImageButton>(R.id.play_pause_button).performClick()
+
+        assertTrue(Tab.mediaPause.testHasValue())
+        assertEquals(1, Tab.mediaPause.testGetValue().size)
+        assertNull(Tab.mediaPause.testGetValue().single().extra)
+
+        verify { mediaSessionController.pause() }
+    }
+
     @Suppress("LongParameterList")
     class TestTabTrayViewHolder(
         itemView: View,
@@ -82,10 +166,9 @@ class AbstractBrowserTabViewHolderTest {
         trayStore: TabsTrayStore,
         selectionHolder: SelectionHolder<TabSessionState>?,
         store: BrowserStore,
-        metrics: MetricController,
         override val browserTrayInteractor: BrowserTrayInteractor,
         featureName: String = "Test"
-    ) : AbstractBrowserTabViewHolder(itemView, imageLoader, trayStore, selectionHolder, featureName, store, metrics) {
+    ) : AbstractBrowserTabViewHolder(itemView, imageLoader, trayStore, selectionHolder, featureName, store) {
         override val thumbnailSize: Int
             get() = 30