From 0a4927a4957c614fe02356bbeb5f1082a9b45149 Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Tue, 13 Apr 2021 23:09:02 -0400 Subject: [PATCH] Issue #18934: Do not nullify adapter on window detached Previously, to fix a memory leak, we were removing the adapter reference entirely in order to have the `onDetachedFromRecyclerView` callback invoked. This causes a side-effect where we can no longer reference the adapter any more when we re-attach. The simpler solution is to just invoke the needed callback directly instead. --- .../tabstray/browser/BaseBrowserTrayList.kt | 8 +++-- .../browser/BaseBrowserTrayListTest.kt | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayListTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt index 78774a959..81029500f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.tabstray.browser import android.content.Context import android.util.AttributeSet +import androidx.annotation.VisibleForTesting import androidx.recyclerview.widget.RecyclerView import mozilla.components.feature.tabs.tabstray.TabsFeature import org.mozilla.fenix.ext.components @@ -82,14 +83,15 @@ abstract class BaseBrowserTrayList @JvmOverloads constructor( touchHelper.attachToRecyclerView(this) } - override fun onDetachedFromWindow() { + @VisibleForTesting + public override fun onDetachedFromWindow() { super.onDetachedFromWindow() tabsFeature.stop() swipeToDelete.stop() - // Release the adapter so that `onDetachedFromRecyclerView` will be called in the adapter. - adapter = null + // Notify the adapter that it is released from the view preemptively. + adapter?.onDetachedFromRecyclerView(this) touchHelper.attachToRecyclerView(null) } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayListTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayListTest.kt new file mode 100644 index 000000000..fe9f19402 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayListTest.kt @@ -0,0 +1,35 @@ +/* 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.tabstray.browser + +import android.content.Context +import io.mockk.mockk +import io.mockk.verify +import mozilla.components.support.test.robolectric.testContext +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.tabstray.TabsTrayStore + +@RunWith(FenixRobolectricTestRunner::class) +class BaseBrowserTrayListTest { + + @Test + fun `WHEN recyclerview detaches from window THEN notify adapter`() { + val trayList = TestBaseBrowserTrayList(testContext) + val adapter = mockk(relaxed = true) + + trayList.adapter = adapter + trayList.tabsTrayStore = TabsTrayStore() + + trayList.onDetachedFromWindow() + + verify { adapter.onDetachedFromRecyclerView(trayList) } + } + + class TestBaseBrowserTrayList(context: Context) : BaseBrowserTrayList(context) { + override val configuration = Configuration(BrowserTabType.PRIVATE) + } +}