From f49fc6dad26ddde5817d25920ecb455d99d94bb6 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Tue, 7 Apr 2020 14:41:57 -0700 Subject: [PATCH] For #8803: hook up frameworkStart metric. --- .../org/mozilla/fenix/FenixApplication.kt | 11 ++++ .../java/org/mozilla/fenix/HomeActivity.kt | 6 ++- .../org/mozilla/fenix/perf/StartupTimeline.kt | 50 ++++++++++++++++++- .../fenix/MigratingFenixApplication.kt | 3 +- ...tartupHomeActivityLifecycleObserverTest.kt | 38 ++++++++++++++ 5 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/perf/StartupHomeActivityLifecycleObserverTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 8753ce6eb..dad8eb95d 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -38,6 +38,7 @@ import org.mozilla.fenix.FeatureFlags.webPushIntegration import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.metrics.MetricServiceType import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.perf.StartupTimeline import org.mozilla.fenix.push.PushFxaIntegration import org.mozilla.fenix.push.WebPushEngineIntegration import org.mozilla.fenix.session.NotificationSessionObserver @@ -49,6 +50,10 @@ import org.mozilla.fenix.utils.Settings @SuppressLint("Registered") @Suppress("TooManyFunctions", "LargeClass") open class FenixApplication : LocaleAwareApplication() { + init { + recordOnInit() // DO NOT MOVE ANYTHING ABOVE HERE: the timing of this measurement is critical. + } + private val logger = Logger("FenixApplication") open val components by lazy { Components(this) } @@ -365,4 +370,10 @@ open class FenixApplication : LocaleAwareApplication() { Logger.error("Failed to initialize web extension support", e) } } + + protected fun recordOnInit() { + // This gets called by more than one process. Ideally we'd only run this in the main process + // but the code to check which process we're in crashes because the Context isn't valid yet. + StartupTimeline.onApplicationInit() + } } diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 627826a3a..30c897e58 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -33,6 +33,7 @@ import mozilla.components.concept.engine.EngineView import mozilla.components.feature.contextmenu.ext.DefaultSelectionActionDelegate import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.support.base.feature.UserInteractionHandler +import mozilla.components.support.ktx.android.arch.lifecycle.addObservers import mozilla.components.support.ktx.android.content.share import mozilla.components.support.ktx.kotlin.isUrl import mozilla.components.support.ktx.kotlin.toNormalizedUrl @@ -146,7 +147,10 @@ open class HomeActivity : LocaleAwareAppCompatActivity() { } supportActionBar?.hide() - lifecycle.addObserver(webExtensionPopupFeature) + lifecycle.addObservers( + webExtensionPopupFeature, + StartupTimeline.homeActivityLifecycleObserver + ) StartupTimeline.onActivityCreateEndHome(this) } diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupTimeline.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupTimeline.kt index f2561476b..a761520ba 100644 --- a/app/src/main/java/org/mozilla/fenix/perf/StartupTimeline.kt +++ b/app/src/main/java/org/mozilla/fenix/perf/StartupTimeline.kt @@ -5,8 +5,17 @@ package org.mozilla.fenix.perf import androidx.annotation.UiThread +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleObserver +import androidx.lifecycle.OnLifecycleEvent +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.launch +import mozilla.components.service.glean.private.NoReasonCodes +import mozilla.components.service.glean.private.PingType +import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.home.sessioncontrol.viewholders.topsites.TopSiteItemViewHolder +import org.mozilla.fenix.perf.StartupTimeline.onApplicationInit import org.mozilla.fenix.perf.StartupTimelineStateMachine.StartupActivity import org.mozilla.fenix.perf.StartupTimelineStateMachine.StartupDestination import org.mozilla.fenix.perf.StartupTimelineStateMachine.StartupState @@ -20,12 +29,25 @@ import org.mozilla.fenix.perf.StartupTimelineStateMachine.StartupState * This class, and its dependencies, may need to be modified for any changes in startup. * * This class is not thread safe and should only be called from the main thread. + * + * [onApplicationInit] is called from multiple processes. To minimize overhead, the class + * dependencies are lazily initialized. */ @UiThread object StartupTimeline { private var state: StartupState = StartupState.Cold(StartupDestination.UNKNOWN) - private val reportFullyDrawn = StartupReportFullyDrawn() + + private val reportFullyDrawn by lazy { StartupReportFullyDrawn() } + private val frameworkStartMeasurement by lazy { StartupFrameworkStartMeasurement() } + internal val homeActivityLifecycleObserver by lazy { + StartupHomeActivityLifecycleObserver(frameworkStartMeasurement) + } + + fun onApplicationInit() { + // This gets called from multiple processes: don't do anything expensive. See call site for details. + frameworkStartMeasurement.onApplicationInit() + } fun onActivityCreateEndIntentReceiver() { advanceState(StartupActivity.INTENT_RECEIVER) @@ -45,3 +67,29 @@ object StartupTimeline { state = StartupTimelineStateMachine.getNextState(state, startingActivity) } } + +/** + * A [LifecycleObserver] for [HomeActivity] focused on startup performance measurement. + */ +internal class StartupHomeActivityLifecycleObserver( + private val frameworkStartMeasurement: StartupFrameworkStartMeasurement, + private val startupTimeline: PingType = Pings.startupTimeline +) : LifecycleObserver { + + @OnLifecycleEvent(Lifecycle.Event.ON_STOP) + fun onStop() { + GlobalScope.launch { // use background thread due to expensive metrics. + // Ensure any last metrics are set before submission. + frameworkStartMeasurement.setExpensiveMetric() + + // Startup metrics placed in the Activity should be re-recorded each time the Activity + // is started so we need to clear the ping lifetime by submitting once per each startup. + // It's less complex to add it here rather than the visual completeness task manager. + // + // N.B.: this submission location may need to be changed if we add metrics outside of the + // HomeActivity startup path (e.g. if the user goes directly to a separate activity and + // closes the app, they will never hit this) to appropriately adjust for the ping lifetimes. + startupTimeline.submit() + } + } +} diff --git a/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt b/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt index 258643448..f6de4e521 100644 --- a/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt +++ b/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt @@ -13,8 +13,9 @@ import org.mozilla.fenix.session.PerformanceActivityLifecycleCallbacks * An application class which knows how to migrate Fennec data. */ class MigratingFenixApplication : FenixApplication() { - init { + recordOnInit() // DO NOT MOVE ANYTHING ABOVE HERE: the timing of this measurement is critical. + PerformanceActivityLifecycleCallbacks.isTransientActivityInMigrationVariant = { if (it is MigrationDecisionActivity) true else false } diff --git a/app/src/test/java/org/mozilla/fenix/perf/StartupHomeActivityLifecycleObserverTest.kt b/app/src/test/java/org/mozilla/fenix/perf/StartupHomeActivityLifecycleObserverTest.kt new file mode 100644 index 000000000..7d4203c40 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/perf/StartupHomeActivityLifecycleObserverTest.kt @@ -0,0 +1,38 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.perf + +import io.mockk.MockKAnnotations +import io.mockk.impl.annotations.MockK +import io.mockk.verifySequence +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runBlockingTest +import mozilla.components.service.glean.private.NoReasonCodes +import mozilla.components.service.glean.private.PingType +import org.junit.Before +import org.junit.Test + +@ExperimentalCoroutinesApi +class StartupHomeActivityLifecycleObserverTest { + + private lateinit var observer: StartupHomeActivityLifecycleObserver + @MockK(relaxed = true) private lateinit var frameworkStartMeasurement: StartupFrameworkStartMeasurement + @MockK(relaxed = true) private lateinit var startupTimeline: PingType + + @Before + fun setUp() { + MockKAnnotations.init(this) + observer = StartupHomeActivityLifecycleObserver(frameworkStartMeasurement, startupTimeline) + } + + @Test + fun `WHEN onStop is called THEN the metrics are set and the ping is submitted`() = runBlockingTest { + observer.onStop() + verifySequence { + frameworkStartMeasurement.setExpensiveMetric() + startupTimeline.submit() + } + } +}