Clearing pendingImages when the cache is cleared or evicted. (#23860)

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

@@ -26,7 +26,7 @@ const int _kDefaultSizeBytes = 100 << 20; // 100 MiB
 /// Generally this class is not used directly. The [ImageProvider] class and its
 /// subclasses automatically handle the caching of images.
 class ImageCache {
-  final Map<Object, ImageStreamCompleter> _pendingImages = <Object, ImageStreamCompleter>{};
+  final Map<Object, _PendingImage> _pendingImages = <Object, _PendingImage>{};
   final Map<Object, _CachedImage> _cache = <Object, _CachedImage>{};
 
   /// Maximum number of entries to store in the cache.
@@ -48,8 +48,7 @@ class ImageCache {
       return;
     _maximumSize = value;
     if (maximumSize == 0) {
-      _cache.clear();
-      _currentSizeBytes = 0;
+      clear();
     } else {
       _checkCacheSize();
     }
@@ -78,8 +77,7 @@ class ImageCache {
       return;
     _maximumSizeBytes = value;
     if (_maximumSizeBytes == 0) {
-      _cache.clear();
-      _currentSizeBytes = 0;
+      clear();
     } else {
       _checkCacheSize();
     }
@@ -98,10 +96,15 @@ class ImageCache {
   /// cache, and when they complete they will be inserted as normal.
   void clear() {
     _cache.clear();
+    _pendingImages.clear();
     _currentSizeBytes = 0;
   }
 
   /// Evicts a single entry from the cache, returning true if successful.
+  /// Pending images waiting for completion are removed as well, returning true if successful.
+  ///
+  /// When a pending image is removed the listener on it is removed as well to prevent
+  /// it from adding itself to the cache if it eventually completes.
   ///
   /// The [key] must be equal to an object used to cache an image in
   /// [ImageCache.putIfAbsent].
@@ -113,6 +116,11 @@ class ImageCache {
   ///
   ///  * [ImageProvider], for providing images to the [Image] widget.
   bool evict(Object key) {
+    final _PendingImage pendingImage = _pendingImages.remove(key);
+    if (pendingImage != null) {
+      pendingImage.removeListener();
+      return true;
+    }
     final _CachedImage image = _cache.remove(key);
     if (image != null) {
       _currentSizeBytes -= image.sizeBytes;
@@ -134,7 +142,7 @@ class ImageCache {
   ImageStreamCompleter putIfAbsent(Object key, ImageStreamCompleter loader(), { ImageErrorListener onError }) {
     assert(key != null);
     assert(loader != null);
-    ImageStreamCompleter result = _pendingImages[key];
+    ImageStreamCompleter result = _pendingImages[key]?.completer;
     // Nothing needs to be done because the image hasn't loaded yet.
     if (result != null)
       return result;
@@ -166,13 +174,16 @@ class ImageCache {
         _maximumSizeBytes = imageSize + 1000;
       }
       _currentSizeBytes += imageSize;
-      _pendingImages.remove(key);
+      final _PendingImage pendingImage = _pendingImages.remove(key);
+      if (pendingImage != null) {
+        pendingImage.removeListener();
+      }
+
       _cache[key] = image;
-      result.removeListener(listener);
       _checkCacheSize();
     }
     if (maximumSize > 0 && maximumSizeBytes > 0) {
-      _pendingImages[key] = result;
+      _pendingImages[key] = _PendingImage(result, listener);
       result.addListener(listener);
     }
     return result;
@@ -199,3 +210,14 @@ class _CachedImage {
   final ImageStreamCompleter completer;
   final int sizeBytes;
 }
+
+class _PendingImage {
+  _PendingImage(this.completer, this.listener);
+
+  final ImageStreamCompleter completer;
+  final ImageListener listener;
+
+  void removeListener() {
+    completer.removeListener(listener);
+  }
+}

packages/flutter/test/painting/image_cache_test.dart

@@ -86,7 +86,6 @@ void main() {
       expect(p.value, equals(16));
 
       // cache has three entries: 3(14), 4(15), 1(16)
-
     });
 
     test('clear removes all images and resets cache size', () async {
@@ -139,7 +138,72 @@ void main() {
       expect(result, null);
       expect(caughtError, true);
     });
+
+    test('already pending image is returned when it is put into the cache again', () async {
+      const TestImage testImage = TestImage(width: 8, height: 8);
+
+      final TestImageStreamCompleter completer1 = TestImageStreamCompleter();
+      final TestImageStreamCompleter completer2 = TestImageStreamCompleter();
+
+      final TestImageStreamCompleter resultingCompleter1 = imageCache.putIfAbsent(testImage, () {
+        return completer1;
+      });
+      final TestImageStreamCompleter resultingCompleter2 = imageCache.putIfAbsent(testImage, () {
+        return completer2;
+      });
+
+      expect(resultingCompleter1, completer1);
+      expect(resultingCompleter2, completer1);
+    });
+
+    test('pending image is removed when cache is cleared', () async {
+      const TestImage testImage = TestImage(width: 8, height: 8);
+
+      final TestImageStreamCompleter completer1 = TestImageStreamCompleter();
+      final TestImageStreamCompleter completer2 = TestImageStreamCompleter();
+
+      final TestImageStreamCompleter resultingCompleter1 = imageCache.putIfAbsent(testImage, () {
+        return completer1;
+      });
+
+      imageCache.clear();
+
+      final TestImageStreamCompleter resultingCompleter2 = imageCache.putIfAbsent(testImage, () {
+        return completer2;
+      });
+
+      expect(resultingCompleter1, completer1);
+      expect(resultingCompleter2, completer2);
+    });
+
+    test('pending image is removed when image is evicted', () async {
+      const TestImage testImage = TestImage(width: 8, height: 8);
+
+      final TestImageStreamCompleter completer1 = TestImageStreamCompleter();
+      final TestImageStreamCompleter completer2 = TestImageStreamCompleter();
+
+      final TestImageStreamCompleter resultingCompleter1 = imageCache.putIfAbsent(testImage, () {
+        return completer1;
+      });
+
+      imageCache.evict(testImage);
+
+      final TestImageStreamCompleter resultingCompleter2 = imageCache.putIfAbsent(testImage, () {
+        return completer2;
+      });
+
+      expect(resultingCompleter1, completer1);
+      expect(resultingCompleter2, completer2);
+    });
+
+    test('failed image can successfully be removed from the cache\'s pending images', () async {
+      const TestImage testImage = TestImage(width: 8, height: 8);
+
+      const FailingTestImageProvider(1, 1, image: testImage).resolve(ImageConfiguration.empty).addListener((ImageInfo image, bool synchronousCall){}, onError: (dynamic exception, StackTrace stackTrace) {
+        final bool evicationResult = imageCache.evict(1);
+        expect(evicationResult, isTrue);
+      });
+    });
   });
 }
 
-

packages/flutter/test/painting/mocks_for_image_cache.dart

@@ -47,6 +47,15 @@ class TestImageProvider extends ImageProvider<int> {
   String toString() => '$runtimeType($key, $imageValue)';
 }
 
+class FailingTestImageProvider extends TestImageProvider {
+  const FailingTestImageProvider(int key, int imageValue, { ui.Image image }) : super(key, imageValue, image: image);
+
+  @override
+  ImageStreamCompleter load(int key) {
+    return OneFrameImageStreamCompleter(Future<ImageInfo>.sync(() => Future<ImageInfo>.error('loading failed!')));
+  }
+}
+
 Future<ImageInfo> extractOneFrame(ImageStream stream) {
   final Completer<ImageInfo> completer = Completer<ImageInfo>();
   void listener(ImageInfo image, bool synchronousCall) {
@@ -84,3 +93,5 @@ class ErrorImageProvider extends ImageProvider<ErrorImageProvider> {
     return SynchronousFuture<ErrorImageProvider>(this);
   }
 }
+
+class TestImageStreamCompleter extends ImageStreamCompleter {}