Unverified Commit 66659229 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Show an X when images can't load. (#74972)

parent 69882d96
...@@ -10,6 +10,15 @@ ...@@ -10,6 +10,15 @@
/// Since this is a const value, it can be used to indicate to the compiler that /// Since this is a const value, it can be used to indicate to the compiler that
/// a particular block of code will not be executed in release mode, and hence /// a particular block of code will not be executed in release mode, and hence
/// can be removed. /// can be removed.
///
/// Generally it is better to use [kDebugMode] or `assert` to gate code, since
/// using [kReleaseMode] will introduce differences between release and profile
/// builds, which makes performance testing less representative.
///
/// See also:
///
/// * [kDebugMode], which is true in debug builds.
/// * [kProfileMode], which is true in profile builds.
const bool kReleaseMode = bool.fromEnvironment('dart.vm.product', defaultValue: false); const bool kReleaseMode = bool.fromEnvironment('dart.vm.product', defaultValue: false);
/// A constant that is true if the application was compiled in profile mode. /// A constant that is true if the application was compiled in profile mode.
...@@ -20,6 +29,11 @@ const bool kReleaseMode = bool.fromEnvironment('dart.vm.product', defaultValue: ...@@ -20,6 +29,11 @@ const bool kReleaseMode = bool.fromEnvironment('dart.vm.product', defaultValue:
/// Since this is a const value, it can be used to indicate to the compiler that /// Since this is a const value, it can be used to indicate to the compiler that
/// a particular block of code will not be executed in profile mode, an hence /// a particular block of code will not be executed in profile mode, an hence
/// can be removed. /// can be removed.
///
/// See also:
///
/// * [kDebugMode], which is true in debug builds.
/// * [kReleaseMode], which is true in release builds.
const bool kProfileMode = bool.fromEnvironment('dart.vm.profile', defaultValue: false); const bool kProfileMode = bool.fromEnvironment('dart.vm.profile', defaultValue: false);
/// A constant that is true if the application was compiled in debug mode. /// A constant that is true if the application was compiled in debug mode.
...@@ -30,6 +44,20 @@ const bool kProfileMode = bool.fromEnvironment('dart.vm.profile', defaultValue: ...@@ -30,6 +44,20 @@ const bool kProfileMode = bool.fromEnvironment('dart.vm.profile', defaultValue:
/// Since this is a const value, it can be used to indicate to the compiler that /// Since this is a const value, it can be used to indicate to the compiler that
/// a particular block of code will not be executed in debug mode, and hence /// a particular block of code will not be executed in debug mode, and hence
/// can be removed. /// can be removed.
///
/// An alternative strategy is to use asserts, as in:
///
/// ```dart
/// assert(() {
/// // ...debug-only code here...
/// return true;
/// }());
/// ```
///
/// See also:
///
/// * [kReleaseMode], which is true in release builds.
/// * [kProfileMode], which is true in profile builds.
const bool kDebugMode = !kReleaseMode && !kProfileMode; const bool kDebugMode = !kReleaseMode && !kProfileMode;
/// The epsilon of tolerable double precision error. /// The epsilon of tolerable double precision error.
......
...@@ -194,6 +194,15 @@ class ImageStreamListener { ...@@ -194,6 +194,15 @@ class ImageStreamListener {
/// ///
/// If an error occurs during loading, [onError] will be called instead of /// If an error occurs during loading, [onError] will be called instead of
/// [onImage]. /// [onImage].
///
/// If [onError] is called and does not throw, then the error is considered to
/// be handled. An error handler can explicitly rethrow the exception reported
/// to it to safely indicate that it did not handle the exception.
///
/// If an image stream has no listeners that handled the error when the error
/// was first encountered, then the error is reported using
/// [FlutterError.reportError], with the [FlutterErrorDetails.silent] flag set
/// to true.
final ImageErrorListener? onError; final ImageErrorListener? onError;
@override @override
...@@ -504,18 +513,20 @@ abstract class ImageStreamCompleter with Diagnosticable { ...@@ -504,18 +513,20 @@ abstract class ImageStreamCompleter with Diagnosticable {
if (_currentError != null && listener.onError != null) { if (_currentError != null && listener.onError != null) {
try { try {
listener.onError!(_currentError!.exception, _currentError!.stack); listener.onError!(_currentError!.exception, _currentError!.stack);
} catch (exception, stack) { } catch (newException, newStack) {
if (newException != _currentError!.exception) {
FlutterError.reportError( FlutterError.reportError(
FlutterErrorDetails( FlutterErrorDetails(
exception: exception, exception: newException,
library: 'image resource service', library: 'image resource service',
context: ErrorDescription('by a synchronously-called image error listener'), context: ErrorDescription('by a synchronously-called image error listener'),
stack: stack, stack: newStack,
), ),
); );
} }
} }
} }
}
int _keepAliveHandles = 0; int _keepAliveHandles = 0;
/// Creates an [ImageStreamCompleterHandle] that will prevent this stream from /// Creates an [ImageStreamCompleterHandle] that will prevent this stream from
...@@ -630,7 +641,9 @@ abstract class ImageStreamCompleter with Diagnosticable { ...@@ -630,7 +641,9 @@ abstract class ImageStreamCompleter with Diagnosticable {
/// occurred while resolving the image. /// occurred while resolving the image.
/// ///
/// If no error listeners (listeners with an [ImageStreamListener.onError] /// If no error listeners (listeners with an [ImageStreamListener.onError]
/// specified) are attached, a [FlutterError] will be reported instead. /// specified) are attached, or if the handlers all rethrow the exception
/// verbatim (with `throw exception`), a [FlutterError] will be reported using
/// [FlutterError.reportError].
/// ///
/// The `context` should be a string describing where the error was caught, in /// 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", /// a form that will make sense in English when following the word "thrown",
...@@ -677,24 +690,27 @@ abstract class ImageStreamCompleter with Diagnosticable { ...@@ -677,24 +690,27 @@ abstract class ImageStreamCompleter with Diagnosticable {
.whereType<ImageErrorListener>() .whereType<ImageErrorListener>()
.toList(); .toList();
if (localErrorListeners.isEmpty) { bool handled = false;
FlutterError.reportError(_currentError!);
} else {
for (final ImageErrorListener errorListener in localErrorListeners) { for (final ImageErrorListener errorListener in localErrorListeners) {
try { try {
errorListener(exception, stack); errorListener(exception, stack);
} catch (exception, stack) { handled = true;
} catch (newException, newStack) {
if (newException != exception) {
FlutterError.reportError( FlutterError.reportError(
FlutterErrorDetails( FlutterErrorDetails(
context: ErrorDescription('when reporting an error to an image listener'), context: ErrorDescription('when reporting an error to an image listener'),
library: 'image resource service', library: 'image resource service',
exception: exception, exception: newException,
stack: stack, stack: newStack,
), ),
); );
} }
} }
} }
if (!handled) {
FlutterError.reportError(_currentError!);
}
} }
/// Calls all the registered [ImageChunkListener]s (listeners with an /// Calls all the registered [ImageChunkListener]s (listeners with an
......
...@@ -17,7 +17,9 @@ import 'disposable_build_context.dart'; ...@@ -17,7 +17,9 @@ import 'disposable_build_context.dart';
import 'framework.dart'; import 'framework.dart';
import 'localizations.dart'; import 'localizations.dart';
import 'media_query.dart'; import 'media_query.dart';
import 'placeholder.dart';
import 'scroll_aware_image_provider.dart'; import 'scroll_aware_image_provider.dart';
import 'text.dart';
import 'ticker_provider.dart'; import 'ticker_provider.dart';
export 'package:flutter/painting.dart' show export 'package:flutter/painting.dart' show
...@@ -471,7 +473,6 @@ class Image extends StatefulWidget { ...@@ -471,7 +473,6 @@ class Image extends StatefulWidget {
assert(isAntiAlias != null), assert(isAntiAlias != null),
super(key: key); super(key: key);
// TODO(ianh): Implement the following (see ../services/image_resolution.dart): // TODO(ianh): Implement the following (see ../services/image_resolution.dart):
// //
// * If [width] and [height] are both specified, and [scale] is not, then // * If [width] and [height] are both specified, and [scale] is not, then
...@@ -1176,12 +1177,17 @@ class _ImageState extends State<Image> with WidgetsBindingObserver { ...@@ -1176,12 +1177,17 @@ class _ImageState extends State<Image> with WidgetsBindingObserver {
_imageStreamListener = ImageStreamListener( _imageStreamListener = ImageStreamListener(
_handleImageFrame, _handleImageFrame,
onChunk: widget.loadingBuilder == null ? null : _handleImageChunk, onChunk: widget.loadingBuilder == null ? null : _handleImageChunk,
onError: widget.errorBuilder != null onError: widget.errorBuilder != null || kDebugMode
? (dynamic error, StackTrace? stackTrace) { ? (Object error, StackTrace? stackTrace) {
setState(() { setState(() {
_lastException = error; _lastException = error;
_lastStack = stackTrace; _lastStack = stackTrace;
}); });
assert(() {
if (widget.errorBuilder == null)
throw error; // Ensures the error message is printed to the console.
return true;
}());
} }
: null, : null,
); );
...@@ -1268,11 +1274,41 @@ class _ImageState extends State<Image> with WidgetsBindingObserver { ...@@ -1268,11 +1274,41 @@ class _ImageState extends State<Image> with WidgetsBindingObserver {
_isListeningToStream = false; _isListeningToStream = false;
} }
Widget _debugBuildErrorWidget(BuildContext context, Object error) {
return Stack(
alignment: Alignment.center,
children: <Widget>[
const Positioned.fill(
child: Placeholder(
color: Color(0xCF8D021F),
),
),
Padding(
padding: const EdgeInsets.all(4.0),
child: FittedBox(
child: Text(
'$error',
textAlign: TextAlign.center,
textDirection: TextDirection.ltr,
style: const TextStyle(
shadows: <Shadow>[
Shadow(blurRadius: 1.0),
],
),
),
),
),
],
);
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
if (_lastException != null) { if (_lastException != null) {
assert(widget.errorBuilder != null); if (widget.errorBuilder != null)
return widget.errorBuilder!(context, _lastException!, _lastStack); return widget.errorBuilder!(context, _lastException!, _lastStack);
if (kDebugMode)
return _debugBuildErrorWidget(context, _lastException!);
} }
Widget result = RawImage( Widget result = RawImage(
......
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:async';
import 'dart:typed_data';
import 'dart:ui' as ui;
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
testWidgets('Image at default filterQuality', (WidgetTester tester) async {
await testImageQuality(tester, null);
});
testWidgets('Image at high filterQuality', (WidgetTester tester) async {
await testImageQuality(tester, ui.FilterQuality.high);
});
testWidgets('Image at none filterQuality', (WidgetTester tester) async {
await testImageQuality(tester, ui.FilterQuality.none);
});
}
Future<void> testImageQuality(WidgetTester tester, ui.FilterQuality? quality) async {
await tester.binding.setSurfaceSize(const ui.Size(3, 3));
// A 3x3 image encoded as PNG with white background and black pixels on the diagonal:
// ┌──────┐
// │▓▓ │
// │ ▓▓ │
// │ ▓▓│
// └──────┘
// At different levels of quality these pixels are blurred differently.
final Uint8List test3x3Image = Uint8List.fromList(<int>[
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d,
0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x03,
0x08, 0x02, 0x00, 0x00, 0x00, 0xd9, 0x4a, 0x22, 0xe8, 0x00, 0x00, 0x00,
0x1b, 0x49, 0x44, 0x41, 0x54, 0x08, 0xd7, 0x63, 0x64, 0x60, 0x60, 0xf8,
0xff, 0xff, 0x3f, 0x03, 0x9c, 0xfa, 0xff, 0xff, 0x3f, 0xc3, 0xff, 0xff,
0xff, 0x21, 0x1c, 0x00, 0xcb, 0x70, 0x0e, 0xf3, 0x5d, 0x11, 0xc2, 0xf8,
0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
]);
final ui.Image image = (await tester.runAsync(() async {
final ui.Codec codec = await ui.instantiateImageCodec(test3x3Image);
return (await codec.getNextFrame()).image;
}))!;
expect(image.width, 3);
expect(image.height, 3);
final _TestImageStreamCompleter streamCompleter = _TestImageStreamCompleter();
streamCompleter.setData(imageInfo: ImageInfo(image: image));
final _TestImageProvider imageProvider = _TestImageProvider(streamCompleter: streamCompleter);
await tester.pumpWidget(
quality == null
? Image(image: imageProvider)
: Image(
image: imageProvider,
filterQuality: quality,
),
);
await expectLater(
find.byType(Image),
matchesGoldenFile('image_quality_${quality ?? 'default'}.png'),
);
}
class _TestImageStreamCompleter extends ImageStreamCompleter {
_TestImageStreamCompleter([this._currentImage]);
ImageInfo? _currentImage;
final Set<ImageStreamListener> listeners = <ImageStreamListener>{};
@override
void addListener(ImageStreamListener listener) {
listeners.add(listener);
if (_currentImage != null) {
listener.onImage(_currentImage!.clone(), true);
}
}
@override
void removeListener(ImageStreamListener listener) {
listeners.remove(listener);
}
void setData({
ImageInfo? imageInfo,
ImageChunkEvent? chunkEvent,
}) {
if (imageInfo != null) {
_currentImage?.dispose();
_currentImage = imageInfo;
}
final List<ImageStreamListener> localListeners = listeners.toList();
for (final ImageStreamListener listener in localListeners) {
if (imageInfo != null) {
listener.onImage(imageInfo.clone(), false);
}
if (chunkEvent != null && listener.onChunk != null) {
listener.onChunk!(chunkEvent);
}
}
}
void setError({
required Object exception,
StackTrace? stackTrace,
}) {
final List<ImageStreamListener> localListeners = listeners.toList();
for (final ImageStreamListener listener in localListeners) {
if (listener.onError != null) {
listener.onError!(exception, stackTrace);
}
}
}
}
class _TestImageProvider extends ImageProvider<Object> {
_TestImageProvider({ImageStreamCompleter? streamCompleter}) {
_streamCompleter = streamCompleter
?? OneFrameImageStreamCompleter(_completer.future);
}
final Completer<ImageInfo> _completer = Completer<ImageInfo>();
late ImageStreamCompleter _streamCompleter;
bool get loadCalled => _loadCallCount > 0;
int get loadCallCount => _loadCallCount;
int _loadCallCount = 0;
@override
Future<Object> obtainKey(ImageConfiguration configuration) {
return SynchronousFuture<_TestImageProvider>(this);
}
@override
ImageStreamCompleter load(Object key, DecoderCallback decode) {
_loadCallCount += 1;
return _streamCompleter;
}
void complete(ui.Image image) {
_completer.complete(ImageInfo(image: image));
}
void fail(Object exception, StackTrace? stackTrace) {
_completer.completeError(exception, stackTrace);
}
@override
String toString() => '${describeIdentity(this)}()';
}
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