From 2cce1c0a5ca8377484518067634f31fa0f472644 Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Thu, 16 Jun 2022 18:26:07 +0530 Subject: [PATCH 1/3] issue_487: Fix WebView related memory leaks in TOSFragment.kt --- .../e/apps/setup/tos/TOSFragment.kt | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/setup/tos/TOSFragment.kt b/app/src/main/java/foundation/e/apps/setup/tos/TOSFragment.kt index 2909d8c29..920a70f49 100644 --- a/app/src/main/java/foundation/e/apps/setup/tos/TOSFragment.kt +++ b/app/src/main/java/foundation/e/apps/setup/tos/TOSFragment.kt @@ -3,6 +3,7 @@ package foundation.e.apps.setup.tos import android.content.res.Configuration import android.os.Bundle import android.view.View +import android.webkit.WebView import androidx.constraintlayout.widget.ConstraintSet import androidx.fragment.app.Fragment import androidx.fragment.app.viewModels @@ -18,11 +19,18 @@ class TOSFragment : Fragment(R.layout.fragment_tos) { private var _binding: FragmentTosBinding? = null private val binding get() = _binding!! + /* + * Fix memory leaks related to WebView. + * Issue: https://gitlab.e.foundation/e/os/backlog/-/issues/485 + */ + private var webView: WebView? = null + private val viewModel: TOSViewModel by viewModels() override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) _binding = FragmentTosBinding.bind(view) + webView = binding.tosWebView var canNavigate = false viewModel.tocStatus.observe(viewLifecycleOwner) { @@ -30,7 +38,7 @@ class TOSFragment : Fragment(R.layout.fragment_tos) { view.findNavController().navigate(R.id.action_TOSFragment_to_signInFragment) } - if (it == true) { + if (it == true && webView != null) { binding.TOSWarning.visibility = View.GONE binding.TOSButtons.visibility = View.GONE binding.toolbar.visibility = View.VISIBLE @@ -38,7 +46,7 @@ class TOSFragment : Fragment(R.layout.fragment_tos) { val constraintSet = ConstraintSet() constraintSet.clone(binding.root) constraintSet.connect( - binding.tosWebView.id, + webView!!.id, ConstraintSet.TOP, binding.acceptDateTV.id, ConstraintSet.BOTTOM, @@ -66,6 +74,16 @@ class TOSFragment : Fragment(R.layout.fragment_tos) { override fun onDestroyView() { super.onDestroyView() + /* + * Fix WebView memory leaks. https://stackoverflow.com/a/19391512 + * Issue: https://gitlab.e.foundation/e/os/backlog/-/issues/485 + */ + webView?.run { + setOnScrollChangeListener(null) + removeAllViews() + destroy() + } + webView = null _binding = null } @@ -92,12 +110,12 @@ class TOSFragment : Fragment(R.layout.fragment_tos) { ) .append(body.toString()) .append("") - binding.tosWebView.loadDataWithBaseURL( + webView?.loadDataWithBaseURL( "file:///android_asset/", sb.toString(), "text/html", "utf-8", null ) - binding.tosWebView.setOnScrollChangeListener { _, scrollX, scrollY, _, _ -> + webView?.setOnScrollChangeListener { _, scrollX, scrollY, _, _ -> if (scrollX == 0 && scrollY == 0 && viewModel.tocStatus.value == true) { binding.acceptDateTV.visibility = View.VISIBLE } else { -- GitLab From e8bc368cb431bbf4b024f22a1c02848ee2fd3581 Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Thu, 16 Jun 2022 18:52:34 +0530 Subject: [PATCH 2/3] issue_487: Fix adapter related memory leaks in HomeFragment.kt --- .../java/foundation/e/apps/home/HomeFragment.kt | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/home/HomeFragment.kt b/app/src/main/java/foundation/e/apps/home/HomeFragment.kt index 69b9a2fc7..e3af670ba 100644 --- a/app/src/main/java/foundation/e/apps/home/HomeFragment.kt +++ b/app/src/main/java/foundation/e/apps/home/HomeFragment.kt @@ -54,7 +54,11 @@ import javax.inject.Inject @AndroidEntryPoint class HomeFragment : TimeoutFragment(R.layout.fragment_home), FusedAPIInterface { - private lateinit var homeParentRVAdapter: HomeParentRVAdapter + /* + * Make adapter nullable to avoid memory leaks. + * Issue: https://gitlab.e.foundation/e/os/backlog/-/issues/485 + */ + private var homeParentRVAdapter: HomeParentRVAdapter? = null private var _binding: FragmentHomeBinding? = null private val binding get() = _binding!! @@ -155,7 +159,7 @@ class HomeFragment : TimeoutFragment(R.layout.fragment_home), FusedAPIInterface stopLoadingUI() if (it.second == ResultStatus.OK) { dismissTimeoutDialog() - homeParentRVAdapter.setData(it.first) + homeParentRVAdapter?.setData(it.first) } else { onTimeout() } @@ -211,10 +215,10 @@ class HomeFragment : TimeoutFragment(R.layout.fragment_home), FusedAPIInterface } private fun updateProgressOfDownloadingAppItemViews( - homeParentRVAdapter: HomeParentRVAdapter, + homeParentRVAdapter: HomeParentRVAdapter?, downloadProgress: DownloadProgress ) { - homeParentRVAdapter.currentList.forEach { fusedHome -> + homeParentRVAdapter?.currentList?.forEach { fusedHome -> val viewHolder = binding.parentRV.findViewHolderForAdapterPosition( homeParentRVAdapter.currentList.indexOf(fusedHome) ) @@ -269,6 +273,11 @@ class HomeFragment : TimeoutFragment(R.layout.fragment_home), FusedAPIInterface override fun onDestroyView() { super.onDestroyView() _binding = null + /* + * Nullify adapter to avoid leaks. + * Issue: https://gitlab.e.foundation/e/os/backlog/-/issues/485 + */ + homeParentRVAdapter = null } override fun getApplication(app: FusedApp, appIcon: ImageView?) { -- GitLab From a734a8b233e65c815d6b91dab70b089092b54f54 Mon Sep 17 00:00:00 2001 From: Hasib Prince Date: Fri, 17 Jun 2022 14:11:22 +0600 Subject: [PATCH 3/3] App Lounge: fixed memory leak for HomePage Adapters --- .../e/apps/home/model/HomeChildRVAdapter.kt | 29 ++++++++++++------- .../e/apps/home/model/HomeParentRVAdapter.kt | 25 ++++++++++++---- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/home/model/HomeChildRVAdapter.kt b/app/src/main/java/foundation/e/apps/home/model/HomeChildRVAdapter.kt index 59fe1b908..ae17f2793 100644 --- a/app/src/main/java/foundation/e/apps/home/model/HomeChildRVAdapter.kt +++ b/app/src/main/java/foundation/e/apps/home/model/HomeChildRVAdapter.kt @@ -48,14 +48,14 @@ import foundation.e.apps.utils.enums.User import foundation.e.apps.utils.modules.PWAManagerModule class HomeChildRVAdapter( - private val fusedAPIInterface: FusedAPIInterface, + private var fusedAPIInterface: FusedAPIInterface?, private val pkgManagerModule: PkgManagerModule, private val pwaManagerModule: PWAManagerModule, private val appInfoFetchViewModel: AppInfoFetchViewModel, private val mainActivityViewModel: MainActivityViewModel, private val user: User, - private val lifecycleOwner: LifecycleOwner, - private val paidAppHandler: ((FusedApp) -> Unit)? = null + private var lifecycleOwner: LifecycleOwner?, + private var paidAppHandler: ((FusedApp) -> Unit)? = null ) : ListAdapter(HomeChildFusedAppDiffUtil()) { private val shimmer = Shimmer.ColorHighlightBuilder() @@ -293,11 +293,13 @@ class HomeChildRVAdapter( materialButton.isEnabled = false materialButton.text = "" homeChildListItemBinding.progressBarInstall.visibility = View.VISIBLE - appInfoFetchViewModel.isAppPurchased(homeApp).observe(lifecycleOwner) { - materialButton.isEnabled = true - homeChildListItemBinding.progressBarInstall.visibility = View.GONE - materialButton.text = - if (it) materialButton.context.getString(R.string.install) else homeApp.price + lifecycleOwner?.let { + appInfoFetchViewModel.isAppPurchased(homeApp).observe(it) { + materialButton.isEnabled = true + homeChildListItemBinding.progressBarInstall.visibility = View.GONE + materialButton.text = + if (it) materialButton.context.getString(R.string.install) else homeApp.price + } } } } @@ -308,10 +310,17 @@ class HomeChildRVAdapter( } private fun installApplication(homeApp: FusedApp, appIcon: ImageView) { - fusedAPIInterface.getApplication(homeApp, appIcon) + fusedAPIInterface?.getApplication(homeApp, appIcon) } private fun cancelDownload(homeApp: FusedApp) { - fusedAPIInterface.cancelDownload(homeApp) + fusedAPIInterface?.cancelDownload(homeApp) + } + + override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) { + super.onDetachedFromRecyclerView(recyclerView) + lifecycleOwner = null + paidAppHandler = null + fusedAPIInterface = null } } diff --git a/app/src/main/java/foundation/e/apps/home/model/HomeParentRVAdapter.kt b/app/src/main/java/foundation/e/apps/home/model/HomeParentRVAdapter.kt index 7e81364fa..1af6ce558 100644 --- a/app/src/main/java/foundation/e/apps/home/model/HomeParentRVAdapter.kt +++ b/app/src/main/java/foundation/e/apps/home/model/HomeParentRVAdapter.kt @@ -41,12 +41,12 @@ class HomeParentRVAdapter( private val user: User, private val mainActivityViewModel: MainActivityViewModel, private val appInfoFetchViewModel: AppInfoFetchViewModel, - private val lifecycleOwner: LifecycleOwner, + private var lifecycleOwner: LifecycleOwner?, private val paidAppHandler: ((FusedApp) -> Unit)? = null ) : ListAdapter(FusedHomeDiffUtil()) { private val viewPool = RecyclerView.RecycledViewPool() - private var isDownloadObserverAdded = false + private var isDetachedFromRecyclerView = false inner class ViewHolder(val binding: HomeParentListItemBinding) : RecyclerView.ViewHolder(binding.root) @@ -91,13 +91,28 @@ class HomeParentRVAdapter( fusedHome: FusedHome, homeChildRVAdapter: RecyclerView.Adapter<*>? ) { - mainActivityViewModel.downloadList.observe(lifecycleOwner) { - mainActivityViewModel.updateStatusOfFusedApps(fusedHome.list, it) - (homeChildRVAdapter as HomeChildRVAdapter).setData(fusedHome.list) + lifecycleOwner?.let { + mainActivityViewModel.downloadList.observe(it) { + mainActivityViewModel.updateStatusOfFusedApps(fusedHome.list, it) + (homeChildRVAdapter as HomeChildRVAdapter).setData(fusedHome.list) + } } } fun setData(newList: List) { submitList(newList.map { it.copy() }) } + + override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) { + super.onDetachedFromRecyclerView(recyclerView) + isDetachedFromRecyclerView = true + lifecycleOwner = null + } + + override fun onViewDetachedFromWindow(holder: ViewHolder) { + super.onViewDetachedFromWindow(holder) + if(isDetachedFromRecyclerView) { + holder.binding.childRV.adapter = null + } + } } -- GitLab