Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Change FxNimbus/Nimbus startup sequence #25266

Merged
merged 3 commits into from
May 18, 2022
Merged

Change FxNimbus/Nimbus startup sequence #25266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4a6c6c
Change FxNimbus initialization sequence
jhugman May 3, 2022
96e90ab
Change ordering of message restoration and Nimbus initialization
jhugman May 17, 2022
a627077
Address reviewer comments
jhugman May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions app/src/main/java/org/mozilla/fenix/FenixApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ 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
Expand Down Expand Up @@ -72,6 +73,7 @@ import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.ext.isCustomEngine
import org.mozilla.fenix.ext.isKnownSearchDomain
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.nimbus.FxNimbus
import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks
import org.mozilla.fenix.perf.ProfilerMarkerFactProcessor
import org.mozilla.fenix.perf.StartupTimeline
Expand Down Expand Up @@ -393,16 +395,38 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
private fun setupMegazord(): Deferred<Unit> {
// 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 }
return GlobalScope.async(Dispatchers.IO) {
// ... but RustHttpConfig.setClient() and RustLog.enable() can be called later.
RustHttpConfig.setClient(lazy { components.core.client })
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.initialize()
components.analytics.experiments.apply {
initialize()
setupNimbusObserver(this)
}
}
}

private fun setupNimbusObserver(nimbus: Observable<NimbusInterface.Observer>) {
nimbus.register(object : NimbusInterface.Observer {
override fun onUpdatesApplied(updated: List<EnrolledExperiment>) {
onNimbusStartupAndUpdate()
}
})

onNimbusStartupAndUpdate()
}

private fun onNimbusStartupAndUpdate() {
val settings = settings()
if (FeatureFlags.messagingFeature && settings.isExperimentationEnabled) {
components.appStore.dispatch(AppAction.MessagingAction.Restore)
}
reportHomeScreenSectionMetrics(settings)
}

override fun onTrimMemory(level: Int) {
Expand Down Expand Up @@ -738,6 +762,11 @@ open class FenixApplication : LocaleAwareApplication(), Provider {

@VisibleForTesting
internal fun reportHomeScreenMetrics(settings: Settings) {
reportOpeningScreenMetrics(settings)
reportHomeScreenSectionMetrics(settings)
}

private fun reportOpeningScreenMetrics(settings: Settings) {
CustomizeHome.openingScreen.set(
when {
settings.alwaysOpenTheHomepageWhenOpeningTheApp -> "homepage"
Expand All @@ -746,21 +775,18 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
else -> ""
}
)
components.analytics.experiments.register(object : NimbusInterface.Observer {
override fun onExperimentsFetched() {
if (FeatureFlags.messagingFeature && settings().isExperimentationEnabled) {
components.appStore.dispatch(AppAction.MessagingAction.Restore)
}
}
override fun onUpdatesApplied(updated: List<EnrolledExperiment>) {
CustomizeHome.jumpBackIn.set(settings.showRecentTabsFeature)
CustomizeHome.recentlySaved.set(settings.showRecentBookmarksFeature)
CustomizeHome.mostVisitedSites.set(settings.showTopSitesFeature)
CustomizeHome.recentlyVisited.set(settings.historyMetadataUIFeature)
CustomizeHome.pocket.set(settings.showPocketRecommendationsFeature)
CustomizeHome.contile.set(settings.showContileFeature)
}
})
}

private fun reportHomeScreenSectionMetrics(settings: Settings) {
// These settings are backed by Nimbus features.
// We break them out here so they can be recorded when
// `nimbus.applyPendingExperiments()` is called.
CustomizeHome.jumpBackIn.set(settings.showRecentTabsFeature)
CustomizeHome.recentlySaved.set(settings.showRecentBookmarksFeature)
CustomizeHome.mostVisitedSites.set(settings.showTopSitesFeature)
CustomizeHome.recentlyVisited.set(settings.historyMetadataUIFeature)
CustomizeHome.pocket.set(settings.showPocketRecommendationsFeature)
CustomizeHome.contile.set(settings.showContileFeature)
}

protected fun recordOnInit() {
Expand Down
4 changes: 1 addition & 3 deletions app/src/main/java/org/mozilla/fenix/components/Analytics.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,7 @@ class Analytics(
}

val experiments: NimbusApi by lazyMonitored {
createNimbus(context, BuildConfig.NIMBUS_ENDPOINT).also { api ->
FxNimbus.api = api
}
createNimbus(context, BuildConfig.NIMBUS_ENDPOINT)
}

val messagingStorage by lazyMonitored {
Expand Down
19 changes: 19 additions & 0 deletions app/src/main/java/org/mozilla/fenix/experiments/NimbusSetup.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,26 @@ import mozilla.components.service.nimbus.NimbusAppInfo
import mozilla.components.service.nimbus.NimbusDisabled
import mozilla.components.service.nimbus.NimbusServerSettings
import mozilla.components.support.base.log.logger.Logger
import org.mozilla.experiments.nimbus.NimbusInterface
import org.mozilla.experiments.nimbus.internal.EnrolledExperiment
import org.mozilla.experiments.nimbus.internal.NimbusException
import org.mozilla.fenix.BuildConfig
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.nimbus.FxNimbus

/**
* Fenix specific observer of Nimbus events.
*
* The generated code `FxNimbus` provides a cache which should be invalidated
* when the experiments recipes are updated.
*/
private val observer = object : NimbusInterface.Observer {
override fun onUpdatesApplied(updated: List<EnrolledExperiment>) {
FxNimbus.invalidateCachedValues()
}
}

@Suppress("TooGenericExceptionCaught")
fun createNimbus(context: Context, url: String?): NimbusApi {
Expand Down Expand Up @@ -69,6 +84,10 @@ fun createNimbus(context: Context, url: String?): NimbusApi {
)
)
Nimbus(context, appInfo, serverSettings, errorReporter).apply {
// We register our own internal observer for housekeeping the Nimbus SDK and
// generated code.
register(observer)

// This performs the minimal amount of work required to load branch and enrolment data
// into memory. If `getExperimentBranch` is called from another thread between here
// and the next nimbus disk write (setting `globalUserParticipation` or
Expand Down
4 changes: 0 additions & 4 deletions app/src/test/java/org/mozilla/fenix/settings/SettingsFragmentTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import io.mockk.mockk
import io.mockk.mockkObject
import kotlinx.coroutines.test.advanceUntilIdle
import mozilla.components.concept.fetch.Client
import mozilla.components.service.nimbus.NimbusDisabled
import mozilla.components.support.test.robolectric.testContext
import mozilla.components.support.test.rule.MainCoroutineRule
import mozilla.components.support.test.rule.runTestOnMain
Expand All @@ -30,7 +29,6 @@ import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getPreferenceKey
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.nimbus.FxNimbus
import org.mozilla.fenix.utils.Settings
import org.robolectric.Robolectric
import java.io.IOException
Expand All @@ -56,8 +54,6 @@ class SettingsFragmentTest {
mockkObject(Config)
every { Config.channel } returns ReleaseChannel.Nightly

FxNimbus.api = NimbusDisabled(testContext)

Comment on lines -59 to -60
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does removing this do exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The api property is no longer public; this is no longer the way we want to initialize the FxNimbus object.

val activity = Robolectric.buildActivity(FragmentActivity::class.java).create().get()
activity.supportFragmentManager.beginTransaction()
.add(settingsFragment, "test")
Expand Down