Close #7313: Use storage/loader to get thumbnails

We were seeing odd bugs and performance issues from trying to map the
disk cache into the TabsTrayPresenter.

A better solution, would be to load the thumbnails straight from the
cache, and incremental updates via the store.

components/browser/icons/src/main/java/mozilla/components/browser/icons/BrowserIcons.kt

@@ -40,7 +40,7 @@ import mozilla.components.browser.icons.preparer.TippyTopIconPreparer
 import mozilla.components.browser.icons.processor.DiskIconProcessor
 import mozilla.components.browser.icons.processor.IconProcessor
 import mozilla.components.browser.icons.processor.MemoryIconProcessor
-import mozilla.components.browser.icons.utils.CancelOnDetach
+import mozilla.components.support.images.CancelOnDetach
 import mozilla.components.browser.icons.utils.IconDiskCache
 import mozilla.components.browser.icons.utils.IconMemoryCache
 import mozilla.components.browser.state.state.BrowserState

components/browser/tabstray/build.gradle

@@ -32,6 +32,7 @@ dependencies {
     implementation project(':ui-icons')
     implementation project(':ui-colors')
     implementation project(':support-base')
+    implementation project(':support-images')
     implementation project(':support-ktx')
 
     implementation Dependencies.androidx_appcompat

components/browser/tabstray/src/main/java/mozilla/components/browser/tabstray/TabViewHolder.kt

@@ -14,6 +14,7 @@ import mozilla.components.browser.tabstray.thumbnail.TabThumbnailView
 import mozilla.components.concept.tabstray.Tab
 import mozilla.components.concept.tabstray.TabsTray
 import mozilla.components.support.base.observer.Observable
+import mozilla.components.support.images.loader.ImageLoader
 import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl
 
 /**
@@ -36,7 +37,8 @@ abstract class TabViewHolder(view: View) : RecyclerView.ViewHolder(view) {
  */
 class DefaultTabViewHolder(
     itemView: View,
-    private val tabsTray: BrowserTabsTray
+    private val tabsTray: BrowserTabsTray,
+    private val thumbnailLoader: ImageLoader? = null
 ) : TabViewHolder(itemView) {
     private val iconView: ImageView? = itemView.findViewById(R.id.mozac_browser_tabstray_icon)
     private val titleView: TextView = itemView.findViewById(R.id.mozac_browser_tabstray_title)
@@ -79,7 +81,12 @@ class DefaultTabViewHolder(
             closeView.imageTintList = ColorStateList.valueOf(tabsTray.styling.itemTextColor)
         }
 
-        thumbnailView.setImageBitmap(tab.thumbnail)
+        // In the final else case, we have no cache or fresh screenshot; do nothing instead of clearing the image.
+        if (thumbnailLoader != null && tab.thumbnail == null) {
+            thumbnailLoader.loadIntoView(thumbnailView, tab.id)
+        } else if (tab.thumbnail != null) {
+            thumbnailView.setImageBitmap(tab.thumbnail)
+        }
 
         iconView?.setImageBitmap(tab.icon)
     }

components/browser/tabstray/src/main/java/mozilla/components/browser/tabstray/TabsAdapter.kt

@@ -11,6 +11,7 @@ import mozilla.components.concept.tabstray.Tabs
 import mozilla.components.concept.tabstray.TabsTray
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
+import mozilla.components.support.images.loader.ImageLoader
 
 /**
  * Function responsible for creating a `TabViewHolder` in the `TabsAdapter`.
@@ -24,16 +25,18 @@ typealias ViewHolderProvider = (ViewGroup, BrowserTabsTray) -> TabViewHolder
  */
 @Suppress("TooManyFunctions")
 open class TabsAdapter(
-    delegate: Observable<TabsTray.Observer> = ObserverRegistry(),
+    thumbnailLoader: ImageLoader? = null,
     private val viewHolderProvider: ViewHolderProvider = { parent, tabsTray ->
         DefaultTabViewHolder(
                 LayoutInflater.from(parent.context).inflate(
                         R.layout.mozac_browser_tabstray_item,
                         parent,
                         false),
-                tabsTray
+                tabsTray,
+                thumbnailLoader
         )
-    }
+    },
+    delegate: Observable<TabsTray.Observer> = ObserverRegistry()
 ) : RecyclerView.Adapter<TabViewHolder>(),
     TabsTray,
     Observable<TabsTray.Observer> by delegate {

components/browser/tabstray/src/test/java/mozilla/components/browser/tabstray/TabViewHolderTest.kt → components/browser/tabstray/src/test/java/mozilla/components/browser/tabstray/DefaultTabViewHolderTest.kt

@@ -13,17 +13,22 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.concept.tabstray.Tab
 import mozilla.components.concept.tabstray.TabsTray
 import mozilla.components.support.base.observer.ObserverRegistry
+import mozilla.components.support.images.loader.ImageLoader
+import mozilla.components.support.test.any
+import mozilla.components.support.test.eq
 import mozilla.components.support.test.mock
+import mozilla.components.support.test.nullable
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.doReturn
+import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
 
 @RunWith(AndroidJUnit4::class)
-class TabViewHolderTest {
+class DefaultTabViewHolderTest {
 
     @Test
     fun `URL from session is assigned to view`() {
@@ -146,6 +151,38 @@ class TabViewHolderTest {
         assertTrue(thumbnailView.drawable != null)
     }
 
+    @Test
+    fun `thumbnail is set from loader`() {
+        val view = LayoutInflater.from(testContext).inflate(R.layout.mozac_browser_tabstray_item, null)
+        val loader: ImageLoader = mock()
+        val viewHolder = DefaultTabViewHolder(view, mockTabsTrayWithStyles(), loader)
+        val tabWithThumbnail = Tab("123", "https://example.com", thumbnail = mock())
+        val tab = Tab("123", "https://example.com")
+
+        viewHolder.bind(tabWithThumbnail, false, mock())
+
+        verify(loader, never()).loadIntoView(any(), eq("123"), nullable(), nullable())
+
+        viewHolder.bind(tab, false, mock())
+
+        verify(loader).loadIntoView(any(), eq("123"), nullable(), nullable())
+    }
+
+    @Test
+    fun `thumbnailView does not change when there is no cache or new thumbnail`() {
+        val view = LayoutInflater.from(testContext).inflate(R.layout.mozac_browser_tabstray_item, null)
+        val viewHolder = DefaultTabViewHolder(view, mockTabsTrayWithStyles())
+        val tab = Tab("123", "https://example.com")
+        val thumbnailView = view.findViewById<ImageView>(R.id.mozac_browser_tabstray_thumbnail)
+
+        thumbnailView.setImageBitmap(mock())
+        val drawable = thumbnailView.drawable
+
+        viewHolder.bind(tab, false, mock())
+
+        assertEquals(drawable, thumbnailView.drawable)
+    }
+
     companion object {
         fun mockTabsTrayWithStyles(): BrowserTabsTray {
             val styles: TabsTrayStyling = mock()

components/browser/tabstray/src/test/java/mozilla/components/browser/tabstray/TabTouchCallbackTest.kt

@@ -7,6 +7,7 @@ package mozilla.components.browser.tabstray
 import android.view.LayoutInflater
 import androidx.recyclerview.widget.ItemTouchHelper
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import mozilla.components.browser.tabstray.DefaultTabViewHolderTest.Companion.mockTabsTrayWithStyles
 import mozilla.components.concept.tabstray.TabsTray
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
@@ -48,7 +49,7 @@ class TabTouchCallbackTest {
     @Test
     fun `onChildDraw alters alpha of ViewHolder on swipe gesture`() {
         val view = LayoutInflater.from(testContext).inflate(R.layout.mozac_browser_tabstray_item, null)
-        val holder = DefaultTabViewHolder(view, TabViewHolderTest.mockTabsTrayWithStyles())
+        val holder = DefaultTabViewHolder(view, mockTabsTrayWithStyles())
         val callback = TabTouchCallback(mock())
 
         holder.itemView.alpha = 0f

components/browser/tabstray/src/test/java/mozilla/components/browser/tabstray/TabsAdapterTest.kt

@@ -40,7 +40,7 @@ class TabsAdapterTest {
 
     @Test
     fun `onCreateViewHolder will create whatever TabViewHolder is provided`() {
-        val adapter = TabsAdapter { _, _ -> TestTabViewHolder(View(testContext)) }
+        val adapter = TabsAdapter(viewHolderProvider = { _, _ -> TestTabViewHolder(View(testContext)) })
         adapter.tabsTray = mock()
 
         val type = adapter.onCreateViewHolder(FrameLayout(testContext), 0)

components/browser/thumbnails/src/main/java/mozilla/components/browser/thumbnails/loader/ThumbnailLoader.kt

+/* 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 mozilla.components.browser.thumbnails.loader
+
+import android.graphics.drawable.Drawable
+import android.widget.ImageView
+import androidx.annotation.MainThread
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.launch
+import mozilla.components.browser.thumbnails.R
+import mozilla.components.browser.thumbnails.storage.ThumbnailStorage
+import mozilla.components.support.images.CancelOnDetach
+import mozilla.components.support.images.loader.ImageLoader
+import java.lang.ref.WeakReference
+
+/**
+ * An implementation of [ImageLoader] for loading thumbnails into a [ImageView].
+ */
+class ThumbnailLoader(private val storage: ThumbnailStorage) : ImageLoader {
+
+    override fun loadIntoView(
+        view: ImageView,
+        id: String,
+        placeholder: Drawable?,
+        error: Drawable?
+    ) {
+        CoroutineScope(Dispatchers.Main).launch {
+            loadIntoViewInternal(WeakReference(view), id, placeholder, error)
+        }
+    }
+
+    @MainThread
+    private suspend fun loadIntoViewInternal(
+        view: WeakReference<ImageView>,
+        id: String,
+        placeholder: Drawable?,
+        error: Drawable?
+    ) {
+        // If we previously started loading into the view, cancel the job.
+        val existingJob = view.get()?.getTag(R.id.mozac_browser_thumbnails_tag_job) as? Job
+        existingJob?.cancel()
+
+        view.get()?.setImageDrawable(placeholder)
+
+        // Create a loading job
+        val deferredThumbnail = storage.loadThumbnail(id)
+
+        view.get()?.setTag(R.id.mozac_browser_thumbnails_tag_job, deferredThumbnail)
+        val onAttachStateChangeListener = CancelOnDetach(deferredThumbnail).also {
+            view.get()?.addOnAttachStateChangeListener(it)
+        }
+
+        try {
+            val thumbnail = deferredThumbnail.await()
+            view.get()?.setImageBitmap(thumbnail)
+        } catch (e: CancellationException) {
+            view.get()?.setImageDrawable(error)
+        } finally {
+            view.get()?.removeOnAttachStateChangeListener(onAttachStateChangeListener)
+            view.get()?.setTag(R.id.mozac_browser_thumbnails_tag_job, null)
+        }
+    }
+}

components/browser/thumbnails/src/main/res/values/tags.xml

+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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/. -->
+
+<resources>
+    <item name="mozac_browser_thumbnails_tag_job" type="id" />
+</resources>

components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/loader/ThumbnailLoaderTest.kt

+/* 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 mozilla.components.browser.thumbnails.loader
+
+import android.graphics.Bitmap
+import android.graphics.drawable.Drawable
+import android.widget.ImageView
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.test.TestCoroutineDispatcher
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.setMain
+import mozilla.components.browser.thumbnails.R
+import mozilla.components.browser.thumbnails.storage.ThumbnailStorage
+import mozilla.components.support.test.any
+import mozilla.components.support.test.eq
+import mozilla.components.support.test.mock
+import mozilla.components.support.test.rule.MainCoroutineRule
+import org.junit.After
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mockito.doReturn
+import org.mockito.Mockito.never
+import org.mockito.Mockito.spy
+import org.mockito.Mockito.verify
+
+@RunWith(AndroidJUnit4::class)
+class ThumbnailLoaderTest {
+    private val testDispatcher = TestCoroutineDispatcher()
+
+    @get:Rule
+    val coroutinesTestRule = MainCoroutineRule(testDispatcher)
+
+    @Before
+    fun setup() {
+        Dispatchers.setMain(testDispatcher)
+    }
+
+    @After
+    fun teardown() {
+        Dispatchers.resetMain()
+        testDispatcher.cleanupTestCoroutines()
+    }
+
+    @Test
+    fun `automatically load thumbnails into image view`() {
+        val mockedBitmap: Bitmap = mock()
+        val result = CompletableDeferred<Bitmap>()
+        val view: ImageView = mock()
+        val storage: ThumbnailStorage = mock()
+        val loader = spy(ThumbnailLoader(storage))
+
+        doReturn(result).`when`(storage).loadThumbnail("123")
+
+        loader.loadIntoView(view, "123")
+
+        verify(view).setImageDrawable(null)
+        verify(view).addOnAttachStateChangeListener(any())
+        verify(view).setTag(eq(R.id.mozac_browser_thumbnails_tag_job), any())
+        verify(view, never()).setImageBitmap(any())
+
+        result.complete(mockedBitmap)
+
+        verify(view).setImageBitmap(mockedBitmap)
+        verify(view).removeOnAttachStateChangeListener(any())
+        verify(view).setTag(R.id.mozac_browser_thumbnails_tag_job, null)
+    }
+
+    @Test
+    fun `loadIntoView sets drawable to error if cancelled`() {
+        val result = CompletableDeferred<Bitmap>()
+        val view: ImageView = mock()
+        val placeholder: Drawable = mock()
+        val error: Drawable = mock()
+        val storage: ThumbnailStorage = mock()
+        val loader = spy(ThumbnailLoader(storage))
+
+        doReturn(result).`when`(storage).loadThumbnail("123")
+
+        loader.loadIntoView(view, "123", placeholder = placeholder, error = error)
+
+        verify(view).setImageDrawable(placeholder)
+
+        result.cancel()
+
+        verify(view).setImageDrawable(error)
+        verify(view).removeOnAttachStateChangeListener(any())
+        verify(view).setTag(R.id.mozac_browser_thumbnails_tag_job, null)
+    }
+
+    @Test
+    fun `loadIntoView cancels previous jobs`() {
+        val result = CompletableDeferred<Bitmap>()
+        val view: ImageView = mock()
+        val previousJob: Job = mock()
+        val storage: ThumbnailStorage = mock()
+        val loader = spy(ThumbnailLoader(storage))
+
+        doReturn(previousJob).`when`(view).getTag(R.id.mozac_browser_thumbnails_tag_job)
+        doReturn(result).`when`(storage).loadThumbnail("123")
+
+        loader.loadIntoView(view, "123")
+
+        verify(previousJob).cancel()
+
+        result.cancel()
+    }
+}

components/concept/tabstray/src/main/java/mozilla/components/concept/tabstray/Tab.kt

@@ -24,17 +24,4 @@ data class Tab(
     val icon: Bitmap? = null,
     val thumbnail: Bitmap? = null,
     val mediaState: Media.State? = null
-) {
-    override fun equals(other: Any?): Boolean {
-        if (javaClass != other?.javaClass) return false
-
-        other as Tab
-
-        return id == other.id &&
-            url == other.url &&
-            title == other.title &&
-            icon?.generationId == other.icon?.generationId &&
-            thumbnail?.generationId == other.thumbnail?.generationId &&
-            mediaState == other.mediaState
-    }
-}
+)

components/feature/tabs/src/main/java/mozilla/components/feature/tabs/tabstray/TabsTrayPresenter.kt

@@ -45,16 +45,6 @@ class TabsTrayPresenter(
 
     private suspend fun collect(flow: Flow<BrowserState>) {
         flow.map { it.toTabs(tabsFilter) }
-            .map { tabs ->
-                if (thumbnailsUseCases != null) {
-                    // Load the tab thumbnail from the memory or disk caches.
-                    tabs.copy(list = tabs.list.map { tab ->
-                        tab.copy(thumbnail = thumbnailsUseCases.loadThumbnail(tab.id))
-                    })
-                } else {
-                    tabs
-                }
-            }
             .ifChanged()
             .collect { tabs ->
                 // Do not invoke the callback on start if this is the initial state.

components/support/images/build.gradle

@@ -39,6 +39,7 @@ dependencies {
 
     implementation Dependencies.androidx_annotation
     implementation Dependencies.kotlin_stdlib
+    implementation Dependencies.kotlin_coroutines
 
     testImplementation project(':support-test')
 

components/browser/icons/src/main/java/mozilla/components/browser/icons/utils/CancelOnDetach.kt → components/support/images/src/main/java/mozilla/components/support/images/CancelOnDetach.kt

@@ -2,7 +2,7 @@
  * 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 mozilla.components.browser.icons.utils
+package mozilla.components.support.images
 
 import android.view.View
 import kotlinx.coroutines.Job
@@ -10,7 +10,7 @@ import kotlinx.coroutines.Job
 /**
  * Cancels the provided job when a view is detached from the window
  */
-internal class CancelOnDetach(private val job: Job) : View.OnAttachStateChangeListener {
+class CancelOnDetach(private val job: Job) : View.OnAttachStateChangeListener {
 
     override fun onViewAttachedToWindow(v: View?) = Unit
 

components/support/images/src/main/java/mozilla/components/support/images/loader/ImageLoader.kt

+/* 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 mozilla.components.support.images.loader
+
+import android.graphics.drawable.Drawable
+import android.widget.ImageView
+import androidx.annotation.MainThread
+
+/**
+ * A loader that can load an image from an ID directly into an [ImageView].
+ */
+interface ImageLoader {
+
+    /**
+     * Loads an image asynchronously and then displays it in the [ImageView].
+     * If the view is detached from the window before loading is completed, then loading is cancelled.
+     *
+     * @param view [ImageView] to load the image into.
+     * @param id Load image for this given ID.
+     * @param placeholder [Drawable] to display while image is loading.
+     * @param error [Drawable] to display if loading fails.
+     */
+    @MainThread
+    fun loadIntoView(
+        view: ImageView,
+        id: String,
+        placeholder: Drawable? = null,
+        error: Drawable? = null
+    )
+}

components/browser/icons/src/test/java/mozilla/components/browser/icons/utils/CancelOnDetachTest.kt → components/support/images/src/test/java/mozilla/components/support/images/CancelOnDetachTest.kt

@@ -2,7 +2,7 @@
  * 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 mozilla.components.browser.icons.utils
+package mozilla.components.support.images
 
 import android.view.View
 import kotlinx.coroutines.Job