Unverified Commit 168d6e3e authored by xster's avatar xster Committed by GitHub

Follow up on #18424 comments (#20125)

parent 686d8f8a
...@@ -151,8 +151,6 @@ class ImageStream extends Diagnosticable { ...@@ -151,8 +151,6 @@ class ImageStream extends Diagnosticable {
/// Many `listener`s can have the same `onError` and one `listener` can also /// Many `listener`s can have the same `onError` and one `listener` can also
/// have multiple `onError` by invoking [addListener] multiple times with /// have multiple `onError` by invoking [addListener] multiple times with
/// a different `onError` each time. /// a different `onError` each time.
///
/// Repeated `onError` will only be called once when an error occurs.
void addListener(ImageListener listener, { ImageErrorListener onError }) { void addListener(ImageListener listener, { ImageErrorListener onError }) {
if (_completer != null) if (_completer != null)
return _completer.addListener(listener, onError: onError); return _completer.addListener(listener, onError: onError);
...@@ -166,9 +164,12 @@ class ImageStream extends Diagnosticable { ...@@ -166,9 +164,12 @@ class ImageStream extends Diagnosticable {
if (_completer != null) if (_completer != null)
return _completer.removeListener(listener); return _completer.removeListener(listener);
assert(_listeners != null); assert(_listeners != null);
_listeners.removeWhere((_ImageListenerPair listenerPair) { for (int i = 0; i < _listeners.length; ++i) {
return listenerPair.listener == listener; if (_listeners[i].listener == listener) {
}); _listeners.removeAt(i);
continue;
}
}
} }
/// Returns an object which can be used with `==` to determine if this /// Returns an object which can be used with `==` to determine if this
...@@ -242,26 +243,29 @@ abstract class ImageStreamCompleter extends Diagnosticable { ...@@ -242,26 +243,29 @@ abstract class ImageStreamCompleter extends Diagnosticable {
} }
if (_currentError != null && onError != null) { if (_currentError != null && onError != null) {
try { try {
onError(_currentError.exception, _currentError.stack); onError(_currentError.exception, _currentError.stack);
} catch (exception, stack) { } catch (exception, stack) {
FlutterError.reportError( FlutterError.reportError(
new FlutterErrorDetails( new FlutterErrorDetails(
exception: exception, exception: exception,
library: 'image resource service', library: 'image resource service',
context: 'by a synchronously-called image error listener', context: 'by a synchronously-called image error listener',
stack: stack, stack: stack,
), ),
); );
} }
} }
} }
/// Stop listening for new concrete [ImageInfo] objects and errors from /// Stop listening for new concrete [ImageInfo] objects and errors from
/// its associated [ImageErrorListener]. /// its associated [ImageErrorListener].
void removeListener(ImageListener listener) { void removeListener(ImageListener listener) {
_listeners.removeWhere((_ImageListenerPair listenerPair) { for (int i = 0; i < _listeners.length; ++i) {
return listenerPair.listener == listener; if (_listeners[i].listener == listener) {
}); _listeners.removeAt(i);
continue;
}
}
} }
/// Calls all the registered listeners to notify them of a new image. /// Calls all the registered listeners to notify them of a new image.
...@@ -289,9 +293,6 @@ abstract class ImageStreamCompleter extends Diagnosticable { ...@@ -289,9 +293,6 @@ abstract class ImageStreamCompleter extends Diagnosticable {
/// Calls all the registered error listeners to notify them of an error that /// Calls all the registered error listeners to notify them of an error that
/// occurred while resolving the image. /// occurred while resolving the image.
/// ///
/// If the same error listener is attached with multiple listeners, that
/// error listener will only be notified once.
///
/// If no error listeners are attached, a [FlutterError] will be reported /// If no error listeners are attached, a [FlutterError] will be reported
/// instead. /// instead.
@protected @protected
...@@ -311,7 +312,6 @@ abstract class ImageStreamCompleter extends Diagnosticable { ...@@ -311,7 +312,6 @@ abstract class ImageStreamCompleter extends Diagnosticable {
silent: silent, silent: silent,
); );
// Many listeners can have the same error listener. De-duplicate.
final List<ImageErrorListener> localErrorListeners = final List<ImageErrorListener> localErrorListeners =
_listeners.map<ImageErrorListener>( _listeners.map<ImageErrorListener>(
(_ImageListenerPair listenerPair) => listenerPair.errorListener (_ImageListenerPair listenerPair) => listenerPair.errorListener
......
...@@ -541,21 +541,15 @@ void main() { ...@@ -541,21 +541,15 @@ void main() {
expect(capturedImage, isNull); // The image stream listeners should never be called. expect(capturedImage, isNull); // The image stream listeners should never be called.
}); });
testWidgets('Removing duplicate listeners removes error listeners', (WidgetTester tester) async { testWidgets('Removing listener removes one listener and error listener', (WidgetTester tester) async {
bool errorListenerCalled = false; int errorListenerCalled = 0;
dynamic reportedException;
StackTrace reportedStackTrace;
ImageInfo capturedImage; ImageInfo capturedImage;
final ImageErrorListener errorListener = (dynamic exception, StackTrace stackTrace) { final ImageErrorListener errorListener = (dynamic exception, StackTrace stackTrace) {
errorListenerCalled = true; errorListenerCalled++;
}; };
final ImageListener listener = (ImageInfo info, bool synchronous) { final ImageListener listener = (ImageInfo info, bool synchronous) {
capturedImage = info; capturedImage = info;
}; };
FlutterError.onError = (FlutterErrorDetails flutterError) {
reportedException = flutterError.exception;
reportedStackTrace = flutterError.stack;
};
final Exception testException = new Exception('cannot resolve host'); final Exception testException = new Exception('cannot resolve host');
final StackTrace testStack = StackTrace.current; final StackTrace testStack = StackTrace.current;
...@@ -563,7 +557,7 @@ void main() { ...@@ -563,7 +557,7 @@ void main() {
imageProvider._streamCompleter.addListener(listener, onError: errorListener); imageProvider._streamCompleter.addListener(listener, onError: errorListener);
// Duplicates the same set of listener and errorListener. // Duplicates the same set of listener and errorListener.
imageProvider._streamCompleter.addListener(listener, onError: errorListener); imageProvider._streamCompleter.addListener(listener, onError: errorListener);
// Now remove all specified listeners and associated error listeners. // Now remove one entry of the specified listener and associated error listener.
// Don't explicitly remove the error listener. // Don't explicitly remove the error listener.
imageProvider._streamCompleter.removeListener(listener); imageProvider._streamCompleter.removeListener(listener);
ImageConfiguration configuration; ImageConfiguration configuration;
...@@ -583,10 +577,7 @@ void main() { ...@@ -583,10 +577,7 @@ void main() {
await tester.idle(); // Let the failed completer's future hit the stream completer. await tester.idle(); // Let the failed completer's future hit the stream completer.
expect(tester.binding.microtaskCount, 0); expect(tester.binding.microtaskCount, 0);
expect(errorListenerCalled, false); expect(errorListenerCalled, 1);
// Since the error listener is removed, bubble up to FlutterError.
expect(reportedException, testException);
expect(reportedStackTrace, testStack);
expect(capturedImage, isNull); // The image stream listeners should never be called. expect(capturedImage, isNull); // The image stream listeners should never be called.
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment