refactor!: multiple fixes & additions to NetworkTileProvider (and u…

…nderlying `ImageProvider`) (#1742)

analysis_options.yaml

@@ -36,7 +36,6 @@ linter:
     noop_primitive_operations: true
     avoid_void_async: true
     avoid_redundant_argument_values: true
-    avoid_types_on_closure_parameters: true
     unnecessary_null_checks: true
     prefer_single_quotes: true
     unnecessary_parenthesis: true

lib/src/layer/marker_layer.dart

@@ -104,7 +104,6 @@ class MarkerLayer extends StatelessWidget {
 
     return MobileLayerTransformer(
       child: Stack(
-        // ignore: avoid_types_on_closure_parameters
         children: (List<Marker> markers) sync* {
           for (final m in markers) {
             // Resolve real alignment

lib/src/layer/tile_layer/tile_image.dart

@@ -222,7 +222,6 @@ class TileImage extends ChangeNotifier {
 
     if (evictImageFromCache) {
       try {
-        // ignore: avoid_types_on_closure_parameters
         imageProvider.evict().catchError((Object e) {
           debugPrint(e.toString());
           return false;

lib/src/layer/tile_layer/tile_provider/base_tile_provider.dart

@@ -124,6 +124,9 @@ abstract class TileProvider {
   }
 
   /// Called when the [TileLayer] is disposed
+  ///
+  /// When disposing resources, ensure that they are not currently being used
+  /// by tiles in progress.
   void dispose() {}
 
   /// Regex that describes the format of placeholders in a `urlTemplate`

lib/src/layer/tile_layer/tile_provider/network_image_provider.dart

@@ -3,6 +3,7 @@ import 'dart:ui';
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/painting.dart';
+import 'package:flutter_map/flutter_map.dart';
 import 'package:http/http.dart';
 
 /// Dedicated [ImageProvider] to fetch tiles from the network
@@ -24,15 +25,31 @@ class FlutterMapNetworkImageProvider
   /// image provider will not be cached in memory.
   final String? fallbackUrl;
 
+  /// The headers to include with the tile fetch request
+  ///
+  /// Not included in [operator==].
+  final Map<String, String> headers;
+
   /// The HTTP client to use to make network requests
   ///
   /// Not included in [operator==].
   final BaseClient httpClient;
 
-  /// The headers to include with the tile fetch request
+  /// Whether to ignore exceptions and errors that occur whilst fetching tiles
+  /// over the network, and just return a transparent tile
+  final bool silenceExceptions;
+
+  /// Function invoked when the image starts loading (not from cache)
   ///
-  /// Not included in [operator==].
-  final Map<String, String> headers;
+  /// Used with [finishedLoadingBytes] to safely dispose of the [httpClient] only
+  /// after all tiles have loaded.
+  final void Function() startedLoading;
+
+  /// Function invoked when the image completes loading bytes from the network
+  ///
+  /// Used with [finishedLoadingBytes] to safely dispose of the [httpClient] only
+  /// after all tiles have loaded.
+  final void Function() finishedLoadingBytes;
 
   /// Create a dedicated [ImageProvider] to fetch tiles from the network
   ///
@@ -44,60 +61,59 @@ class FlutterMapNetworkImageProvider
     required this.fallbackUrl,
     required this.headers,
     required this.httpClient,
+    required this.silenceExceptions,
+    required this.startedLoading,
+    required this.finishedLoadingBytes,
   });
 
   @override
   ImageStreamCompleter loadImage(
     FlutterMapNetworkImageProvider key,
     ImageDecoderCallback decode,
-  ) {
-    final chunkEvents = StreamController<ImageChunkEvent>();
-
-    return MultiFrameImageStreamCompleter(
-      codec: _loadAsync(key, chunkEvents, decode),
-      chunkEvents: chunkEvents.stream,
-      scale: 1,
-      debugLabel: url,
-      informationCollector: () => [
-        DiagnosticsProperty('URL', url),
-        DiagnosticsProperty('Fallback URL', fallbackUrl),
-        DiagnosticsProperty('Current provider', key),
-      ],
-    );
-  }
-
-  @override
-  Future<FlutterMapNetworkImageProvider> obtainKey(
-    ImageConfiguration configuration,
   ) =>
-      SynchronousFuture<FlutterMapNetworkImageProvider>(this);
+      MultiFrameImageStreamCompleter(
+        codec: _load(key, decode),
+        scale: 1,
+        debugLabel: url,
+        informationCollector: () => [
+          DiagnosticsProperty('URL', url),
+          DiagnosticsProperty('Fallback URL', fallbackUrl),
+          DiagnosticsProperty('Current provider', key),
+        ],
+      );
 
-  Future<Codec> _loadAsync(
+  Future<Codec> _load(
     FlutterMapNetworkImageProvider key,
-    StreamController<ImageChunkEvent> chunkEvents,
     ImageDecoderCallback decode, {
     bool useFallback = false,
-  }) async {
-    try {
-      return decode(
-        await ImmutableBuffer.fromUint8List(
-          await httpClient.readBytes(
-            Uri.parse(useFallback ? fallbackUrl ?? '' : url),
-            headers: headers,
-          ),
-        ),
-      ).catchError((dynamic e) {
-        // ignore: only_throw_errors
-        if (useFallback || fallbackUrl == null) throw e as Object;
-        return _loadAsync(key, chunkEvents, decode, useFallback: true);
-      });
-    } catch (_) {
-      // This redundancy necessary, do not remove
-      if (useFallback || fallbackUrl == null) rethrow;
-      return _loadAsync(key, chunkEvents, decode, useFallback: true);
-    }
+  }) {
+    startedLoading();
+
+    return httpClient
+        .readBytes(
+          Uri.parse(useFallback ? fallbackUrl ?? '' : url),
+          headers: headers,
+        )
+        .whenComplete(finishedLoadingBytes)
+        .then(ImmutableBuffer.fromUint8List)
+        .then(decode)
+        .onError<Exception>((err, stack) {
+      scheduleMicrotask(() => PaintingBinding.instance.imageCache.evict(key));
+      if (useFallback || fallbackUrl == null) {
+        if (!silenceExceptions) throw err;
+        return ImmutableBuffer.fromUint8List(TileProvider.transparentImage)
+            .then(decode);
+      }
+      return _load(key, decode, useFallback: true);
+    });
   }
 
+  @override
+  SynchronousFuture<FlutterMapNetworkImageProvider> obtainKey(
+    ImageConfiguration configuration,
+  ) =>
+      SynchronousFuture(this);
+
   @override
   bool operator ==(Object other) =>
       identical(this, other) ||

lib/src/layer/tile_layer/tile_provider/network_tile_provider.dart

@@ -1,3 +1,6 @@
+import 'dart:async';
+import 'dart:collection';
+
 import 'package:flutter/rendering.dart';
 import 'package:flutter_map/src/layer/tile_layer/tile_coordinates.dart';
 import 'package:flutter_map/src/layer/tile_layer/tile_layer.dart';
@@ -34,23 +37,48 @@ class NetworkTileProvider extends TileProvider {
   NetworkTileProvider({
     super.headers,
     BaseClient? httpClient,
-  }) : httpClient = httpClient ?? RetryClient(Client());
+    this.silenceExceptions = false,
+  }) : _httpClient = httpClient ?? RetryClient(Client());
+
+  /// Whether to ignore exceptions and errors that occur whilst fetching tiles
+  /// over the network, and just return a transparent tile
+  final bool silenceExceptions;
+
+  /// Long living client used to make all tile requests by
+  /// [FlutterMapNetworkImageProvider] for the duration that this provider is
+  /// alive
+  final BaseClient _httpClient;
 
-  /// The HTTP client used to make network requests for tiles
-  final BaseClient httpClient;
+  /// Each [Completer] is completed once the corresponding tile has finished
+  /// loading
+  ///
+  /// Used to avoid disposing of [_httpClient] whilst HTTP requests are still
+  /// underway.
+  ///
+  /// Does not include tiles loaded from session cache.
+  final _tilesInProgress = HashMap<TileCoordinates, Completer<void>>();
 
   @override
   ImageProvider getImage(TileCoordinates coordinates, TileLayer options) =>
       FlutterMapNetworkImageProvider(
         url: getTileUrl(coordinates, options),
         fallbackUrl: getTileFallbackUrl(coordinates, options),
         headers: headers,
-        httpClient: httpClient,
+        httpClient: _httpClient,
+        silenceExceptions: silenceExceptions,
+        startedLoading: () => _tilesInProgress[coordinates] = Completer(),
+        finishedLoadingBytes: () {
+          _tilesInProgress[coordinates]?.complete();
+          _tilesInProgress.remove(coordinates);
+        },
       );
 
   @override
-  void dispose() {
-    httpClient.close();
+  Future<void> dispose() async {
+    if (_tilesInProgress.isNotEmpty) {
+      await Future.wait(_tilesInProgress.values.map((c) => c.future));
+    }
+    _httpClient.close();
     super.dispose();
   }
 }