From a6b07afa95c49e62987dbf7adbec1a4da74f31ef Mon Sep 17 00:00:00 2001 From: ekager Date: Thu, 20 Feb 2020 18:51:12 -0800 Subject: [PATCH] No issue: Fix LeakCanary detected memory leaks Co-authored-by: Emily Kager Co-authored-by: Pierre-Yves Ricau --- .../fenix/browser/BaseBrowserFragment.kt | 26 +++++++++++++++---- .../components/toolbar/BrowserToolbarView.kt | 7 ++--- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 33179eb4e..16b8f08b8 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -98,8 +98,14 @@ import org.mozilla.fenix.theme.ThemeManager @Suppress("TooManyFunctions", "LargeClass") abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, SessionManager.Observer { protected lateinit var browserFragmentStore: BrowserFragmentStore - protected lateinit var browserInteractor: BrowserToolbarViewInteractor - protected lateinit var browserToolbarView: BrowserToolbarView + + private var _browserInteractor: BrowserToolbarViewInteractor? = null + protected val browserInteractor: BrowserToolbarViewInteractor + get() = _browserInteractor!! + + private var _browserToolbarView: BrowserToolbarView? = null + protected val browserToolbarView: BrowserToolbarView + get() = _browserToolbarView!! protected val readerViewFeature = ViewBoundFeatureWrapper() @@ -191,15 +197,16 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session topSiteStorage = requireComponents.core.topSiteStorage ) - browserInteractor = BrowserInteractor( + _browserInteractor = BrowserInteractor( browserToolbarController = browserToolbarController ) - browserToolbarView = BrowserToolbarView( + _browserToolbarView = BrowserToolbarView( container = view.browserLayout, shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar, interactor = browserInteractor, - customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) } + customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) }, + lifecycleOwner = this.viewLifecycleOwner ) toolbarIntegration.set( @@ -764,6 +771,15 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } } + /* + * Dereference these views when the fragment view is destroyed to prevent memory leaks + */ + override fun onDestroyView() { + super.onDestroyView() + _browserToolbarView = null + _browserInteractor = null + } + companion object { private const val KEY_CUSTOM_TAB_SESSION_ID = "custom_tab_session_id" private const val REQUEST_CODE_DOWNLOAD_PERMISSIONS = 1 diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index f329aa5f6..df4ad2177 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt @@ -11,9 +11,9 @@ import android.view.ViewGroup import android.widget.LinearLayout import android.widget.PopupWindow import androidx.annotation.LayoutRes -import androidx.appcompat.app.AppCompatActivity import androidx.core.content.ContextCompat import androidx.core.view.isVisible +import androidx.lifecycle.LifecycleOwner import com.google.android.material.snackbar.Snackbar import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.* @@ -45,7 +45,8 @@ class BrowserToolbarView( private val container: ViewGroup, private val shouldUseBottomToolbar: Boolean, private val interactor: BrowserToolbarViewInteractor, - private val customTabSession: Session? + private val customTabSession: Session?, + private val lifecycleOwner: LifecycleOwner ) : LayoutContainer { override val containerView: View? @@ -188,7 +189,7 @@ class BrowserToolbarView( hasAccountProblem = components.backgroundServices.accountManager.accountNeedsReauth(), shouldReverseItems = !shouldUseBottomToolbar, onItemTapped = { interactor.onBrowserToolbarMenuItemTapped(it) }, - lifecycleOwner = container.context as AppCompatActivity, + lifecycleOwner = lifecycleOwner, sessionManager = sessionManager, bookmarksStorage = bookmarkStorage )