Unverified Commit aeddab42 authored by fzyzcjy's avatar fzyzcjy Committed by GitHub

When resized network image has error, all future unrelated images using the...

When resized network image has error, all future unrelated images using the same url will fail, even if the network becomes OK (#127456)

Close #127265

The CI fails because of simple analyzer errors. Thus, I would like to hear your opinions first!
parent 27dd111a
......@@ -1355,6 +1355,7 @@ class ResizeImage extends ImageProvider<ResizeImageKey> {
if (!kReleaseMode) {
completer.debugLabel = '${completer.debugLabel} - Resized(${key._width}×${key._height})';
}
_configureErrorListener(completer, key);
return completer;
}
......@@ -1377,6 +1378,7 @@ class ResizeImage extends ImageProvider<ResizeImageKey> {
if (!kReleaseMode) {
completer.debugLabel = '${completer.debugLabel} - Resized(${key._width}×${key._height})';
}
_configureErrorListener(completer, key);
return completer;
}
......@@ -1446,9 +1448,22 @@ class ResizeImage extends ImageProvider<ResizeImageKey> {
if (!kReleaseMode) {
completer.debugLabel = '${completer.debugLabel} - Resized(${key._width}×${key._height})';
}
_configureErrorListener(completer, key);
return completer;
}
void _configureErrorListener(ImageStreamCompleter completer, ResizeImageKey key) {
completer.addEphemeralErrorListener((Object exception, StackTrace? stackTrace) {
// The microtask is scheduled because of the same reason as NetworkImage:
// Depending on where the exception was thrown, the image cache may not
// have had a chance to track the key in the cache at all.
// Schedule a microtask to give the cache a chance to add the key.
scheduleMicrotask(() {
PaintingBinding.instance.imageCache.evict(key);
});
});
}
@override
Future<ResizeImageKey> obtainKey(ImageConfiguration configuration) {
Completer<ResizeImageKey>? completer;
......
......@@ -468,6 +468,7 @@ class ImageStreamCompleterHandle {
/// configure it with the right [ImageStreamCompleter] when possible.
abstract class ImageStreamCompleter with Diagnosticable {
final List<ImageStreamListener> _listeners = <ImageStreamListener>[];
final List<ImageErrorListener> _ephemeralErrorListeners = <ImageErrorListener>[];
ImageInfo? _currentImage;
FlutterErrorDetails? _currentError;
......@@ -489,6 +490,9 @@ abstract class ImageStreamCompleter with Diagnosticable {
/// and similarly, by overriding [removeListener], checking if [hasListeners]
/// is false after calling `super.removeListener()`, and if so, stopping that
/// same work.
///
/// The ephemeral error listeners (added through [addEphemeralErrorListener])
/// will not be taken into consideration in this property.
@protected
@visibleForTesting
bool get hasListeners => _listeners.isNotEmpty;
......@@ -515,6 +519,11 @@ abstract class ImageStreamCompleter with Diagnosticable {
/// this listener's [ImageStreamListener.onImage] will fire multiple times.
///
/// {@macro flutter.painting.imageStream.addListener}
///
/// See also:
///
/// * [addEphemeralErrorListener], which adds an error listener that is
/// automatically removed after first image load or error.
void addListener(ImageStreamListener listener) {
_checkDisposed();
_hadAtLeastOneListener = true;
......@@ -548,6 +557,58 @@ abstract class ImageStreamCompleter with Diagnosticable {
}
}
/// Adds an error listener callback that is called when the first error is reported.
///
/// The callback will be removed automatically after the first successful
/// image load or the first error - that is why it is called "ephemeral".
///
/// If a concrete image is already available, the listener will be discarded
/// synchronously. If an error has been already reported, the listener
/// will be notified synchronously.
///
/// The presence of a listener will affect neither the lifecycle of this object
/// nor what [hasListeners] reports.
///
/// It is different from [addListener] in a few points: Firstly, this one only
/// listens to errors, while [addListener] listens to all kinds of events.
/// Secondly, this listener will be automatically removed according to the
/// rules mentioned above, while [addListener] will need manual removal.
/// Thirdly, this listener will not affect how this object is disposed, while
/// any non-removed listener added via [addListener] will forbid this object
/// from disposal.
///
/// When you want to know full information and full control, use [addListener].
/// When you only want to get notified for an error ephemerally, use this function.
///
/// See also:
///
/// * [addListener], which adds a full-featured listener and needs manual
/// removal.
void addEphemeralErrorListener(ImageErrorListener listener) {
_checkDisposed();
if (_currentError != null) {
// immediately fire the listener, and no need to add to _ephemeralErrorListeners
try {
listener(_currentError!.exception, _currentError!.stack);
} catch (newException, newStack) {
if (newException != _currentError!.exception) {
FlutterError.reportError(
FlutterErrorDetails(
exception: newException,
library: 'image resource service',
context: ErrorDescription('by a synchronously-called image error listener'),
stack: newStack,
),
);
}
}
} else if (_currentImage == null) {
// add to _ephemeralErrorListeners to wait for the error,
// only if no image has been loaded
_ephemeralErrorListeners.add(listener);
}
}
int _keepAliveHandles = 0;
/// Creates an [ImageStreamCompleterHandle] that will prevent this stream from
/// being disposed at least until the handle is disposed.
......@@ -595,6 +656,7 @@ abstract class ImageStreamCompleter with Diagnosticable {
return;
}
_ephemeralErrorListeners.clear();
_currentImage?.dispose();
_currentImage = null;
_disposed = true;
......@@ -640,6 +702,8 @@ abstract class ImageStreamCompleter with Diagnosticable {
_currentImage?.dispose();
_currentImage = image;
_ephemeralErrorListeners.clear();
if (_listeners.isEmpty) {
return;
}
......@@ -707,10 +771,14 @@ abstract class ImageStreamCompleter with Diagnosticable {
);
// Make a copy to allow for concurrent modification.
final List<ImageErrorListener> localErrorListeners = _listeners
.map<ImageErrorListener?>((ImageStreamListener listener) => listener.onError)
.whereType<ImageErrorListener>()
.toList();
final List<ImageErrorListener> localErrorListeners = <ImageErrorListener>[
..._listeners
.map<ImageErrorListener?>((ImageStreamListener listener) => listener.onError)
.whereType<ImageErrorListener>(),
..._ephemeralErrorListeners,
];
_ephemeralErrorListeners.clear();
bool handled = false;
for (final ImageErrorListener errorListener in localErrorListeners) {
......@@ -764,6 +832,11 @@ abstract class ImageStreamCompleter with Diagnosticable {
_listeners,
ifPresent: '${_listeners.length} listener${_listeners.length == 1 ? "" : "s" }',
));
description.add(ObjectFlagProperty<List<ImageErrorListener>>(
'ephemeralErrorListeners',
_ephemeralErrorListeners,
ifPresent: '${_ephemeralErrorListeners.length} ephemeralErrorListener${_ephemeralErrorListeners.length == 1 ? "" : "s" }',
));
description.add(FlagProperty('disposed', value: _disposed, ifTrue: '<disposed>'));
}
}
......
......@@ -73,6 +73,42 @@ void main() {
expect(httpClient.request.response.drained, true);
}, skip: isBrowser); // [intended] Browser implementation does not use HTTP client but an <img> tag.
test('Expect thrown exception with statusCode - evicts from cache and drains, when using ResizeImage', () async {
const int errorStatusCode = HttpStatus.notFound;
const String requestUrl = 'foo-url';
httpClient.request.response.statusCode = errorStatusCode;
final Completer<dynamic> caughtError = Completer<dynamic>();
final ImageProvider imageProvider = ResizeImage(NetworkImage(nonconst(requestUrl)), width: 5, height: 5);
expect(imageCache.pendingImageCount, 0);
expect(imageCache.statusForKey(imageProvider).untracked, true);
final ImageStream result = imageProvider.resolve(ImageConfiguration.empty);
expect(imageCache.pendingImageCount, 1);
result.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {},
onError: (dynamic error, StackTrace? stackTrace) {
caughtError.complete(error);
}));
final Object? err = await caughtError.future;
await Future<void>.delayed(Duration.zero);
expect(imageCache.pendingImageCount, 0);
expect(imageCache.statusForKey(imageProvider).untracked, true);
expect(
err,
isA<NetworkImageLoadException>()
.having((NetworkImageLoadException e) => e.statusCode, 'statusCode', errorStatusCode)
.having((NetworkImageLoadException e) => e.uri, 'uri', Uri.base.resolve(requestUrl)),
);
expect(httpClient.request.response.drained, true);
}, skip: isBrowser); // [intended] Browser implementation does not use HTTP client but an <img> tag.
test('Uses the HttpClient provided by debugNetworkImageHttpClientProvider if set', () async {
httpClient.thrownError = 'client1';
final List<dynamic> capturedErrors = <dynamic>[];
......@@ -180,7 +216,7 @@ void main() {
},
));
final dynamic err = await caughtError.future;
final Object? err = await caughtError.future;
expect(err, isA<SocketException>());
......
......@@ -501,12 +501,12 @@ void main() {
final _TestImageProvider imageProvider = _TestImageProvider();
await tester.pumpWidget(Image(image: imageProvider, excludeFromSemantics: true));
final State<Image> image = tester.state/*State<Image>*/(find.byType(Image));
expect(image.toString(), equalsIgnoringHashCodes('_ImageState#00000(stream: ImageStream#00000(OneFrameImageStreamCompleter#00000, unresolved, 2 listeners), pixels: null, loadingProgress: null, frameNumber: null, wasSynchronouslyLoaded: false)'));
expect(image.toString(), equalsIgnoringHashCodes('_ImageState#00000(stream: ImageStream#00000(OneFrameImageStreamCompleter#00000, unresolved, 2 listeners, 0 ephemeralErrorListeners), pixels: null, loadingProgress: null, frameNumber: null, wasSynchronouslyLoaded: false)'));
imageProvider.complete(image100x100);
await tester.pump();
expect(image.toString(), equalsIgnoringHashCodes('_ImageState#00000(stream: ImageStream#00000(OneFrameImageStreamCompleter#00000, $imageString @ 1.0x, 1 listener), pixels: $imageString @ 1.0x, loadingProgress: null, frameNumber: 0, wasSynchronouslyLoaded: false)'));
expect(image.toString(), equalsIgnoringHashCodes('_ImageState#00000(stream: ImageStream#00000(OneFrameImageStreamCompleter#00000, $imageString @ 1.0x, 1 listener, 0 ephemeralErrorListeners), pixels: $imageString @ 1.0x, loadingProgress: null, frameNumber: 0, wasSynchronouslyLoaded: false)'));
await tester.pumpWidget(Container());
expect(image.toString(), equalsIgnoringHashCodes('_ImageState#00000(lifecycle state: defunct, not mounted, stream: ImageStream#00000(OneFrameImageStreamCompleter#00000, $imageString @ 1.0x, 0 listeners), pixels: null, loadingProgress: null, frameNumber: 0, wasSynchronouslyLoaded: false)'));
expect(image.toString(), equalsIgnoringHashCodes('_ImageState#00000(lifecycle state: defunct, not mounted, stream: ImageStream#00000(OneFrameImageStreamCompleter#00000, $imageString @ 1.0x, 0 listeners, 0 ephemeralErrorListeners), pixels: null, loadingProgress: null, frameNumber: 0, wasSynchronouslyLoaded: false)'));
});
testWidgets('Stream completer errors can be listened to by attaching before resolving', (WidgetTester tester) async {
......
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