Close #18668: Add stable ID cache for TabAdapter.getItemId

app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt

@@ -45,10 +45,6 @@ class BrowserTabsAdapter(
 
     private val imageLoader = ThumbnailLoader(context.components.core.thumbnailStorage)
 
-    init {
-        setHasStableIds(true)
-    }
-
     override fun getItemViewType(position: Int): Int {
         return if (context.settings().gridTabView) {
             ViewType.GRID.ordinal
@@ -64,8 +60,6 @@ class BrowserTabsAdapter(
         }
     }
 
-    override fun getItemId(position: Int) = position.toLong()
-
     override fun onBindViewHolder(holder: TabsTrayViewHolder, position: Int) {
         super.onBindViewHolder(holder, position)
 
@@ -79,7 +73,7 @@ class BrowserTabsAdapter(
             }
 
             tracker?.let {
-                holder.showTabIsMultiSelectEnabled(it.isSelected(position.toLong()))
+                holder.showTabIsMultiSelectEnabled(it.isSelected(getItemId(position)))
             }
         }
     }

app/src/main/java/org/mozilla/fenix/tabstray/browser/TabAdapterIdStorage.kt

@@ -0,0 +1,45 @@
+/* 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.util.LruCache
+import androidx.recyclerview.widget.RecyclerView
+import mozilla.components.concept.tabstray.Tab
+
+internal const val INITIAL_NUMBER_OF_TABS = 20
+internal const val CACHE_SIZE_MULTIPLIER = 1.5
+
+/**
+ * Storage for Browser tabs that need a stable ID for each item in a [RecyclerView.Adapter].
+ * This ID is commonly needed by [RecyclerView.Adapter.getItemId] when
+ * enabling [RecyclerView.Adapter.setHasStableIds].
+ */
+internal class TabAdapterIdStorage(initialSize: Int = INITIAL_NUMBER_OF_TABS) {
+    private val uniqueTabIds = LruCache<String, Long>(initialSize)
+    private var lastUsedSuggestionId = 0L
+
+    /**
+     * Returns a unique tab ID for the given [Tab].
+     */
+    fun getStableId(tab: Tab): Long {
+        val key = tab.id
+        return uniqueTabIds[key] ?: run {
+            lastUsedSuggestionId += 1
+            uniqueTabIds.put(key, lastUsedSuggestionId)
+            lastUsedSuggestionId
+        }
+    }
+
+    /**
+     * Resizes the internal cache size if the [count] is larger than what is currently available.
+     */
+    fun resizeCacheIfNeeded(count: Int) {
+        val currentMaxSize = uniqueTabIds.maxSize()
+        if (count > currentMaxSize) {
+            val newMaxSize = (count * CACHE_SIZE_MULTIPLIER).toInt()
+            uniqueTabIds.resize(newMaxSize)
+        }
+    }
+}

app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsAdapter.kt

@@ -13,11 +13,17 @@ import mozilla.components.concept.tabstray.TabsTray
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
 
-// The previous tabs adapter was very restrictive and required Fenix to jump through
-// may hoops to access and update certain methods. An abstract adapter is easier to manage
-// for Android UI APIs.
-//
-// TODO Let's upstream this to AC with tests.
+/**
+ * RecyclerView adapter implementation to display a list/grid of tabs.
+ *
+ * The previous tabs adapter was very restrictive and required Fenix to jump through
+ * may hoops to access and update certain methods. An abstract adapter is easier to manage
+ * for Android UI APIs.
+ *
+ * TODO Let's upstream this to AC with tests.
+ *
+ * @param delegate TabsTray.Observer registry to allow `TabsAdapter` to conform to `Observable<TabsTray.Observer>`.
+ */
 abstract class TabsAdapter<T : TabViewHolder>(
     delegate: Observable<TabsTray.Observer> = ObserverRegistry()
 ) : RecyclerView.Adapter<T>(), TabsTray, Observable<TabsTray.Observer> by delegate {
@@ -25,10 +31,18 @@ abstract class TabsAdapter<T : TabViewHolder>(
     protected var tabs: Tabs? = null
     protected var styling: TabsTrayStyling = TabsTrayStyling()
 
+    private val idStorage = TabAdapterIdStorage()
+
+    init {
+        setHasStableIds(true)
+    }
+
     @CallSuper
     override fun updateTabs(tabs: Tabs) {
         this.tabs = tabs
 
+        idStorage.resizeCacheIfNeeded(tabs.list.size)
+
         notifyObservers { onTabsUpdated() }
     }
 
@@ -39,6 +53,13 @@ abstract class TabsAdapter<T : TabViewHolder>(
         holder.bind(tabs.list[position], isTabSelected(tabs, position), styling, this)
     }
 
+    override fun getItemId(position: Int): Long {
+        val key = tabs?.list?.get(position)
+            ?: throw IllegalStateException("Unknown tab for position $position")
+
+        return idStorage.getStableId(key)
+    }
+
     override fun getItemCount(): Int = tabs?.list?.size ?: 0
 
     final override fun isTabSelected(tabs: Tabs, position: Int): Boolean =

app/src/test/java/org/mozilla/fenix/tabstray/browser/TabAdapterIdStorageTest.kt

@@ -0,0 +1,85 @@
+/* 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 mozilla.components.concept.tabstray.Tab
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
+import java.util.UUID
+
+@RunWith(FenixRobolectricTestRunner::class)
+class TabAdapterIdStorageTest {
+
+    @Test
+    fun `the same ID is returned when queried multiple times`() {
+        val storage = TabAdapterIdStorage()
+        val tab = createTab()
+
+        val id1 = storage.getStableId(tab)
+        val id2 = storage.getStableId(tab)
+
+        assertEquals(id1, id2)
+    }
+
+    @Test
+    fun `the same ID is returned when the cache is at max`() {
+        val storage = TabAdapterIdStorage(2)
+        val tab1 = createTab()
+        val tab2 = createTab()
+
+        val id1 = storage.getStableId(tab1)
+        val id2 = storage.getStableId(tab2)
+        val id1Again = storage.getStableId(tab1)
+
+        assertEquals(id1, id1Again)
+        assertNotEquals(id1, id2)
+    }
+
+    @Test
+    fun `the same ID is NOT returned if the cache is over max`() {
+        val storage = TabAdapterIdStorage(2)
+        val tab1 = createTab()
+        val tab2 = createTab()
+        val tab3 = createTab()
+
+        val id1 = storage.getStableId(tab1)
+        val id2 = storage.getStableId(tab2)
+        val id3 = storage.getStableId(tab3)
+        val id1Again = storage.getStableId(tab1)
+
+        assertNotEquals(id1, id1Again)
+        assertNotEquals(id1, id2)
+        assertNotEquals(id1, id3)
+    }
+
+    @Test
+    fun `the same ID is returned if the cache is resized when full`() {
+        val storage = TabAdapterIdStorage(2)
+        val tab1 = createTab()
+        val tab2 = createTab()
+        val tab3 = createTab()
+
+        val id1 = storage.getStableId(tab1)
+        val id2 = storage.getStableId(tab2)
+
+        storage.resizeCacheIfNeeded(3)
+
+        val id3 = storage.getStableId(tab3)
+        val id1Again = storage.getStableId(tab1)
+
+        assertEquals(id1, id1Again)
+        assertNotEquals(id1, id2)
+        assertNotEquals(id1, id3)
+        assertNotEquals(id2, id3)
+    }
+}
+
+fun createTab() = Tab(
+    UUID.randomUUID().toString(),
+    "https://mozilla.org"
+)

app/src/test/java/org/mozilla/fenix/tabstray/browser/TabsAdapterTest.kt

@@ -0,0 +1,76 @@
+/* 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.view.View
+import android.view.ViewGroup
+import mozilla.components.browser.tabstray.TabViewHolder
+import mozilla.components.browser.tabstray.TabsTrayStyling
+import mozilla.components.concept.tabstray.Tab
+import mozilla.components.concept.tabstray.Tabs
+import mozilla.components.concept.tabstray.TabsTray
+import mozilla.components.support.base.observer.Observable
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
+
+@RunWith(FenixRobolectricTestRunner::class)
+class TabsAdapterTest {
+
+    lateinit var adapter: TabsAdapter<TestTabsAdapter.ViewHolder>
+
+    @Before
+    fun setup() {
+        adapter = TestTabsAdapter()
+    }
+
+    @Test
+    fun `getItemId gives a new ID for each position`() {
+        val (tab1, tab2, tab3) = Triple(createTab(), createTab(), createTab())
+        val tabs = Tabs(
+            list = listOf(tab1, tab2, tab3),
+            selectedIndex = 0
+        )
+
+        adapter.updateTabs(tabs)
+
+        val id1 = adapter.getItemId(0)
+        val id2 = adapter.getItemId(1)
+        val id3 = adapter.getItemId(2)
+        val id1Again = adapter.getItemId(0)
+
+        assertEquals(id1, id1Again)
+        assertNotEquals(id1, id2)
+        assertNotEquals(id1, id3)
+        assertNotEquals(id2, id3)
+    }
+
+    @Test(expected = IllegalStateException::class)
+    fun `getItemId throws if a tab does not exist for the position`() {
+        adapter.getItemId(4)
+    }
+
+    class TestTabsAdapter : TabsAdapter<TestTabsAdapter.ViewHolder>() {
+
+        inner class ViewHolder(view: View) : TabViewHolder(view) {
+            override var tab: Tab? = null
+
+            override fun bind(
+                tab: Tab,
+                isSelected: Boolean,
+                styling: TabsTrayStyling,
+                observable: Observable<TabsTray.Observer>
+            ) = throw UnsupportedOperationException()
+        }
+
+        override fun onCreateViewHolder(
+            parent: ViewGroup,
+            viewType: Int
+        ): ViewHolder = throw UnsupportedOperationException()
+    }
+}