From 9c28cb632c3fc2e980315ca6ee9cb01460f7b0f8 Mon Sep 17 00:00:00 2001 From: Will Hawkins Date: Sat, 4 Jan 2020 04:35:46 -0500 Subject: [PATCH] Issue #7425 (et al): Cache the list of installed browsers Cache the list of installed browsers. Calling `Browsers.all` the application directly redundantly recalculates the list. Accessing the list of installed browsers through this cache will reduce that overhead. --- .../java/org/mozilla/fenix/HomeActivity.kt | 11 ++ .../components/metrics/GleanMetricsService.kt | 4 +- .../metrics/MozillaProductDetector.kt | 4 +- .../settings/DefaultBrowserPreference.kt | 4 +- .../DefaultBrowserSettingsFragment.kt | 4 +- .../org/mozilla/fenix/utils/BrowsersCache.kt | 46 ++++++ .../java/org/mozilla/fenix/utils/Settings.kt | 3 +- .../mozilla/fenix/utils/BrowsersCacheTest.kt | 155 ++++++++++++++++++ 8 files changed, 221 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/utils/BrowsersCache.kt create mode 100644 app/src/test/java/org/mozilla/fenix/utils/BrowsersCacheTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 58ff88c44..78b36fe81 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -60,6 +60,7 @@ import org.mozilla.fenix.settings.SettingsFragmentDirections import org.mozilla.fenix.settings.TrackingProtectionFragmentDirections import org.mozilla.fenix.theme.DefaultThemeManager import org.mozilla.fenix.theme.ThemeManager +import org.mozilla.fenix.utils.BrowsersCache @SuppressWarnings("TooManyFunctions", "LargeClass") open class HomeActivity : LocaleAwareAppCompatActivity() { @@ -139,6 +140,16 @@ open class HomeActivity : LocaleAwareAppCompatActivity() { hotStartMonitor.onPostResumeFinalMethodCall() } + final override fun onPause() { + super.onPause() + + // Every time the application goes into the background, it is possible that the user + // is about to change the browsers installed on their system. Therefore, we reset the cache of + // all the installed browsers. + // + // NB: There are ways for the user to install new products without leaving the browser. + BrowsersCache.resetAll() + } /** * Handles intents received when the activity is open. */ diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 3e330ad32..f0b8425b6 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -13,7 +13,6 @@ import mozilla.components.service.glean.Glean import mozilla.components.service.glean.config.Configuration import mozilla.components.service.glean.net.ConceptFetchHttpUploader import mozilla.components.service.glean.private.NoExtraKeys -import mozilla.components.support.utils.Browsers import org.mozilla.fenix.GleanMetrics.BookmarksManagement import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.ContextMenu @@ -45,6 +44,7 @@ import org.mozilla.fenix.GleanMetrics.ToolbarSettings import org.mozilla.fenix.GleanMetrics.TrackingProtection import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.utils.BrowsersCache private class EventWrapper>( private val recorder: ((Map?) -> Unit), @@ -504,7 +504,7 @@ class GleanMetricsService(private val context: Context) : MetricsService { internal fun setStartupMetrics() { Metrics.apply { - defaultBrowser.set(Browsers.all(context).isDefaultBrowser) + defaultBrowser.set(BrowsersCache.all(context).isDefaultBrowser) MozillaProductDetector.getMozillaBrowserDefault(context)?.also { defaultMozBrowser.set(it) } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MozillaProductDetector.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MozillaProductDetector.kt index 75ef1398f..2aeb830a8 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MozillaProductDetector.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MozillaProductDetector.kt @@ -6,7 +6,7 @@ package org.mozilla.fenix.components.metrics import android.content.Context import android.content.pm.PackageManager -import mozilla.components.support.utils.Browsers +import org.mozilla.fenix.utils.BrowsersCache object MozillaProductDetector { enum class MozillaProducts(val productName: String) { @@ -57,7 +57,7 @@ object MozillaProductDetector { * Returns the default browser if and only if it is a Mozilla product. */ fun getMozillaBrowserDefault(context: Context): String? { - val browserPackageName = Browsers.all(context).defaultBrowser?.packageName + val browserPackageName = BrowsersCache.all(context).defaultBrowser?.packageName return if (isMozillaProduct(browserPackageName)) { browserPackageName } else { null } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserPreference.kt index 41367f374..c2ef16ebb 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserPreference.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserPreference.kt @@ -9,8 +9,8 @@ import android.util.AttributeSet import android.widget.Switch import androidx.preference.Preference import androidx.preference.PreferenceViewHolder -import mozilla.components.support.utils.Browsers import org.mozilla.fenix.R +import org.mozilla.fenix.utils.BrowsersCache class DefaultBrowserPreference @JvmOverloads constructor( context: Context, @@ -31,7 +31,7 @@ class DefaultBrowserPreference @JvmOverloads constructor( } fun updateSwitch() { - val browsers = Browsers.all(context) + val browsers = BrowsersCache.all(context) switchView?.isChecked = browsers.isDefaultBrowser } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserSettingsFragment.kt index 43282efd1..018ce06e2 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/DefaultBrowserSettingsFragment.kt @@ -12,13 +12,13 @@ import android.provider.Settings.ACTION_MANAGE_DEFAULT_APPS_SETTINGS import androidx.preference.CheckBoxPreference import androidx.preference.Preference import androidx.preference.PreferenceFragmentCompat -import mozilla.components.support.utils.Browsers import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar +import org.mozilla.fenix.utils.BrowsersCache /** * Lets the user control their default browser preferences @@ -55,7 +55,7 @@ class DefaultBrowserSettingsFragment : PreferenceFragmentCompat() { settings.unsetOpenLinksInAPrivateTabIfNecessary() findPreference(getPreferenceKey(R.string.pref_key_open_links_in_a_private_tab))?.apply { - isEnabled = Browsers.all(requireContext()).isDefaultBrowser + isEnabled = BrowsersCache.all(requireContext()).isDefaultBrowser isChecked = settings.openLinksInAPrivateTab onPreferenceChangeListener = SharedPreferenceUpdater() } diff --git a/app/src/main/java/org/mozilla/fenix/utils/BrowsersCache.kt b/app/src/main/java/org/mozilla/fenix/utils/BrowsersCache.kt new file mode 100644 index 000000000..391a9a0f0 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/utils/BrowsersCache.kt @@ -0,0 +1,46 @@ +/* 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.utils + +import android.content.Context +import androidx.annotation.VisibleForTesting +import mozilla.components.support.utils.Browsers + +/** + * Caches the list of browsers installed on a user's device. + * + * BrowsersCache caches the list of installed browsers is gathered lazily when it is first accessed + * after initial creation or invalidation. For that reason, a context is required every time + * the cache is accessed. + * + * Users are responsible for invalidating the cache at the appropriate time. It is left up to the + * user to determine appropriate policies for maintaining the validity of the cache. If, when the + * cache is accessed, it is filled, the contents will be returned. As mentioned above, the cache + * will be lazily refilled after invalidation. In other words, invalidation is O(1). + * + * This cache is threadsafe. + */ +object BrowsersCache { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var cachedBrowsers: Browsers? = null + + @Synchronized + fun all(context: Context): Browsers { + run { + val cachedBrowsers = cachedBrowsers + if (cachedBrowsers != null) { + return cachedBrowsers + } + } + return Browsers.all(context).also { + this.cachedBrowsers = it + } + } + + @Synchronized + fun resetAll() { + cachedBrowsers = null + } +} 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 ed38a0b08..61ed68460 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -19,7 +19,6 @@ import mozilla.components.support.ktx.android.content.floatPreference import mozilla.components.support.ktx.android.content.intPreference import mozilla.components.support.ktx.android.content.longPreference import mozilla.components.support.ktx.android.content.stringPreference -import mozilla.components.support.utils.Browsers import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.FeatureFlags @@ -375,7 +374,7 @@ class Settings private constructor( } fun unsetOpenLinksInAPrivateTabIfNecessary() { - if (Browsers.all(appContext).isDefaultBrowser) { + if (BrowsersCache.all(appContext).isDefaultBrowser) { return } diff --git a/app/src/test/java/org/mozilla/fenix/utils/BrowsersCacheTest.kt b/app/src/test/java/org/mozilla/fenix/utils/BrowsersCacheTest.kt new file mode 100644 index 000000000..a1f9e6bf0 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/utils/BrowsersCacheTest.kt @@ -0,0 +1,155 @@ +/* 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/. */ +@file:Suppress("DEPRECATION") + +package org.mozilla.fenix.utils + +import android.content.Intent +import android.content.pm.ActivityInfo +import android.content.pm.PackageInfo +import android.content.pm.ResolveInfo +import android.net.Uri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.utils.Browsers +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.TestApplication +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +@RunWith(AndroidJUnit4::class) +@Config(application = TestApplication::class) +class BrowsersCacheTest { + + // NB: There is always one more browser than pretendBrowsersAreInstalled installs because + // the application we are testing is recognized as a browser itself! + + @Test + fun `cached list of browsers match before-after installation when cache is not invalidated`() { + BrowsersCache.resetAll() + pretendBrowsersAreInstalled( + browsers = listOf( + Browsers.KnownBrowser.FIREFOX_NIGHTLY.packageName, + Browsers.KnownBrowser.REFERENCE_BROWSER.packageName + ) + ) + + val initialBrowserList = BrowsersCache.all(testContext) + assertEquals(3, initialBrowserList.installedBrowsers.size) + + pretendBrowsersAreInstalled( + browsers = listOf( + Browsers.KnownBrowser.FIREFOX_NIGHTLY.packageName, + Browsers.KnownBrowser.FIREFOX.packageName, + Browsers.KnownBrowser.CHROME.packageName, + Browsers.KnownBrowser.SAMSUNG_INTERNET.packageName, + Browsers.KnownBrowser.DUCKDUCKGO.packageName, + Browsers.KnownBrowser.REFERENCE_BROWSER.packageName + ) + ) + val updatedBrowserList = BrowsersCache.all(testContext) + assertEquals(3, updatedBrowserList.installedBrowsers.size) + } + + @Test + fun `cached list of browsers change before-after installation when cache is invalidated`() { + BrowsersCache.resetAll() + pretendBrowsersAreInstalled( + browsers = listOf( + Browsers.KnownBrowser.FIREFOX_NIGHTLY.packageName, + Browsers.KnownBrowser.REFERENCE_BROWSER.packageName + ) + ) + + val initialBrowserList = BrowsersCache.all(testContext) + assertEquals(3, initialBrowserList.installedBrowsers.size) + + pretendBrowsersAreInstalled( + browsers = listOf( + Browsers.KnownBrowser.FIREFOX_NIGHTLY.packageName, + Browsers.KnownBrowser.FIREFOX.packageName, + Browsers.KnownBrowser.CHROME.packageName, + Browsers.KnownBrowser.SAMSUNG_INTERNET.packageName, + Browsers.KnownBrowser.DUCKDUCKGO.packageName, + Browsers.KnownBrowser.REFERENCE_BROWSER.packageName + ) + ) + + BrowsersCache.resetAll() + + val updatedBrowserList = BrowsersCache.all(testContext) + assertEquals(7, updatedBrowserList.installedBrowsers.size) + } + + @Test + fun `resetting the cache should empty it`() { + BrowsersCache.resetAll() + + BrowsersCache.all(testContext) + + assertNotNull(BrowsersCache.cachedBrowsers) + + BrowsersCache.resetAll() + + assertNull(BrowsersCache.cachedBrowsers) + } + + // pretendBrowsersAreInstalled was taken, verbatim, from a-c. + // See support/utils/src/test/java/mozilla/components/support/utils/BrowsersTest.kt + private fun pretendBrowsersAreInstalled( + browsers: List = listOf(), + defaultBrowser: String? = null, + url: String = "http://www.mozilla.org", + browsersExported: Boolean = true, + defaultBrowserExported: Boolean = true + ) { + val packageManager = testContext.packageManager + val shadow = shadowOf(packageManager) + + browsers.forEach { packageName -> + val intent = Intent(Intent.ACTION_VIEW) + intent.`package` = packageName + intent.data = Uri.parse(url) + + val packageInfo = PackageInfo().apply { + this.packageName = packageName + } + + shadow.installPackage(packageInfo) + + val activityInfo = ActivityInfo().apply { + exported = browsersExported + this.packageName = packageName + } + + val resolveInfo = ResolveInfo().apply { + resolvePackageName = packageName + this.activityInfo = activityInfo + } + + shadow.addResolveInfoForIntent(intent, resolveInfo) + } + + if (defaultBrowser != null) { + val intent = Intent(Intent.ACTION_VIEW) + intent.data = Uri.parse(url) + + val activityInfo = ActivityInfo().apply { + exported = defaultBrowserExported + packageName = defaultBrowser + } + + val resolveInfo = ResolveInfo().apply { + resolvePackageName = defaultBrowser + this.activityInfo = activityInfo + } + + shadow.addResolveInfoForIntent(intent, resolveInfo) + } + } +}