Skip to content

Commit

Permalink
When resized network image has error, all future unrelated images usi…
Browse files Browse the repository at this point in the history 
…ng 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!
  • Loading branch information
@fzyzcjy
fzyzcjy authored Aug 21, 2023
1 parent 27dd111 commit aeddab4
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 8 deletions.
15 changes: 15 additions & 0 deletions packages/flutter/lib/src/painting/image_provider.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
81 changes: 77 additions & 4 deletions packages/flutter/lib/src/painting/image_stream.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -595,6 +656,7 @@ abstract class ImageStreamCompleter with Diagnosticable {
return;
}

_ephemeralErrorListeners.clear();
_currentImage?.dispose();
_currentImage = null;
_disposed = true;
Expand Down Expand Up @@ -640,6 +702,8 @@ abstract class ImageStreamCompleter with Diagnosticable {
_currentImage?.dispose();
_currentImage = image;

_ephemeralErrorListeners.clear();

if (_listeners.isEmpty) {
return;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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>'));
}
}
Expand Down
38 changes: 37 additions & 1 deletion packages/flutter/test/painting/image_provider_network_image_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>[];
Expand Down Expand Up @@ -180,7 +216,7 @@ void main() {
},
));

final dynamic err = await caughtError.future;
final Object? err = await caughtError.future;

expect(err, isA<SocketException>());

Expand Down
6 changes: 3 additions & 3 deletions packages/flutter/test/widgets/image_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit aeddab4

Please sign in to comment.