From e803cc61a2187ddf0d567485b677159de5d5945b Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Thu, 8 Apr 2021 14:34:34 -0700 Subject: [PATCH] For #18836: add isWarmStartForStartedActivity, tests. --- .../fenix/perf/StartupStateProvider.kt | 30 ++++++++++ .../fenix/perf/StartupStateProviderTest.kt | 57 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt index 21fdeab7e..76a5fde75 100644 --- a/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt @@ -59,4 +59,34 @@ class StartupStateProvider( * your own risk. */ fun shouldShortCircuitColdStart(): Boolean = startupLog.log.contains(LogEntry.AppStopped) + + /** + * Returns true if the current startup state is WARM and the currently started activity is the + * first started activity for this start up (i.e. we can use it for performance measurements). + * + * This method must be called after the foreground activity is STARTED. + */ + fun isWarmStartForStartedActivity(activityClass: Class): Boolean { + // A warm start means: + // - the app was backgrounded and has since been started + // - the first started activity since the app was started is still active. + // - that activity was created before being started + // + // For the activity log, we expect: + // [... App-STOPPED, ... Activity-CREATED, Activity-STARTED, App-STARTED] + // where: + // - App-STOPPED is the last STOPPED seen + // - we're assuming App-STARTED will only be last if one activity is started (as observed) + if (!startupLog.log.contains(LogEntry.AppStopped)) { + return false // if the app hasn't been stopped, it's not a warm start. + } + val afterLastStopped = startupLog.log.takeLastWhile { it != LogEntry.AppStopped } + + val isLastActivityCreatedStillStarted = afterLastStopped.takeLast(3) == listOf( + LogEntry.ActivityCreated(activityClass), + LogEntry.ActivityStarted(activityClass), + LogEntry.AppStarted + ) + return isLastActivityCreatedStillStarted + } } diff --git a/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt b/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt index fd98d2b23..d7b822e8d 100644 --- a/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt @@ -60,6 +60,27 @@ class StartupStateProviderTest { } } + @Test + fun `GIVEN the app started for an activity WHEN is cold start THEN warm start is false`() { + forEachColdStartEntries { index -> + assertFalse("$index", provider.isWarmStartForStartedActivity(homeActivityClass)) + } + } + + @Test + fun `GIVEN the app started for an activity WHEN is warm start THEN warm start is true`() { + forEachWarmStartEntries { index -> + assertTrue("$index", provider.isWarmStartForStartedActivity(homeActivityClass)) + } + } + + @Test + fun `GIVEN the app started for an activity WHEN is hot start THEN warm start is false` () { + forEachHotStartEntries { index -> + assertFalse("$index", provider.isWarmStartForStartedActivity(homeActivityClass)) + } + } + @Test fun `GIVEN the app started for an activity WHEN we launched HA through a drawing IntentRA THEN start up is not cold`() { // These entries mimic observed behavior for local code changes. @@ -74,6 +95,22 @@ class StartupStateProviderTest { assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) } + @Test + fun `GIVEN the app started for an activity WHEN we launched HA through a drawing IntentRA THEN start up is not warm`() { + // These entries mimic observed behavior for local code changes. + logEntries.addAll(listOf( + LogEntry.AppStopped, + LogEntry.ActivityStopped(homeActivityClass), + LogEntry.ActivityCreated(irActivityClass), + LogEntry.ActivityStarted(irActivityClass), + LogEntry.AppStarted, + LogEntry.ActivityCreated(homeActivityClass), + LogEntry.ActivityStarted(homeActivityClass), + LogEntry.ActivityStopped(irActivityClass) + )) + assertFalse(provider.isWarmStartForStartedActivity(homeActivityClass)) + } + @Test fun `GIVEN the app started for an activity WHEN two HomeActivities are created THEN start up is not cold`() { // We're making an assumption about how this would work based on previous observed patterns. @@ -124,6 +161,26 @@ class StartupStateProviderTest { assertFalse(provider.shouldShortCircuitColdStart()) } + @Test + fun `GIVEN the app has not been stopped WHEN an activity has not been created THEN it's not a warm start`() { + assertFalse(provider.isWarmStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app has been stopped WHEN an activity has not been created THEN it's not a warm start`() { + logEntries.add(LogEntry.AppStopped) + assertFalse(provider.isWarmStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app has been stopped WHEN an activity has not been started THEN it's not a warm start`() { + logEntries.addAll(listOf( + LogEntry.AppStopped, + LogEntry.ActivityCreated(homeActivityClass) + )) + assertFalse(provider.isWarmStartForStartedActivity(homeActivityClass)) + } + private fun forEachColdStartEntries(block: (index: Int) -> Unit) { // These entries mimic observed behavior. //