For #19804: Restore logic to original behavior

upstream-sync
Marc Leclair 3 years ago committed by mergify[bot]
parent 871cde764e
commit 298ec5cce1

@ -311,7 +311,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
} }
} }
trackDefaultBrowser() isFenixTheDefaultBrowser()
} }
override fun onStart() = PerfStartup.homeActivityOnStart.measureNoInline { 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 // Launch this on a background thread so as not to affect startup performance
lifecycleScope.launch(IO) { lifecycleScope.launch(IO) {
if ( if (
settings().checkDefaultBrowserAndSet() settings().checkIfFenixIsDefaultBrowserOnAppResume()
) { ) {
metrics.track(Event.ChangedToDefaultBrowser) metrics.track(Event.ChangedToDefaultBrowser)
} }

@ -30,7 +30,7 @@ class FenixReviewSettings(
get() = settings.numberOfAppLaunches get() = settings.numberOfAppLaunches
set(value) { settings.numberOfAppLaunches = value } set(value) { settings.numberOfAppLaunches = value }
override val isDefaultBrowser: Boolean override val isDefaultBrowser: Boolean
get() = settings.isDefaultBrowser() get() = settings.isDefaultBrowserBlocking()
override var lastReviewPromptTimeInMillis: Long override var lastReviewPromptTimeInMillis: Long
get() = settings.lastReviewPromptTimeInMillis get() = settings.lastReviewPromptTimeInMillis
set(value) { settings.lastReviewPromptTimeInMillis = value } set(value) { settings.lastReviewPromptTimeInMillis = value }

@ -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 * Declared as a function for performance purposes. This could be declared as a variable using
* isDefaultBrowser function takes approx. 30-40ms. * 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 prefKey = appContext.getPreferenceKey(R.string.pref_key_default_browser)
val isDefaultBrowserNow = isDefaultBrowser() val isDefaultBrowserNow = isDefaultBrowserBlocking()
val isDefaultBrowserBefore = this.preferences.getBoolean(prefKey, isDefaultBrowserNow) val wasDefaultBrowserOnLastResume = this.preferences.getBoolean(prefKey, isDefaultBrowserNow)
this.preferences.edit().putBoolean(prefKey, isDefaultBrowserNow).apply() 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 * This function is "blocking" since calling this can take approx. 30-40ms (timing taken on a
* G5+). * G5+).
*/ */
private fun isDefaultBrowserBlocking(): Boolean { fun isDefaultBrowserBlocking(): Boolean {
val browsers = BrowsersCache.all(appContext) val browsers = BrowsersCache.all(appContext)
return browsers.isDefaultBrowser return browsers.isDefaultBrowser
} }
@ -497,7 +499,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
) )
fun shouldShowDefaultBrowserNotification(): Boolean { fun shouldShowDefaultBrowserNotification(): Boolean {
return !defaultBrowserNotificationDisplayed && !isDefaultBrowser() return !defaultBrowserNotificationDisplayed && !isDefaultBrowserBlocking()
} }
val shouldUseAutoBatteryTheme by booleanPreference( val shouldUseAutoBatteryTheme by booleanPreference(

@ -749,7 +749,7 @@ class SettingsTest {
@Test @Test
fun `GIVEN shownDefaultBrowserNotification and isDefaultBrowser WHEN calling shouldShowDefaultBrowserNotification THEN return correct value`() { fun `GIVEN shownDefaultBrowserNotification and isDefaultBrowser WHEN calling shouldShowDefaultBrowserNotification THEN return correct value`() {
val localSetting = spyk(settings) val localSetting = spyk(settings)
every { localSetting.isDefaultBrowser() } returns false every { localSetting.isDefaultBrowserBlocking() } returns false
localSetting.defaultBrowserNotificationDisplayed = false localSetting.defaultBrowserNotificationDisplayed = false
assert(localSetting.shouldShowDefaultBrowserNotification()) assert(localSetting.shouldShowDefaultBrowserNotification())
@ -757,7 +757,7 @@ class SettingsTest {
localSetting.defaultBrowserNotificationDisplayed = true localSetting.defaultBrowserNotificationDisplayed = true
assertFalse(localSetting.shouldShowDefaultBrowserNotification()) assertFalse(localSetting.shouldShowDefaultBrowserNotification())
every { localSetting.isDefaultBrowser() } returns true every { localSetting.isDefaultBrowserBlocking() } returns true
localSetting.defaultBrowserNotificationDisplayed = false localSetting.defaultBrowserNotificationDisplayed = false
assertFalse(localSetting.shouldShowDefaultBrowserNotification()) assertFalse(localSetting.shouldShowDefaultBrowserNotification())

Loading…
Cancel
Save