Avoid caching image load failures that are retriable (#51398)

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

@@ -9,6 +9,7 @@ import 'dart:ui' as ui;
 
 import 'package:flutter/foundation.dart';
 
+import 'binding.dart';
 import 'debug.dart';
 import 'image_provider.dart' as image_provider;
 import 'image_stream.dart';
@@ -86,8 +87,13 @@ class NetworkImage extends image_provider.ImageProvider<image_provider.NetworkIm
         request.headers.add(name, value);
       });
       final HttpClientResponse response = await request.close();
-      if (response.statusCode != HttpStatus.ok)
+      if (response.statusCode != HttpStatus.ok) {
+        // The network may be only temporarily unavailable, or the file will be
+        // added on the server later. Avoid having future calls to resolve
+        // fail to check the network again.
+        PaintingBinding.instance.imageCache.evict(key);
         throw image_provider.NetworkImageLoadException(statusCode: response.statusCode, uri: resolved);
+      }
 
       final Uint8List bytes = await consolidateHttpClientResponseBytes(
         response,

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

@@ -642,9 +642,19 @@ abstract class AssetBundleImageProvider extends ImageProvider<AssetBundleImageKe
   /// This function is used by [load].
   @protected
   Future<ui.Codec> _loadAsync(AssetBundleImageKey key, DecoderCallback decode) async {
-    final ByteData data = await key.bundle.load(key.name);
-    if (data == null)
-      throw 'Unable to read data';
+    ByteData data;
+    // Hot reload/restart could change whether an asset bundle or key in a
+    // bundle are available, or if it is a network backed bundle.
+    try {
+      data = await key.bundle.load(key.name);
+    } on FlutterError {
+      PaintingBinding.instance.imageCache.evict(key);
+      rethrow;
+    }
+    if (data == null) {
+      PaintingBinding.instance.imageCache.evict(key);
+      throw StateError('Unable to read data');
+    }
     return await decode(data.buffer.asUint8List());
   }
 }
@@ -827,8 +837,12 @@ class FileImage extends ImageProvider<FileImage> {
     assert(key == this);
 
     final Uint8List bytes = await file.readAsBytes();
-    if (bytes.lengthInBytes == 0)
+
+    if (bytes.lengthInBytes == 0) {
+      // The file may become available later.
+      PaintingBinding.instance.imageCache.evict(key);
       throw StateError('$file is empty and cannot be loaded as an image.');
+    }
 
     return await decode(bytes);
   }

packages/flutter/test/painting/image_provider_test.dart

@@ -10,6 +10,7 @@ import 'dart:typed_data';
 import 'package:file/memory.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/painting.dart';
+import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:mockito/mockito.dart';
 
@@ -34,6 +35,8 @@ void main() {
 
   tearDown(() {
     FlutterError.onError = oldError;
+    PaintingBinding.instance.imageCache.clear();
+    PaintingBinding.instance.imageCache.clearLiveImages();
   });
 
   group('ImageProvider', () {
@@ -42,6 +45,50 @@ void main() {
         imageCache.clear();
       });
 
+      test('AssetImageProvider - evicts on failure to load', () async {
+        final Completer<FlutterError> error = Completer<FlutterError>();
+        FlutterError.onError = (FlutterErrorDetails details) {
+          error.complete(details.exception as FlutterError);
+        };
+
+        const ImageProvider provider = ExactAssetImage('does-not-exist');
+        final Object key = await provider.obtainKey(ImageConfiguration.empty);
+        expect(imageCache.statusForKey(provider).untracked, true);
+        expect(imageCache.pendingImageCount, 0);
+
+        provider.resolve(ImageConfiguration.empty);
+
+        expect(imageCache.statusForKey(key).pending, true);
+        expect(imageCache.pendingImageCount, 1);
+
+        await error.future;
+
+        expect(imageCache.statusForKey(provider).untracked, true);
+        expect(imageCache.pendingImageCount, 0);
+      }, skip: isBrowser);
+
+      test('AssetImageProvider - evicts on null load', () async {
+        final Completer<StateError> error = Completer<StateError>();
+        FlutterError.onError = (FlutterErrorDetails details) {
+          error.complete(details.exception as StateError);
+        };
+
+        final ImageProvider provider = ExactAssetImage('does-not-exist', bundle: TestAssetBundle());
+        final Object key = await provider.obtainKey(ImageConfiguration.empty);
+        expect(imageCache.statusForKey(provider).untracked, true);
+        expect(imageCache.pendingImageCount, 0);
+
+        provider.resolve(ImageConfiguration.empty);
+
+        expect(imageCache.statusForKey(key).pending, true);
+        expect(imageCache.pendingImageCount, 1);
+
+        await error.future;
+
+        expect(imageCache.statusForKey(provider).untracked, true);
+        expect(imageCache.pendingImageCount, 0);
+      }, skip: isBrowser);
+
       test('ImageProvider can evict images', () async {
         final Uint8List bytes = Uint8List.fromList(kTransparentImage);
         final MemoryImage imageProvider = MemoryImage(bytes);
@@ -168,7 +215,7 @@ void main() {
       expect(uncaught, false);
     });
 
-    test('File image with empty file throws expected error - (image cache)', () async {
+    test('File image with empty file throws expected error and evicts from cache', () async {
       final Completer<StateError> error = Completer<StateError>();
       FlutterError.onError = (FlutterErrorDetails details) {
         error.complete(details.exception as StateError);
@@ -177,9 +224,17 @@ void main() {
       final File file = fs.file('/empty.png')..createSync(recursive: true);
       final FileImage provider = FileImage(file);
 
+      expect(imageCache.statusForKey(provider).untracked, true);
+      expect(imageCache.pendingImageCount, 0);
+
       provider.resolve(ImageConfiguration.empty);
 
+      expect(imageCache.statusForKey(provider).pending, true);
+      expect(imageCache.pendingImageCount, 1);
+
       expect(await error.future, isStateError);
+      expect(imageCache.statusForKey(provider).untracked, true);
+      expect(imageCache.pendingImageCount, 0);
     });
 
     group('NetworkImage', () {
@@ -194,7 +249,7 @@ void main() {
         debugNetworkImageHttpClientProvider = null;
       });
 
-      test('Expect thrown exception with statusCode', () async {
+      test('Expect thrown exception with statusCode - evicts from cache', () async {
         final int errorStatusCode = HttpStatus.notFound;
         const String requestUrl = 'foo-url';
 
@@ -207,13 +262,24 @@ void main() {
         final Completer<dynamic> caughtError = Completer<dynamic>();
 
         final ImageProvider imageProvider = NetworkImage(nonconst(requestUrl));
+        expect(imageCache.pendingImageCount, 0);
+        expect(imageCache.statusForKey(imageProvider).untracked, true);
+
         final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
+
+        expect(imageCache.pendingImageCount, 1);
+        expect(imageCache.statusForKey(imageProvider).pending, true);
+
         result.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {
         }, onError: (dynamic error, StackTrace stackTrace) {
           caughtError.complete(error);
         }));
 
         final dynamic err = await caughtError.future;
+
+        expect(imageCache.pendingImageCount, 0);
+        expect(imageCache.statusForKey(imageProvider).untracked, true);
+
         expect(
           err,
           isA<NetworkImageLoadException>()
@@ -465,3 +531,10 @@ class AsyncKeyMemoryImage extends MemoryImage {
 class MockHttpClient extends Mock implements HttpClient {}
 class MockHttpClientRequest extends Mock implements HttpClientRequest {}
 class MockHttpClientResponse extends Mock implements HttpClientResponse {}
+
+class TestAssetBundle extends CachingAssetBundle {
+  @override
+  Future<ByteData> load(String key) async {
+    return null;
+  }
+}