Fixes: #26320 Reorganize Nimbus Startup

This commit is contained in:
James Hugman 2022-11-01 20:42:44 +00:00 committed by mergify[bot]
parent 79521d692b
commit b3ec3062cc
4 changed files with 91 additions and 96 deletions

View File

@ -33,7 +33,7 @@ import org.mozilla.fenix.helpers.HomeActivityTestRule
*
* Say no to main thread IO! 🙅
*/
private const val EXPECTED_SUPPRESSION_COUNT = 19
private const val EXPECTED_SUPPRESSION_COUNT = 18
/**
* The number of times we call the `runBlocking` coroutine method on the main thread during this

View File

@ -50,7 +50,6 @@ import mozilla.components.service.glean.net.ConceptFetchHttpUploader
import mozilla.components.support.base.facts.register
import mozilla.components.support.base.log.Log
import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.base.observer.Observable
import mozilla.components.support.ktx.android.content.isMainProcess
import mozilla.components.support.ktx.android.content.runOnlyInMainProcess
import mozilla.components.support.locale.LocaleAwareApplication
@ -59,8 +58,6 @@ import mozilla.components.support.rusthttp.RustHttpConfig
import mozilla.components.support.rustlog.RustLog
import mozilla.components.support.utils.logElapsedTime
import mozilla.components.support.webextensions.WebExtensionSupport
import org.mozilla.experiments.nimbus.NimbusInterface
import org.mozilla.experiments.nimbus.internal.EnrolledExperiment
import org.mozilla.fenix.GleanMetrics.Addons
import org.mozilla.fenix.GleanMetrics.AndroidAutofill
import org.mozilla.fenix.GleanMetrics.CustomizeHome
@ -134,6 +131,10 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
return
}
// We can initialize Nimbus before Glean because Glean will queue messages
// before it's initialized.
initializeNimbus()
// We need to always initialize Glean and do it early here.
initializeGlean()
@ -201,7 +202,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
run {
// Attention: Do not invoke any code from a-s in this scope.
val megazordSetup = setupMegazord()
val megazordSetup = finishSetupMegazord()
setDayNightTheme()
components.strictMode.enableStrictMode(true)
@ -222,6 +223,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
}
restoreBrowserState()
restoreDownloads()
restoreMessaging()
// Just to make sure it is impossible for any application-services pieces
// to invoke parts of itself that require complete megazord initialization
@ -415,6 +417,15 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
.install(this)
}
protected open fun initializeNimbus() {
beginSetupMegazord()
// This lazily constructs the Nimbus object…
val nimbus = components.analytics.experiments
// … which we then can populate the feature configuration.
FxNimbus.initialize { nimbus }
}
/**
* Initiate Megazord sequence! Megazord Battle Mode!
*
@ -424,54 +435,40 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
* Documentation on what megazords are, and why they're needed:
* - https://github.com/mozilla/application-services/blob/master/docs/design/megazords.md
* - https://mozilla.github.io/application-services/docs/applications/consuming-megazord-libraries.html
*
* This is the initialization of the megazord without setting up networking, i.e. needing the
* engine for networking. This should do the minimum work necessary as it is done on the main
* thread, early in the app startup sequence.
*/
@OptIn(DelicateCoroutinesApi::class) // GlobalScope usage
private fun setupMegazord(): Deferred<Unit> {
private fun beginSetupMegazord() {
// Note: Megazord.init() must be called as soon as possible ...
Megazord.init()
// Give the generated FxNimbus a closure to lazily get the Nimbus object
FxNimbus.initialize { components.analytics.experiments }
initializeRustErrors(components.analytics.crashReporter)
// ... but RustHttpConfig.setClient() and RustLog.enable() can be called later.
// Once application-services has switched to using the new
// error reporting system, RustLog shouldn't input a CrashReporter
// anymore.
// (https://github.com/mozilla/application-services/issues/4981).
RustLog.enable(components.analytics.crashReporter)
}
@OptIn(DelicateCoroutinesApi::class) // GlobalScope usage
private fun finishSetupMegazord(): Deferred<Unit> {
return GlobalScope.async(Dispatchers.IO) {
initializeRustErrors(components.analytics.crashReporter)
// ... but RustHttpConfig.setClient() and RustLog.enable() can be called later.
RustHttpConfig.setClient(lazy { components.core.client })
// Once application-services has switched to using the new
// error reporting system, RustLog shouldn't input a CrashReporter
// anymore.
// (https://github.com/mozilla/application-services/issues/4981).
RustLog.enable(components.analytics.crashReporter)
// We want to ensure Nimbus is initialized as early as possible so we can
// experiment on features close to startup.
// But we need viaduct (the RustHttp client) to be ready before we do.
components.analytics.experiments.apply {
setupNimbusObserver(this)
}
// Now viaduct (the RustHttp client) is initialized we can ask Nimbus to fetch
// experiments recipes from the server.
components.analytics.experiments.fetchExperiments()
}
}
private fun setupNimbusObserver(nimbus: Observable<NimbusInterface.Observer>) {
nimbus.register(
object : NimbusInterface.Observer {
override fun onUpdatesApplied(updated: List<EnrolledExperiment>) {
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 (settings.isExperimentationEnabled) {
private fun restoreMessaging() {
if (settings().isExperimentationEnabled) {
components.appStore.dispatch(AppAction.MessagingAction.Restore)
}
reportHomeScreenSectionMetrics(settings)
}
override fun onTrimMemory(level: Int) {

View File

@ -6,7 +6,6 @@ package org.mozilla.fenix.experiments
import android.content.Context
import android.net.Uri
import android.os.StrictMode
import mozilla.components.service.nimbus.Nimbus
import mozilla.components.service.nimbus.NimbusApi
import mozilla.components.service.nimbus.NimbusAppInfo
@ -46,6 +45,9 @@ private const val TIME_OUT_LOADING_EXPERIMENT_FROM_DISK_MS = 200L
@Suppress("TooGenericExceptionCaught")
fun createNimbus(context: Context, url: String?): NimbusApi {
// Once application-services has switched to using the new
// error reporting system, we may not need this anymore.
// https://mozilla-hub.atlassian.net/browse/EXP-2868
val errorReporter: ((String, Throwable) -> Unit) = reporter@{ message, e ->
Logger.error("Nimbus error: $message", e)
@ -55,75 +57,69 @@ fun createNimbus(context: Context, url: String?): NimbusApi {
context.components.analytics.crashReporter.submitCaughtException(e)
}
return try {
// Eventually we'll want to use `NimbusDisabled` when we have no NIMBUS_ENDPOINT.
// but we keep this here to not mix feature flags and how we configure Nimbus.
val serverSettings = if (!url.isNullOrBlank()) {
if (context.settings().nimbusUsePreview) {
NimbusServerSettings(url = Uri.parse(url), collection = "nimbus-preview")
} else {
NimbusServerSettings(url = Uri.parse(url))
}
// Eventually we'll want to use `NimbusDisabled` when we have no NIMBUS_ENDPOINT.
// but we keep this here to not mix feature flags and how we configure Nimbus.
val serverSettings = if (!url.isNullOrBlank()) {
if (context.settings().nimbusUsePreview) {
NimbusServerSettings(url = Uri.parse(url), collection = "nimbus-preview")
} else {
null
NimbusServerSettings(url = Uri.parse(url))
}
} else {
null
}
// Global opt out state is stored in Nimbus, and shouldn't be toggled to `true`
// from the app unless the user does so from a UI control.
// However, the user may have opt-ed out of mako experiments already, so
// we should respect that setting here.
val enabled =
context.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) {
context.settings().isExperimentationEnabled
}
val isFirstNimbusRun = context.settings().isFirstNimbusRun
if (isFirstNimbusRun) {
context.settings().isFirstNimbusRun = false
}
// The name "fenix" here corresponds to the app_name defined for the family of apps
// that encompasses all of the channels for the Fenix app. This is defined upstream in
// the telemetry system. For more context on where the app_name come from see:
// https://probeinfo.telemetry.mozilla.org/v2/glean/app-listings
// and
// https://github.com/mozilla/probe-scraper/blob/master/repositories.yaml
val appInfo = NimbusAppInfo(
appName = "fenix",
// Note: Using BuildConfig.BUILD_TYPE is important here so that it matches the value
// passed into Glean. `Config.channel.toString()` turned out to be non-deterministic
// and would mostly produce the value `Beta` and rarely would produce `beta`.
channel = BuildConfig.BUILD_TYPE,
customTargetingAttributes = mapOf(
"isFirstRun" to context.settings().isFirstNimbusRun.toString(),
),
)
// The name "fenix" here corresponds to the app_name defined for the family of apps
// that encompasses all of the channels for the Fenix app. This is defined upstream in
// the telemetry system. For more context on where the app_name come from see:
// https://probeinfo.telemetry.mozilla.org/v2/glean/app-listings
// and
// https://github.com/mozilla/probe-scraper/blob/master/repositories.yaml
val appInfo = NimbusAppInfo(
appName = "fenix",
// Note: Using BuildConfig.BUILD_TYPE is important here so that it matches the value
// passed into Glean. `Config.channel.toString()` turned out to be non-deterministic
// and would mostly produce the value `Beta` and rarely would produce `beta`.
channel = BuildConfig.BUILD_TYPE.let { if (it == "debug") "developer" else it },
customTargetingAttributes = mapOf(
"isFirstRun" to isFirstNimbusRun.toString(),
),
)
return try {
Nimbus(context, appInfo, serverSettings, errorReporter).apply {
// We register our own internal observer for housekeeping the Nimbus SDK and
// generated code.
register(observer)
val isFirstNimbusRun = context.settings().isFirstNimbusRun
// Apply any experiment recipes we downloaded last time, or
// if this is the first time, we load the ones bundled in the res/raw
// directory.
val job = if (isFirstNimbusRun || url.isNullOrBlank()) {
applyLocalExperiments(R.raw.initial_experiments)
} else {
applyPendingExperiments()
}
// We always want `Nimbus.initialize` to happen ASAP and before any features (engine/UI)
// We always want initialize Nimbus to happen ASAP and before any features (engine/UI)
// have been initialized. For that reason, we use runBlocking here to avoid
// inconsistency in the experiments.
// We can safely do this because Nimbus does most of it's work on background threads,
// except for loading the initial experiments from disk. For this reason, we have a
// We can safely do this because Nimbus does most of its work on background threads,
// including the loading the initial experiments from disk. For this reason, we have a
// `joinOrTimeout` to limit the blocking until TIME_OUT_LOADING_EXPERIMENT_FROM_DISK_MS.
runBlockingIncrement {
val job = initialize(
isFirstNimbusRun || url.isNullOrBlank(),
R.raw.initial_experiments,
)
// We only read from disk when loading first-run experiments. This is the only time
// that we should join and block. Otherwise, we don't want to wait.
if (isFirstNimbusRun) {
context.settings().isFirstNimbusRun = false
job.joinOrTimeout(TIME_OUT_LOADING_EXPERIMENT_FROM_DISK_MS)
}
}
if (!enabled) {
// This opts out of nimbus experiments. It involves writing to disk, so does its
// work on the db thread.
globalUserParticipation = enabled
job.joinOrTimeout(TIME_OUT_LOADING_EXPERIMENT_FROM_DISK_MS)
}
// By now, on this thread, we have a fully initialized Nimbus object, ready for use:
// * we gave a 200ms timeout to the loading of a file from res/raw
// * on completion or cancellation, applyPendingExperiments or initialize was
// called, and this thread waited for that to complete.
}
} catch (e: Throwable) {
// Something went wrong. We'd like not to, but stability of the app is more important than

View File

@ -23,6 +23,8 @@ class FenixRobolectricTestApplication : FenixApplication() {
override val components = mockk<Components>()
override fun initializeNimbus() = Unit
override fun initializeGlean() = Unit
override fun setupInAllProcesses() = Unit