diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index c298571ed9..e3441cc24a 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -311,7 +311,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } } - trackDefaultBrowser() + isFenixTheDefaultBrowser() } override fun onStart() = PerfStartup.homeActivityOnStart.measureNoInline { @@ -946,11 +946,11 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } } - private fun trackDefaultBrowser(){ + private fun isFenixTheDefaultBrowser() { // Launch this on a background thread so as not to affect startup performance lifecycleScope.launch(IO) { if ( - settings().checkDefaultBrowserAndSet() + settings().checkIfFenixIsDefaultBrowserOnAppResume() ) { metrics.track(Event.ChangedToDefaultBrowser) } diff --git a/app/src/main/java/org/mozilla/fenix/components/ReviewPromptController.kt b/app/src/main/java/org/mozilla/fenix/components/ReviewPromptController.kt index 2e8c185be4..9793c0112f 100644 --- a/app/src/main/java/org/mozilla/fenix/components/ReviewPromptController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/ReviewPromptController.kt @@ -30,7 +30,7 @@ class FenixReviewSettings( get() = settings.numberOfAppLaunches set(value) { settings.numberOfAppLaunches = value } override val isDefaultBrowser: Boolean - get() = settings.isDefaultBrowser() + get() = settings.isDefaultBrowserBlocking() override var lastReviewPromptTimeInMillis: Long get() = settings.lastReviewPromptTimeInMillis set(value) { settings.lastReviewPromptTimeInMillis = value } 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 1a722a49f8..76d16897ad 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -471,22 +471,24 @@ class Settings(private val appContext: Context) : PreferencesHolder { ) /** - * Declared as a function for performance purposes. When this class is first initialized, the - * isDefaultBrowser function takes approx. 30-40ms. + * Declared as a function for performance purposes. This could be declared as a variable using + * booleanPreference like other members of this class. However, doing so will make it so it will + * be initialized once Settings.kt is first called, which in turn will call `isDefaultBrowserBlocking()`. + * This will lead to a performance regression since that function can be expensive to call. */ - fun checkDefaultBrowserAndSet(): Boolean { + fun checkIfFenixIsDefaultBrowserOnAppResume(): Boolean { val prefKey = appContext.getPreferenceKey(R.string.pref_key_default_browser) - val isDefaultBrowserNow = isDefaultBrowser() - val isDefaultBrowserBefore = this.preferences.getBoolean(prefKey, isDefaultBrowserNow) + val isDefaultBrowserNow = isDefaultBrowserBlocking() + val wasDefaultBrowserOnLastResume = this.preferences.getBoolean(prefKey, isDefaultBrowserNow) this.preferences.edit().putBoolean(prefKey, isDefaultBrowserNow).apply() - return isDefaultBrowserNow != isDefaultBrowserBefore + return isDefaultBrowserNow && !wasDefaultBrowserOnLastResume } /** * This function is "blocking" since calling this can take approx. 30-40ms (timing taken on a * G5+). */ - private fun isDefaultBrowserBlocking(): Boolean { + fun isDefaultBrowserBlocking(): Boolean { val browsers = BrowsersCache.all(appContext) return browsers.isDefaultBrowser } @@ -497,7 +499,7 @@ class Settings(private val appContext: Context) : PreferencesHolder { ) fun shouldShowDefaultBrowserNotification(): Boolean { - return !defaultBrowserNotificationDisplayed && !isDefaultBrowser() + return !defaultBrowserNotificationDisplayed && !isDefaultBrowserBlocking() } val shouldUseAutoBatteryTheme by booleanPreference( diff --git a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt index 968e787077..d4298043fc 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt @@ -749,7 +749,7 @@ class SettingsTest { @Test fun `GIVEN shownDefaultBrowserNotification and isDefaultBrowser WHEN calling shouldShowDefaultBrowserNotification THEN return correct value`() { val localSetting = spyk(settings) - every { localSetting.isDefaultBrowser() } returns false + every { localSetting.isDefaultBrowserBlocking() } returns false localSetting.defaultBrowserNotificationDisplayed = false assert(localSetting.shouldShowDefaultBrowserNotification()) @@ -757,7 +757,7 @@ class SettingsTest { localSetting.defaultBrowserNotificationDisplayed = true assertFalse(localSetting.shouldShowDefaultBrowserNotification()) - every { localSetting.isDefaultBrowser() } returns true + every { localSetting.isDefaultBrowserBlocking() } returns true localSetting.defaultBrowserNotificationDisplayed = false assertFalse(localSetting.shouldShowDefaultBrowserNotification())