Fix LRUness of ScrollAwareImageProvider (#50242)

packages/flutter/lib/src/painting/image_provider.dart

@@ -411,6 +411,18 @@ abstract class ImageProvider<T> {
   /// [ImageCache].
   @protected
   void resolveStreamForKey(ImageConfiguration configuration, ImageStream stream, T key, ImageErrorListener handleError) {
+    // This is an unusual edge case where someone has told us that they found
+    // the image we want before getting to this method. We should avoid calling
+    // load again, but still update the image cache with LRU information.
+    if (stream.completer != null) {
+      final ImageStreamCompleter completer = PaintingBinding.instance.imageCache.putIfAbsent(
+        key,
+        () => stream.completer,
+        onError: handleError,
+      );
+      assert(identical(completer, stream.completer));
+      return;
+    }
     final ImageStreamCompleter completer = PaintingBinding.instance.imageCache.putIfAbsent(
       key,
       () => load(key, PaintingBinding.instance.instantiateImageCodec),

packages/flutter/lib/src/widgets/scroll_aware_image_provider.dart

@@ -74,12 +74,16 @@ class ScrollAwareImageProvider<T> extends ImageProvider<T> {
     T key,
     ImageErrorListener handleError,
   ) {
-    // Something managed to complete the stream. Nothing left to do.
-    if (stream.completer != null) {
-      return;
-    }
-    // Something else got this image into the cache. Return it.
-    if (PaintingBinding.instance.imageCache.containsKey(key)) {
+    // Something managed to complete the stream, or it's already in the image
+    // cache. Notify the wrapped provider and expect it to behave by not
+    // reloading the image since it's already resolved.
+    // Do this even if the context has gone out of the tree, since it will
+    // update LRU information about the cache. Even though we never showed the
+    // image, it was still touched more recently.
+    // Do this before checking scrolling, so that if the bytes are available we
+    // render them even though we're scrolling fast - there's no additional
+    // allocations to do for texture memory, it's already there.
+    if (stream.completer != null || PaintingBinding.instance.imageCache.containsKey(key)) {
       imageProvider.resolveStreamForKey(configuration, stream, key, handleError);
       return;
     }

packages/flutter/test/painting/image_test_utils.dart

@@ -18,6 +18,7 @@ class TestImageProvider extends ImageProvider<TestImageProvider> {
 
   final Completer<ImageInfo> _completer = Completer<ImageInfo>.sync();
   ImageConfiguration configuration;
+  int loadCallCount = 0;
 
   @override
   Future<TestImageProvider> obtainKey(ImageConfiguration configuration) {
@@ -31,8 +32,10 @@ class TestImageProvider extends ImageProvider<TestImageProvider> {
   }
 
   @override
-  ImageStreamCompleter load(TestImageProvider key, DecoderCallback decode) =>
-      OneFrameImageStreamCompleter(_completer.future);
+  ImageStreamCompleter load(TestImageProvider key, DecoderCallback decode) {
+    loadCallCount += 1;
+    return OneFrameImageStreamCompleter(_completer.future);
+  }
 
   ImageInfo complete() {
     final ImageInfo imageInfo = ImageInfo(image: testImage);

packages/flutter/test/widgets/scroll_aware_image_provider_test.dart

@@ -327,8 +327,74 @@ void main() {
 
     expect(imageCache.containsKey(testImageProvider), true);
     expect(imageCache.currentSize, 1);
+    expect(testImageProvider.loadCallCount, 1);
     expect(stream.completer, null);
   });
+
+  testWidgets('ScrollAwareImageProvider does not block LRU updates to image cache', (WidgetTester tester) async {
+    final int oldSize = imageCache.maximumSize;
+    imageCache.maximumSize = 1;
+
+    final GlobalKey<TestWidgetState> key = GlobalKey<TestWidgetState>();
+    final ScrollController scrollController = ScrollController();
+    await tester.pumpWidget(Directionality(
+      textDirection: TextDirection.ltr,
+      child: SingleChildScrollView(
+        physics: ControllablePhysics(),
+        controller: scrollController,
+        child: TestWidget(key),
+      ),
+    ));
+
+    final DisposableBuildContext context = DisposableBuildContext(key.currentState);
+    const TestImage testImage = TestImage(width: 10, height: 10);
+    final TestImageProvider testImageProvider = TestImageProvider(testImage);
+    final ScrollAwareImageProvider<TestImageProvider> imageProvider = ScrollAwareImageProvider<TestImageProvider>(
+      context: context,
+      imageProvider: testImageProvider,
+    );
+
+    expect(testImageProvider.configuration, null);
+    expect(imageCache.containsKey(testImageProvider), false);
+
+    final ControllablePhysics physics = _findPhysics<ControllablePhysics>(tester);
+    physics.recommendDeferredLoadingValue = true;
+
+    final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
+
+    expect(testImageProvider.configuration, null);
+    expect(stream.completer, null);
+    expect(imageCache.currentSize, 0);
+
+    // Occupy the only slot in the cache with another image.
+    final TestImageProvider testImageProvider2 = TestImageProvider(const TestImage());
+    testImageProvider2.complete();
+    await precacheImage(testImageProvider2, context.context);
+    expect(imageCache.containsKey(testImageProvider), false);
+    expect(imageCache.containsKey(testImageProvider2), true);
+    expect(imageCache.currentSize, 1);
+
+    // Complete the original image while we're still scrolling fast.
+    testImageProvider.complete();
+    stream.setCompleter(testImageProvider.load(testImageProvider, PaintingBinding.instance.instantiateImageCodec));
+
+    // Verify that this hasn't chagned the cache state yet
+    expect(imageCache.containsKey(testImageProvider), false);
+    expect(imageCache.containsKey(testImageProvider2), true);
+    expect(imageCache.currentSize, 1);
+    expect(testImageProvider.loadCallCount, 1);
+
+    await tester.pump();
+
+    // After pumping a frame, the original image should be in the cache because
+    // it took the LRU slot.
+    expect(imageCache.containsKey(testImageProvider), true);
+    expect(imageCache.containsKey(testImageProvider2), false);
+    expect(imageCache.currentSize, 1);
+    expect(testImageProvider.loadCallCount, 1);
+
+    imageCache.maximumSize = oldSize;
+  });
 }
 
 class TestWidget extends StatefulWidget {