From af3a5b0a1736e2a2e5a8222c1f699678221acab6 Mon Sep 17 00:00:00 2001
From: Jonathan Almeida <jalmeida@mozilla.com>
Date: Mon, 5 Apr 2021 23:10:29 +0400
Subject: [PATCH] No issue - Make TabLayoutMediator lifecycle aware (#18779)

---
 .../mozilla/fenix/tabstray/TabLayoutMediator.kt  | 13 ++++++++++---
 .../mozilla/fenix/tabstray/TabsTrayFragment.kt   | 16 +++++++++++-----
 .../fenix/tabstray/TabLayoutMediatorTest.kt      | 15 +++++++++++++++
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt
index 3726636a05..25f1d6b624 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt
@@ -8,6 +8,7 @@ import androidx.annotation.VisibleForTesting
 import com.google.android.material.tabs.TabLayout
 import mozilla.components.browser.state.selector.selectedTab
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.support.base.feature.LifecycleAwareFeature
 import org.mozilla.fenix.tabstray.TrayPagerAdapter.Companion.POSITION_NORMAL_TABS
 import org.mozilla.fenix.tabstray.TrayPagerAdapter.Companion.POSITION_PRIVATE_TABS
 
@@ -19,17 +20,23 @@ class TabLayoutMediator(
     private val tabLayout: TabLayout,
     private val interactor: TabsTrayInteractor,
     private val store: BrowserStore
-) {
+) : LifecycleAwareFeature {
+
+    private val observer = TabLayoutObserver(interactor)
 
     /**
      * Start observing the [TabLayout] and select the current tab for initial state.
      */
-    fun attach() {
-        tabLayout.addOnTabSelectedListener(TabLayoutObserver(interactor))
+    override fun start() {
+        tabLayout.addOnTabSelectedListener(observer)
 
         selectActivePage()
     }
 
+    override fun stop() {
+        tabLayout.removeOnTabSelectedListener(observer)
+    }
+
     @VisibleForTesting
     internal fun selectActivePage() {
         val selectedTab = store.state.selectedTab ?: return
diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
index 596be401e7..a2dea00aed 100644
--- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
@@ -19,6 +19,7 @@ import org.mozilla.fenix.HomeActivity
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import mozilla.components.browser.state.selector.normalTabs
 import mozilla.components.lib.state.ext.consumeFrom
+import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.ui.tabcounter.TabCounter
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.requireComponents
@@ -32,6 +33,8 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor {
 
     lateinit var behavior: BottomSheetBehavior<ConstraintLayout>
 
+    private val tabLayoutMediator = ViewBoundFeatureWrapper<TabLayoutMediator>()
+
     private val selectTabUseCase by lazy {
         SelectTabUseCaseWrapper(
             requireComponents.analytics.metrics,
@@ -85,11 +88,14 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor {
 
         setupPager(view.context, this, browserTrayInteractor, syncedTabsTrayInteractor)
 
-        TabLayoutMediator(
-            tabLayout = tab_layout,
-            interactor = this,
-            store = requireComponents.core.store
-        ).attach()
+        tabLayoutMediator.set(
+            feature = TabLayoutMediator(
+                tabLayout = tab_layout,
+                interactor = this,
+                store = requireComponents.core.store
+            ), owner = this,
+            view = view
+        )
 
         consumeFrom(requireComponents.core.store) {
             view.findViewById<TabCounter>(R.id.tab_counter)?.apply {
diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutMediatorTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutMediatorTest.kt
index 67b3ad78df..3ccb737d23 100644
--- a/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutMediatorTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutMediatorTest.kt
@@ -49,6 +49,21 @@ class TabLayoutMediatorTest {
         verify { tab.select() }
     }
 
+    @Test
+    fun `lifecycle methods adds and removes observer`() {
+        val store = createState("456")
+        val tabLayout: TabLayout = mockk(relaxed = true)
+        val mediator = TabLayoutMediator(tabLayout, mockk(relaxed = true), store)
+
+        mediator.start()
+
+        verify { tabLayout.addOnTabSelectedListener(any()) }
+
+        mediator.stop()
+
+        verify { tabLayout.removeOnTabSelectedListener(any()) }
+    }
+
     private fun createState(selectedId: String) = BrowserStore(
         initialState = BrowserState(
             tabs = listOf(