Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Commit

Permalink
For #15369: Add synced tabs usage metrics.
Browse files Browse the repository at this point in the history
  • Loading branch information
mcarare committed Dec 15, 2020
1 parent 686aab5 commit 931b232
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ sealed class Event {

object TabSettingsOpened : Event()

object SyncedTabOpened : Event()

object RecentlyClosedTabsOpened : Event()

// Interaction events with extras
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,10 @@ private val Event.wrapper: EventWrapper<*>?
{ ProgressiveWebApp.backgroundKeys.valueOf(it) }
)

is Event.SyncedTabOpened -> EventWrapper<NoExtraKeys>(
{ Events.syncedTabOpened.record(it) }
)

is Event.RecentlyClosedTabsOpened -> EventWrapper<NoExtraKeys>(
{ Events.recentlyClosedTabsOpened.record(it) }
)
Expand Down
8 changes: 7 additions & 1 deletion app/src/main/java/org/mozilla/fenix/sync/SyncedTabsLayout.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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() {
Expand All @@ -118,5 +123,6 @@ class ListenerDelegate(

override fun onTabClicked(tab: Tab) {
listener.invoke()?.onTabClicked(tab)
metrics.track(Event.SyncedTabOpened)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
3 changes: 2 additions & 1 deletion app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<SyncedTabsFeature>()

private var hasLoaded = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -22,5 +26,6 @@ class ListenerDelegateTest {
delegate.onTabClicked(mockk())

verify { listener?.onTabClicked(any()) }
verify { metrics.track(Event.SyncedTabOpened) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 931b232

Please sign in to comment.