From c488bb0e110450297495d931dbd9eab6e51afcec Mon Sep 17 00:00:00 2001
From: hwinnemo <hwinnemo@students.uni-mainz.de>
Date: Mon, 1 Feb 2021 21:10:58 +0100
Subject: [PATCH] For #17724: Wait for tab to be fully created before showing
 ETP Onboarding if ETP is active

---
 .../TrackingProtectionOverlay.kt                 | 14 +++++++++-----
 .../TrackingProtectionOverlayTest.kt             | 16 ++++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt b/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt
index 6e098fa123..167c78d188 100644
--- a/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt
+++ b/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt
@@ -62,9 +62,9 @@ class TrackingProtectionOverlay(
             }.ifChanged { tab ->
                 tab.content.loading
             }
-            .collect { tab ->
-                onLoadingStateChanged(tab)
-            }
+                .collect { tab ->
+                    onLoadingStateChanged(tab)
+                }
         }
     }
 
@@ -77,7 +77,10 @@ class TrackingProtectionOverlay(
 
     @VisibleForTesting
     internal fun onLoadingStateChanged(tab: SessionState) {
-        if (!tab.content.loading && shouldShowTrackingProtectionOnboarding(tab)) {
+        if (shouldShowTrackingProtectionOnboarding(tab) &&
+            tab.content.progress == FULL_PROGRESS &&
+            settings.shouldUseTrackingProtection
+        ) {
             showTrackingProtectionOnboarding()
         }
     }
@@ -85,7 +88,7 @@ class TrackingProtectionOverlay(
     private fun shouldShowTrackingProtectionOnboarding(tab: SessionState) =
         tab.trackingProtection.enabled &&
                 tab.trackingProtection.blockedTrackers.isNotEmpty() &&
-            settings.shouldShowTrackingProtectionCfr
+                settings.shouldShowTrackingProtectionCfr
 
     @Suppress("MagicNumber", "InflateParams")
     private fun showTrackingProtectionOnboarding() {
@@ -184,6 +187,7 @@ class TrackingProtectionOverlay(
     }
 
     private companion object {
+        private const val FULL_PROGRESS = 100
         private const val BUTTON_INCREASE_DPS = 12
     }
 }
diff --git a/app/src/test/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlayTest.kt b/app/src/test/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlayTest.kt
index 59f5753c76..98da76499b 100644
--- a/app/src/test/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlayTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlayTest.kt
@@ -156,7 +156,9 @@ class TrackingProtectionOverlayTest {
     fun `show onboarding when trackers are blocked`() {
         every { toolbar.hasWindowFocus() } returns true
         every { settings.shouldShowTrackingProtectionCfr } returns true
+        every { session.content.progress } returns 100
         every { session.content.loading } returns false
+        every { settings.shouldUseTrackingProtection } returns true
         every { session.trackingProtection } returns TrackingProtectionState(
             enabled = true,
             blockedTrackers = listOf(mockk())
@@ -165,6 +167,20 @@ class TrackingProtectionOverlayTest {
         verify { settings.incrementTrackingProtectionOnboardingCount() }
     }
 
+    @Test
+    fun `no-op when trackers are blocked but not finished loading`() {
+        every { toolbar.hasWindowFocus() } returns true
+        every { settings.shouldShowTrackingProtectionCfr } returns true
+        every { session.content.progress } returns 50
+        every { session.content.loading } returns false
+        every { session.trackingProtection } returns TrackingProtectionState(
+            enabled = true,
+            blockedTrackers = listOf(mockk())
+        )
+        overlay.onLoadingStateChanged(session)
+        verify(exactly = 0) { settings.incrementTrackingProtectionOnboardingCount() }
+    }
+
     @Test
     fun `no-op when toolbar doesn't have focus`() {
         every { toolbar.hasWindowFocus() } returns false