Add some sanity to the ImageStream listener API (#32936)

The current API was broken in that you registered multiple
callbacks at once, but when you removed listeners, only the
primary listener was used to determine what was removed.
This led to unintuitive cases where the caller could get
unexpected behavior.

This updates the API to add and remove listeners using
a newly introduced [ImageStreamListener] object, a value
object that has references to the individual callbacks
that may fire.

flutter/flutter#24722
flutter/flutter#32374
flutter/flutter#32935

dev/integration_tests/image_loading/lib/main.dart

@@ -44,9 +44,17 @@ class _ImageLoaderState extends State<ImageLoader> {
     // http client.
     final NetworkImage image = NetworkImage('https://github.com/flutter/flutter');
     final ImageStream stream = image.resolve(ImageConfiguration.empty);
-    stream.addListener((ImageInfo info, bool syncCall) {}, onError: (dynamic error, StackTrace stackTrace) {
-      print('ERROR caught by framework');
-    });
+    ImageStreamListener listener;
+    listener = ImageStreamListener(
+      (ImageInfo info, bool syncCall) {
+        stream.removeListener(listener);
+      },
+      onError: (dynamic error, StackTrace stackTrace) {
+        print('ERROR caught by framework');
+        stream.removeListener(listener);
+      },
+    );
+    stream.addListener(listener);
     super.initState();
   }
 

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

@@ -238,9 +238,10 @@ class DecorationImagePainter {
 
     final ImageStream newImageStream = _details.image.resolve(configuration);
     if (newImageStream.key != _imageStream?.key) {
-      _imageStream?.removeListener(_imageListener);
+      final ImageStreamListener listener = ImageStreamListener(_handleImage);
+      _imageStream?.removeListener(listener);
       _imageStream = newImageStream;
-      _imageStream.addListener(_imageListener);
+      _imageStream.addListener(listener);
     }
     if (_image == null)
       return;
@@ -268,7 +269,7 @@ class DecorationImagePainter {
       canvas.restore();
   }
 
-  void _imageListener(ImageInfo value, bool synchronousCall) {
+  void _handleImage(ImageInfo value, bool synchronousCall) {
     if (_image == value)
       return;
     _image = value;
@@ -284,7 +285,7 @@ class DecorationImagePainter {
   /// After this method has been called, the object is no longer usable.
   @mustCallSuper
   void dispose() {
-    _imageStream?.removeListener(_imageListener);
+    _imageStream?.removeListener(ImageStreamListener(_handleImage));
   }
 
   @override

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

@@ -183,8 +183,10 @@ class ImageCache {
       _checkCacheSize();
     }
     if (maximumSize > 0 && maximumSizeBytes > 0) {
-      _pendingImages[key] = _PendingImage(result, listener);
-      result.addListener(listener);
+      final ImageStreamListener streamListener = ImageStreamListener(listener);
+      _pendingImages[key] = _PendingImage(result, streamListener);
+      // Listener is removed in [_PendingImage.removeListener].
+      result.addListener(streamListener);
     }
     return result;
   }
@@ -215,7 +217,7 @@ class _PendingImage {
   _PendingImage(this.completer, this.listener);
 
   final ImageStreamCompleter completer;
-  final ImageListener listener;
+  final ImageStreamListener listener;
 
   void removeListener() {
     completer.removeListener(listener);

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

@@ -218,8 +218,9 @@ class ImageConfiguration {
 ///       // If the keys are the same, then we got the same image back, and so we don't
 ///       // need to update the listeners. If the key changed, though, we must make sure
 ///       // to switch our listeners to the new image stream.
-///       oldImageStream?.removeListener(_updateImage);
-///       _imageStream.addListener(_updateImage);
+///       final ImageStreamListener listener = ImageStreamListener(_updateImage);
+///       oldImageStream?.removeListener(listener);
+///       _imageStream.addListener(listener);
 ///     }
 ///   }
 ///
@@ -232,7 +233,7 @@ class ImageConfiguration {
 ///
 ///   @override
 ///   void dispose() {
-///     _imageStream.removeListener(_updateImage);
+///     _imageStream.removeListener(ImageStreamListener(_updateImage));
 ///     super.dispose();
 ///   }
 ///

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

@@ -56,9 +56,64 @@ class ImageInfo {
   }
 }
 
+/// Interface for receiving notifications about the loading of an image.
+///
+/// This class overrides `operator ==` and `hashCode` to compare the individual
+/// callbacks in the listener, meaning that if you add an instance of this class
+/// as a listener (e.g. via [ImageStream.addListener]), you can instantiate a
+/// _different_ instance of this class when you remove the listener, and the
+/// listener will be properly removed as long all associated callbacks are
+/// equal.
+///
+/// Used by [ImageStream] and [ImageStreamCompleter].
+@immutable
+class ImageStreamListener {
+  /// Creates a new [ImageStreamListener].
+  ///
+  /// The [onImage] parameter must not be null.
+  const ImageStreamListener(
+    this.onImage, {
+    this.onError,
+  }) : assert(onImage != null);
+
+  /// Callback for getting notified that an image is available.
+  ///
+  /// This callback may fire multiple times (e.g. if the [ImageStreamCompleter]
+  /// that drives the notifications fires multiple times). An example of such a
+  /// case would be an image with multiple frames within it (such as an animated
+  /// GIF).
+  ///
+  /// For more information on how to interpret the parameters to the callback,
+  /// see the documentation on [ImageListener].
+  ///
+  /// See also:
+  ///
+  ///  * [onError], which will be called instead of [onImage] if an error occurs
+  ///    during loading.
+  final ImageListener onImage;
+
+  /// Callback for getting notified when an error occurs while loading an image.
+  ///
+  /// If an error occurs during loading, [onError] will be called instead of
+  /// [onImage].
+  final ImageErrorListener onError;
+
+  @override
+  int get hashCode => hashValues(onImage, onError);
+
+  @override
+  bool operator ==(dynamic other) {
+    if (other.runtimeType != runtimeType)
+      return false;
+    final ImageStreamListener typedOther = other;
+    return onImage == typedOther.onImage
+        && onError == typedOther.onError;
+  }
+}
+
 /// Signature for callbacks reporting that an image is available.
 ///
-/// Used by [ImageStream].
+/// Used in [ImageStreamListener].
 ///
 /// The `synchronousCall` argument is true if the listener is being invoked
 /// during the call to `addListener`. This can be useful if, for example,
@@ -70,15 +125,10 @@ typedef ImageListener = void Function(ImageInfo image, bool synchronousCall);
 
 /// Signature for reporting errors when resolving images.
 ///
-/// Used by [ImageStream] and [precacheImage] to report errors.
+/// Used in [ImageStreamListener], as well as by [ImageCache.putIfAbsent] and
+/// [precacheImage], to report errors.
 typedef ImageErrorListener = void Function(dynamic exception, StackTrace stackTrace);
 
-class _ImageListenerPair {
-  _ImageListenerPair(this.listener, this.errorListener);
-  final ImageListener listener;
-  final ImageErrorListener errorListener;
-}
-
 /// A handle to an image resource.
 ///
 /// ImageStream represents a handle to a [dart:ui.Image] object and its scale
@@ -107,7 +157,7 @@ class ImageStream extends Diagnosticable {
   ImageStreamCompleter get completer => _completer;
   ImageStreamCompleter _completer;
 
-  List<_ImageListenerPair> _listeners;
+  List<ImageStreamListener> _listeners;
 
   /// Assigns a particular [ImageStreamCompleter] to this [ImageStream].
   ///
@@ -121,14 +171,9 @@ class ImageStream extends Diagnosticable {
     assert(_completer == null);
     _completer = value;
     if (_listeners != null) {
-      final List<_ImageListenerPair> initialListeners = _listeners;
+      final List<ImageStreamListener> initialListeners = _listeners;
       _listeners = null;
-      for (_ImageListenerPair listenerPair in initialListeners) {
-        _completer.addListener(
-          listenerPair.listener,
-          onError: listenerPair.errorListener,
-        );
-      }
+      initialListeners.forEach(_completer.addListener);
     }
   }
 
@@ -139,52 +184,34 @@ class ImageStream extends Diagnosticable {
   /// If the assigned [completer] completes multiple images over its lifetime,
   /// this listener will fire multiple times.
   ///
+  /// {@template flutter.painting.imageStream.addListener}
   /// The listener will be passed a flag indicating whether a synchronous call
   /// occurred. If the listener is added within a render object paint function,
   /// then use this flag to avoid calling [RenderObject.markNeedsPaint] during
   /// a paint.
   ///
-  /// An [ImageErrorListener] can also optionally be added along with the
-  /// `listener`. If an error occurred, `onError` will be called instead of
-  /// `listener`.
-  ///
-  /// If a `listener` or `onError` handler is registered multiple times, then it
-  /// will be called multiple times when the image stream completes (whether
-  /// because a new image is available or because an error occurs,
-  /// respectively). In general, registering a listener multiple times is
-  /// discouraged because [removeListener] will remove the first instance that
-  /// was added, even if it was added with a different `onError` than the
-  /// intended paired `addListener` call.
-  void addListener(ImageListener listener, { ImageErrorListener onError }) {
+  /// If a duplicate `listener` is registered N times, then it will be called N
+  /// times when the image stream completes (whether because a new image is
+  /// available or because an error occurs). Likewise, to remove all instances
+  /// of the listener, [removeListener] would need to called N times as well.
+  /// {@endtemplate}
+  void addListener(ImageStreamListener listener) {
     if (_completer != null)
-      return _completer.addListener(listener, onError: onError);
-    _listeners ??= <_ImageListenerPair>[];
-    _listeners.add(_ImageListenerPair(listener, onError));
+      return _completer.addListener(listener);
+    _listeners ??= <ImageStreamListener>[];
+    _listeners.add(listener);
   }
 
-  /// Stop listening for new concrete [ImageInfo] objects and errors from
-  /// the `listener`'s associated [ImageErrorListener].
-  ///
-  /// If `listener` has been added multiple times, this removes the first
-  /// instance of the listener, along with the `onError` listener that was
-  /// registered with that first instance. This might not be the instance that
-  /// the `addListener` corresponding to this `removeListener` had added.
-  ///
-  /// For example, if one widget calls [addListener] with a global static
-  /// function and a private error handler, and another widget calls
-  /// [addListener] with the same global static function but a different private
-  /// error handler, then the second widget is disposed and removes the image
-  /// listener (the aforementioned global static function), it will remove the
-  /// error handler from the first widget, not the second. If an error later
-  /// occurs, the first widget, which is still supposedly listening, will not
-  /// receive any messages, while the second, which is supposedly disposed, will
-  /// have its callback invoked.
-  void removeListener(ImageListener listener) {
+  /// Stops listening for events from this stream's [ImageStreamCompleter].
+  ///
+  /// If [listener] has been added multiple times, this removes the _first_
+  /// instance of the listener.
+  void removeListener(ImageStreamListener listener) {
     if (_completer != null)
       return _completer.removeListener(listener);
     assert(_listeners != null);
     for (int i = 0; i < _listeners.length; i += 1) {
-      if (_listeners[i].listener == listener) {
+      if (_listeners[i] == listener) {
         _listeners.removeAt(i);
         break;
       }
@@ -213,7 +240,7 @@ class ImageStream extends Diagnosticable {
       ifPresent: _completer?.toStringShort(),
       ifNull: 'unresolved',
     ));
-    properties.add(ObjectFlagProperty<List<_ImageListenerPair>>(
+    properties.add(ObjectFlagProperty<List<ImageStreamListener>>(
       'listeners',
       _listeners,
       ifPresent: '${_listeners?.length} listener${_listeners?.length == 1 ? "" : "s" }',
@@ -231,7 +258,7 @@ class ImageStream extends Diagnosticable {
 /// [ImageProvider] subclass will return an [ImageStream] and automatically
 /// configure it with the right [ImageStreamCompleter] when possible.
 abstract class ImageStreamCompleter extends Diagnosticable {
-  final List<_ImageListenerPair> _listeners = <_ImageListenerPair>[];
+  final List<ImageStreamListener> _listeners = <ImageStreamListener>[];
   ImageInfo _currentImage;
   FlutterErrorDetails _currentError;
 
@@ -246,42 +273,27 @@ abstract class ImageStreamCompleter extends Diagnosticable {
   ///
   /// Typically this is used by overriding [addListener], checking if
   /// [hasListeners] is false before calling `super.addListener()`, and if so,
-  /// starting whatever work is needed to determine when to call
-  /// [notifyListeners]; and similarly, by overriding [removeListener], checking
-  /// if [hasListeners] is false after calling `super.removeListener()`, and if
-  /// so, stopping that same work.
+  /// starting whatever work is needed to determine when to notify listeners;
+  /// and similarly, by overriding [removeListener], checking if [hasListeners]
+  /// is false after calling `super.removeListener()`, and if so, stopping that
+  /// same work.
   @protected
   bool get hasListeners => _listeners.isNotEmpty;
 
   /// Adds a listener callback that is called whenever a new concrete [ImageInfo]
   /// object is available or an error is reported. If a concrete image is
   /// already available, or if an error has been already reported, this object
-  /// will call the listener or error listener synchronously.
+  /// will notify the listener synchronously.
   ///
   /// If the [ImageStreamCompleter] completes multiple images over its lifetime,
-  /// this listener will fire multiple times.
-  ///
-  /// The listener will be passed a flag indicating whether a synchronous call
-  /// occurred. If the listener is added within a render object paint function,
-  /// then use this flag to avoid calling [RenderObject.markNeedsPaint] during
-  /// a paint.
+  /// this listener's [ImageStreamListener.onImage] will fire multiple times.
   ///
-  /// An [ImageErrorListener] can also optionally be added along with the
-  /// `listener`. If an error occurred, `onError` will be called instead of
-  /// `listener`.
-  ///
-  /// If a `listener` or `onError` handler is registered multiple times, then it
-  /// will be called multiple times when the image stream completes (whether
-  /// because a new image is available or because an error occurs,
-  /// respectively). In general, registering a listener multiple times is
-  /// discouraged because [removeListener] will remove the first instance that
-  /// was added, even if it was added with a different `onError` than the
-  /// intended paired `addListener` call.
-  void addListener(ImageListener listener, { ImageErrorListener onError }) {
-    _listeners.add(_ImageListenerPair(listener, onError));
+  /// {@macro flutter.painting.imageStream.addListener}
+  void addListener(ImageStreamListener listener) {
+    _listeners.add(listener);
     if (_currentImage != null) {
       try {
-        listener(_currentImage, true);
+        listener.onImage(_currentImage, true);
       } catch (exception, stack) {
         reportError(
           context: ErrorDescription('by a synchronously-called image listener'),
@@ -290,9 +302,9 @@ abstract class ImageStreamCompleter extends Diagnosticable {
         );
       }
     }
-    if (_currentError != null && onError != null) {
+    if (_currentError != null && listener.onError != null) {
       try {
-        onError(_currentError.exception, _currentError.stack);
+        listener.onError(_currentError.exception, _currentError.stack);
       } catch (exception, stack) {
         FlutterError.reportError(
           FlutterErrorDetails(
@@ -306,16 +318,13 @@ abstract class ImageStreamCompleter extends Diagnosticable {
     }
   }
 
-  /// Stop listening for new concrete [ImageInfo] objects and errors from
-  /// its associated [ImageErrorListener].
+  /// Stops the specified [listener] from receiving image stream events.
   ///
-  /// If `listener` has been added multiple times, this removes the first
-  /// instance of the listener, along with the `onError` listener that was
-  /// registered with that first instance. This might not be the instance that
-  /// the `addListener` corresponding to this `removeListener` had added.
-  void removeListener(ImageListener listener) {
+  /// If [listener] has been added multiple times, this removes the _first_
+  /// instance of the listener.
+  void removeListener(ImageStreamListener listener) {
     for (int i = 0; i < _listeners.length; i += 1) {
-      if (_listeners[i].listener == listener) {
+      if (_listeners[i] == listener) {
         _listeners.removeAt(i);
         break;
       }
@@ -328,12 +337,11 @@ abstract class ImageStreamCompleter extends Diagnosticable {
     _currentImage = image;
     if (_listeners.isEmpty)
       return;
-    final List<ImageListener> localListeners = _listeners.map<ImageListener>(
-      (_ImageListenerPair listenerPair) => listenerPair.listener
-    ).toList();
-    for (ImageListener listener in localListeners) {
+    final List<ImageStreamListener> localListeners =
+        List<ImageStreamListener>.from(_listeners);
+    for (ImageStreamListener listener in localListeners) {
       try {
-        listener(image, false);
+        listener.onImage(image, false);
       } catch (exception, stack) {
         reportError(
           context: ErrorDescription('by an image listener'),
@@ -347,8 +355,8 @@ abstract class ImageStreamCompleter extends Diagnosticable {
   /// Calls all the registered error listeners to notify them of an error that
   /// occurred while resolving the image.
   ///
-  /// If no error listeners are attached, a [FlutterError] will be reported
-  /// instead.
+  /// If no error listeners (listeners with an [ImageStreamListener.onError]
+  /// specified) are attached, a [FlutterError] will be reported instead.
   ///
   /// The `context` should be a string describing where the error was caught, in
   /// a form that will make sense in English when following the word "thrown",
@@ -389,12 +397,10 @@ abstract class ImageStreamCompleter extends Diagnosticable {
       silent: silent,
     );
 
-    final List<ImageErrorListener> localErrorListeners =
-      _listeners.map<ImageErrorListener>(
-        (_ImageListenerPair listenerPair) => listenerPair.errorListener
-      ).where(
-        (ImageErrorListener errorListener) => errorListener != null
-      ).toList();
+    final List<ImageErrorListener> localErrorListeners = _listeners
+        .map<ImageErrorListener>((ImageStreamListener listener) => listener.onError)
+        .where((ImageErrorListener errorListener) => errorListener != null)
+        .toList();
 
     if (localErrorListeners.isEmpty) {
       FlutterError.reportError(_currentError);
@@ -422,7 +428,7 @@ abstract class ImageStreamCompleter extends Diagnosticable {
   void debugFillProperties(DiagnosticPropertiesBuilder description) {
     super.debugFillProperties(description);
     description.add(DiagnosticsProperty<ImageInfo>('current', _currentImage, ifNull: 'unresolved', showName: false));
-    description.add(ObjectFlagProperty<List<_ImageListenerPair>>(
+    description.add(ObjectFlagProperty<List<ImageStreamListener>>(
       'listeners',
       _listeners,
       ifPresent: '${_listeners?.length} listener${_listeners?.length == 1 ? "" : "s" }',
@@ -610,14 +616,14 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
   }
 
   @override
-  void addListener(ImageListener listener, { ImageErrorListener onError }) {
+  void addListener(ImageStreamListener listener) {
     if (!hasListeners && _codec != null)
       _decodeNextFrameAndSchedule();
-    super.addListener(listener, onError: onError);
+    super.addListener(listener);
   }
 
   @override
-  void removeListener(ImageListener listener) {
+  void removeListener(ImageStreamListener listener) {
     super.removeListener(listener);
     if (!hasListeners) {
       _timer?.cancel();

packages/flutter/lib/src/widgets/fade_in_image.dart

@@ -374,8 +374,9 @@ class _ImageProviderResolver {
     assert(_imageStream != null);
 
     if (_imageStream.key != oldImageStream?.key) {
-      oldImageStream?.removeListener(_handleImageChanged);
-      _imageStream.addListener(_handleImageChanged);
+      final ImageStreamListener listener = ImageStreamListener(_handleImageChanged);
+      oldImageStream?.removeListener(listener);
+      _imageStream.addListener(listener);
     }
   }
 
@@ -385,7 +386,7 @@ class _ImageProviderResolver {
   }
 
   void stopListening() {
-    _imageStream?.removeListener(_handleImageChanged);
+    _imageStream?.removeListener(ImageStreamListener(_handleImageChanged));
   }
 }
 

packages/flutter/lib/src/widgets/image.dart

@@ -82,26 +82,29 @@ Future<void> precacheImage(
   final ImageConfiguration config = createLocalImageConfiguration(context, size: size);
   final Completer<void> completer = Completer<void>();
   final ImageStream stream = provider.resolve(config);
-  void listener(ImageInfo image, bool sync) {
-    completer.complete();
-    stream.removeListener(listener);
-  }
-  void errorListener(dynamic exception, StackTrace stackTrace) {
-    completer.complete();
-    stream.removeListener(listener);
-    if (onError != null) {
-      onError(exception, stackTrace);
-    } else {
-      FlutterError.reportError(FlutterErrorDetails(
-        context: ErrorDescription('image failed to precache'),
-        library: 'image resource service',
-        exception: exception,
-        stack: stackTrace,
-        silent: true,
-      ));
-    }
-  }
-  stream.addListener(listener, onError: errorListener);
+  ImageStreamListener listener;
+  listener = ImageStreamListener(
+    (ImageInfo image, bool sync) {
+      completer.complete();
+      stream.removeListener(listener);
+    },
+    onError: (dynamic exception, StackTrace stackTrace) {
+      completer.complete();
+      stream.removeListener(listener);
+      if (onError != null) {
+        onError(exception, stackTrace);
+      } else {
+        FlutterError.reportError(FlutterErrorDetails(
+          context: ErrorDescription('image failed to precache'),
+          library: 'image resource service',
+          exception: exception,
+          stack: stackTrace,
+          silent: true,
+        ));
+      }
+    },
+  );
+  stream.addListener(listener);
   return completer.future;
 }
 
@@ -656,28 +659,30 @@ class _ImageState extends State<Image> {
     if (_imageStream?.key == newStream?.key)
       return;
 
+    final ImageStreamListener listener = ImageStreamListener(_handleImageChanged);
+
     if (_isListeningToStream)
-      _imageStream.removeListener(_handleImageChanged);
+      _imageStream.removeListener(listener);
 
     if (!widget.gaplessPlayback)
       setState(() { _imageInfo = null; });
 
     _imageStream = newStream;
     if (_isListeningToStream)
-      _imageStream.addListener(_handleImageChanged);
+      _imageStream.addListener(listener);
   }
 
   void _listenToStream() {
     if (_isListeningToStream)
       return;
-    _imageStream.addListener(_handleImageChanged);
+    _imageStream.addListener(ImageStreamListener(_handleImageChanged));
     _isListeningToStream = true;
   }
 
   void _stopListeningToStream() {
     if (!_isListeningToStream)
       return;
-    _imageStream.removeListener(_handleImageChanged);
+    _imageStream.removeListener(ImageStreamListener(_handleImageChanged));
     _isListeningToStream = false;
   }
 

packages/flutter/test/painting/image_cache_test.dart

@@ -199,10 +199,15 @@ void main() {
     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);
-      });
+      const FailingTestImageProvider(1, 1, image: testImage)
+          .resolve(ImageConfiguration.empty)
+          .addListener(ImageStreamListener(
+            (ImageInfo image, bool synchronousCall) { },
+            onError: (dynamic exception, StackTrace stackTrace) {
+              final bool evicationResult = imageCache.evict(1);
+              expect(evicationResult, isTrue);
+            },
+          ));
     });
   });
 }

packages/flutter/test/painting/image_provider_test.dart

@@ -31,7 +31,7 @@ void main() {
         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());
+        stream.addListener(ImageStreamListener((ImageInfo info, bool syncCall) => completer.complete()));
         await completer.future;
 
         expect(imageCache.currentSize, 1);
@@ -46,7 +46,7 @@ void main() {
         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());
+        stream.addListener(ImageStreamListener((ImageInfo info, bool syncCall) => completer.complete()));
         await completer.future;
 
         expect(otherCache.currentSize, 1);
@@ -63,11 +63,11 @@ void main() {
           caughtError.complete(false);
         };
         final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
-        stream.addListener((ImageInfo info, bool syncCall) {
+        stream.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {
           caughtError.complete(false);
         }, onError: (dynamic error, StackTrace stackTrace) {
           caughtError.complete(true);
-        });
+        }));
         expect(await caughtError.future, true);
       });
     });
@@ -79,11 +79,11 @@ void main() {
         caughtError.complete(false);
       };
       final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
-      stream.addListener((ImageInfo info, bool syncCall) {
+      stream.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {
         caughtError.complete(false);
       }, onError: (dynamic error, StackTrace stackTrace) {
         caughtError.complete(true);
-      });
+      }));
       expect(await caughtError.future, true);
     });
 
@@ -101,10 +101,10 @@ void main() {
           throw Error();
         };
         final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
-        result.addListener((ImageInfo info, bool syncCall) {
+        result.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {
         }, onError: (dynamic error, StackTrace stackTrace) {
           caughtError.complete(true);
-        });
+        }));
         expect(await caughtError.future, true);
       });
       expect(uncaught, false);
@@ -124,10 +124,10 @@ void main() {
           throw Error();
         };
         final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
-        result.addListener((ImageInfo info, bool syncCall) {
+        result.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {
         }, onError: (dynamic error, StackTrace stackTrace) {
           caughtError.complete(true);
-        });
+        }));
         expect(await caughtError.future, true);
       });
       expect(uncaught, false);
@@ -158,12 +158,12 @@ void main() {
         Future<void> loadNetworkImage() async {
           final NetworkImage networkImage = NetworkImage(nonconst('foo'));
           final ImageStreamCompleter completer = networkImage.load(networkImage);
-          completer.addListener(
+          completer.addListener(ImageStreamListener(
             (ImageInfo image, bool synchronousCall) { },
             onError: (dynamic error, StackTrace stackTrace) {
               capturedErrors.add(error);
             },
-          );
+          ));
           await Future<void>.value();
         }
 
@@ -187,10 +187,10 @@ void main() {
             throw Error();
           };
           final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
-          result.addListener((ImageInfo info, bool syncCall) {
+          result.addListener(ImageStreamListener((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) {

packages/flutter/test/painting/image_stream_test.dart

@@ -103,7 +103,7 @@ void main() {
     expect(mockCodec.numFramesAsked, 0);
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
     await tester.idle();
     expect(mockCodec.numFramesAsked, 1);
   });
@@ -118,7 +118,7 @@ void main() {
     );
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
     await tester.idle();
     expect(mockCodec.numFramesAsked, 0);
 
@@ -138,7 +138,7 @@ void main() {
     );
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
     codecCompleter.complete(mockCodec);
     // MultiFrameImageStreamCompleter only sets an error handler for the next
     // frame future after the codec future has completed.
@@ -163,9 +163,9 @@ void main() {
     );
 
     final List<ImageInfo> emittedImages = <ImageInfo>[];
-    imageStream.addListener((ImageInfo image, bool synchronousCall) {
+    imageStream.addListener(ImageStreamListener((ImageInfo image, bool synchronousCall) {
       emittedImages.add(image);
-    });
+    }));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -189,9 +189,9 @@ void main() {
     );
 
     final List<ImageInfo> emittedImages = <ImageInfo>[];
-    imageStream.addListener((ImageInfo image, bool synchronousCall) {
+    imageStream.addListener(ImageStreamListener((ImageInfo image, bool synchronousCall) {
       emittedImages.add(image);
-    });
+    }));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -237,9 +237,9 @@ void main() {
     );
 
     final List<ImageInfo> emittedImages = <ImageInfo>[];
-    imageStream.addListener((ImageInfo image, bool synchronousCall) {
+    imageStream.addListener(ImageStreamListener((ImageInfo image, bool synchronousCall) {
       emittedImages.add(image);
-    });
+    }));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -280,9 +280,9 @@ void main() {
     );
 
     final List<ImageInfo> emittedImages = <ImageInfo>[];
-    imageStream.addListener((ImageInfo image, bool synchronousCall) {
+    imageStream.addListener(ImageStreamListener((ImageInfo image, bool synchronousCall) {
       emittedImages.add(image);
-    });
+    }));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -320,7 +320,7 @@ void main() {
     );
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -332,7 +332,7 @@ void main() {
     await tester.idle(); // let nextFrameFuture complete
     await tester.pump(); // first animation frame shows on first app frame.
     mockCodec.completeNextFrame(frame2);
-    imageStream.removeListener(listener);
+    imageStream.removeListener(ImageStreamListener(listener));
     await tester.idle(); // let nextFrameFuture complete
     await tester.pump(const Duration(milliseconds: 400)); // emit 2nd frame.
 
@@ -340,7 +340,7 @@ void main() {
     // listeners to the stream
     expect(mockCodec.numFramesAsked, 2);
 
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
     await tester.idle(); // let nextFrameFuture complete
     expect(mockCodec.numFramesAsked, 3);
   });
@@ -364,8 +364,8 @@ void main() {
     final ImageListener listener2 = (ImageInfo image, bool synchronousCall) {
       emittedImages2.add(image);
     };
-    imageStream.addListener(listener1);
-    imageStream.addListener(listener2);
+    imageStream.addListener(ImageStreamListener(listener1));
+    imageStream.addListener(ImageStreamListener(listener2));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -382,7 +382,7 @@ void main() {
     mockCodec.completeNextFrame(frame2);
     await tester.idle(); // let nextFrameFuture complete
     await tester.pump(); // next app frame will schedule a timer.
-    imageStream.removeListener(listener1);
+    imageStream.removeListener(ImageStreamListener(listener1));
 
     await tester.pump(const Duration(milliseconds: 400)); // emit 2nd frame.
     expect(emittedImages1, equals(<ImageInfo>[ImageInfo(image: frame1.image)]));
@@ -404,7 +404,7 @@ void main() {
     );
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -420,7 +420,7 @@ void main() {
     await tester.idle(); // let nextFrameFuture complete
     await tester.pump();
 
-    imageStream.removeListener(listener);
+    imageStream.removeListener(ImageStreamListener(listener));
     // The test framework will fail this if there are pending timers at this
     // point.
   });
@@ -437,7 +437,7 @@ void main() {
     );
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
 
     codecCompleter.complete(mockCodec);
     await tester.idle();
@@ -476,10 +476,10 @@ void main() {
       capturedException = exception;
     };
 
-    streamUnderTest.addListener(
+    streamUnderTest.addListener(ImageStreamListener(
       (ImageInfo image, bool synchronousCall) { },
       onError: errorListener,
-    );
+    ));
 
     codecCompleter.complete(mockCodec);
     // MultiFrameImageStreamCompleter only sets an error handler for the next
@@ -508,14 +508,14 @@ void main() {
     );
 
     final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-    imageStream.addListener(listener);
+    imageStream.addListener(ImageStreamListener(listener));
 
     codecCompleter.complete(mockCodec);
 
     await tester.idle(); // let nextFrameFuture complete
 
-    imageStream.removeListener(listener);
-    imageStream.addListener(listener);
+    imageStream.removeListener(ImageStreamListener(listener));
+    imageStream.addListener(ImageStreamListener(listener));
 
 
     final FrameInfo frame1 = FakeFrameInfo(20, 10, const Duration(milliseconds: 200));
@@ -541,7 +541,7 @@ void main() {
   //   );
   //
   //   final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
-  //   imageStream.addListener(listener);
+  //   imageStream.addListener(ImageLoadingListener(listener));
   //
   //   codecCompleter.complete(mockCodec);
   //   await tester.idle();
@@ -560,7 +560,7 @@ void main() {
   //   tester.flushTimers();
   //
   //   imageStream.removeListener(listener);
-  //   imageStream.addListener(listener);
+  //   imageStream.addListener(ImageLoadingListener(listener));
   //
   //   mockCodec.completeNextFrame(frame3);
   //   await tester.idle(); // let nextFrameFuture complete

packages/flutter/test/painting/mocks_for_image_cache.dart

@@ -58,10 +58,11 @@ class FailingTestImageProvider extends TestImageProvider {
 
 Future<ImageInfo> extractOneFrame(ImageStream stream) {
   final Completer<ImageInfo> completer = Completer<ImageInfo>();
-  void listener(ImageInfo image, bool synchronousCall) {
+  ImageStreamListener listener;
+  listener = ImageStreamListener((ImageInfo image, bool synchronousCall) {
     completer.complete(image);
     stream.removeListener(listener);
-  }
+  });
   stream.addListener(listener);
   return completer.future;
 }

packages/flutter/test/widgets/image_test.dart

@@ -331,7 +331,7 @@ void main() {
     final Exception testException = Exception('cannot resolve host');
     final StackTrace testStack = StackTrace.current;
     final TestImageProvider imageProvider = TestImageProvider();
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     ImageConfiguration configuration;
     await tester.pumpWidget(
       Builder(
@@ -399,7 +399,7 @@ void main() {
     expect(reportedException, testException);
     expect(reportedStackTrace, testStack);
 
-    streamUnderTest.addListener(listener, onError: errorListener);
+    streamUnderTest.addListener(ImageStreamListener(listener, onError: errorListener));
 
     expect(capturedImage, isNull); // The image stream listeners should never be called.
     // The image stream error handler should have the original exception.
@@ -422,9 +422,9 @@ void main() {
     final Exception testException = Exception('cannot resolve host');
     final StackTrace testStack = StackTrace.current;
     final TestImageProvider imageProvider = TestImageProvider();
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     // Add the exact same listener a second time without the errorListener.
-    imageProvider._streamCompleter.addListener(listener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener));
     ImageConfiguration configuration;
     await tester.pumpWidget(
       Builder(
@@ -466,9 +466,9 @@ void main() {
     final Exception testException = Exception('cannot resolve host');
     final StackTrace testStack = StackTrace.current;
     final TestImageProvider imageProvider = TestImageProvider();
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     // Add the exact same errorListener a second time.
-    imageProvider._streamCompleter.addListener(null, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     ImageConfiguration configuration;
     await tester.pumpWidget(
       Builder(
@@ -494,29 +494,27 @@ void main() {
     expect(tester.takeException(), isNull);
   });
 
-  testWidgets('Error listeners are removed along with listeners', (WidgetTester tester) async {
+  testWidgets('Listeners are only removed if callback tuple matches', (WidgetTester tester) async {
     bool errorListenerCalled = false;
     dynamic reportedException;
     StackTrace reportedStackTrace;
     ImageInfo capturedImage;
     final ImageErrorListener errorListener = (dynamic exception, StackTrace stackTrace) {
       errorListenerCalled = true;
+      reportedException = exception;
+      reportedStackTrace = stackTrace;
     };
     final ImageListener listener = (ImageInfo info, bool synchronous) {
       capturedImage = info;
     };
-    FlutterError.onError = (FlutterErrorDetails flutterError) {
-      reportedException = flutterError.exception;
-      reportedStackTrace = flutterError.stack;
-    };
 
     final Exception testException = Exception('cannot resolve host');
     final StackTrace testStack = StackTrace.current;
     final TestImageProvider imageProvider = TestImageProvider();
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     // Now remove the listener the error listener is attached to.
     // Don't explicitly remove the error listener.
-    imageProvider._streamCompleter.removeListener(listener);
+    imageProvider._streamCompleter.removeListener(ImageStreamListener(listener));
     ImageConfiguration configuration;
     await tester.pumpWidget(
       Builder(
@@ -534,8 +532,7 @@ void main() {
     await tester.idle(); // Let the failed completer's future hit the stream completer.
     expect(tester.binding.microtaskCount, 0);
 
-    expect(errorListenerCalled, false);
-    // Since the error listener is removed, bubble up to FlutterError.
+    expect(errorListenerCalled, true);
     expect(reportedException, testException);
     expect(reportedStackTrace, testStack);
     expect(capturedImage, isNull); // The image stream listeners should never be called.
@@ -554,12 +551,12 @@ void main() {
     final Exception testException = Exception('cannot resolve host');
     final StackTrace testStack = StackTrace.current;
     final TestImageProvider imageProvider = TestImageProvider();
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     // Duplicates the same set of listener and errorListener.
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener);
+    imageProvider._streamCompleter.addListener(ImageStreamListener(listener, onError: errorListener));
     // Now remove one entry of the specified listener and associated error listener.
     // Don't explicitly remove the error listener.
-    imageProvider._streamCompleter.removeListener(listener);
+    imageProvider._streamCompleter.removeListener(ImageStreamListener(listener, onError: errorListener));
     ImageConfiguration configuration;
     await tester.pumpWidget(
       Builder(
@@ -581,58 +578,6 @@ void main() {
     expect(capturedImage, isNull); // The image stream listeners should never be called.
   });
 
-  testWidgets('Removing listener FIFO removes exactly one listener and error listener', (WidgetTester tester) async {
-    // To make sure that a single listener removal doesn't only happen
-    // accidentally as described in https://github.com/flutter/flutter/pull/25865#discussion_r244851565.
-    int errorListener1Called = 0;
-    int errorListener2Called = 0;
-    int errorListener3Called = 0;
-    ImageInfo capturedImage;
-    final ImageErrorListener errorListener1 = (dynamic exception, StackTrace stackTrace) {
-      errorListener1Called++;
-    };
-    final ImageErrorListener errorListener2 = (dynamic exception, StackTrace stackTrace) {
-      errorListener2Called++;
-    };
-    final ImageErrorListener errorListener3 = (dynamic exception, StackTrace stackTrace) {
-      errorListener3Called++;
-    };
-    final ImageListener listener = (ImageInfo info, bool synchronous) {
-      capturedImage = info;
-    };
-
-    final Exception testException = Exception('cannot resolve host');
-    final StackTrace testStack = StackTrace.current;
-    final TestImageProvider imageProvider = TestImageProvider();
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener1);
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener2);
-    imageProvider._streamCompleter.addListener(listener, onError: errorListener3);
-    // Remove listener. It should remove exactly the first one and the associated
-    // errorListener1.
-    imageProvider._streamCompleter.removeListener(listener);
-    ImageConfiguration configuration;
-    await tester.pumpWidget(
-      Builder(
-        builder: (BuildContext context) {
-          configuration = createLocalImageConfiguration(context);
-          return Container();
-        },
-      ),
-    );
-    imageProvider.resolve(configuration);
-
-    imageProvider.fail(testException, testStack);
-
-    expect(tester.binding.microtaskCount, 1);
-    await tester.idle(); // Let the failed completer's future hit the stream completer.
-    expect(tester.binding.microtaskCount, 0);
-
-    expect(errorListener1Called, 0);
-    expect(errorListener2Called, 1);
-    expect(errorListener3Called, 1);
-    expect(capturedImage, isNull); // The image stream listeners should never be called.
-  });
-
   testWidgets('Image.memory control test', (WidgetTester tester) async {
     await tester.pumpWidget(Image.memory(Uint8List.fromList(kTransparentImage), excludeFromSemantics: true,));
   });
@@ -669,7 +614,7 @@ void main() {
     // Check that a second resolve of the same image is synchronous.
     final ImageStream stream = provider.resolve(provider._lastResolvedConfiguration);
     bool isSync;
-    stream.addListener((ImageInfo image, bool sync) { isSync = sync; });
+    stream.addListener(ImageStreamListener((ImageInfo image, bool sync) { isSync = sync; }));
     expect(isSync, isTrue);
   });
 
@@ -687,10 +632,10 @@ void main() {
     );
 
     expect(imageStreamCompleter.listeners.length, 2);
-    imageStreamCompleter.listeners.keys.toList()[1](null, null);
+    imageStreamCompleter.listeners.toList()[1].onImage(null, null);
 
     expect(imageStreamCompleter.listeners.length, 1);
-    imageStreamCompleter.listeners.keys.toList()[0](null, null);
+    imageStreamCompleter.listeners.toList()[0].onImage(null, null);
 
     expect(imageStreamCompleter.listeners.length, 0);
   });
@@ -902,15 +847,15 @@ class TestImageProvider extends ImageProvider<TestImageProvider> {
 }
 
 class TestImageStreamCompleter extends ImageStreamCompleter {
-  final Map<ImageListener, ImageErrorListener> listeners = <ImageListener, ImageErrorListener>{};
+  final Set<ImageStreamListener> listeners = <ImageStreamListener>{};
 
   @override
-  void addListener(ImageListener listener, { ImageErrorListener onError }) {
-    listeners[listener] = onError;
+  void addListener(ImageStreamListener listener) {
+    listeners.add(listener);
   }
 
   @override
-  void removeListener(ImageListener listener) {
+  void removeListener(ImageStreamListener listener) {
     listeners.remove(listener);
   }
 }