Fix LRUness of ScrollAwareImageProvider (#50242)
diff --git a/packages/flutter/lib/src/painting/image_provider.dart b/packages/flutter/lib/src/painting/image_provider.dart
index 8c0ac71..04ec59f 100644
--- a/packages/flutter/lib/src/painting/image_provider.dart
+++ b/packages/flutter/lib/src/painting/image_provider.dart
@@ -411,6 +411,18 @@
/// [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),
diff --git a/packages/flutter/lib/src/widgets/scroll_aware_image_provider.dart b/packages/flutter/lib/src/widgets/scroll_aware_image_provider.dart
index 90f301a..851aa1e 100644
--- a/packages/flutter/lib/src/widgets/scroll_aware_image_provider.dart
+++ b/packages/flutter/lib/src/widgets/scroll_aware_image_provider.dart
@@ -74,12 +74,16 @@
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;
}
diff --git a/packages/flutter/test/painting/image_test_utils.dart b/packages/flutter/test/painting/image_test_utils.dart
index c690f76..4fbd7a2 100644
--- a/packages/flutter/test/painting/image_test_utils.dart
+++ b/packages/flutter/test/painting/image_test_utils.dart
@@ -18,6 +18,7 @@
final Completer<ImageInfo> _completer = Completer<ImageInfo>.sync();
ImageConfiguration configuration;
+ int loadCallCount = 0;
@override
Future<TestImageProvider> obtainKey(ImageConfiguration configuration) {
@@ -31,8 +32,10 @@
}
@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);
diff --git a/packages/flutter/test/widgets/scroll_aware_image_provider_test.dart b/packages/flutter/test/widgets/scroll_aware_image_provider_test.dart
index e86179a..1f2b56c 100644
--- a/packages/flutter/test/widgets/scroll_aware_image_provider_test.dart
+++ b/packages/flutter/test/widgets/scroll_aware_image_provider_test.dart
@@ -327,8 +327,74 @@
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 {
