Bug 1815579 - Improve performance of image loading in tab items

2024-11-19 09:25:34 +00:00 · 2023-06-26 10:42:15 -07:00 · 2023-06-26 10:42:15 -07:00 · d37e510b9a
commit d37e510b9a
parent 24c795ce5c
8 changed files with 302 additions and 84 deletions
--- a/app/src/main/java/org/mozilla/fenix/compose/TabThumbnail.kt
+++ b/app/src/main/java/org/mozilla/fenix/compose/TabThumbnail.kt
@ -0,0 +1,101 @@
+/* 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.compose
+
+import androidx.compose.foundation.Image
+import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.shape.RoundedCornerShape
+import androidx.compose.material.Card
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.draw.clip
+import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.graphics.asImageBitmap
+import androidx.compose.ui.layout.ContentScale
+import androidx.compose.ui.tooling.preview.Preview
+import androidx.compose.ui.unit.Dp
+import androidx.compose.ui.unit.dp
+import mozilla.components.browser.state.state.TabSessionState
+import mozilla.components.browser.state.state.createTab
+import org.mozilla.fenix.theme.FirefoxTheme
+
+private const val THUMBNAIL_SIZE = 108
+private const val FALLBACK_ICON_SIZE = 36
+
+/**
+ * Thumbnail belonging to a [tab]. If a thumbnail is not available, the favicon
+ * will be displayed until the thumbnail is loaded.
+ *
+ * @param tab The given [TabSessionState] to render a thumbnail for.
+ * @param size [Dp] size of the thumbnail.
+ * @param backgroundColor [Color] used for the background of the favicon.
+ * @param modifier [Modifier] used to draw the image content.
+ * @param contentDescription Text used by accessibility services
+ * to describe what this image represents.
+ * @param contentScale [ContentScale] used to draw image content.
+ * @param alignment [Alignment] used to draw the image content.
+ */
+@Composable
+@Suppress("LongParameterList")
+fun TabThumbnail(
+    tab: TabSessionState,
+    modifier: Modifier = Modifier,
+    size: Dp = THUMBNAIL_SIZE.dp,
+    backgroundColor: Color = FirefoxTheme.colors.layer2,
+    contentDescription: String? = null,
+    contentScale: ContentScale = ContentScale.FillWidth,
+    alignment: Alignment = Alignment.TopCenter,
+) {
+    Card(
+        modifier = modifier,
+        backgroundColor = backgroundColor,
+    ) {
+        ThumbnailImage(
+            key = tab.id,
+            size = size,
+            modifier = modifier,
+            contentScale = contentScale,
+            alignment = alignment,
+        ) {
+            Box(
+                modifier = Modifier.size(FALLBACK_ICON_SIZE.dp),
+                contentAlignment = Alignment.Center,
+            ) {
+                val icon = tab.content.icon
+                if (icon != null) {
+                    icon.prepareToDraw()
+                    Image(
+                        bitmap = icon.asImageBitmap(),
+                        contentDescription = contentDescription,
+                        modifier = Modifier
+                            .size(FALLBACK_ICON_SIZE.dp)
+                            .clip(RoundedCornerShape(8.dp)),
+                        contentScale = contentScale,
+                    )
+                } else {
+                    Favicon(
+                        url = tab.content.url,
+                        size = FALLBACK_ICON_SIZE.dp,
+                    )
+                }
+            }
+        }
+    }
+}
+
+@Preview
+@Composable
+private fun ThumbnailCardPreview() {
+    FirefoxTheme {
+        TabThumbnail(
+            tab = createTab(url = "www.mozilla.com", title = "Mozilla"),
+            modifier = Modifier
+                .size(THUMBNAIL_SIZE.dp, 80.dp)
+                .clip(RoundedCornerShape(8.dp)),
+        )
+    }
+}
--- a/app/src/main/java/org/mozilla/fenix/compose/ThumbnailCard.kt
+++ b/app/src/main/java/org/mozilla/fenix/compose/ThumbnailCard.kt
@ -11,28 +11,23 @@ import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.shape.RoundedCornerShape
 import androidx.compose.material.Card
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.graphics.Color
-import androidx.compose.ui.graphics.ImageBitmap
-import androidx.compose.ui.graphics.asImageBitmap
-import androidx.compose.ui.graphics.painter.BitmapPainter
 import androidx.compose.ui.layout.ContentScale
-import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
 import mozilla.components.browser.icons.compose.Loader
 import mozilla.components.browser.icons.compose.Placeholder
 import mozilla.components.browser.icons.compose.WithIcon
-import mozilla.components.concept.base.images.ImageLoadRequest
 import org.mozilla.fenix.components.components
 import org.mozilla.fenix.theme.FirefoxTheme

+private const val THUMBNAIL_SIZE = 108
+private const val FALLBACK_ICON_SIZE = 36
+
 /**
 * Card which will display a thumbnail. If a thumbnail is not available for [url], the favicon
 * will be displayed until the thumbnail is loaded.
@ -51,7 +46,7 @@ import org.mozilla.fenix.theme.FirefoxTheme
 fun ThumbnailCard(
    url: String,
    key: String,
-    size: Dp = 108.dp,
+    size: Dp = THUMBNAIL_SIZE.dp,
    backgroundColor: Color = FirefoxTheme.colors.layer2,
    modifier: Modifier = Modifier,
    contentDescription: String? = null,
@ -62,74 +57,36 @@ fun ThumbnailCard(
        modifier = modifier,
        backgroundColor = backgroundColor,
    ) {
-        if (inComposePreview) {
-            Box(
-                modifier = Modifier.background(color = FirefoxTheme.colors.layer3),
-            )
-        } else {
-            components.core.icons.Loader(url) {
-                Placeholder {
-                    Box(
-                        modifier = Modifier.background(color = FirefoxTheme.colors.layer3),
-                    )
-                }
-
-                WithIcon { icon ->
-                    Box(
-                        modifier = Modifier.size(36.dp),
-                        contentAlignment = Alignment.Center,
-                    ) {
-                        Image(
-                            painter = icon.painter,
-                            contentDescription = contentDescription,
-                            modifier = Modifier
-                                .size(36.dp)
-                                .clip(RoundedCornerShape(8.dp)),
-                            contentScale = contentScale,
-                        )
-                    }
-                }
-            }
-
        ThumbnailImage(
            key = key,
            size = size,
            modifier = modifier,
            contentScale = contentScale,
            alignment = alignment,
-            )
-        }
-    }
-}
-
-@Composable
-private fun ThumbnailImage(
-    key: String,
-    size: Dp,
-    modifier: Modifier,
-    contentScale: ContentScale,
-    alignment: Alignment,
-) {
-    val rememberBitmap = remember(key) { mutableStateOf<ImageBitmap?>(null) }
-    val thumbnailSize = LocalDensity.current.run { size.toPx().toInt() }
-    val request = ImageLoadRequest(key, thumbnailSize)
-    val storage = components.core.thumbnailStorage
-    val bitmap = rememberBitmap.value
-
-    LaunchedEffect(key) {
-        rememberBitmap.value = storage.loadThumbnail(request).await()?.asImageBitmap()
+        ) {
+            components.core.icons.Loader(url) {
+                Placeholder {
+                    Box(modifier = Modifier.background(color = FirefoxTheme.colors.layer3))
                }

-    if (bitmap != null) {
-        val painter = BitmapPainter(bitmap)
+                WithIcon { icon ->
+                    Box(
+                        modifier = Modifier.size(FALLBACK_ICON_SIZE.dp),
+                        contentAlignment = Alignment.Center,
+                    ) {
                        Image(
-            painter = painter,
-            contentDescription = null,
-            modifier = modifier,
+                            painter = icon.painter,
+                            contentDescription = contentDescription,
+                            modifier = Modifier
+                                .size(FALLBACK_ICON_SIZE.dp)
+                                .clip(RoundedCornerShape(8.dp)),
                            contentScale = contentScale,
-            alignment = alignment,
                        )
                    }
+                }
+            }
+        }
+    }
 }

@Preview
@ -140,7 +97,7 @@ private fun ThumbnailCardPreview() {
            url = "https://mozilla.com",
            key = "123",
            modifier = Modifier
-                .size(108.dp, 80.dp)
+                .size(THUMBNAIL_SIZE.dp)
                .clip(RoundedCornerShape(8.dp)),
        )
    }
--- a/app/src/main/java/org/mozilla/fenix/compose/ThumbnailImage.kt
+++ b/app/src/main/java/org/mozilla/fenix/compose/ThumbnailImage.kt
@ -0,0 +1,127 @@
+/* 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.compose
+
+import android.graphics.Bitmap
+import android.os.Parcelable
+import androidx.compose.foundation.Image
+import androidx.compose.foundation.background
+import androidx.compose.foundation.layout.Box
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.DisposableEffect
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.saveable.rememberSaveable
+import androidx.compose.runtime.setValue
+import androidx.compose.ui.Alignment
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.graphics.asImageBitmap
+import androidx.compose.ui.graphics.painter.BitmapPainter
+import androidx.compose.ui.layout.ContentScale
+import androidx.compose.ui.platform.LocalDensity
+import androidx.compose.ui.tooling.preview.Preview
+import androidx.compose.ui.unit.Dp
+import androidx.compose.ui.unit.dp
+import kotlinx.coroutines.launch
+import kotlinx.parcelize.Parcelize
+import mozilla.components.concept.base.images.ImageLoadRequest
+import org.mozilla.fenix.components.components
+import org.mozilla.fenix.theme.FirefoxTheme
+
+/**
+ * Thumbnail belonging to a [key]. Asynchronously fetches the bitmap from storage.
+ *
+ * @param key Key used to remember the thumbnail for future compositions.
+ * @param size [Dp] size of the thumbnail.
+ * @param modifier [Modifier] used to draw the image content.
+ * @param contentScale [ContentScale] used to draw image content.
+ * @param alignment [Alignment] used to draw the image content.
+ */
+@Composable
+@Suppress("LongParameterList")
+fun ThumbnailImage(
+    key: String,
+    size: Dp,
+    modifier: Modifier,
+    contentScale: ContentScale,
+    alignment: Alignment,
+    fallbackContent: @Composable () -> Unit,
+) {
+    if (inComposePreview) {
+        Box(modifier = Modifier.background(color = FirefoxTheme.colors.layer3))
+    } else {
+        val thumbnailSize = LocalDensity.current.run { size.toPx().toInt() }
+        val request = ImageLoadRequest(key, thumbnailSize)
+        val storage = components.core.thumbnailStorage
+        var state by rememberSaveable { mutableStateOf(ThumbnailImageState(null, false)) }
+        val scope = rememberCoroutineScope()
+
+        DisposableEffect(Unit) {
+            if (!state.hasLoaded) {
+                scope.launch {
+                    val thumbnailBitmap = storage.loadThumbnail(request).await()
+                    thumbnailBitmap?.prepareToDraw()
+                    state = ThumbnailImageState(
+                        bitmap = thumbnailBitmap,
+                        hasLoaded = true,
+                    )
+                }
+            }
+
+            onDispose {
+                // Recycle the bitmap to liberate the RAM. Without this, a list of [ThumbnailImage]
+                // will bloat the memory. This is a trade-off, however, as the bitmap
+                // will be re-fetched if this Composable is disposed and re-loaded.
+                state.bitmap?.recycle()
+                state = ThumbnailImageState(
+                    bitmap = null,
+                    hasLoaded = false,
+                )
+            }
+        }
+
+        if (state.bitmap == null && state.hasLoaded) {
+            fallbackContent()
+        } else {
+            state.bitmap?.let { bitmap ->
+                Image(
+                    painter = BitmapPainter(bitmap.asImageBitmap()),
+                    contentDescription = null,
+                    modifier = modifier,
+                    contentScale = contentScale,
+                    alignment = alignment,
+                )
+            }
+        }
+    }
+}
+
+/**
+ * State wrapper for [ThumbnailImage].
+ */
+@Parcelize
+private data class ThumbnailImageState(
+    val bitmap: Bitmap?,
+    val hasLoaded: Boolean,
+) : Parcelable
+
+/**
+ * This preview does not demo anything. This is to ensure that [ThumbnailImage] does not break other previews.
+*/
+@Preview
+@Composable
+private fun ThumbnailImagePreview() {
+    FirefoxTheme {
+        ThumbnailImage(
+            key = "",
+            size = 1.dp,
+            modifier = Modifier,
+            contentScale = ContentScale.Crop,
+            alignment = Alignment.Center,
+            fallbackContent = {},
+        )
+    }
+}
--- a/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt
+++ b/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt
@ -5,6 +5,7 @@
 package org.mozilla.fenix.compose.list

 import android.content.res.Configuration
+import androidx.compose.foundation.Image
 import androidx.compose.foundation.background
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Box
@ -86,6 +87,7 @@ fun TextListItem(
 *
 * @param label The label in the list item.
 * @param description An optional description text below the label.
+ * @param faviconPainter Optional painter to use when fetching a new favicon is unnecessary.
 * @param onClick Called when the user clicks on the item.
 * @param url Website [url] for which the favicon will be shown.
 * @param iconPainter [Painter] used to display an [IconButton] after the list item.
@ -96,6 +98,7 @@ fun TextListItem(
 fun FaviconListItem(
    label: String,
    description: String? = null,
+    faviconPainter: Painter? = null,
    onClick: (() -> Unit)? = null,
    url: String,
    iconPainter: Painter? = null,
@ -107,11 +110,21 @@ fun FaviconListItem(
        description = description,
        onClick = onClick,
        beforeListAction = {
+            if (faviconPainter != null) {
+                Image(
+                    painter = faviconPainter,
+                    contentDescription = null,
+                    modifier = Modifier
+                        .padding(horizontal = 16.dp)
+                        .size(ICON_SIZE),
+                )
+            } else {
                Favicon(
                    url = url,
                    size = ICON_SIZE,
                    modifier = Modifier.padding(horizontal = 16.dp),
                )
+            }
        },
        afterListAction = {
            if (iconPainter != null && onIconClick != null) {
@ -325,7 +338,7 @@ private fun IconListItemWithRightIconPreview() {
 )
 private fun FaviconListItemPreview() {
    FirefoxTheme {
-        Box(Modifier.background(FirefoxTheme.colors.layer1)) {
+        Column(Modifier.background(FirefoxTheme.colors.layer1)) {
            FaviconListItem(
                label = "Favicon + right icon + clicks",
                description = "Description text",
@ -334,6 +347,14 @@ private fun FaviconListItemPreview() {
                iconPainter = painterResource(R.drawable.ic_menu),
                onIconClick = { println("icon click") },
            )
+
+            FaviconListItem(
+                label = "Favicon + painter",
+                description = "Description text",
+                faviconPainter = painterResource(id = R.drawable.ic_tab_collection),
+                onClick = { println("list item click") },
+                url = "",
+            )
        }
    }
 }
--- a/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabGridItem.kt
+++ b/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabGridItem.kt
@ -6,6 +6,7 @@ package org.mozilla.fenix.compose.tabstray

 import androidx.compose.foundation.BorderStroke
 import androidx.compose.foundation.ExperimentalFoundationApi
+import androidx.compose.foundation.Image
 import androidx.compose.foundation.background
 import androidx.compose.foundation.border
 import androidx.compose.foundation.clickable
@ -13,12 +14,14 @@ import androidx.compose.foundation.combinedClickable
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.requiredHeight
 import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.layout.wrapContentHeight
 import androidx.compose.foundation.layout.wrapContentSize
 import androidx.compose.foundation.shape.CircleShape
@ -33,6 +36,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clipToBounds
+import androidx.compose.ui.graphics.asImageBitmap
 import androidx.compose.ui.platform.LocalConfiguration
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.res.colorResource
@ -52,10 +56,9 @@ import mozilla.components.browser.state.state.createTab
 import mozilla.components.support.ktx.kotlin.MAX_URI_LENGTH
 import org.mozilla.fenix.R
 import org.mozilla.fenix.compose.Divider
-import org.mozilla.fenix.compose.Favicon
 import org.mozilla.fenix.compose.HorizontalFadingEdgeBox
 import org.mozilla.fenix.compose.SwipeToDismiss
-import org.mozilla.fenix.compose.ThumbnailCard
+import org.mozilla.fenix.compose.TabThumbnail
 import org.mozilla.fenix.compose.annotation.LightDarkPreview
 import org.mozilla.fenix.tabstray.TabsTrayTestTag
 import org.mozilla.fenix.tabstray.ext.toDisplayTitle
@ -146,13 +149,18 @@ fun TabGridItem(
                            .fillMaxWidth()
                            .wrapContentHeight(),
                    ) {
-                        Favicon(
-                            url = tab.content.url,
-                            size = 16.dp,
+                        Spacer(modifier = Modifier.width(8.dp))
+
+                        tab.content.icon?.let { icon ->
+                            icon.prepareToDraw()
+                            Image(
+                                bitmap = icon.asImageBitmap(),
+                                contentDescription = null,
                                modifier = Modifier
                                    .align(Alignment.CenterVertically)
-                                .padding(start = 8.dp),
+                                    .size(16.dp),
                            )
+                        }

                        HorizontalFadingEdgeBox(
                            modifier = Modifier
@ -230,9 +238,8 @@ private fun Thumbnail(
                testTag = TabsTrayTestTag.tabItemThumbnail
            },
    ) {
-        ThumbnailCard(
-            url = tab.content.url,
-            key = tab.id,
+        TabThumbnail(
+            tab = tab,
            size = LocalConfiguration.current.screenWidthDp.dp,
            modifier = Modifier.fillMaxSize(),
        )
--- a/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabListItem.kt
+++ b/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabListItem.kt
@ -41,7 +41,7 @@ import mozilla.components.browser.state.state.createTab
 import mozilla.components.support.ktx.kotlin.MAX_URI_LENGTH
 import org.mozilla.fenix.R
 import org.mozilla.fenix.compose.SwipeToDismiss
-import org.mozilla.fenix.compose.ThumbnailCard
+import org.mozilla.fenix.compose.TabThumbnail
 import org.mozilla.fenix.compose.annotation.LightDarkPreview
 import org.mozilla.fenix.ext.toShortUrl
 import org.mozilla.fenix.tabstray.TabsTrayTestTag
@ -174,9 +174,8 @@ private fun Thumbnail(
    onMediaIconClicked: ((TabSessionState) -> Unit),
 ) {
    Box {
-        ThumbnailCard(
-            url = tab.content.url,
-            key = tab.id,
+        TabThumbnail(
+            tab = tab,
            modifier = Modifier
                .size(width = 92.dp, height = 72.dp)
                .semantics(mergeDescendants = true) {
--- a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt
+++ b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt
@ -56,7 +56,7 @@ import org.mozilla.fenix.components.components
 import org.mozilla.fenix.compose.ContextualMenu
 import org.mozilla.fenix.compose.Image
 import org.mozilla.fenix.compose.MenuItem
-import org.mozilla.fenix.compose.ThumbnailCard
+import org.mozilla.fenix.compose.TabThumbnail
 import org.mozilla.fenix.compose.annotation.LightDarkPreview
 import org.mozilla.fenix.compose.inComposePreview
 import org.mozilla.fenix.home.recenttabs.RecentTab
@ -228,9 +228,8 @@ fun RecentTabImage(
                contentScale = ContentScale.Crop,
            )
        }
-        else -> ThumbnailCard(
-            url = tab.state.content.url,
-            key = tab.state.id,
+        else -> TabThumbnail(
+            tab = tab.state,
            modifier = modifier,
            contentScale = contentScale,
        )
--- a/app/src/main/java/org/mozilla/fenix/tabstray/inactivetabs/InactiveTabs.kt
+++ b/app/src/main/java/org/mozilla/fenix/tabstray/inactivetabs/InactiveTabs.kt
@ -29,6 +29,8 @@ import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.graphics.asImageBitmap
+import androidx.compose.ui.graphics.painter.BitmapPainter
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.Preview
@ -101,10 +103,15 @@ fun InactiveTabsList(
                Column {
                    inactiveTabs.forEach { tab ->
                        val tabUrl = tab.content.url.toShortUrl()
+                        val faviconPainter = tab.content.icon?.run {
+                            prepareToDraw()
+                            BitmapPainter(asImageBitmap())
+                        }

                        FaviconListItem(
                            label = tab.toDisplayTitle(),
                            description = tabUrl,
+                            faviconPainter = faviconPainter,
                            onClick = { onTabClick(tab) },
                            url = tabUrl,
                            iconPainter = painterResource(R.drawable.mozac_ic_close),