From be7bd527f68a4a128be5f3f27f93650da0fc6f3e Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Wed, 5 Feb 2020 14:22:45 -0500 Subject: [PATCH] For issue #8097: Run add-ons callbacks only when the fragment is attached. --- .../fenix/addons/AddonsManagementFragment.kt | 15 ++-- .../org/mozilla/fenix/addons/Extensions.kt | 12 +++ .../addons/InstalledAddonDetailsFragment.kt | 89 ++++++++++++------- 3 files changed, 77 insertions(+), 39 deletions(-) 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 ead13842d..c72f3181f 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt @@ -9,12 +9,13 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.fragment.app.Fragment +import androidx.lifecycle.lifecycleScope import androidx.navigation.Navigation import androidx.recyclerview.widget.LinearLayoutManager import kotlinx.android.synthetic.main.fragment_add_ons_management.* import kotlinx.android.synthetic.main.fragment_add_ons_management.view.* -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.launch import mozilla.components.feature.addons.Addon import mozilla.components.feature.addons.AddonManagerException @@ -31,7 +32,6 @@ import org.mozilla.fenix.ext.showToolbar */ @Suppress("TooManyFunctions") class AddonsManagementFragment : Fragment(), AddonsManagerAdapterDelegate { - private val scope = CoroutineScope(Dispatchers.IO) override fun onCreateView( inflater: LayoutInflater, @@ -77,11 +77,11 @@ class AddonsManagementFragment : Fragment(), AddonsManagerAdapterDelegate { private fun bindRecyclerView(view: View) { val recyclerView = view.add_ons_list recyclerView.layoutManager = LinearLayoutManager(requireContext()) - scope.launch { + lifecycleScope.launch(IO) { try { val addons = requireContext().components.addonManager.getAddons() - scope.launch(Dispatchers.Main) { + lifecycleScope.launch(Dispatchers.Main) { val adapter = AddonsManagerAdapter( requireContext().components.addonCollectionProvider, this@AddonsManagementFragment, @@ -90,7 +90,7 @@ class AddonsManagementFragment : Fragment(), AddonsManagerAdapterDelegate { recyclerView.adapter = adapter } } catch (e: AddonManagerException) { - scope.launch(Dispatchers.Main) { + lifecycleScope.launch(Dispatchers.Main) { showSnackBar(view, getString(R.string.mozac_feature_addons_failed_to_query_add_ons)) } } @@ -153,15 +153,14 @@ class AddonsManagementFragment : Fragment(), AddonsManagerAdapterDelegate { ) ) bindRecyclerView(view) + addonProgressOverlay?.visibility = View.GONE } - - addonProgressOverlay?.visibility = View.GONE }, onError = { _, _ -> this@AddonsManagementFragment.view?.let { view -> showSnackBar(view, getString(R.string.mozac_feature_addons_failed_to_install, addon.translatedName)) + addonProgressOverlay?.visibility = View.GONE } - addonProgressOverlay?.visibility = View.GONE } ) } diff --git a/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt b/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt index 306a6d29e..ded5b7785 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.addons import android.view.View +import androidx.fragment.app.Fragment import org.mozilla.fenix.components.FenixSnackbar import java.text.NumberFormat import java.util.Locale @@ -29,3 +30,14 @@ internal fun showSnackBar(view: View, text: String) { .setText(text) .show() } + +/** + * Run the [block] only if the [Fragment] is attached. + * + * @param block A callback to be executed if the container [Fragment] is attached. + */ +internal inline fun Fragment.runIfFragmentIsAttached(block: () -> Unit) { + context?.let { + block() + } +} diff --git a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt index e8dde6b9f..45ef1f3e5 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt @@ -62,36 +62,56 @@ class InstalledAddonDetailsFragment : Fragment() { requireContext().components.addonManager.enableAddon( addon, onSuccess = { - switch.setState(true) - this.addon = it - showSnackBar( - view, - getString(R.string.mozac_feature_addons_successfully_enabled, addon.translatedName) - ) + runIfFragmentIsAttached { + switch.setState(true) + this.addon = it + showSnackBar( + view, + getString( + R.string.mozac_feature_addons_successfully_enabled, + addon.translatedName + ) + ) + } }, onError = { - showSnackBar( - view, - getString(R.string.mozac_feature_addons_failed_to_enable, addon.translatedName) - ) + runIfFragmentIsAttached { + showSnackBar( + view, + getString( + R.string.mozac_feature_addons_failed_to_enable, + addon.translatedName + ) + ) + } } ) } else { requireContext().components.addonManager.disableAddon( addon, onSuccess = { - switch.setState(false) - this.addon = it - showSnackBar( - view, - getString(R.string.mozac_feature_addons_successfully_disabled, addon.translatedName) - ) + runIfFragmentIsAttached { + switch.setState(false) + this.addon = it + showSnackBar( + view, + getString( + R.string.mozac_feature_addons_successfully_disabled, + addon.translatedName + ) + ) + } }, onError = { - showSnackBar( - view, - getString(R.string.mozac_feature_addons_failed_to_disable, addon.translatedName) - ) + runIfFragmentIsAttached { + showSnackBar( + view, + getString( + R.string.mozac_feature_addons_failed_to_disable, + addon.translatedName + ) + ) + } } ) } @@ -136,20 +156,27 @@ class InstalledAddonDetailsFragment : Fragment() { requireContext().components.addonManager.uninstallAddon( addon, onSuccess = { - showSnackBar( - view, - getString(R.string.mozac_feature_addons_successfully_uninstalled, addon.translatedName) - ) - view.findNavController().popBackStack() + runIfFragmentIsAttached { + showSnackBar( + view, + getString( + R.string.mozac_feature_addons_successfully_uninstalled, + addon.translatedName + ) + ) + view.findNavController().popBackStack() + } }, onError = { _, _ -> - showSnackBar( - view, - getString( - R.string.mozac_feature_addons_failed_to_uninstall, - addon.translatedName + runIfFragmentIsAttached { + showSnackBar( + view, + getString( + R.string.mozac_feature_addons_failed_to_uninstall, + addon.translatedName + ) ) - ) + } } ) }