From 1d9b80bad8984a6b2c4d6277e162de07ded7bd41 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Fri, 17 Feb 2023 15:25:15 -0800 Subject: Add image caching to ImagePreviewImageLoader ScrollableImagePreviewView being a RecyclerView may reattache its children multiple times and rely on the ImageLoader to implement any image retrival optimizations. Fix: 269797062 Test: manual test, unit tests Change-Id: I256f4a78a677e939f717fee5dd82492ec572bc65 --- .../chooser_image_preview_view_internals.xml | 12 +-- java/res/layout/image_preview_image_item.xml | 4 +- java/res/values/dimens.xml | 2 + .../android/intentresolver/ChooserActivity.java | 11 ++- .../intentresolver/ChooserContentPreviewUi.java | 2 + java/src/com/android/intentresolver/ImageLoader.kt | 1 + .../intentresolver/ImagePreviewImageLoader.kt | 46 ++++++++-- .../intentresolver/ImagePreviewImageLoaderTest.kt | 101 +++++++++++++++++++++ .../android/intentresolver/MockitoKotlinHelpers.kt | 3 + .../intentresolver/TestPreviewImageLoader.kt | 1 + 10 files changed, 166 insertions(+), 17 deletions(-) create mode 100644 java/tests/src/com/android/intentresolver/ImagePreviewImageLoaderTest.kt (limited to 'java') diff --git a/java/res/layout/chooser_image_preview_view_internals.xml b/java/res/layout/chooser_image_preview_view_internals.xml index 8730fc30..2b93edf8 100644 --- a/java/res/layout/chooser_image_preview_view_internals.xml +++ b/java/res/layout/chooser_image_preview_view_internals.xml @@ -26,8 +26,8 @@ diff --git a/java/res/values/dimens.xml b/java/res/values/dimens.xml index 93cb4637..87eec7fb 100644 --- a/java/res/values/dimens.xml +++ b/java/res/values/dimens.xml @@ -25,6 +25,8 @@ 24dp 20sp 1dp + 120dp + 104dp 200dp -1px 4dp diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index 32b10f23..910eb885 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -85,6 +85,7 @@ import com.android.intentresolver.chooser.MultiDisplayResolveInfo; import com.android.intentresolver.chooser.TargetInfo; import com.android.intentresolver.flags.FeatureFlagRepository; import com.android.intentresolver.flags.FeatureFlagRepositoryFactory; +import com.android.intentresolver.flags.Flags; import com.android.intentresolver.grid.ChooserGridAdapter; import com.android.intentresolver.grid.DirectShareViewHolder; import com.android.intentresolver.model.AbstractResolverComparator; @@ -1338,7 +1339,15 @@ public class ChooserActivity extends ResolverActivity implements @VisibleForTesting protected ImageLoader createPreviewImageLoader() { - return new ImagePreviewImageLoader(this, getLifecycle()); + final int cacheSize; + if (mFeatureFlagRepository.isEnabled(Flags.SHARESHEET_SCROLLABLE_IMAGE_PREVIEW)) { + float chooserWidth = getResources().getDimension(R.dimen.chooser_width); + float imageWidth = getResources().getDimension(R.dimen.chooser_preview_image_width); + cacheSize = (int) (Math.ceil(chooserWidth / imageWidth) + 2); + } else { + cacheSize = 3; + } + return new ImagePreviewImageLoader(this, getLifecycle(), cacheSize); } private void handleScroll(View view, int x, int y, int oldx, int oldy) { diff --git a/java/src/com/android/intentresolver/ChooserContentPreviewUi.java b/java/src/com/android/intentresolver/ChooserContentPreviewUi.java index aa147853..60ea0122 100644 --- a/java/src/com/android/intentresolver/ChooserContentPreviewUi.java +++ b/java/src/com/android/intentresolver/ChooserContentPreviewUi.java @@ -17,6 +17,7 @@ package com.android.intentresolver; import static android.content.ContentProvider.getUserIdFromUri; + import static java.lang.annotation.RetentionPolicy.SOURCE; import android.animation.ObjectAnimator; @@ -413,6 +414,7 @@ public final class ChooserContentPreviewUi { actionFactory); imagePreview.setTransitionElementStatusCallback(transitionElementStatusCallback); imagePreview.setImages(imageUris, imageLoader); + imageLoader.prePopulate(imageUris); return contentPreviewLayout; } diff --git a/java/src/com/android/intentresolver/ImageLoader.kt b/java/src/com/android/intentresolver/ImageLoader.kt index 13b1dd9c..0ed8b122 100644 --- a/java/src/com/android/intentresolver/ImageLoader.kt +++ b/java/src/com/android/intentresolver/ImageLoader.kt @@ -22,4 +22,5 @@ import java.util.function.Consumer interface ImageLoader : suspend (Uri) -> Bitmap? { fun loadImage(uri: Uri, callback: Consumer) + fun prePopulate(uris: List) } diff --git a/java/src/com/android/intentresolver/ImagePreviewImageLoader.kt b/java/src/com/android/intentresolver/ImagePreviewImageLoader.kt index 40081c87..7b6651a2 100644 --- a/java/src/com/android/intentresolver/ImagePreviewImageLoader.kt +++ b/java/src/com/android/intentresolver/ImagePreviewImageLoader.kt @@ -20,21 +20,34 @@ import android.content.Context import android.graphics.Bitmap import android.net.Uri import android.util.Size +import androidx.annotation.GuardedBy +import androidx.annotation.VisibleForTesting +import androidx.collection.LruCache import androidx.lifecycle.Lifecycle import androidx.lifecycle.coroutineScope +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.isActive import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import java.util.function.Consumer -internal class ImagePreviewImageLoader @JvmOverloads constructor( +@VisibleForTesting +class ImagePreviewImageLoader @JvmOverloads constructor( private val context: Context, private val lifecycle: Lifecycle, + cacheSize: Int, private val dispatcher: CoroutineDispatcher = Dispatchers.IO ) : ImageLoader { + private val thumbnailSize: Size = + context.resources.getDimensionPixelSize(R.dimen.chooser_preview_image_max_dimen).let { + Size(it, it) + } + + @GuardedBy("self") + private val cache = LruCache>(cacheSize) + override suspend fun invoke(uri: Uri): Bitmap? = loadImageAsync(uri) override fun loadImage(uri: Uri, callback: Consumer) { @@ -46,12 +59,29 @@ internal class ImagePreviewImageLoader @JvmOverloads constructor( } } - private suspend fun loadImageAsync(uri: Uri): Bitmap? { - val size = context.resources.getDimensionPixelSize(R.dimen.chooser_preview_image_max_dimen) - return withContext(dispatcher) { - runCatching { - context.contentResolver.loadThumbnail(uri, Size(size, size), null) - }.getOrNull() + override fun prePopulate(uris: List) { + uris.asSequence().take(cache.maxSize()).forEach { uri -> + lifecycle.coroutineScope.launch { + loadImageAsync(uri) + } } } + + private suspend fun loadImageAsync(uri: Uri): Bitmap? { + return synchronized(cache) { + cache.get(uri) ?: CompletableDeferred().also { result -> + cache.put(uri, result) + lifecycle.coroutineScope.launch(dispatcher) { + result.loadBitmap(uri) + } + } + }.await() + } + + private fun CompletableDeferred.loadBitmap(uri: Uri) { + val bitmap = runCatching { + context.contentResolver.loadThumbnail(uri, thumbnailSize, null) + }.getOrNull() + complete(bitmap) + } } diff --git a/java/tests/src/com/android/intentresolver/ImagePreviewImageLoaderTest.kt b/java/tests/src/com/android/intentresolver/ImagePreviewImageLoaderTest.kt new file mode 100644 index 00000000..f327e19e --- /dev/null +++ b/java/tests/src/com/android/intentresolver/ImagePreviewImageLoaderTest.kt @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver + +import android.content.ContentResolver +import android.content.Context +import android.content.res.Resources +import android.net.Uri +import android.util.Size +import androidx.lifecycle.Lifecycle +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito.never +import org.mockito.Mockito.times +import org.mockito.Mockito.verify + +@OptIn(ExperimentalCoroutinesApi::class) +class ImagePreviewImageLoaderTest { + private val imageSize = Size(300, 300) + private val uriOne = Uri.parse("content://org.package.app/image-1.png") + private val uriTwo = Uri.parse("content://org.package.app/image-2.png") + private val contentResolver = mock() + private val resources = mock { + whenever(getDimensionPixelSize(R.dimen.chooser_preview_image_max_dimen)) + .thenReturn(imageSize.width) + } + private val context = mock { + whenever(this.resources).thenReturn(this@ImagePreviewImageLoaderTest.resources) + whenever(this.contentResolver).thenReturn(this@ImagePreviewImageLoaderTest.contentResolver) + } + private val scheduler = TestCoroutineScheduler() + private val lifecycleOwner = TestLifecycleOwner() + private val dispatcher = UnconfinedTestDispatcher(scheduler) + private val testSubject = ImagePreviewImageLoader( + context, lifecycleOwner.lifecycle, 1, dispatcher + ) + + @Before + fun setup() { + Dispatchers.setMain(dispatcher) + lifecycleOwner.state = Lifecycle.State.CREATED + } + + @After + fun cleanup() { + lifecycleOwner.state = Lifecycle.State.DESTROYED + Dispatchers.resetMain() + } + + @Test + fun test_prePopulate() = runTest { + testSubject.prePopulate(listOf(uriOne, uriTwo)) + + verify(contentResolver, times(1)).loadThumbnail(uriOne, imageSize, null) + verify(contentResolver, never()).loadThumbnail(uriTwo, imageSize, null) + + testSubject(uriOne) + verify(contentResolver, times(1)).loadThumbnail(uriOne, imageSize, null) + } + + @Test + fun test_invoke_return_cached_image() = runTest { + testSubject(uriOne) + testSubject(uriOne) + + verify(contentResolver, times(1)).loadThumbnail(any(), any(), anyOrNull()) + } + + @Test + fun test_invoke_old_records_evicted_from_the_cache() = runTest { + testSubject(uriOne) + testSubject(uriTwo) + testSubject(uriTwo) + testSubject(uriOne) + + verify(contentResolver, times(2)).loadThumbnail(uriOne, imageSize, null) + verify(contentResolver, times(1)).loadThumbnail(uriTwo, imageSize, null) + } +} diff --git a/java/tests/src/com/android/intentresolver/MockitoKotlinHelpers.kt b/java/tests/src/com/android/intentresolver/MockitoKotlinHelpers.kt index 159c6d6a..aaa7a282 100644 --- a/java/tests/src/com/android/intentresolver/MockitoKotlinHelpers.kt +++ b/java/tests/src/com/android/intentresolver/MockitoKotlinHelpers.kt @@ -26,6 +26,7 @@ package com.android.intentresolver import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatcher +import org.mockito.ArgumentMatchers import org.mockito.Mockito import org.mockito.stubbing.OngoingStubbing @@ -144,3 +145,5 @@ inline fun withArgCaptor(block: KotlinArgumentCaptor.() -> */ inline fun captureMany(block: KotlinArgumentCaptor.() -> Unit): List = kotlinArgumentCaptor().apply{ block() }.allValues + +inline fun anyOrNull() = ArgumentMatchers.argThat(ArgumentMatcher { true }) diff --git a/java/tests/src/com/android/intentresolver/TestPreviewImageLoader.kt b/java/tests/src/com/android/intentresolver/TestPreviewImageLoader.kt index fd617fdd..cfe041dd 100644 --- a/java/tests/src/com/android/intentresolver/TestPreviewImageLoader.kt +++ b/java/tests/src/com/android/intentresolver/TestPreviewImageLoader.kt @@ -34,4 +34,5 @@ internal class TestPreviewImageLoader( } override suspend fun invoke(uri: Uri): Bitmap? = imageOverride() ?: imageLoader(uri) + override fun prePopulate(uris: List) = Unit } -- cgit v1.2.3-59-g8ed1b