Add debugNetworkImageHttpClientProvider (#32857)

Currently, the fact that NetworkImage uses a static HttpClient
makes it impossible to properly test, as a mock in one test will
be reused in another test. This change fixes that.

https://github.com/flutter/flutter/issues/32374

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

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:io';
+
 import 'package:flutter/foundation.dart';
 
 /// Whether to replace all shadows with solid color blocks.
@@ -11,6 +13,22 @@ import 'package:flutter/foundation.dart';
 /// version to version (or even from run to run).
 bool debugDisableShadows = false;
 
+/// Signature for a method that returns an [HttpClient].
+///
+/// Used by [debugNetworkImageHttpClientProvider].
+typedef HttpClientProvider = HttpClient Function();
+
+/// Provider from which [NetworkImage] will get its [HttpClient] in debug builds.
+///
+/// If this value is unset, [NetworkImage] will use its own internally-managed
+/// [HttpClient].
+///
+/// This setting can be overridden for testing to ensure that each test receives
+/// a mock client that hasn't been affected by other tests.
+///
+/// This value is ignored in non-debug builds.
+HttpClientProvider debugNetworkImageHttpClientProvider;
+
 /// Returns true if none of the painting library debug variables have been changed.
 ///
 /// This function is used by the test framework to ensure that debug variables
@@ -24,7 +42,8 @@ bool debugDisableShadows = false;
 /// test framework itself overrides this value in some cases.)
 bool debugAssertAllPaintingVarsUnset(String reason, { bool debugDisableShadowsOverride = false }) {
   assert(() {
-    if (debugDisableShadows != debugDisableShadowsOverride) {
+    if (debugDisableShadows != debugDisableShadowsOverride ||
+        debugNetworkImageHttpClientProvider != null) {
       throw FlutterError(reason);
     }
     return true;

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

@@ -12,6 +12,7 @@ import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
 
 import 'binding.dart';
+import 'debug.dart';
 import 'image_cache.dart';
 import 'image_stream.dart';
 
@@ -510,7 +511,18 @@ class NetworkImage extends ImageProvider<NetworkImage> {
     );
   }
 
-  static final HttpClient _httpClient = HttpClient();
+  // Do not access this field directly; use [_httpClient] instead.
+  static final HttpClient _sharedHttpClient = HttpClient();
+
+  static HttpClient get _httpClient {
+    HttpClient client = _sharedHttpClient;
+    assert(() {
+      if (debugNetworkImageHttpClientProvider != null)
+        client = debugNetworkImageHttpClientProvider();
+      return true;
+    }());
+    return client;
+  }
 
   Future<ui.Codec> _loadAsync(NetworkImage key) async {
     assert(key == this);

packages/flutter/test/painting/image_provider_test.dart

@@ -16,50 +16,64 @@ import 'image_data.dart';
 import 'mocks_for_image_cache.dart';
 
 void main() {
-  TestRenderingFlutterBinding(); // initializes the imageCache
   group(ImageProvider, () {
-    tearDown(() {
-      imageCache.clear();
+    setUpAll(() {
+      TestRenderingFlutterBinding(); // initializes the imageCache
     });
 
-    test('NetworkImage non-null url test', () {
-      expect(() {
-        NetworkImage(nonconst(null));
-      }, throwsAssertionError);
-    });
+    group('Image cache', () {
+      tearDown(() {
+        imageCache.clear();
+      });
 
-    test('ImageProvider can evict images', () async {
-      final Uint8List bytes = Uint8List.fromList(kTransparentImage);
-      final MemoryImage imageProvider = MemoryImage(bytes);
-      final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
-      final Completer<void> completer = Completer<void>();
-      stream.addListener((ImageInfo info, bool syncCall) => completer.complete());
-      await completer.future;
+      test('ImageProvider can evict images', () async {
+        final Uint8List bytes = Uint8List.fromList(kTransparentImage);
+        final MemoryImage imageProvider = MemoryImage(bytes);
+        final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
+        final Completer<void> completer = Completer<void>();
+        stream.addListener((ImageInfo info, bool syncCall) => completer.complete());
+        await completer.future;
+
+        expect(imageCache.currentSize, 1);
+        expect(await MemoryImage(bytes).evict(), true);
+        expect(imageCache.currentSize, 0);
+      });
 
-      expect(imageCache.currentSize, 1);
-      expect(await MemoryImage(bytes).evict(), true);
-      expect(imageCache.currentSize, 0);
-    });
+      test('ImageProvider.evict respects the provided ImageCache', () async {
+        final ImageCache otherCache = ImageCache();
+        final Uint8List bytes = Uint8List.fromList(kTransparentImage);
+        final MemoryImage imageProvider = MemoryImage(bytes);
+        otherCache.putIfAbsent(imageProvider, () => imageProvider.load(imageProvider));
+        final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
+        final Completer<void> completer = Completer<void>();
+        stream.addListener((ImageInfo info, bool syncCall) => completer.complete());
+        await completer.future;
+
+        expect(otherCache.currentSize, 1);
+        expect(imageCache.currentSize, 1);
+        expect(await imageProvider.evict(cache: otherCache), true);
+        expect(otherCache.currentSize, 0);
+        expect(imageCache.currentSize, 1);
+      });
 
-    test('ImageProvider.evict respects the provided ImageCache', () async {
-      final ImageCache otherCache = ImageCache();
-      final Uint8List bytes = Uint8List.fromList(kTransparentImage);
-      final MemoryImage imageProvider = MemoryImage(bytes);
-      otherCache.putIfAbsent(imageProvider, () => imageProvider.load(imageProvider));
-      final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
-      final Completer<void> completer = Completer<void>();
-      stream.addListener((ImageInfo info, bool syncCall) => completer.complete());
-      await completer.future;
-
-      expect(otherCache.currentSize, 1);
-      expect(imageCache.currentSize, 1);
-      expect(await imageProvider.evict(cache: otherCache), true);
-      expect(otherCache.currentSize, 0);
-      expect(imageCache.currentSize, 1);
+      test('ImageProvider errors can always be caught', () async {
+        final ErrorImageProvider imageProvider = ErrorImageProvider();
+        final Completer<bool> caughtError = Completer<bool>();
+        FlutterError.onError = (FlutterErrorDetails details) {
+          caughtError.complete(false);
+        };
+        final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
+        stream.addListener((ImageInfo info, bool syncCall) {
+          caughtError.complete(false);
+        }, onError: (dynamic error, StackTrace stackTrace) {
+          caughtError.complete(true);
+        });
+        expect(await caughtError.future, true);
+      });
     });
 
-    test('ImageProvider errors can always be caught', () async {
-      final ErrorImageProvider imageProvider = ErrorImageProvider();
+    test('obtainKey errors will be caught', () async {
+      final ImageProvider imageProvider = ObtainKeyErrorImageProvider();
       final Completer<bool> caughtError = Completer<bool>();
       FlutterError.onError = (FlutterErrorDetails details) {
         caughtError.complete(false);
@@ -72,93 +86,121 @@ void main() {
       });
       expect(await caughtError.future, true);
     });
-  });
 
-  test('ImageProvide.obtainKey errors will be caught', () async {
-    final ImageProvider imageProvider = ObtainKeyErrorImageProvider();
-    final Completer<bool> caughtError = Completer<bool>();
-    FlutterError.onError = (FlutterErrorDetails details) {
-      caughtError.complete(false);
-    };
-    final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
-    stream.addListener((ImageInfo info, bool syncCall) {
-      caughtError.complete(false);
-    }, onError: (dynamic error, StackTrace stackTrace) {
-      caughtError.complete(true);
+    test('resolve sync errors will be caught', () async {
+      bool uncaught = false;
+      final Zone testZone = Zone.current.fork(specification: ZoneSpecification(
+        handleUncaughtError: (Zone zone, ZoneDelegate zoneDelegate, Zone parent, Object error, StackTrace stackTrace) {
+          uncaught = true;
+        },
+      ));
+      await testZone.run(() async {
+        final ImageProvider imageProvider = LoadErrorImageProvider();
+        final Completer<bool> caughtError = Completer<bool>();
+        FlutterError.onError = (FlutterErrorDetails details) {
+          throw Error();
+        };
+        final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
+        result.addListener((ImageInfo info, bool syncCall) {
+        }, onError: (dynamic error, StackTrace stackTrace) {
+          caughtError.complete(true);
+        });
+        expect(await caughtError.future, true);
+      });
+      expect(uncaught, false);
     });
-    expect(await caughtError.future, true);
-  });
 
-  test('ImageProvider.resolve sync errors will be caught', () async {
-    bool uncaught = false;
-    final Zone testZone = Zone.current.fork(specification: ZoneSpecification(
-      handleUncaughtError: (Zone zone, ZoneDelegate zoneDelegate, Zone parent, Object error, StackTrace stackTrace) {
-        uncaught = true;
-      }
-    ));
-    await testZone.run(() async {
-      final ImageProvider imageProvider = LoadErrorImageProvider();
-      final Completer<bool> caughtError = Completer<bool>();
-      FlutterError.onError = (FlutterErrorDetails details) {
-        throw Error();
-      };
-      final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
-      result.addListener((ImageInfo info, bool syncCall) {
-      }, onError: (dynamic error, StackTrace stackTrace) {
-        caughtError.complete(true);
+    test('resolve errors in the completer will be caught', () async {
+      bool uncaught = false;
+      final Zone testZone = Zone.current.fork(specification: ZoneSpecification(
+        handleUncaughtError: (Zone zone, ZoneDelegate zoneDelegate, Zone parent, Object error, StackTrace stackTrace) {
+          uncaught = true;
+        },
+      ));
+      await testZone.run(() async {
+        final ImageProvider imageProvider = LoadErrorCompleterImageProvider();
+        final Completer<bool> caughtError = Completer<bool>();
+        FlutterError.onError = (FlutterErrorDetails details) {
+          throw Error();
+        };
+        final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
+        result.addListener((ImageInfo info, bool syncCall) {
+        }, onError: (dynamic error, StackTrace stackTrace) {
+          caughtError.complete(true);
+        });
+        expect(await caughtError.future, true);
       });
-      expect(await caughtError.future, true);
+      expect(uncaught, false);
     });
-    expect(uncaught, false);
-  });
 
-   test('ImageProvider.resolve errors in the completer will be caught', () async {
-    bool uncaught = false;
-    final Zone testZone = Zone.current.fork(specification: ZoneSpecification(
-      handleUncaughtError: (Zone zone, ZoneDelegate zoneDelegate, Zone parent, Object error, StackTrace stackTrace) {
-        uncaught = true;
-      }
-    ));
-    await testZone.run(() async {
-      final ImageProvider imageProvider = LoadErrorCompleterImageProvider();
-      final Completer<bool> caughtError = Completer<bool>();
-      FlutterError.onError = (FlutterErrorDetails details) {
-        throw Error();
-      };
-      final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
-      result.addListener((ImageInfo info, bool syncCall) {
-      }, onError: (dynamic error, StackTrace stackTrace) {
-        caughtError.complete(true);
+    group(NetworkImage, () {
+      MockHttpClient httpClient;
+
+      setUp(() {
+        httpClient = MockHttpClient();
+        debugNetworkImageHttpClientProvider = () => httpClient;
       });
-      expect(await caughtError.future, true);
-    });
-    expect(uncaught, false);
-  });
 
-  test('ImageProvider.resolve errors in the http client will be caught', () async {
-    bool uncaught = false;
-    final HttpClientMock httpClientMock = HttpClientMock();
-    when(httpClientMock.getUrl(any)).thenThrow(Error());
+      tearDown(() {
+        debugNetworkImageHttpClientProvider = null;
+      });
 
-    await HttpOverrides.runZoned(() async {
-      const ImageProvider imageProvider = NetworkImage('asdasdasdas');
-      final Completer<bool> caughtError = Completer<bool>();
-      FlutterError.onError = (FlutterErrorDetails details) {
-        throw Error();
-      };
-      final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
-      result.addListener((ImageInfo info, bool syncCall) {
-      }, onError: (dynamic error, StackTrace stackTrace) {
-        caughtError.complete(true);
+      test('Disallows null urls', () {
+        expect(() {
+          NetworkImage(nonconst(null));
+        }, throwsAssertionError);
       });
-      expect(await caughtError.future, true);
-    }, createHttpClient: (SecurityContext context) => httpClientMock, zoneSpecification: ZoneSpecification(
-      handleUncaughtError: (Zone zone, ZoneDelegate zoneDelegate, Zone parent, Object error, StackTrace stackTrace) {
-        uncaught = true;
-      }
-    ));
-    expect(uncaught, false);
+
+      test('Uses the HttpClient provided by debugNetworkImageHttpClientProvider if set', () async {
+        when(httpClient.getUrl(any)).thenThrow('client1');
+        final List<dynamic> capturedErrors = <dynamic>[];
+
+        Future<void> loadNetworkImage() async {
+          final NetworkImage networkImage = NetworkImage(nonconst('foo'));
+          final ImageStreamCompleter completer = networkImage.load(networkImage);
+          completer.addListener(
+            (ImageInfo image, bool synchronousCall) { },
+            onError: (dynamic error, StackTrace stackTrace) {
+              capturedErrors.add(error);
+            },
+          );
+          await Future<void>.value();
+        }
+
+        await loadNetworkImage();
+        expect(capturedErrors, <dynamic>['client1']);
+        final MockHttpClient client2 = MockHttpClient();
+        when(client2.getUrl(any)).thenThrow('client2');
+        debugNetworkImageHttpClientProvider = () => client2;
+        await loadNetworkImage();
+        expect(capturedErrors, <dynamic>['client1', 'client2']);
+      });
+
+      test('Propagates http client errors during resolve()', () async {
+        when(httpClient.getUrl(any)).thenThrow(Error());
+        bool uncaught = false;
+
+        await runZoned(() async {
+          const ImageProvider imageProvider = NetworkImage('asdasdasdas');
+          final Completer<bool> caughtError = Completer<bool>();
+          FlutterError.onError = (FlutterErrorDetails details) {
+            throw Error();
+          };
+          final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
+          result.addListener((ImageInfo info, bool syncCall) {
+          }, onError: (dynamic error, StackTrace stackTrace) {
+            caughtError.complete(true);
+          });
+          expect(await caughtError.future, true);
+        }, zoneSpecification: ZoneSpecification(
+          handleUncaughtError: (Zone zone, ZoneDelegate zoneDelegate, Zone parent, Object error, StackTrace stackTrace) {
+            uncaught = true;
+          },
+        ));
+        expect(uncaught, false);
+      });
+    });
   });
 }
 
-class HttpClientMock extends Mock implements HttpClient {}
+class MockHttpClient extends Mock implements HttpClient {}