From 0911a1f3e04535f7cff81a1bcd9fe6cb230dee79 Mon Sep 17 00:00:00 2001 From: Adam Novak Date: Tue, 10 May 2022 17:28:44 -0400 Subject: [PATCH] Fix #440 by reattaching settings --- .../PagedAddonCollectionProvider.kt | 56 +++++++++++++------ .../fenix/addons/AddonsManagementFragment.kt | 17 +++++- .../mozilla/fenix/components/Components.kt | 38 +++---------- .../fenix/settings/CustomizationFragment.kt | 34 ++++++++++- .../fenix/settings/SharedPreferenceUpdater.kt | 11 +++- 5 files changed, 101 insertions(+), 55 deletions(-) diff --git a/app/src/main/java/io/github/forkmaintainers/iceraven/components/PagedAddonCollectionProvider.kt b/app/src/main/java/io/github/forkmaintainers/iceraven/components/PagedAddonCollectionProvider.kt index cc1777759965..7050282f5007 100644 --- a/app/src/main/java/io/github/forkmaintainers/iceraven/components/PagedAddonCollectionProvider.kt +++ b/app/src/main/java/io/github/forkmaintainers/iceraven/components/PagedAddonCollectionProvider.kt @@ -23,6 +23,8 @@ import mozilla.components.support.ktx.util.writeString import org.json.JSONArray import org.json.JSONException import org.json.JSONObject +import org.mozilla.fenix.Config +import org.mozilla.fenix.ext.settings import java.io.File import java.io.IOException import java.util.Date @@ -30,8 +32,6 @@ import java.util.concurrent.TimeUnit internal const val API_VERSION = "api/v4" internal const val DEFAULT_SERVER_URL = "https://addons.mozilla.org" -internal const val DEFAULT_COLLECTION_ACCOUNT = "mozilla" -internal const val DEFAULT_COLLECTION_NAME = "7e8d6dc651b54ab385fb8791bf9dac" internal const val COLLECTION_FILE_NAME = "%s_components_addon_collection_%s.json" internal const val MINUTE_IN_MS = 60 * 1000 internal const val DEFAULT_READ_TIMEOUT_IN_SECONDS = 20L @@ -48,10 +48,6 @@ internal const val DEFAULT_READ_TIMEOUT_IN_SECONDS = 20L * * @property serverURL The url of the endpoint to interact with e.g production, staging * or testing. Defaults to [DEFAULT_SERVER_URL]. - * @property collectionAccount The account owning the collection to access, defaults - * to [DEFAULT_COLLECTION_ACCOUNT]. - * @property collectionName The name of the collection to access, defaults - * to [DEFAULT_COLLECTION_NAME]. * @property maxCacheAgeInMinutes maximum time (in minutes) the collection cache * should remain valid. Defaults to -1, meaning no cache is being used by default. * @property client A reference of [Client] for interacting with the AMO HTTP api. @@ -61,8 +57,6 @@ class PagedAddonCollectionProvider( private val context: Context, private val client: Client, private val serverURL: String = DEFAULT_SERVER_URL, - private var collectionAccount: String = DEFAULT_COLLECTION_ACCOUNT, - private var collectionName: String = DEFAULT_COLLECTION_NAME, private val maxCacheAgeInMinutes: Long = -1 ) : AddonsProvider { @@ -70,12 +64,30 @@ class PagedAddonCollectionProvider( private val diskCacheLock = Any() - fun setCollectionAccount(account: String) { - collectionAccount = account + /** + * Get the account we should be fetching addons from. + */ + private fun getCollectionAccount(): String { + var result = context.settings().customAddonsAccount + if (Config.channel.isNightlyOrDebug && context.settings().amoCollectionOverrideConfigured()) { + result = context.settings().overrideAmoUser + } + + logger.info("Determined collection account: ${result}") + return result } - - fun setCollectionName(collection: String) { - collectionName = collection + + /** + * Get the collection name we should be fetching addons from. + */ + private fun getCollectionName(): String { + var result = context.settings().customAddonsCollection + if (Config.channel.isNightlyOrDebug && context.settings().amoCollectionOverrideConfigured()) { + result = context.settings().overrideAmoCollection + } + + logger.info("Determined collection name: ${result}") + return result } /** @@ -103,10 +115,15 @@ class PagedAddonCollectionProvider( } else { null } - + + val collectionAccount = getCollectionAccount() + val collectionName = getCollectionName() + if (cachedAddons != null) { + logger.info("Providing cached list of addons for ${collectionAccount} collection ${collectionName}") return cachedAddons } else { + logger.info("Fetching fresh list of addons for ${collectionAccount} collection ${collectionName}") return getAllPages( listOf( serverURL, @@ -199,6 +216,7 @@ class PagedAddonCollectionProvider( @VisibleForTesting internal fun writeToDiskCache(collectionResponse: String) { + logger.info("Storing cache file") synchronized(diskCacheLock) { getCacheFile(context).writeString { collectionResponse } } @@ -206,6 +224,7 @@ class PagedAddonCollectionProvider( @VisibleForTesting internal fun readFromDiskCache(): List? { + logger.info("Loading cache file") synchronized(diskCacheLock) { return getCacheFile(context).readAndDeserialize { JSONObject(it).getAddons() @@ -229,12 +248,17 @@ class PagedAddonCollectionProvider( } private fun getBaseCacheFile(context: Context): File { + val collectionAccount = getCollectionAccount() + val collectionName = getCollectionName() return File(context.filesDir, COLLECTION_FILE_NAME.format(collectionAccount, collectionName)) } fun deleteCacheFile(context: Context): Boolean { - val file = getBaseCacheFile(context) - return if (file.exists()) file.delete() else false + logger.info("Clearing cache file") + synchronized(diskCacheLock) { + val file = getBaseCacheFile(context) + return if (file.exists()) file.delete() else false + } } } diff --git a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt index 3483b9552bc3..b2deb62ce593 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt @@ -30,6 +30,7 @@ import mozilla.components.feature.addons.Addon import mozilla.components.feature.addons.AddonManagerException import mozilla.components.feature.addons.ui.PermissionsDialogFragment import mozilla.components.feature.addons.ui.translateName +import mozilla.components.support.base.log.logger.Logger import io.github.forkmaintainers.iceraven.components.PagedAddonInstallationDialogFragment import io.github.forkmaintainers.iceraven.components.PagedAddonsManagerAdapter import org.mozilla.fenix.R @@ -51,7 +52,9 @@ import java.util.concurrent.CancellationException */ @Suppress("TooManyFunctions", "LargeClass") class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) { - + + private val logger = Logger("AddonsManagementFragment") + private val args by navArgs() private var binding: FragmentAddOnsManagementBinding? = null @@ -70,6 +73,7 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) container: ViewGroup?, savedInstanceState: Bundle? ): View? { + logger.info("Creating view for AddonsManagementFragment") setHasOptionsMenu(true) return super.onCreateView(inflater, container, savedInstanceState) } @@ -83,6 +87,7 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + logger.info("View created for AddonsManagementFragment") super.onViewCreated(view, savedInstanceState) binding = FragmentAddOnsManagementBinding.bind(view) bindRecyclerView() @@ -139,11 +144,13 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) } override fun onResume() { + logger.info("Resumed AddonsManagementFragment") super.onResume() showToolbar(getString(R.string.preferences_addons)) } override fun onStart() { + logger.info("Started AddonsManagementFragment") super.onStart() findPreviousDialogFragment()?.let { dialog -> dialog.onPositiveButtonClicked = onPositiveButtonClicked @@ -151,6 +158,7 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) } override fun onDestroyView() { + logger.info("Destroyed view for AddonsManagementFragment") super.onDestroyView() // letting go of the resources to avoid memory leak. adapter = null @@ -158,20 +166,25 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) } private fun bindRecyclerView() { + logger.info("Binding recycler view for AddonsManagementFragment") + val managementView = AddonsManagementView( navController = findNavController(), showPermissionDialog = ::showPermissionDialog ) - + val recyclerView = binding?.addOnsList recyclerView?.layoutManager = LinearLayoutManager(requireContext()) val shouldRefresh = adapter != null + + logger.info("AddonsManagementFragment should refresh? ${shouldRefresh}") // If the fragment was launched to install an "external" add-on from AMO, we deactivate // the cache to get the most up-to-date list of add-ons to match against. val allowCache = args.installAddonId == null || installExternalAddonComplete lifecycleScope.launch(IO) { try { + logger.info("AddonsManagementFragment asking for addons") addons = requireContext().components.addonManager.getAddons(allowCache = allowCache) lifecycleScope.launch(Dispatchers.Main) { runIfFragmentIsAttached { 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 e29237b950e7..5a1fa4b3fae5 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -103,26 +103,12 @@ class Components(private val context: Context) { } val addonCollectionProvider by lazyMonitored { - // Check if we have a customized (overridden) AMO collection (only supported in Nightly) - if (Config.channel.isNightlyOrDebug && context.settings().amoCollectionOverrideConfigured()) { - PagedAddonCollectionProvider( - context, - core.client, - collectionAccount = context.settings().overrideAmoUser, - collectionName = context.settings().overrideAmoCollection - ) - } - // Use Iceraven settings config otherwise - else { - PagedAddonCollectionProvider( - context, - core.client, - serverURL = BuildConfig.AMO_SERVER_URL, - collectionAccount = context.settings().customAddonsAccount, - collectionName = context.settings().customAddonsCollection, - maxCacheAgeInMinutes = AMO_COLLECTION_MAX_CACHE_AGE - ) - } + PagedAddonCollectionProvider( + context, + core.client, + serverURL = BuildConfig.AMO_SERVER_URL, + maxCacheAgeInMinutes = AMO_COLLECTION_MAX_CACHE_AGE + ) } @Suppress("MagicNumber") @@ -146,18 +132,8 @@ class Components(private val context: Context) { AddonManager(core.store, core.engine, addonCollectionProvider, addonUpdater) } - - /** - * Tell the addon-finding logic that it needs to go download the list of - * addons, from a source that may have changed. - */ - fun updateAddonManager() { + fun clearAddonCache() { addonCollectionProvider.deleteCacheFile(context) - - val addonsAccount = context.settings().customAddonsAccount - val addonsCollection = context.settings().customAddonsCollection - addonCollectionProvider.setCollectionAccount(addonsAccount) - addonCollectionProvider.setCollectionName(addonsCollection) } val analytics by lazyMonitored { Analytics(context) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt index 1910ed30269f..0ad69e450b80 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt @@ -9,6 +9,7 @@ import android.os.Build import android.os.Build.VERSION.SDK_INT import android.os.Bundle import androidx.appcompat.app.AppCompatDelegate +import androidx.preference.EditTextPreference import androidx.preference.PreferenceFragmentCompat import androidx.preference.SwitchPreference import org.mozilla.fenix.FeatureFlags @@ -52,6 +53,14 @@ class CustomizationFragment : PreferenceFragmentCompat() { setupRadioGroups() setupToolbarCategory() setupGesturesCategory() + setupAddonsCustomizationCategory() + setupSystemBehaviorCategory() + + requirePreference(R.string.pref_key_strip_url).apply { + isChecked = requireContext().settings().shouldStripUrl + + onPreferenceChangeListener = SharedPreferenceUpdater() + } } private fun setupRadioGroups() { @@ -142,19 +151,38 @@ class CustomizationFragment : PreferenceFragmentCompat() { private fun setupGesturesCategory() { requirePreference(R.string.pref_key_website_pull_to_refresh).apply { isVisible = FeatureFlags.pullToRefreshEnabled - isChecked = context.settings().isPullToRefreshEnabledInBrowser + isChecked = requireContext().settings().isPullToRefreshEnabledInBrowser onPreferenceChangeListener = SharedPreferenceUpdater() } requirePreference(R.string.pref_key_dynamic_toolbar).apply { - isChecked = context.settings().isDynamicToolbarEnabled + isChecked = requireContext().settings().isDynamicToolbarEnabled onPreferenceChangeListener = SharedPreferenceUpdater() } requirePreference(R.string.pref_key_swipe_toolbar_switch_tabs).apply { - isChecked = context.settings().isSwipeToolbarToSwitchTabsEnabled + isChecked = requireContext().settings().isSwipeToolbarToSwitchTabsEnabled + onPreferenceChangeListener = SharedPreferenceUpdater() + } + } + + private fun setupAddonsCustomizationCategory() { + requirePreference(R.string.pref_key_addons_custom_account).apply { + text = requireContext().settings().customAddonsAccount + onPreferenceChangeListener = SharedPreferenceUpdater() + } + + requirePreference(R.string.pref_key_addons_custom_collection).apply { + text = requireContext().settings().customAddonsCollection onPreferenceChangeListener = SharedPreferenceUpdater() } } + private fun setupSystemBehaviorCategory() { + requirePreference(R.string.pref_key_relinquish_memory_under_pressure).apply { + isChecked = requireContext().settings().shouldRelinquishMemoryUnderPressure + onPreferenceChangeListener = SharedPreferenceUpdater() + } + } + companion object { // Used to send telemetry data about toolbar position changes enum class Position { TOP, BOTTOM } diff --git a/app/src/main/java/org/mozilla/fenix/settings/SharedPreferenceUpdater.kt b/app/src/main/java/org/mozilla/fenix/settings/SharedPreferenceUpdater.kt index 950a718620be..8c39d3c7750f 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SharedPreferenceUpdater.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/SharedPreferenceUpdater.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.settings import androidx.core.content.edit import androidx.preference.Preference +import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.R import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings @@ -15,7 +16,9 @@ import org.mozilla.fenix.ext.settings * The preference key is used as the shared preference key. */ open class SharedPreferenceUpdater : Preference.OnPreferenceChangeListener { - + + private val logger = Logger("SharedPreferenceUpdater") + override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean { if (newValue is Boolean) { preference.context.settings().preferences.edit { @@ -25,12 +28,14 @@ open class SharedPreferenceUpdater : Preference.OnPreferenceChangeListener { preference.context.settings().preferences.edit { putString(preference.key, newValue) } - + logger.info("Set string preference ${preference.key} to ${newValue}") if (preference.key == preference.context.getString(R.string.pref_key_addons_custom_account) || preference.key == preference.context.getString(R.string.pref_key_addons_custom_collection) ) { - preference.context.components.updateAddonManager() + logger.info("Preference suggests clearing add-on cache") + preference.context.components.clearAddonCache() } + } return true