From b7ecc058110e7f074c76c94191f84dcf01c45183 Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Wed, 5 May 2021 14:47:10 -0400 Subject: [PATCH] [fenix] Issue https://github.com/mozilla-mobile/fenix/issues/19147: Move set startup metrics off main thread --- .../main/java/org/mozilla/fenix/FenixApplication.kt | 11 ++++++----- .../mozilla/fenix/components/BackgroundServices.kt | 2 ++ app/src/main/java/org/mozilla/fenix/utils/Settings.kt | 5 +++++ app/src/main/res/values/preference_keys.xml | 2 ++ .../java/org/mozilla/fenix/FenixApplicationTest.kt | 1 + .../fenix/components/BackgroundServicesTest.kt | 10 ++++++++++ 6 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 87d9349a85..3c7ed6a593 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -141,7 +141,11 @@ open class FenixApplication : LocaleAwareApplication(), Provider { buildInfo = GleanBuildInfo.buildInfo ) - setStartupMetrics(components.core.store, settings()) + // We avoid blocking the main thread on startup by setting startup metrics on the background thread. + val store = components.core.store + GlobalScope.launch(Dispatchers.IO) { + setStartupMetrics(store, settings()) + } } @CallSuper @@ -622,10 +626,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { privateSearchSuggestions.set(settings.shouldShowSearchSuggestionsInPrivate) voiceSearchEnabled.set(settings.shouldShowVoiceSearch) openLinksInAppEnabled.set(settings.openLinksInExternalApp) - - val isLoggedIn = - components.backgroundServices.accountManager.accountProfile() != null - signedInSync.set(isLoggedIn) + signedInSync.set(settings.signedInFxaAccount) val syncedItems = SyncEnginesStorage(applicationContext).getStatus().entries.filter { it.value diff --git a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt index fabf86d629..b2faacf5df 100644 --- a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt +++ b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt @@ -188,6 +188,7 @@ internal class TelemetryAccountObserver( private val metricController: MetricController ) : AccountObserver { override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { + settings.signedInFxaAccount = true when (authType) { // User signed-in into an existing FxA account. AuthType.Signin -> Event.SyncAuthSignIn @@ -220,5 +221,6 @@ internal class TelemetryAccountObserver( override fun onLoggedOut() { metricController.track(Event.SyncAuthSignOut) + settings.signedInFxaAccount = false } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 5f80df3036..915cd0288b 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -1062,4 +1062,9 @@ class Settings(private val appContext: Context) : PreferencesHolder { appContext.getPreferenceKey(R.string.pref_key_open_next_tab_desktop_mode), default = false ) + + var signedInFxaAccount by booleanPreference( + appContext.getPreferenceKey(R.string.pref_key_fxa_signed_in), + default = false + ) } diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 0a87d060e4..6409596a5c 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -242,6 +242,8 @@ pref_key_default_browser + pref_key_fxa_signed_in + pref_key_login_exceptions pref_key_show_collections_home diff --git a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt index 492c24f140..55f16ae2e2 100644 --- a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt +++ b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt @@ -127,6 +127,7 @@ class FenixApplicationTest { every { settings.switchServiceIsEnabled } returns true every { settings.touchExplorationIsEnabled } returns true every { settings.shouldUseLightTheme } returns true + every { settings.signedInFxaAccount } returns true application.setStartupMetrics(browserStore, settings, browsersCache, mozillaProductDetector) diff --git a/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt b/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt index eaf915a60b..7fc6a89ba7 100644 --- a/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt @@ -38,6 +38,7 @@ class BackgroundServicesTest { fun setup() { MockKAnnotations.init(this) every { metrics.track(any()) } just Runs + every { settings.signedInFxaAccount = any() } just Runs observer = TelemetryAccountObserver(settings, metrics) registry = ObserverRegistry().apply { register(observer) } @@ -49,6 +50,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.Signin) } verify { metrics.track(Event.SyncAuthSignIn) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -58,6 +60,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.Signup) } verify { metrics.track(Event.SyncAuthSignUp) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -67,6 +70,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.Pairing) } verify { metrics.track(Event.SyncAuthPaired) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -76,6 +80,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.MigratedReuse) } verify { metrics.track(Event.SyncAuthFromSharedReuse) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) registry.notifyObservers { onAuthenticated(account, AuthType.MigratedCopy) } @@ -88,6 +93,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.Recovered) } verify { metrics.track(Event.SyncAuthRecovered) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -97,6 +103,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.OtherExternal(null)) } verify { metrics.track(Event.SyncAuthOtherExternal) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -106,6 +113,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.OtherExternal("someAction")) } verify { metrics.track(Event.SyncAuthOtherExternal) } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -115,6 +123,7 @@ class BackgroundServicesTest { registry.notifyObservers { onAuthenticated(account, AuthType.Existing) } verify { metrics wasNot Called } + verify { settings.signedInFxaAccount = true } confirmVerified(metrics, settings) } @@ -122,6 +131,7 @@ class BackgroundServicesTest { fun `telemetry account observer tracks sign out event`() { registry.notifyObservers { onLoggedOut() } verify { metrics.track(Event.SyncAuthSignOut) } + verify { settings.signedInFxaAccount = false } confirmVerified(metrics, settings) } }