Closes mozilla-mobile#9787 separate content blocking setting from Tra…

…ckingProtectionState.enabled
Amejia481 · Mar 1, 2021 · 4f3edd8 · 4f3edd8
1 parent b57a1aa
commit 4f3edd8
Show file tree

Hide file tree

Showing 14 changed files with 410 additions and 272 deletions.
diff --git a/...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -245,14 +245,19 @@ class GeckoEngineSession(
     }
 
     /**
-     * See [EngineSession.enableTrackingProtection]
+     * See [EngineSession.updateTrackingProtection]
      */
-    override fun enableTrackingProtection(policy: TrackingProtectionPolicy) {
-        val enabled = if (privateMode) {
-            policy.useForPrivateSessions
-        } else {
-            policy.useForRegularSessions
+    override fun updateTrackingProtection(policy: TrackingProtectionPolicy) {
+        updateContentBlocking(policy)
+        val enabled = policy != TrackingProtectionPolicy.none()
+        etpEnabled = enabled
+        notifyObservers {
+            onTrackerBlockingEnabledChange(this, enabled)
         }
+    }
+
+    @VisibleForTesting
+    internal fun updateContentBlocking(policy: TrackingProtectionPolicy) {
         /**
          * As described on https://bugzilla.mozilla.org/show_bug.cgi?id=1579264,useTrackingProtection
          * is a misleading setting. When is set to true is blocking content (scripts/sub-resources).
@@ -261,19 +266,20 @@ class GeckoEngineSession(
          * [TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES].
          */
         val shouldBlockContent =
-            policy.contains(TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)
+                policy.contains(TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)
 
-        geckoSession.settings.useTrackingProtection = shouldBlockContent && enabled
-        etpEnabled = enabled
-        notifyObservers {
-            onTrackerBlockingEnabledChange(this, enabled)
+        val enabledInBrowsingMode = if (privateMode) {
+            policy.useForPrivateSessions
+        } else {
+            policy.useForRegularSessions
         }
+        geckoSession.settings.useTrackingProtection = enabledInBrowsingMode && shouldBlockContent
     }
 
     // This is a temporary solution to address
     // https://github.com/mozilla-mobile/android-components/issues/8431
     // until we eventually delete [EngineObserver] then this will not be needed.
-    private var etpEnabled: Boolean? = null
+    @VisibleForTesting internal var etpEnabled: Boolean? = null
 
     override fun register(observer: Observer) {
         super.register(observer)
@@ -292,14 +298,6 @@ class GeckoEngineSession(
         }
     }
 
-    /**
-     * See [EngineSession.disableTrackingProtection]
-     */
-    override fun disableTrackingProtection() {
-        geckoSession.settings.useTrackingProtection = false
-        notifyObservers { onTrackerBlockingEnabledChange(false) }
-    }
-
     /**
      * Indicates if this [EngineSession] should be ignored the tracking protection policies.
      * @param onResult A callback to inform if this [EngineSession] is in
@@ -1052,7 +1050,7 @@ class GeckoEngineSession(
     private fun createGeckoSession(shouldOpen: Boolean = true) {
         this.geckoSession = geckoSessionProvider()
 
-        defaultSettings?.trackingProtectionPolicy?.let { enableTrackingProtection(it) }
+        defaultSettings?.trackingProtectionPolicy?.let { updateTrackingProtection(it) }
         defaultSettings?.requestInterceptor?.let { settings.requestInterceptor = it }
         defaultSettings?.historyTrackingDelegate?.let { settings.historyTrackingDelegate = it }
         defaultSettings?.testingModeEnabled?.let { geckoSession.settings.fullAccessibilityTree = it }

diff --git a/...ecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt b/...ecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt
@@ -1127,85 +1127,125 @@ class GeckoEngineSessionTest {
     }
 
     @Test
-    fun enableTrackingProtection() {
+    fun `WHEN updating tracking protection with a recommended policy THEN etpEnabled should be enabled`() {
         whenever(runtime.settings).thenReturn(mock())
         whenever(runtime.settings.contentBlocking).thenReturn(mock())
-        val session = GeckoEngineSession(runtime, geckoSessionProvider = geckoSessionProvider)
-        val privSession = GeckoEngineSession(
-            runtime,
-            geckoSessionProvider = geckoSessionProvider,
-            privateMode = true
-        )
 
+        val session = spy(GeckoEngineSession(runtime, geckoSessionProvider = geckoSessionProvider))
         var trackerBlockingObserved = false
+
         session.register(object : EngineSession.Observer {
             override fun onTrackerBlockingEnabledChange(enabled: Boolean) {
                 trackerBlockingObserved = enabled
             }
         })
-        var privateTrackerBlockingObserved = false
-        privSession.register(object : EngineSession.Observer {
+
+        val policy = TrackingProtectionPolicy.recommended()
+        session.updateTrackingProtection(policy)
+
+        verify(session).updateContentBlocking(policy)
+        assertTrue(session.etpEnabled!!)
+        assertTrue(trackerBlockingObserved)
+    }
+
+    @Test
+    fun `WHEN updating tracking protection with a none policy THEN etpEnabled should be enabled`() {
+        whenever(runtime.settings).thenReturn(mock())
+        whenever(runtime.settings.contentBlocking).thenReturn(mock())
+
+        val session = spy(GeckoEngineSession(runtime, geckoSessionProvider = geckoSessionProvider))
+        var trackerBlockingObserved = false
+
+        session.register(object : EngineSession.Observer {
             override fun onTrackerBlockingEnabledChange(enabled: Boolean) {
-                privateTrackerBlockingObserved = enabled
+                trackerBlockingObserved = enabled
             }
         })
 
-        val allPolicy = TrackingProtectionPolicy.select(
-            trackingCategories = arrayOf(TrackingCategory.AD)
-        )
-        val regularOnlyPolicy = TrackingProtectionPolicy.select(
-            trackingCategories = arrayOf(TrackingCategory.AD)
-        ).forRegularSessionsOnly()
-        val privateOnlyPolicy = TrackingProtectionPolicy.select(
-            trackingCategories = arrayOf(TrackingCategory.AD)
-        ).forPrivateSessionsOnly()
-
-        session.enableTrackingProtection(allPolicy)
-        assertTrue(trackerBlockingObserved)
+        val policy = TrackingProtectionPolicy.none()
+        session.updateTrackingProtection(policy)
 
-        session.enableTrackingProtection(privateOnlyPolicy)
+        verify(session).updateContentBlocking(policy)
+        assertFalse(session.etpEnabled!!)
         assertFalse(trackerBlockingObserved)
-        assertEquals(
-            GeckoCookieBehavior.ACCEPT_ALL,
-            runtime.settings.contentBlocking.cookieBehavior
-        )
+    }
 
-        assertEquals(
-            ContentBlocking.AntiTracking.NONE,
-            runtime.settings.contentBlocking.antiTrackingCategories
-        )
+    @Test
+    fun `WHEN update content blocking with a policy SCRIPTS_AND_SUB_RESOURCES useForPrivateSessions being in privateMode THEN useTrackingProtection should be true`() {
+        val geckoSetting = mock<GeckoSessionSettings>()
+        val geckoSession = mock<GeckoSession>()
+
+        val session = spy(GeckoEngineSession(
+            runtime = runtime,
+            geckoSessionProvider = geckoSessionProvider,
+            privateMode = true
+        ))
 
-        assertFalse(session.geckoSession.settings.useTrackingProtection)
+        whenever(geckoSession.settings).thenReturn(geckoSetting)
 
-        session.enableTrackingProtection(regularOnlyPolicy)
-        assertTrue(trackerBlockingObserved)
+        session.geckoSession = geckoSession
 
-        privSession.enableTrackingProtection(allPolicy)
-        assertTrue(privateTrackerBlockingObserved)
+        val policy = TrackingProtectionPolicy.select(trackingCategories = arrayOf(TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)).forPrivateSessionsOnly()
 
-        privSession.enableTrackingProtection(regularOnlyPolicy)
-        assertFalse(privateTrackerBlockingObserved)
+        session.updateContentBlocking(policy)
 
-        privSession.enableTrackingProtection(privateOnlyPolicy)
-        assertTrue(privateTrackerBlockingObserved)
+        verify(geckoSetting).useTrackingProtection = true
     }
 
     @Test
-    fun disableTrackingProtection() {
-        whenever(runtime.settings.contentBlocking).thenReturn(mock())
+    fun `WHEN calling updateContentBlocking with a policy SCRIPTS_AND_SUB_RESOURCES useForRegularSessions being in privateMode THEN useTrackingProtection should be true`() {
+        val geckoSetting = mock<GeckoSessionSettings>()
+        val geckoSession = mock<GeckoSession>()
 
-        val engineSession = GeckoEngineSession(runtime, geckoSessionProvider = geckoSessionProvider)
+        val session = spy(GeckoEngineSession(
+            runtime = runtime,
+            geckoSessionProvider = geckoSessionProvider,
+            privateMode = false
+        ))
 
-        var trackerBlockingDisabledObserved = false
-        engineSession.register(object : EngineSession.Observer {
-            override fun onTrackerBlockingEnabledChange(enabled: Boolean) {
-                trackerBlockingDisabledObserved = !enabled
-            }
-        })
+        whenever(geckoSession.settings).thenReturn(geckoSetting)
+
+        session.geckoSession = geckoSession
+
+        val policy = TrackingProtectionPolicy.select(trackingCategories = arrayOf(TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)).forRegularSessionsOnly()
+
+        session.updateContentBlocking(policy)
+
+        verify(geckoSetting).useTrackingProtection = true
+    }
+
+    @Test
+    fun `WHEN updating content blocking without a policy SCRIPTS_AND_SUB_RESOURCES for any browsing mode THEN useTrackingProtection should be false`() {
+        val geckoSetting = mock<GeckoSessionSettings>()
+        val geckoSession = mock<GeckoSession>()
+
+        var session = spy(GeckoEngineSession(
+            runtime = runtime,
+            geckoSessionProvider = geckoSessionProvider,
+            privateMode = false
+        ))
+
+        whenever(geckoSession.settings).thenReturn(geckoSetting)
+        session.geckoSession = geckoSession
+
+        val policy = TrackingProtectionPolicy.none()
+
+        session.updateContentBlocking(policy)
+
+        verify(geckoSetting).useTrackingProtection = false
+
+        session = spy(GeckoEngineSession(
+            runtime = runtime,
+            geckoSessionProvider = geckoSessionProvider,
+            privateMode = true
+        ))
+
+        whenever(geckoSession.settings).thenReturn(geckoSetting)
+        session.geckoSession = geckoSession
+
+        session.updateContentBlocking(policy)
 
-        engineSession.disableTrackingProtection()
-        assertTrue(trackerBlockingDisabledObserved)
-        assertFalse(engineSession.geckoSession.settings.useTrackingProtection)
+        verify(geckoSetting, times(2)).useTrackingProtection = false
     }
 
     @Test
@@ -1220,7 +1260,7 @@ class GeckoEngineSessionTest {
             observers.add(spy(object : EngineSession.Observer {}))
         }
 
-        session.enableTrackingProtection(policy)
+        session.updateTrackingProtection(policy)
 
         observers.forEach { session.register(it) }
 
@@ -1230,7 +1270,7 @@ class GeckoEngineSessionTest {
 
         observers.forEach { session.unregister(it) }
 
-        session.enableTrackingProtection(policy.forPrivateSessionsOnly())
+        session.updateTrackingProtection(TrackingProtectionPolicy.none())
 
         observers.forEach { session.register(it) }
 

diff --git a/...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -245,14 +245,19 @@ class GeckoEngineSession(
     }
 
     /**
-     * See [EngineSession.enableTrackingProtection]
+     * See [EngineSession.updateTrackingProtection]
      */
-    override fun enableTrackingProtection(policy: TrackingProtectionPolicy) {
-        val enabled = if (privateMode) {
-            policy.useForPrivateSessions
-        } else {
-            policy.useForRegularSessions
+    override fun updateTrackingProtection(policy: TrackingProtectionPolicy) {
+        updateContentBlocking(policy)
+        val enabled = policy != TrackingProtectionPolicy.none()
+        etpEnabled = enabled
+        notifyObservers {
+            onTrackerBlockingEnabledChange(this, enabled)
         }
+    }
+
+    @VisibleForTesting
+    internal fun updateContentBlocking(policy: TrackingProtectionPolicy) {
         /**
          * As described on https://bugzilla.mozilla.org/show_bug.cgi?id=1579264,useTrackingProtection
          * is a misleading setting. When is set to true is blocking content (scripts/sub-resources).
@@ -263,17 +268,18 @@ class GeckoEngineSession(
         val shouldBlockContent =
             policy.contains(TrackingProtectionPolicy.TrackingCategory.SCRIPTS_AND_SUB_RESOURCES)
 
-        geckoSession.settings.useTrackingProtection = shouldBlockContent && enabled
-        etpEnabled = enabled
-        notifyObservers {
-            onTrackerBlockingEnabledChange(this, enabled)
+        val enabledInBrowsingMode = if (privateMode) {
+            policy.useForPrivateSessions
+        } else {
+            policy.useForRegularSessions
         }
+        geckoSession.settings.useTrackingProtection = enabledInBrowsingMode && shouldBlockContent
     }
 
     // This is a temporary solution to address
     // https://github.com/mozilla-mobile/android-components/issues/8431
     // until we eventually delete [EngineObserver] then this will not be needed.
-    private var etpEnabled: Boolean? = null
+    @VisibleForTesting internal var etpEnabled: Boolean? = null
 
     override fun register(observer: Observer) {
         super.register(observer)
@@ -292,14 +298,6 @@ class GeckoEngineSession(
         }
     }
 
-    /**
-     * See [EngineSession.disableTrackingProtection]
-     */
-    override fun disableTrackingProtection() {
-        geckoSession.settings.useTrackingProtection = false
-        notifyObservers { onTrackerBlockingEnabledChange(false) }
-    }
-
     /**
      * Indicates if this [EngineSession] should be ignored the tracking protection policies.
      * @param onResult A callback to inform if this [EngineSession] is in
@@ -1052,7 +1050,7 @@ class GeckoEngineSession(
     private fun createGeckoSession(shouldOpen: Boolean = true) {
         this.geckoSession = geckoSessionProvider()
 
-        defaultSettings?.trackingProtectionPolicy?.let { enableTrackingProtection(it) }
+        defaultSettings?.trackingProtectionPolicy?.let { updateTrackingProtection(it) }
         defaultSettings?.requestInterceptor?.let { settings.requestInterceptor = it }
         defaultSettings?.historyTrackingDelegate?.let { settings.historyTrackingDelegate = it }
         defaultSettings?.testingModeEnabled?.let { geckoSession.settings.fullAccessibilityTree = it }