From a64540bd066728ad67024cec0c7223300b470128 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Fri, 9 Apr 2021 14:51:29 -0700 Subject: [PATCH] For #18836: add StartupPathProvider + tests. --- .../java/org/mozilla/fenix/HomeActivity.kt | 6 + .../mozilla/fenix/components/Components.kt | 2 +- .../mozilla/fenix/perf/StartupPathProvider.kt | 107 +++++++++ .../fenix/perf/StartupPathProviderTest.kt | 203 ++++++++++++++++++ 4 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 app/src/main/java/org/mozilla/fenix/perf/StartupPathProvider.kt create mode 100644 app/src/test/java/org/mozilla/fenix/perf/StartupPathProviderTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 64762fa68..49381b187 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -99,6 +99,7 @@ import org.mozilla.fenix.perf.NavGraphProvider import org.mozilla.fenix.perf.Performance import org.mozilla.fenix.perf.PerformanceInflater import org.mozilla.fenix.perf.ProfilerMarkers +import org.mozilla.fenix.perf.StartupPathProvider import org.mozilla.fenix.perf.StartupTimeline import org.mozilla.fenix.search.SearchDialogFragmentDirections import org.mozilla.fenix.session.PrivateNotificationService @@ -171,6 +172,8 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { // Tracker for contextual menu (Copy|Search|Select all|etc...) private var actionMode: ActionMode? = null + private val startupPathProvider = StartupPathProvider() + final override fun onCreate(savedInstanceState: Bundle?): Unit = PerfStartup.homeActivityOnCreate.measureNoInline { // DO NOT MOVE ANYTHING ABOVE THIS addMarker CALL. components.core.engine.profiler?.addMarker("Activity.onCreate", "HomeActivity") @@ -262,6 +265,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { captureSnapshotTelemetryMetrics() startupTelemetryOnCreateCalled(intent.toSafeIntent(), savedInstanceState != null) + startupPathProvider.attachOnActivityOnCreate(lifecycle, intent) components.core.requestInterceptor.setNavigationController(navHost.navController) @@ -272,6 +276,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { safeIntent: SafeIntent, hasSavedInstanceState: Boolean ) { + // This function gets overridden by subclasses. components.appStartupTelemetry.onHomeActivityOnCreate( safeIntent, hasSavedInstanceState, @@ -472,6 +477,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { intent?.let { handleNewIntent(it) } + startupPathProvider.onIntentReceived(intent) } open fun handleNewIntent(intent: Intent) { diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 4115fc609..ef2fb1274 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -24,13 +24,13 @@ import org.mozilla.fenix.Config import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.autofill.AutofillUnlockActivity -import org.mozilla.fenix.perf.StrictModeManager import org.mozilla.fenix.components.metrics.AppStartupTelemetry import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.perf.AppStartReasonProvider import org.mozilla.fenix.perf.StartupActivityLog import org.mozilla.fenix.perf.StartupStateProvider +import org.mozilla.fenix.perf.StrictModeManager import org.mozilla.fenix.perf.lazyMonitored import org.mozilla.fenix.utils.ClipboardHandler import org.mozilla.fenix.utils.Mockable diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupPathProvider.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupPathProvider.kt new file mode 100644 index 000000000..f6b6f375f --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/perf/StartupPathProvider.kt @@ -0,0 +1,107 @@ +/* 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 android.app.Activity +import android.content.Intent +import androidx.annotation.VisibleForTesting +import androidx.annotation.VisibleForTesting.NONE +import androidx.annotation.VisibleForTesting.PRIVATE +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner + +/** + * The "path" that this activity started in. See the + * [Fenix perf glossary](https://wiki.mozilla.org/index.php?title=Performance/Fenix/Glossary) + * for specific definitions. + * + * This should be a member variable of [Activity] because its data is tied to the lifecycle of an + * Activity. Call [attachOnActivityOnCreate] & [onIntentReceived] for this class to work correctly. + */ +class StartupPathProvider { + + /** + * The path the application took to + * [Fenix perf glossary](https://wiki.mozilla.org/index.php?title=Performance/Fenix/Glossary) + * for specific definitions. + */ + enum class StartupPath { + MAIN, + VIEW, + + /** + * The start up path if we received an Intent but we're unable to categorize it into other buckets. + */ + UNKNOWN, + + /** + * The start up path has not been set. This state includes: + * - this API is accessed before it is set + * - if no intent is received before the activity is STARTED (e.g. app switcher) + */ + NOT_SET + } + + /** + * Returns the [StartupPath] for the currently started activity. This value will be set + * after an [Intent] is received that causes this activity to move into the STARTED state. + */ + var startupPathForActivity = StartupPath.NOT_SET + private set + + private var wasResumedSinceStartedState = false + + fun attachOnActivityOnCreate(lifecycle: Lifecycle, intent: Intent?) { + lifecycle.addObserver(StartupPathLifecycleObserver()) + onIntentReceived(intent) + } + + // N.B.: this method duplicates the actual logic for determining what page to open. + // Unfortunately, it's difficult to re-use that logic because it occurs in many places throughout + // the code so we do the simple thing for now and duplicate it. It's noticeably different from + // what you might expect: e.g. ACTION_MAIN can open a URL and if ACTION_VIEW provides an invalid + // URL, it'll perform a MAIN action. However, it's fairly representative of what users *intended* + // to do when opening the app and shouldn't change much because it's based on Android system-wide + // conventions, so it's probably fine for our purposes. + private fun getStartupPathFromIntent(intent: Intent): StartupPath = when (intent.action) { + Intent.ACTION_MAIN -> StartupPath.MAIN + Intent.ACTION_VIEW -> StartupPath.VIEW + else -> StartupPath.UNKNOWN + } + + /** + * Expected to be called when a new [Intent] is received by the [Activity]: i.e. + * [Activity.onCreate] and [Activity.onNewIntent]. + */ + fun onIntentReceived(intent: Intent?) { + // We want to set a path only if the intent causes the Activity to move into the STARTED state. + // This means we want to discard any intents that are received when the app is foregrounded. + // However, we can't use the Lifecycle.currentState to determine this because: + // - the app is briefly paused (state becomes STARTED) before receiving the Intent in + // the foreground so we can't say <= STARTED + // - onIntentReceived can be called from the CREATED or STARTED state so we can't say == CREATED + // So we're forced to track this state ourselves. + if (!wasResumedSinceStartedState && intent != null) { + startupPathForActivity = getStartupPathFromIntent(intent) + } + } + + @VisibleForTesting(otherwise = NONE) + fun getTestCallbacks() = StartupPathLifecycleObserver() + + @VisibleForTesting(otherwise = PRIVATE) + inner class StartupPathLifecycleObserver : DefaultLifecycleObserver { + override fun onResume(owner: LifecycleOwner) { + wasResumedSinceStartedState = true + } + + override fun onStop(owner: LifecycleOwner) { + // Clear existing state. + startupPathForActivity = StartupPath.NOT_SET + wasResumedSinceStartedState = false + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/perf/StartupPathProviderTest.kt b/app/src/test/java/org/mozilla/fenix/perf/StartupPathProviderTest.kt new file mode 100644 index 000000000..8d22d86d3 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/perf/StartupPathProviderTest.kt @@ -0,0 +1,203 @@ +/* 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 android.content.Intent +import androidx.lifecycle.Lifecycle +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.perf.StartupPathProvider.StartupPath + +class StartupPathProviderTest { + + private lateinit var provider: StartupPathProvider + private lateinit var callbacks: StartupPathProvider.StartupPathLifecycleObserver + + @MockK private lateinit var intent: Intent + + @Before + fun setUp() { + MockKAnnotations.init(this) + + provider = StartupPathProvider() + callbacks = provider.getTestCallbacks() + } + + @Test + fun `WHEN attach is called THEN the provider is registered to the lifecycle`() { + val lifecycle = mockk(relaxed = true) + provider.attachOnActivityOnCreate(lifecycle, null) + + verify { lifecycle.addObserver(any()) } + } + + @Test + fun `WHEN calling attach THEN the intent is passed to on intent received`() { + // With this test, we're basically saying, "attach..." does the same thing as + // "onIntentReceived" so we don't need to duplicate all the tests we run for + // "onIntentReceived". + val spyProvider = spyk(provider) + every { spyProvider.onIntentReceived(intent) } returns Unit + spyProvider.attachOnActivityOnCreate(mockk(relaxed = true), intent) + + verify { spyProvider.onIntentReceived(intent) } + } + + @Test + fun `GIVEN no intent is received and the activity is not started WHEN getting the start up path THEN it is not set`() { + assertEquals(StartupPath.NOT_SET, provider.startupPathForActivity) + } + + @Test + fun `GIVEN a main intent is received but the activity is not started yet WHEN getting the start up path THEN main is returned`() { + every { intent.action } returns Intent.ACTION_MAIN + provider.onIntentReceived(intent) + assertEquals(StartupPath.MAIN, provider.startupPathForActivity) + } + + @Test + fun `GIVEN a main intent is received and the app is started WHEN getting the start up path THEN it is main`() { + every { intent.action } returns Intent.ACTION_MAIN + callbacks.onCreate(mockk()) + provider.onIntentReceived(intent) + callbacks.onStart(mockk()) + + assertEquals(StartupPath.MAIN, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched from the homescreen WHEN getting the start up path THEN it is main`() { + // There's technically more to a homescreen Intent but it's fine for now. + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + assertEquals(StartupPath.MAIN, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched by app link WHEN getting the start up path THEN it is view`() { + // There's technically more to a homescreen Intent but it's fine for now. + every { intent.action } returns Intent.ACTION_VIEW + launchApp(intent) + assertEquals(StartupPath.VIEW, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched by a send action WHEN getting the start up path THEN it is unknown`() { + every { intent.action } returns Intent.ACTION_SEND + launchApp(intent) + assertEquals(StartupPath.UNKNOWN, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched by a null intent (is this possible) WHEN getting the start up path THEN it is not set`() { + callbacks.onCreate(mockk()) + provider.onIntentReceived(null) + callbacks.onStart(mockk()) + callbacks.onResume(mockk()) + + assertEquals(StartupPath.NOT_SET, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched to the homescreen and stopped WHEN getting the start up path THEN it is not set`() { + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + stopLaunchedApp() + + assertEquals(StartupPath.NOT_SET, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched to the homescreen, stopped, and relaunched warm from app link WHEN getting the start up path THEN it is view`() { + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + stopLaunchedApp() + + every { intent.action } returns Intent.ACTION_VIEW + startStoppedApp(intent) + + assertEquals(StartupPath.VIEW, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched to the homescreen, stopped, and relaunched warm from the app switcher WHEN getting the start up path THEN it is not set`() { + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + stopLaunchedApp() + startStoppedAppFromAppSwitcher() + + assertEquals(StartupPath.NOT_SET, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched to the homescreen, paused, and resumed WHEN getting the start up path THEN it returns the initial intent value`() { + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + callbacks.onPause(mockk()) + callbacks.onResume(mockk()) + + assertEquals(StartupPath.MAIN, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched with an intent and receives an intent while the activity is foregrounded WHEN getting the start up path THEN it returns the initial intent value`() { + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + every { intent.action } returns Intent.ACTION_VIEW + receiveIntentInForeground(intent) + + assertEquals(StartupPath.MAIN, provider.startupPathForActivity) + } + + @Test + fun `GIVEN the app is launched, stopped, started from the app switcher and receives an intent in the foreground WHEN getting the start up path THEN it returns not set`() { + every { intent.action } returns Intent.ACTION_MAIN + launchApp(intent) + stopLaunchedApp() + startStoppedAppFromAppSwitcher() + every { intent.action } returns Intent.ACTION_VIEW + receiveIntentInForeground(intent) + + assertEquals(StartupPath.NOT_SET, provider.startupPathForActivity) + } + + private fun launchApp(intent: Intent) { + callbacks.onCreate(mockk()) + provider.onIntentReceived(intent) + callbacks.onStart(mockk()) + callbacks.onResume(mockk()) + } + + private fun stopLaunchedApp() { + callbacks.onPause(mockk()) + callbacks.onStop(mockk()) + } + + private fun startStoppedApp(intent: Intent) { + callbacks.onStart(mockk()) + provider.onIntentReceived(intent) + callbacks.onResume(mockk()) + } + + private fun startStoppedAppFromAppSwitcher() { + // What makes the app switcher case special is it starts the app without an intent. + callbacks.onStart(mockk()) + callbacks.onResume(mockk()) + } + + private fun receiveIntentInForeground(intent: Intent) { + // To my surprise, the app is paused before receiving an intent on Pixel 2. + callbacks.onPause(mockk()) + provider.onIntentReceived(intent) + callbacks.onResume(mockk()) + } +}