2
0
mirror of https://github.com/fork-maintainers/iceraven-browser synced 2024-11-07 15:20:38 +00:00

Close #26041: Re-set TrackingProtectionPolicy after Nimbus SDK is initialized

There are three issues here that we have uncovered while investigating
this bug:

 1. Settings.kt has a lazy block around `enabledTotalCookieProtection`
    which ends up caching the first result it evaluates.
 3. The `FeatureHolder` within FxNimbus caches the incorrectly
    evaluated value and returns this value hence forth.
 4. Nimbus is not ready to return a result for an engine experiment
    when we need it early on in the dependency tree initialization.

There are multiple systems that require engine to be initialized for
 them to work (e.g. Glean, Profiler, concept-fetch). In our TCP,
 experiment, we need to apply these engine settings during the engine
 initialization. So when we try and evaluate Nimbus that early on, it
 has not had time to initialize itself correctly or even use the
 engine's concept-fetch client to return the correct experiment result.
 This bug is made worse because of the first two caching bugs where we
 are always holding onto a cached value of the wrong result.

Our temporary solution is to:

 1. Remove the `lazy` around `Settings.enabledTotalCookieProtection`.
 2. Set the `FxNimbus.api` value right after we are done initializing
    `FxNimbus` and `NimbusApi` so that all future queries to FxNimbus
    will be made against a real instance of `NimbusApi`. This is a
    short-term fix for the `FeatureHolder` caching bug.
 3. Set a new TrackingProtectionPolicy that will evaluate Nimbus now
    that it is in the correct state when receive the
    `NimbusInterface.Observer.onUpdatesApplied`.

Co-authored-by: jhugman <jhugman@users.noreply.github.com>
Co-authored-by: Christian Sadilek <christian.sadilek@gmail.com>
This commit is contained in:
Jonathan Almeida 2022-08-03 21:01:15 -04:00 committed by mergify[bot]
parent 29a8c33ba5
commit f03ee91ecb
2 changed files with 18 additions and 3 deletions

View File

@ -434,12 +434,28 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
private fun setupNimbusObserver(nimbus: Observable<NimbusInterface.Observer>) {
nimbus.register(object : NimbusInterface.Observer {
override fun onUpdatesApplied(updated: List<EnrolledExperiment>) {
// To workaround a caching bug in Nimbus that appears when we try to query an
// experiment **before** `FxNimbus.initialize` is called we need to set the `api`
// value again so that we can still access the NimbusApi that is wrapped
// in `FxNimbus.initialize.getSdk`.
//
// We set this value here to minimize the race between setting the `api` value and
// the callers of FxNimbus.
// See: https://github.com/mozilla/application-services/issues/5075
FxNimbus.api = components.analytics.experiments
onNimbusStartupAndUpdate()
}
})
}
private fun onNimbusStartupAndUpdate() {
// When Nimbus has successfully started up, we can apply our engine settings experiment.
// Any previous value that was set on the engine will be overridden from those set in
// Core.Engine.DefaultSettings.
// NOTE ⚠️: Any startup experiment we want to run needs to have it's value re-applied here.
components.core.engine.settings.trackingProtectionPolicy =
components.core.trackingProtectionPolicyFactory.createTrackingProtectionPolicy()
val settings = settings()
if (FeatureFlags.messagingFeature && settings.isExperimentationEnabled) {
components.appStore.dispatch(AppAction.MessagingAction.Restore)

View File

@ -581,9 +581,8 @@ class Settings(private val appContext: Context) : PreferencesHolder {
true
)
val enabledTotalCookieProtection: Boolean by lazy {
FxNimbus.features.engineSettings.value().totalCookieProtectionEnabled
}
val enabledTotalCookieProtection: Boolean
get() = FxNimbus.features.engineSettings.value().totalCookieProtectionEnabled
val blockCookiesSelectionInCustomTrackingProtection by stringPreference(
appContext.getPreferenceKey(R.string.pref_key_tracking_protection_custom_cookies_select),