Merge pull request #4957 from sjudd:compose_fix_preloading

PiperOrigin-RevId: 488794440

instrumentation/src/androidTest/java/com/bumptech/glide/DataUriTest.java

@@ -92,6 +92,6 @@ private String getBase64BitmapBytes(CompressFormat format) {
     Bitmap bitmap = ((BitmapDrawable) drawable).getBitmap();
     bitmap.compress(format, 100, bos);
     byte[] data = bos.toByteArray();
-    return Base64.encodeToString(data, /*flags=*/ 0);
+    return Base64.encodeToString(data, /* flags= */ 0);
   }
 }

instrumentation/src/androidTest/java/com/bumptech/glide/ErrorHandlingTest.java

@@ -62,7 +62,8 @@ public void load_whenEncoderFails_callsUncaughtThrowableStrategy() {
     Glide.init(
         context,
         new GlideBuilder()
-            .setAnimationExecutor(GlideExecutor.newAnimationExecutor(/*threadCount=*/ 1, strategy))
+            .setAnimationExecutor(
+                GlideExecutor.newAnimationExecutor(/* threadCount= */ 1, strategy))
             .setSourceExecutor(GlideExecutor.newSourceExecutor(strategy))
             .setDiskCacheExecutor(GlideExecutor.newDiskCacheExecutor(strategy)));
     Glide.get(context).getRegistry().prepend(Bitmap.class, new FailEncoder());
@@ -89,7 +90,8 @@ public void load_whenLoadSucceeds_butEncoderFails_doesNotCallOnLoadFailed() {
     Glide.init(
         context,
         new GlideBuilder()
-            .setAnimationExecutor(GlideExecutor.newAnimationExecutor(/*threadCount=*/ 1, strategy))
+            .setAnimationExecutor(
+                GlideExecutor.newAnimationExecutor(/* threadCount= */ 1, strategy))
             .setSourceExecutor(GlideExecutor.newSourceExecutor(strategy))
             .setDiskCacheExecutor(GlideExecutor.newDiskCacheExecutor(strategy)));
     Glide.get(context).getRegistry().prepend(Bitmap.class, new FailEncoder());

instrumentation/src/androidTest/java/com/bumptech/glide/LoadBytesTest.java

@@ -61,7 +61,7 @@ public void setUp() throws IOException {
 
     imageView = new ImageView(context);
     int[] dimensions = getCanonicalDimensions();
-    imageView.setLayoutParams(new LayoutParams(/*w=*/ dimensions[0], /*h=*/ dimensions[1]));
+    imageView.setLayoutParams(new LayoutParams(/* w= */ dimensions[0], /* h= */ dimensions[1]));
 
     // Writes to the resource disk cache run in a non-blocking manner after the Target is notified.
     // Unless we enforce a single threaded executor, the encode task races with our second decode
@@ -95,11 +95,11 @@ public void loadFromRequestManager_intoImageView_withDifferentByteArrays_loadsDi
     assertThat(firstBitmap).isNotSameInstanceAs(secondBitmap);
 
     Bitmap expectedCanonicalBitmap =
-        BitmapFactory.decodeByteArray(canonicalBytes, /*offset=*/ 0, canonicalBytes.length);
+        BitmapFactory.decodeByteArray(canonicalBytes, /* offset= */ 0, canonicalBytes.length);
     assertThat(firstBitmap).sameAs(expectedCanonicalBitmap);
 
     Bitmap expectedModifiedBitmap =
-        BitmapFactory.decodeByteArray(modifiedBytes, /*offset=*/ 0, modifiedBytes.length);
+        BitmapFactory.decodeByteArray(modifiedBytes, /* offset= */ 0, modifiedBytes.length);
     assertThat(secondBitmap).sameAs(expectedModifiedBitmap);
   }
 
@@ -122,11 +122,11 @@ public void loadFromRequestBuilder_intoImageView_withDifferentByteArrays_loadsDi
     assertThat(firstBitmap).isNotSameInstanceAs(secondBitmap);
 
     Bitmap expectedCanonicalBitmap =
-        BitmapFactory.decodeByteArray(canonicalBytes, /*offset=*/ 0, canonicalBytes.length);
+        BitmapFactory.decodeByteArray(canonicalBytes, /* offset= */ 0, canonicalBytes.length);
     assertThat(firstBitmap).sameAs(expectedCanonicalBitmap);
 
     Bitmap expectedModifiedBitmap =
-        BitmapFactory.decodeByteArray(modifiedBytes, /*offset=*/ 0, modifiedBytes.length);
+        BitmapFactory.decodeByteArray(modifiedBytes, /* offset= */ 0, modifiedBytes.length);
     assertThat(secondBitmap).sameAs(expectedModifiedBitmap);
   }
 
@@ -506,15 +506,15 @@ private Bitmap copyFromImageViewDrawable(ImageView imageView) {
   private int[] getCanonicalDimensions() throws IOException {
     byte[] canonicalBytes = getCanonicalBytes();
     Bitmap bitmap =
-        BitmapFactory.decodeByteArray(canonicalBytes, /*offset=*/ 0, canonicalBytes.length);
+        BitmapFactory.decodeByteArray(canonicalBytes, /* offset= */ 0, canonicalBytes.length);
     return new int[] {bitmap.getWidth(), bitmap.getHeight()};
   }
 
   private byte[] getModifiedBytes() throws IOException {
     int[] dimensions = getCanonicalDimensions();
     Bitmap bitmap = Bitmap.createBitmap(dimensions[0], dimensions[1], Config.ARGB_8888);
     ByteArrayOutputStream os = new ByteArrayOutputStream();
-    bitmap.compress(CompressFormat.PNG, /*quality=*/ 100, os);
+    bitmap.compress(CompressFormat.PNG, /* quality= */ 100, os);
     return os.toByteArray();
   }
 

instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java

@@ -395,7 +395,7 @@ public void load_withApplicationIconResourceNameUri_asDrawable_producesNonNullDr
     for (String packageName : getInstalledPackages()) {
       int iconResourceId = getResourceId(packageName);
 
-      Context toUse = context.createPackageContext(packageName, /*flags=*/ 0);
+      Context toUse = context.createPackageContext(packageName, /* flags= */ 0);
       Resources resources = toUse.getResources();
       Uri uri =
           new Uri.Builder()
@@ -416,7 +416,7 @@ public void load_withApplicationIconResourceNameUri_asDrawable_withTransform_non
     for (String packageName : getInstalledPackages()) {
       int iconResourceId = getResourceId(packageName);
 
-      Context toUse = context.createPackageContext(packageName, /*flags=*/ 0);
+      Context toUse = context.createPackageContext(packageName, /* flags= */ 0);
       Resources resources = toUse.getResources();
       Uri uri =
           new Uri.Builder()
@@ -437,7 +437,7 @@ public void load_withApplicationIconResourceNameUri_asBitmap_producesNonNullBitm
     for (String packageName : getInstalledPackages()) {
       int iconResourceId = getResourceId(packageName);
 
-      Context toUse = context.createPackageContext(packageName, /*flags=*/ 0);
+      Context toUse = context.createPackageContext(packageName, /* flags= */ 0);
       Resources resources = toUse.getResources();
       Uri uri =
           new Uri.Builder()
@@ -458,7 +458,7 @@ public void load_withApplicationIconResourceNameUri_asBitmap_withTransform_nonNu
     for (String packageName : getInstalledPackages()) {
       int iconResourceId = getResourceId(packageName);
 
-      Context toUse = context.createPackageContext(packageName, /*flags=*/ 0);
+      Context toUse = context.createPackageContext(packageName, /* flags= */ 0);
       Resources resources = toUse.getResources();
       Uri uri =
           new Uri.Builder()
@@ -478,7 +478,8 @@ private Set<String> getInstalledPackages() {
     Intent mainIntent = new Intent(Intent.ACTION_MAIN, null);
     mainIntent.addCategory(Intent.CATEGORY_LAUNCHER);
     PackageManager packageManager = context.getPackageManager();
-    List<ResolveInfo> pkgAppsList = packageManager.queryIntentActivities(mainIntent, /*flags=*/ 0);
+    List<ResolveInfo> pkgAppsList =
+        packageManager.queryIntentActivities(mainIntent, /* flags= */ 0);
     Set<String> result = new HashSet<>();
     for (ResolveInfo info : pkgAppsList) {
       String packageName = info.activityInfo.packageName;
@@ -494,7 +495,7 @@ && doesApplicationPackageNameMatchResourcePackageName(packageName, iconResourceI
   private int getResourceId(String packageName) {
     PackageInfo packageInfo;
     try {
-      packageInfo = context.getPackageManager().getPackageInfo(packageName, /*flags=*/ 0);
+      packageInfo = context.getPackageManager().getPackageInfo(packageName, /* flags= */ 0);
     } catch (NameNotFoundException e) {
       return 0;
     }
@@ -536,7 +537,7 @@ private int getResourceId(String packageName) {
   private boolean doesApplicationPackageNameMatchResourcePackageName(
       String applicationPackageName, int iconResourceId) {
     try {
-      Context current = context.createPackageContext(applicationPackageName, /*flags=*/ 0);
+      Context current = context.createPackageContext(applicationPackageName, /* flags= */ 0);
       String resourcePackageName = current.getResources().getResourcePackageName(iconResourceId);
       return applicationPackageName.equals(resourcePackageName);
     } catch (NameNotFoundException e) {

instrumentation/src/androidTest/java/com/bumptech/glide/WideGamutTest.java

@@ -82,7 +82,7 @@ public void load_withWideGamutImage_hardwareAllowed_returnsDecodedBitmap() {
   public void load_withEncodedPngWideGamutImage_decodesWideGamut() {
     Bitmap toCompress =
         Bitmap.createBitmap(
-            100, 100, Bitmap.Config.RGBA_F16, /*hasAlpha=*/ true, ColorSpace.get(Named.DCI_P3));
+            100, 100, Bitmap.Config.RGBA_F16, /* hasAlpha= */ true, ColorSpace.get(Named.DCI_P3));
 
     byte[] data = asPng(toCompress);
 
@@ -97,7 +97,7 @@ public void load_withEncodedJpegWideGamutImage_decodesArgb8888() {
     assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O_MR1);
     Bitmap toCompress =
         Bitmap.createBitmap(
-            100, 100, Bitmap.Config.RGBA_F16, /*hasAlpha=*/ true, ColorSpace.get(Named.DCI_P3));
+            100, 100, Bitmap.Config.RGBA_F16, /* hasAlpha= */ true, ColorSpace.get(Named.DCI_P3));
 
     byte[] data = asJpeg(toCompress);
 
@@ -109,7 +109,7 @@ public void load_withEncodedJpegWideGamutImage_decodesArgb8888() {
   public void load_withEncodedWebpWideGamutImage_decodesArgb8888() {
     Bitmap toCompress =
         Bitmap.createBitmap(
-            100, 100, Bitmap.Config.RGBA_F16, /*hasAlpha=*/ true, ColorSpace.get(Named.DCI_P3));
+            100, 100, Bitmap.Config.RGBA_F16, /* hasAlpha= */ true, ColorSpace.get(Named.DCI_P3));
 
     byte[] data = asWebp(toCompress);
 
@@ -152,7 +152,7 @@ public void roundedCorners_withWideGamutBitmap_producesWideGamutBitmap() {
             GlideApp.with(context)
                 .asBitmap()
                 .load(data)
-                .transform(new RoundedCorners(/*roundingRadius=*/ 10))
+                .transform(new RoundedCorners(/* roundingRadius= */ 10))
                 .submit());
     assertThat(result).isNotNull();
     assertThat(result.getConfig()).isEqualTo(Config.RGBA_F16);

instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java

@@ -526,7 +526,7 @@ private static InputStream openFileStream(int width, int height, int exifOrienta
               ".jpeg",
               ApplicationProvider.getApplicationContext().getCacheDir());
       os = new BufferedOutputStream(new FileOutputStream(tempFile));
-      bitmap.compress(CompressFormat.JPEG, /*quality=*/ 100, os);
+      bitmap.compress(CompressFormat.JPEG, /* quality= */ 100, os);
       bitmap.recycle();
       os.close();
 

instrumentation/src/androidTest/java/com/bumptech/glide/test/BitmapRegressionTester.java

@@ -89,7 +89,7 @@ private SplitBySdk getSplitBySdkValues() {
     SplitBySdk result;
     try {
       Method method =
-          testClass.getMethod(testName.getMethodName(), /* parameterTypes...= */ (Class[]) null);
+          testClass.getMethod(testName.getMethodName(), /* parameterTypes= */ (Class[]) null);
       result = method.getAnnotation(SplitBySdk.class);
     } catch (NoSuchMethodException e) {
       throw new RuntimeException(e);
@@ -151,7 +151,7 @@ private void writeBitmap(Bitmap bitmap) {
     OutputStream os = null;
     try {
       os = new BufferedOutputStream(new FileOutputStream(file));
-      bitmap.compress(CompressFormat.PNG, /*quality=*/ 100, os);
+      bitmap.compress(CompressFormat.PNG, /* quality= */ 100, os);
       os.close();
     } catch (IOException e) {
       throw new RuntimeException(e);

instrumentation/src/androidTest/java/com/bumptech/glide/test/CanonicalBitmap.java

@@ -46,7 +46,7 @@ private Bitmap decodeBitmap() {
               result,
               (int) (result.getWidth() * scaleFactor),
               (int) (result.getHeight() * scaleFactor),
-              /*filter=*/ false);
+              /* filter= */ false);
     }
     // Make sure the Bitmap is immutable.
     return result;

integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlidePreloaderTest.kt

@@ -4,17 +4,26 @@ package com.bumptech.glide.integration.compose
 
 import android.content.Context
 import android.graphics.drawable.Drawable
+import androidx.annotation.DrawableRes
+import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.lazy.LazyListState
 import androidx.compose.foundation.lazy.LazyRow
 import androidx.compose.foundation.lazy.itemsIndexed
 import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.material.Text
+import androidx.compose.material.TextButton
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.geometry.Size
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.test.hasTestTag
+import androidx.compose.ui.test.onNodeWithText
+import androidx.compose.ui.test.performClick
 import androidx.compose.ui.test.performScrollToIndex
+import androidx.compose.ui.unit.dp
 import androidx.test.core.app.ApplicationProvider
 import com.bumptech.glide.Glide
 import com.bumptech.glide.integration.compose.test.GlideComposeRule
@@ -44,9 +53,14 @@ class GlidePreloaderTest {
       PreloadOneItemGlideLazyListPreloader(state)
     }
 
-    val nextPreloadModel: Drawable =
-      Glide.with(context).load(preloadModels[2]).onlyRetrieveFromCache(true).submit().get()
+    assertThatModelIsInMemoryCache(preloadModels[2])
+  }
 
+  fun assertThatModelIsInMemoryCache(@DrawableRes model: Int){
+    // Wait for previous aysnc image loads to finish
+    glideComposeRule.waitForIdle()
+    val nextPreloadModel: Drawable =
+      Glide.with(context).load(model).onlyRetrieveFromCache(true).submit().get()
     assertThat(nextPreloadModel).isNotNull()
   }
 
@@ -70,10 +84,7 @@ class GlidePreloaderTest {
     val scrollToIndex = 1
     glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex)
 
-    val nextPreloadModel: Drawable =
-      Glide.with(context).load(preloadModels[2]).onlyRetrieveFromCache(true).submit().get()
-
-    assertThat(nextPreloadModel).isNotNull()
+    assertThatModelIsInMemoryCache(preloadModels[2])
   }
 
   @Test
@@ -97,18 +108,58 @@ class GlidePreloaderTest {
     val scrollToIndex = 1
     glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex)
 
-    val nextPreloadModel: Drawable =
-      Glide.with(context).load(preloadModels[2]).onlyRetrieveFromCache(true).submit().get()
+    assertThatModelIsInMemoryCache(preloadModels[2])
+  }
 
-    assertThat(nextPreloadModel).isNotNull()
+  @Test
+  fun glideLazyListPreloader_whenDataChanges_onScroll_preloadsUpdatedData() {
+    glideComposeRule.setContent {
+      // Swap both to avoid confusing the preloader. The preloader doesn't notice or take into
+      // account data set changes (this is a bug in the Java preloading API)...
+      val currentPreloadModels = remember { mutableStateOf(listOf<Int>()) }
+      val currentModels = remember { mutableStateOf(listOf<Int>()) }
+      // Use a button to swap data because we can't mutate state in setContent easily from outside
+      // the method, nor can you call setContent multiple times.
+      fun swapData() {
+        currentPreloadModels.value = preloadModels
+        currentModels.value = models
+      }
+      val state = rememberLazyListState()
+      Column {
+        TextButton(onClick = ::swapData) { Text(text = "Swap") }
+        LazyRow(state = state,
+                modifier = Modifier.testTag(listTestTag),
+                horizontalArrangement = Arrangement.spacedBy(10.dp)) {
+          item { Text(text = "Header") }
+          itemsIndexed(currentModels.value) { index, drawableResourceId ->
+            GlideImage(
+              model = drawableResourceId,
+              contentDescription = imageContentDescription(index),
+              Modifier.fillParentMaxWidth(),
+            )
+          }
+        }
+      }
+
+      PreloadOneItemGlideLazyListPreloader(state, data = currentPreloadModels.value)
+    }
+
+    glideComposeRule.onNodeWithText("Swap").performClick()
+    val scrollToIndex = 1
+    glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex)
+
+    assertThatModelIsInMemoryCache(preloadModels[2])
   }
 
   @Suppress("TestFunctionName") // Not a Test...
   @Composable
-  private fun PreloadOneItemGlideLazyListPreloader(state: LazyListState) =
+  private fun PreloadOneItemGlideLazyListPreloader(
+    state: LazyListState,
+    data: List<Int> = preloadModels,
+  ) =
     GlideLazyListPreloader(
       state = state,
-      data = preloadModels,
+      data = data,
       size = Size(Target.SIZE_ORIGINAL.toFloat(), Target.SIZE_ORIGINAL.toFloat()),
       numberOfItemsToPreload = 1,
       requestBuilderTransform = { resourceId, requestBuilder -> requestBuilder.load(resourceId) })