commit 72c331806e9eb09df4f757bd32410681306a75ea
parent 98a7f035de8ad358602f0071d9ca800648493c8b
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Thu, 27 Nov 2025 15:27:49 +0000
Bug 2002724 - Refactor CfrMiddleware to use dependency injection. r=android-reviewers,giorga
This patch refactors `CfrMiddleware` to take `AppStore` and `Settings` as constructor arguments instead of `Context`. This removes the need for `Robolectric` in tests, allowing `CfrMiddlewareTest` to be rewritten as a standard JUnit test with Mockito.
Additionally, the local `isCurrentTabSecure` state in `CfrMiddleware` is removed. The security status is now derived directly from the `BrowserState` when determining whether to show the tracking protection CFR.
Differential Revision: https://phabricator.services.mozilla.com/D274262
Diffstat:
3 files changed, 299 insertions(+), 106 deletions(-)
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/Components.kt
@@ -207,7 +207,7 @@ class Components(
AdsTelemetryMiddleware(adsTelemetry),
BlockedTrackersMiddleware(context),
RecordingDevicesMiddleware(context, notificationsDelegate),
- CfrMiddleware(context),
+ CfrMiddleware(appStore, settings),
FileUploadsDirCleanerMiddleware(fileUploadsDirCleaner),
) + EngineMiddleware.create(
engine,
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/cfr/CfrMiddleware.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/cfr/CfrMiddleware.kt
@@ -4,10 +4,9 @@
package org.mozilla.focus.cfr
-import android.content.Context
+import androidx.annotation.VisibleForTesting
import androidx.core.net.toUri
import mozilla.components.browser.state.action.BrowserAction
-import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.CookieBannerAction
import mozilla.components.browser.state.action.TrackingProtectionAction
import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
@@ -16,23 +15,26 @@ import mozilla.components.concept.engine.EngineSession
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.MiddlewareContext
import mozilla.telemetry.glean.private.NoExtras
+import org.mozilla.experiments.nimbus.internal.FeatureHolder
import org.mozilla.focus.GleanMetrics.CookieBanner
import org.mozilla.focus.cookiebanner.CookieBannerOption
-import org.mozilla.focus.ext.components
-import org.mozilla.focus.ext.settings
import org.mozilla.focus.ext.truncatedHost
import org.mozilla.focus.nimbus.FocusNimbus
import org.mozilla.focus.nimbus.Onboarding
import org.mozilla.focus.state.AppAction
+import org.mozilla.focus.state.AppStore
+import org.mozilla.focus.utils.Settings
/**
* Middleware used to intercept browser store actions in order to decide when should we display a specific CFR
*/
-class CfrMiddleware(private val appContext: Context) : Middleware<BrowserState, BrowserAction> {
- private val onboardingFeature = FocusNimbus.features.onboarding
- private lateinit var onboardingConfig: Onboarding
- private val components = appContext.components
- private var isCurrentTabSecure = false
+class CfrMiddleware(
+ private val appStore: AppStore,
+ private val settings: Settings,
+ private val onboardingProvider: () -> FeatureHolder<Onboarding> = {
+ FocusNimbus.features.onboarding
+ },
+) : Middleware<BrowserState, BrowserAction> {
private var tpExposureAlreadyRecorded = false
override fun invoke(
@@ -40,13 +42,11 @@ class CfrMiddleware(private val appContext: Context) : Middleware<BrowserState,
next: (BrowserAction) -> Unit,
action: BrowserAction,
) {
- onboardingConfig = onboardingFeature.value()
- if (onboardingConfig.isCfrEnabled) {
- next(action)
+ next(action)
+
+ if (onboardingProvider().value().isCfrEnabled) {
showCookieBannerCfr(action)
- showTrackingProtectionCfr(action, context)
- } else {
- next(action)
+ showTrackingProtectionCfr(action, context.state)
}
}
@@ -58,7 +58,7 @@ class CfrMiddleware(private val appContext: Context) : Middleware<BrowserState,
otherCfrHasBeenShown()
) {
CookieBanner.cookieBannerCfrShown.record(NoExtras())
- components.appStore.dispatch(
+ appStore.dispatch(
AppAction.ShowCookieBannerCfrChange(true),
)
}
@@ -66,12 +66,9 @@ class CfrMiddleware(private val appContext: Context) : Middleware<BrowserState,
private fun showTrackingProtectionCfr(
action: BrowserAction,
- context: MiddlewareContext<BrowserState, BrowserAction>,
+ state: BrowserState,
) {
- if (action is ContentAction.UpdateSecurityInfoAction) {
- isCurrentTabSecure = action.securityInfo.secure
- }
- if (shouldShowCfrForTrackingProtection(action = action, browserState = context.state)) {
+ if (shouldShowCfrForTrackingProtection(action = action, browserState = state)) {
if (tpExposureAlreadyRecorded) {
// do not record exposure twice
} else {
@@ -79,7 +76,7 @@ class CfrMiddleware(private val appContext: Context) : Middleware<BrowserState,
tpExposureAlreadyRecorded = true
}
- components.appStore.dispatch(
+ appStore.dispatch(
AppAction.ShowTrackingProtectionCfrChange(
mapOf((action as TrackingProtectionAction.TrackerBlockedAction).tabId to true),
),
@@ -87,38 +84,46 @@ class CfrMiddleware(private val appContext: Context) : Middleware<BrowserState,
}
}
- private fun isMozillaUrl(browserState: BrowserState): Boolean {
+ @VisibleForTesting
+ internal fun isMozillaUrl(browserState: BrowserState): Boolean {
return browserState.findTabOrCustomTabOrSelectedTab(
browserState.selectedTabId,
)?.content?.url?.toUri()?.truncatedHost()?.substringBefore(".") == ("mozilla")
}
- private fun isActionSecure(action: BrowserAction) =
- action is TrackingProtectionAction.TrackerBlockedAction && isCurrentTabSecure
+ private fun isActionSecure(action: BrowserAction, browserState: BrowserState) =
+ action is TrackingProtectionAction.TrackerBlockedAction &&
+ action.tabId == browserState.selectedTabId &&
+ isSessionSecure(browserState)
+
+ private fun isSessionSecure(browserState: BrowserState) =
+ browserState.findTabOrCustomTabOrSelectedTab(
+ browserState.selectedTabId,
+ )?.content?.securityInfo?.secure == true
private fun shouldShowCfrForTrackingProtection(
action: BrowserAction,
browserState: BrowserState,
) = (
- isActionSecure(action = action) &&
- !isMozillaUrl(browserState = browserState) &&
- components.settings.shouldShowCfrForTrackingProtection &&
- !components.appStore.state.showEraseTabsCfr
- )
+ isActionSecure(action = action, browserState = browserState) &&
+ !isMozillaUrl(browserState = browserState) &&
+ settings.shouldShowCfrForTrackingProtection &&
+ !appStore.state.showEraseTabsCfr
+ )
private fun otherCfrHasBeenShown(): Boolean {
return (
- !appContext.settings.shouldShowCfrForTrackingProtection &&
- !components.appStore.state.showEraseTabsCfr
+ !settings.shouldShowCfrForTrackingProtection &&
+ !appStore.state.showEraseTabsCfr
)
}
private fun shouldShowCookieBannerCfr(action: CookieBannerAction.UpdateStatusAction): Boolean {
return (
- !appContext.settings.isFirstRun &&
- appContext.settings.shouldShowCookieBannerCfr &&
- appContext.settings.isCookieBannerEnable &&
- appContext.settings.getCurrentCookieBannerOptionFromSharePref() ==
+ !settings.isFirstRun &&
+ settings.shouldShowCookieBannerCfr &&
+ settings.isCookieBannerEnable &&
+ settings.getCurrentCookieBannerOptionFromSharePref() ==
CookieBannerOption.CookieBannerRejectAll() &&
action.status == EngineSession.CookieBannerHandlingStatus.HANDLED
)
diff --git a/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/cfr/CfrMiddlewareTest.kt b/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/cfr/CfrMiddlewareTest.kt
@@ -4,106 +4,294 @@
package org.mozilla.focus.cfr
import mozilla.components.browser.state.action.ContentAction
+import mozilla.components.browser.state.action.CookieBannerAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.action.TrackingProtectionAction
+import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.SecurityInfoState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.content.blocking.Tracker
-import mozilla.components.support.test.robolectric.testContext
-import org.junit.Assert.assertFalse
-import org.junit.Assert.assertTrue
+import mozilla.components.support.test.any
+import mozilla.components.support.test.whenever
import org.junit.Before
import org.junit.Test
-import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.Mockito.doReturn
+import org.mockito.Mockito.never
+import org.mockito.Mockito.spy
+import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
-import org.mozilla.focus.TestFocusApplication
-import org.mozilla.focus.ext.components
-import org.mozilla.focus.nimbus.FocusNimbus
+import org.mozilla.experiments.nimbus.internal.FeatureHolder
+import org.mozilla.focus.cookiebanner.CookieBannerOption
import org.mozilla.focus.nimbus.Onboarding
+import org.mozilla.focus.state.AppAction
+import org.mozilla.focus.state.AppState
import org.mozilla.focus.state.AppStore
-import org.robolectric.RobolectricTestRunner
-import org.robolectric.annotation.Config
+import org.mozilla.focus.utils.Settings
-@RunWith(RobolectricTestRunner::class)
-@Config(application = TestFocusApplication::class)
class CfrMiddlewareTest {
- private lateinit var onboardingExperiment: Onboarding
- private val browserStore: BrowserStore = testContext.components.store
- private val appStore: AppStore = testContext.components.appStore
+
+ @Mock
+ private lateinit var onboardingExperiment: FeatureHolder<Onboarding>
+
+ @Mock
+ private lateinit var onboardingConfig: Onboarding
+
+ @Mock
+ private lateinit var appStore: AppStore
+
+ @Mock
+ private lateinit var appState: AppState
+
+ @Mock
+ private lateinit var settings: Settings
+ private lateinit var browserStore: BrowserStore
+ private lateinit var cfrMiddleware: CfrMiddleware
@Before
fun setUp() {
MockitoAnnotations.openMocks(this)
- onboardingExperiment = FocusNimbus.features.onboarding.value()
+ whenever(onboardingExperiment.value()).thenReturn(onboardingConfig)
+ whenever(onboardingConfig.isCfrEnabled).thenReturn(true)
+
+ whenever(settings.isFirstRun).thenReturn(false)
+ whenever(settings.shouldShowCfrForTrackingProtection).thenReturn(true)
+ whenever(settings.shouldShowCookieBannerCfr).thenReturn(true)
+ whenever(settings.isCookieBannerEnable).thenReturn(true)
+ whenever(settings.getCurrentCookieBannerOptionFromSharePref()).thenReturn(CookieBannerOption.CookieBannerRejectAll())
+
+ whenever(appStore.state).thenReturn(appState)
+ whenever(appState.showEraseTabsCfr).thenReturn(false)
+
+ cfrMiddleware = spy(CfrMiddleware(appStore, settings) { onboardingExperiment })
+
+ browserStore = BrowserStore(
+ initialState = BrowserState(
+ tabs = listOf(),
+ ),
+ middleware = listOf(cfrMiddleware),
+ )
}
@Test
fun `GIVEN shouldShowCfrForTrackingProtection is true WHEN UpdateSecurityInfoAction is intercepted THEN showTrackingProtectionCfr is changed to true`() {
- if (onboardingExperiment.isCfrEnabled) {
- val updateSecurityInfoAction = ContentAction.UpdateSecurityInfoAction(
- "1",
- SecurityInfoState(
- secure = true,
- host = "test.org",
- issuer = "Test",
- ),
- )
- val trackerBlockedAction = TrackingProtectionAction.TrackerBlockedAction(
- tabId = "1",
- tracker = Tracker(
- url = "test.org",
- trackingCategories = listOf(EngineSession.TrackingProtectionPolicy.TrackingCategory.CRYPTOMINING),
- cookiePolicies = listOf(EngineSession.TrackingProtectionPolicy.CookiePolicy.ACCEPT_NONE),
- ),
- )
-
- browserStore.dispatch(updateSecurityInfoAction)
- browserStore.dispatch(trackerBlockedAction)
-
- assertTrue(appStore.state.showTrackingProtectionCfrForTab.getOrDefault("1", false))
- }
+ doReturn(false).`when`(cfrMiddleware).isMozillaUrl(any())
+
+ val tab = createTab(tabId = 1, isSecure = true)
+ browserStore.dispatch(TabListAction.AddTabAction(tab))
+ browserStore.dispatch(TabListAction.SelectTabAction(tab.id))
+
+ val trackerBlockedAction = TrackingProtectionAction.TrackerBlockedAction(
+ tabId = "1",
+ tracker = Tracker(
+ url = "test.org",
+ trackingCategories = listOf(EngineSession.TrackingProtectionPolicy.TrackingCategory.CRYPTOMINING),
+ cookiePolicies = listOf(EngineSession.TrackingProtectionPolicy.CookiePolicy.ACCEPT_NONE),
+ ),
+ )
+
+ browserStore.dispatch(trackerBlockedAction)
+
+ verify(appStore).dispatch(
+ AppAction.ShowTrackingProtectionCfrChange(mapOf("1" to true)),
+ )
}
@Test
fun `GIVEN insecure tab WHEN UpdateSecurityInfoAction is intercepted THEN showTrackingProtectionCfr is not changed to true`() {
- if (onboardingExperiment.isCfrEnabled) {
- val insecureTab = createTab(isSecure = false)
- val updateSecurityInfoAction = ContentAction.UpdateSecurityInfoAction(
- "1",
- SecurityInfoState(
- secure = false,
- host = "test.org",
- issuer = "Test",
- ),
- )
-
- browserStore.dispatch(TabListAction.AddTabAction(insecureTab))
- browserStore.dispatch(updateSecurityInfoAction)
-
- assertFalse(appStore.state.showTrackingProtectionCfrForTab.getOrDefault("1", false))
- }
+ doReturn(false).`when`(cfrMiddleware).isMozillaUrl(any())
+
+ val insecureTab = createTab(isSecure = false)
+
+ browserStore.dispatch(TabListAction.AddTabAction(insecureTab))
+ browserStore.dispatch(TabListAction.SelectTabAction(insecureTab.id))
+
+ val updateSecurityInfoAction = ContentAction.UpdateSecurityInfoAction(
+ "1",
+ SecurityInfoState(
+ secure = false,
+ host = "test.org",
+ issuer = "Test",
+ ),
+ )
+ browserStore.dispatch(updateSecurityInfoAction)
+
+ val trackerBlockedAction = TrackingProtectionAction.TrackerBlockedAction(
+ tabId = "1",
+ tracker = Tracker(
+ url = "test.org",
+ trackingCategories = listOf(EngineSession.TrackingProtectionPolicy.TrackingCategory.CRYPTOMINING),
+ cookiePolicies = listOf(EngineSession.TrackingProtectionPolicy.CookiePolicy.ACCEPT_NONE),
+ ),
+ )
+ browserStore.dispatch(trackerBlockedAction)
+
+ verify(appStore, never()).dispatch(
+ AppAction.ShowTrackingProtectionCfrChange(mapOf("1" to true)),
+ )
}
@Test
fun `GIVEN mozilla tab WHEN UpdateSecurityInfoAction is intercepted THEN showTrackingProtectionCfr is not changed to true`() {
- if (onboardingExperiment.isCfrEnabled) {
- val mozillaTab = createTab(tabId = 1, tabUrl = "https://www.mozilla.org")
- val updateSecurityInfoAction = ContentAction.UpdateSecurityInfoAction(
- "1",
- SecurityInfoState(
- secure = true,
- host = "test.org",
- issuer = "Test",
- ),
- )
- browserStore.dispatch(TabListAction.AddTabAction(mozillaTab))
- browserStore.dispatch(updateSecurityInfoAction)
-
- assertFalse(appStore.state.showTrackingProtectionCfrForTab.getOrDefault("1", false))
- }
+ doReturn(true).`when`(cfrMiddleware).isMozillaUrl(any())
+
+ val mozillaTab = createTab(tabId = 1, tabUrl = "https://www.mozilla.org", isSecure = true)
+
+ browserStore.dispatch(TabListAction.AddTabAction(mozillaTab))
+ browserStore.dispatch(TabListAction.SelectTabAction(mozillaTab.id))
+
+ val updateSecurityInfoAction = ContentAction.UpdateSecurityInfoAction(
+ "1",
+ SecurityInfoState(
+ secure = true,
+ host = "test.org",
+ issuer = "Test",
+ ),
+ )
+ browserStore.dispatch(updateSecurityInfoAction)
+
+ val trackerBlockedAction = TrackingProtectionAction.TrackerBlockedAction(
+ tabId = "1",
+ tracker = Tracker(
+ url = "test.org",
+ trackingCategories = listOf(EngineSession.TrackingProtectionPolicy.TrackingCategory.CRYPTOMINING),
+ cookiePolicies = listOf(EngineSession.TrackingProtectionPolicy.CookiePolicy.ACCEPT_NONE),
+ ),
+ )
+ browserStore.dispatch(trackerBlockedAction)
+
+ verify(appStore, never()).dispatch(
+ AppAction.ShowTrackingProtectionCfrChange(mapOf("1" to true)),
+ )
+ }
+
+ @Test
+ fun `GIVEN settings prevent CFR WHEN TrackerBlockedAction is intercepted THEN showTrackingProtectionCfr is not dispatched`() {
+ whenever(settings.shouldShowCfrForTrackingProtection).thenReturn(false)
+ doReturn(false).`when`(cfrMiddleware).isMozillaUrl(any())
+
+ val tab = createTab(tabId = 1, isSecure = true)
+ browserStore.dispatch(TabListAction.AddTabAction(tab))
+ browserStore.dispatch(TabListAction.SelectTabAction(tab.id))
+
+ val trackerBlockedAction = TrackingProtectionAction.TrackerBlockedAction(
+ tabId = "1",
+ tracker = Tracker(
+ url = "test.org",
+ trackingCategories = listOf(EngineSession.TrackingProtectionPolicy.TrackingCategory.CRYPTOMINING),
+ cookiePolicies = listOf(EngineSession.TrackingProtectionPolicy.CookiePolicy.ACCEPT_NONE),
+ ),
+ )
+ browserStore.dispatch(trackerBlockedAction)
+
+ verify(appStore, never()).dispatch(
+ AppAction.ShowTrackingProtectionCfrChange(mapOf("1" to true)),
+ )
+ }
+
+ @Test
+ fun `GIVEN erase tabs CFR is shown WHEN TrackerBlockedAction is intercepted THEN showTrackingProtectionCfr is not dispatched`() {
+ whenever(appState.showEraseTabsCfr).thenReturn(true)
+ doReturn(false).`when`(cfrMiddleware).isMozillaUrl(any())
+
+ val tab = createTab(tabId = 1, isSecure = true)
+ browserStore.dispatch(TabListAction.AddTabAction(tab))
+ browserStore.dispatch(TabListAction.SelectTabAction(tab.id))
+
+ val trackerBlockedAction = TrackingProtectionAction.TrackerBlockedAction(
+ tabId = "1",
+ tracker = Tracker(
+ url = "test.org",
+ trackingCategories = listOf(EngineSession.TrackingProtectionPolicy.TrackingCategory.CRYPTOMINING),
+ cookiePolicies = listOf(EngineSession.TrackingProtectionPolicy.CookiePolicy.ACCEPT_NONE),
+ ),
+ )
+ browserStore.dispatch(trackerBlockedAction)
+
+ verify(appStore, never()).dispatch(
+ AppAction.ShowTrackingProtectionCfrChange(mapOf("1" to true)),
+ )
+ }
+
+ @Test
+ fun `GIVEN cookie banner action handled WHEN conditions met THEN show cookie banner CFR`() {
+ val action = CookieBannerAction.UpdateStatusAction(
+ "1",
+ EngineSession.CookieBannerHandlingStatus.HANDLED,
+ )
+
+ whenever(settings.shouldShowCfrForTrackingProtection).thenReturn(false)
+
+ browserStore.dispatch(action)
+
+ verify(appStore).dispatch(AppAction.ShowCookieBannerCfrChange(true))
+ }
+
+ @Test
+ fun `GIVEN cookie banner action not handled WHEN conditions met THEN do not show cookie banner CFR`() {
+ val action = CookieBannerAction.UpdateStatusAction(
+ "1",
+ EngineSession.CookieBannerHandlingStatus.DETECTED,
+ )
+
+ browserStore.dispatch(action)
+
+ verify(appStore, never()).dispatch(AppAction.ShowCookieBannerCfrChange(true))
+ }
+
+ @Test
+ fun `GIVEN cookie banner action handled but first run WHEN conditions met THEN do not show cookie banner CFR`() {
+ whenever(settings.isFirstRun).thenReturn(true)
+ val action = CookieBannerAction.UpdateStatusAction(
+ "1",
+ EngineSession.CookieBannerHandlingStatus.HANDLED,
+ )
+
+ browserStore.dispatch(action)
+
+ verify(appStore, never()).dispatch(AppAction.ShowCookieBannerCfrChange(true))
+ }
+
+ @Test
+ fun `GIVEN cookie banner action handled but setting disabled WHEN conditions met THEN do not show cookie banner CFR`() {
+ whenever(settings.shouldShowCookieBannerCfr).thenReturn(false)
+ val action = CookieBannerAction.UpdateStatusAction(
+ "1",
+ EngineSession.CookieBannerHandlingStatus.HANDLED,
+ )
+
+ browserStore.dispatch(action)
+
+ verify(appStore, never()).dispatch(AppAction.ShowCookieBannerCfrChange(true))
+ }
+
+ @Test
+ fun `GIVEN cookie banner action handled but feature disabled WHEN conditions met THEN do not show cookie banner CFR`() {
+ whenever(settings.isCookieBannerEnable).thenReturn(false)
+ val action = CookieBannerAction.UpdateStatusAction(
+ "1",
+ EngineSession.CookieBannerHandlingStatus.HANDLED,
+ )
+
+ browserStore.dispatch(action)
+
+ verify(appStore, never()).dispatch(AppAction.ShowCookieBannerCfrChange(true))
+ }
+
+ @Test
+ fun `GIVEN cookie banner action handled but option not reject all WHEN conditions met THEN do not show cookie banner CFR`() {
+ whenever(settings.getCurrentCookieBannerOptionFromSharePref()).thenReturn(CookieBannerOption.CookieBannerDisabled())
+ val action = CookieBannerAction.UpdateStatusAction(
+ "1",
+ EngineSession.CookieBannerHandlingStatus.HANDLED,
+ )
+
+ browserStore.dispatch(action)
+
+ verify(appStore, never()).dispatch(AppAction.ShowCookieBannerCfrChange(true))
}
private fun createTab(
