From da6ac8459b3dc132bec168c61d13249afb474c4c Mon Sep 17 00:00:00 2001
From: Jonathan Almeida <jalmeida@mozilla.com>
Date: Mon, 31 May 2021 12:24:59 -0400
Subject: [PATCH] [fenix] Close
 https://github.com/mozilla-mobile/fenix/issues/19731: Track metrics before we
 dismiss the tabs tray

---
 .../org/mozilla/fenix/tabstray/NavigationInteractor.kt     | 1 +
 .../java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt    | 3 ++-
 .../org/mozilla/fenix/tabstray/NavigationInteractorTest.kt | 7 ++++++-
 .../org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt     | 3 +--
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt
index b875a28ac..d619eda80 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt
@@ -105,6 +105,7 @@ class DefaultNavigationInteractor(
 ) : NavigationInteractor {
 
     override fun onTabTrayDismissed() {
+        metrics.track(Event.TabsTrayClosed)
         dismissTabTray()
     }
 
diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
index 195877bd1..c067fd0c2 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
@@ -365,8 +365,9 @@ class TabsTrayFragment : AppCompatDialogFragment() {
 
     @VisibleForTesting
     internal fun dismissTabsTray() {
+        // This should always be the last thing we do because nothing (e.g. telemetry)
+        // is guaranteed after that.
         dismissAllowingStateLoss()
-        requireComponents.analytics.metrics.track(Event.TabsTrayClosed)
     }
 
     companion object {
diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt
index 7b9484fd4..6c1d214ef 100644
--- a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt
@@ -167,7 +167,12 @@ class NavigationInteractorTest {
     @Test
     fun `onTabTrayDismissed calls dismissTabTray on DefaultNavigationInteractor`() {
         navigationInteractor.onTabTrayDismissed()
-        verify(exactly = 1) { dismissTabTray() }
+
+        // We care about the order here; anything after `dismissTabTray` is not guaranteed.
+        verifyOrder {
+            metrics.track(Event.TabsTrayClosed)
+            dismissTabTray()
+        }
     }
 
     @Test
diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt
index 1a2502469..bbdb915bd 100644
--- a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt
@@ -335,7 +335,7 @@ class TabsTrayFragmentTest {
     }
 
     @Test
-    fun `WHEN dismissTabsTray is called THEN it dismisses the tray and record this event`() {
+    fun `WHEN dismissTabsTray is called THEN it dismisses the tray`() {
         every { fragment.dismissAllowingStateLoss() } just Runs
         val metrics: MetricController = mockk(relaxed = true)
         every { context.components.analytics.metrics } returns metrics
@@ -343,6 +343,5 @@ class TabsTrayFragmentTest {
         fragment.dismissTabsTray()
 
         verify { fragment.dismissAllowingStateLoss() }
-        verify { metrics.track(Event.TabsTrayClosed) }
     }
 }