diff --git a/dev/tracing_tests/test/image_cache_tracing_test.dart b/dev/tracing_tests/test/image_cache_tracing_test.dart index c96e4b2e9f9..7121ae7551b 100644 --- a/dev/tracing_tests/test/image_cache_tracing_test.dart +++ b/dev/tracing_tests/test/image_cache_tracing_test.dart @@ -20,7 +20,7 @@ void main() { final developer.ServiceProtocolInfo info = await developer.Service.getInfo(); if (info.serverUri == null) { - throw TestFailure('This test _must_ be run with --enable-vmservice.'); + fail('This test _must_ be run with --enable-vmservice.'); } await timelineObtainer.connect(info.serverUri); await timelineObtainer.setDartFlags(); @@ -58,7 +58,8 @@ void main() { 'name': 'ImageCache.clear', 'args': { 'pendingImages': 1, - 'cachedImages': 0, + 'keepAliveImages': 0, + 'liveImages': 1, 'currentSizeInBytes': 0, 'isolateId': isolateId, } @@ -153,7 +154,7 @@ class TimelineObtainer { Future close() async { expect(_completers, isEmpty); - await _observatorySocket.close(); + await _observatorySocket?.close(); } } diff --git a/packages/flutter/lib/src/painting/binding.dart b/packages/flutter/lib/src/painting/binding.dart index cbdb882dbca..d5f00a981d7 100644 --- a/packages/flutter/lib/src/painting/binding.dart +++ b/packages/flutter/lib/src/painting/binding.dart @@ -96,6 +96,7 @@ mixin PaintingBinding on BindingBase, ServicesBinding { void evict(String asset) { super.evict(asset); imageCache.clear(); + imageCache.clearLiveImages(); } /// Listenable that notifies when the available fonts on the system have diff --git a/packages/flutter/lib/src/painting/image_cache.dart b/packages/flutter/lib/src/painting/image_cache.dart index 63034dbba59..450b9f7b458 100644 --- a/packages/flutter/lib/src/painting/image_cache.dart +++ b/packages/flutter/lib/src/painting/image_cache.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:developer'; +import 'dart:ui' show hashValues; import 'package:flutter/foundation.dart'; @@ -15,18 +16,24 @@ const int _kDefaultSizeBytes = 100 << 20; // 100 MiB /// /// Implements a least-recently-used cache of up to 1000 images, and up to 100 /// MB. The maximum size can be adjusted using [maximumSize] and -/// [maximumSizeBytes]. Images that are actively in use (i.e. to which the -/// application is holding references, either via [ImageStream] objects, -/// [ImageStreamCompleter] objects, [ImageInfo] objects, or raw [dart:ui.Image] -/// objects) may get evicted from the cache (and thus need to be refetched from -/// the network if they are referenced in the [putIfAbsent] method), but the raw -/// bits are kept in memory for as long as the application is using them. +/// [maximumSizeBytes]. +/// +/// The cache also holds a list of "live" references. An image is considered +/// live if its [ImageStreamCompleter]'s listener count has never dropped to +/// zero after adding at least one listener. The cache uses +/// [ImageStreamCompleter.addOnLastListenerRemovedCallback] to determine when +/// this has happened. /// /// The [putIfAbsent] method is the main entry-point to the cache API. It /// returns the previously cached [ImageStreamCompleter] for the given key, if /// available; if not, it calls the given callback to obtain it first. In either /// case, the key is moved to the "most recently used" position. /// +/// A caller can determine whether an image is already in the cache by using +/// [containsKey], which will return true if the image is tracked by the cache +/// in a pending or compelted state. More fine grained information is available +/// by using the [statusForKey] method. +/// /// Generally this class is not used directly. The [ImageProvider] class and its /// subclasses automatically handle the caching of images. /// @@ -71,6 +78,11 @@ const int _kDefaultSizeBytes = 100 << 20; // 100 MiB class ImageCache { final Map _pendingImages = {}; final Map _cache = {}; + /// ImageStreamCompleters with at least one listener. These images may or may + /// not fit into the _pendingImages or _cache objects. + /// + /// Unlike _cache, the [_CachedImage] for this may have a null byte size. + final Map _liveImages = {}; /// Maximum number of entries to store in the cache. /// @@ -163,7 +175,8 @@ class ImageCache { 'ImageCache.clear', arguments: { 'pendingImages': _pendingImages.length, - 'cachedImages': _cache.length, + 'keepAliveImages': _cache.length, + 'liveImages': _liveImages.length, 'currentSizeInBytes': _currentSizeBytes, }, ); @@ -204,7 +217,7 @@ class ImageCache { if (image != null) { if (!kReleaseMode) { Timeline.instantSync('ImageCache.evict', arguments: { - 'type': 'completed', + 'type': 'keepAlive', 'sizeiInBytes': image.sizeBytes, }); } @@ -219,6 +232,36 @@ class ImageCache { return false; } + /// Updates the least recently used image cache with this image, if it is + /// less than the [maximumSizeBytes] of this cache. + /// + /// Resizes the cache as appropriate to maintain the constraints of + /// [maximumSize] and [maximumSizeBytes]. + void _touch(Object key, _CachedImage image, TimelineTask timelineTask) { + assert(timelineTask != null); + if (image.sizeBytes != null && image.sizeBytes <= maximumSizeBytes) { + _currentSizeBytes += image.sizeBytes; + _cache[key] = image; + _checkCacheSize(timelineTask); + } + } + + void _trackLiveImage(Object key, _CachedImage image) { + // Avoid adding unnecessary callbacks to the completer. + if (_liveImages.containsKey(key)) { + assert(identical(_liveImages[key].completer, image.completer)); + return; + } + _liveImages[key] = image; + // Even if no callers to ImageProvider.resolve have listened to the stream, + // the cache is listening to the stream and will remove itself once the + // image completes to move it from pending to keepAlive. + // Even if the cache size is 0, we still add this listener. + image.completer.addOnLastListenerRemovedCallback(() { + _liveImages.remove(key); + }); + } + /// Returns the previously cached [ImageStream] for the given key, if available; /// if not, calls the given callback to obtain it first. In either case, the /// key is moved to the "most recently used" position. @@ -255,14 +298,27 @@ class ImageCache { final _CachedImage image = _cache.remove(key); if (image != null) { if (!kReleaseMode) { - timelineTask.finish(arguments: {'result': 'completed'}); + timelineTask.finish(arguments: {'result': 'keepAlive'}); } + // The image might have been keptAlive but had no listeners. We should + // track it as live again until it has no listeners again. + _trackLiveImage(key, image); _cache[key] = image; return image.completer; } + final _CachedImage liveImage = _liveImages[key]; + if (liveImage != null) { + _touch(key, liveImage, timelineTask); + if (!kReleaseMode) { + timelineTask.finish(arguments: {'result': 'keepAlive'}); + } + return liveImage.completer; + } + try { result = loader(); + _trackLiveImage(key, _CachedImage(result, null)); } catch (error, stackTrace) { if (!kReleaseMode) { timelineTask.finish(arguments: { @@ -282,21 +338,37 @@ class ImageCache { if (!kReleaseMode) { listenerTask = TimelineTask(parent: timelineTask)..start('listener'); } + // If we're doing tracing, we need to make sure that we don't try to finish + // the trace entry multiple times if we get re-entrant calls from a multi- + // frame provider here. bool listenedOnce = false; + + // We shouldn't use the _pendingImages map if the cache is disabled, but we + // will have to listen to the image at least once so we don't leak it in + // the live image tracking. + // If the cache is disabled, this variable will be set. + _PendingImage untrackedPendingImage; void listener(ImageInfo info, bool syncCall) { // Images that fail to load don't contribute to cache size. final int imageSize = info?.image == null ? 0 : info.image.height * info.image.width * 4; + final _CachedImage image = _CachedImage(result, imageSize); - final _PendingImage pendingImage = _pendingImages.remove(key); + if (!_liveImages.containsKey(key)) { + assert(syncCall); + result.addOnLastListenerRemovedCallback(() { + _liveImages.remove(key); + }); + } + _liveImages[key] = image; + final _PendingImage pendingImage = untrackedPendingImage ?? _pendingImages.remove(key); if (pendingImage != null) { pendingImage.removeListener(); } - - if (imageSize <= maximumSizeBytes) { - _currentSizeBytes += imageSize; - _cache[key] = image; - _checkCacheSize(listenerTask); + // Only touch if the cache was enabled when resolve was initially called. + if (untrackedPendingImage == null) { + _touch(key, image, listenerTask); } + if (!kReleaseMode && !listenedOnce) { listenerTask.finish(arguments: { 'syncCall': syncCall, @@ -309,20 +381,58 @@ class ImageCache { } listenedOnce = true; } + + final ImageStreamListener streamListener = ImageStreamListener(listener); if (maximumSize > 0 && maximumSizeBytes > 0) { - final ImageStreamListener streamListener = ImageStreamListener(listener); _pendingImages[key] = _PendingImage(result, streamListener); - // Listener is removed in [_PendingImage.removeListener]. - result.addListener(streamListener); + } else { + untrackedPendingImage = _PendingImage(result, streamListener); } + // Listener is removed in [_PendingImage.removeListener]. + result.addListener(streamListener); + return result; } + /// The [ImageCacheStatus] information for the given `key`. + ImageCacheStatus statusForKey(Object key) { + return ImageCacheStatus._( + pending: _pendingImages.containsKey(key), + keepAlive: _cache.containsKey(key), + live: _liveImages.containsKey(key), + ); + } + /// Returns whether this `key` has been previously added by [putIfAbsent]. bool containsKey(Object key) { return _pendingImages[key] != null || _cache[key] != null; } + /// The number of live images being held by the [ImageCache]. + /// + /// Compare with [ImageCache.currentSize] for keepAlive images. + int get liveImageCount => _liveImages.length; + + /// The number of images being tracked as pending in the [ImageCache]. + /// + /// Compare with [ImageCache.currentSize] for keepAlive images. + int get pendingImageCount => _pendingImages.length; + + /// Clears any live references to images in this cache. + /// + /// An image is considered live if its [ImageStreamCompleter] has never hit + /// zero listeners after adding at least one listener. The + /// [ImageStreamCompleter.addOnLastListenerRemovedCallback] is used to + /// determine when this has happened. + /// + /// This is called after a hot reload to evict any stale references to image + /// data for assets that have changed. Calling this method does not relieve + /// memory pressure, since the live image caching only tracks image instances + /// that are also being held by at least one other object. + void clearLiveImages() { + _liveImages.clear(); + } + // Remove images from the cache until both the length and bytes are below // maximum, or the cache is empty. void _checkCacheSize(TimelineTask timelineTask) { @@ -354,6 +464,73 @@ class ImageCache { } } +/// Information about how the [ImageCache] is tracking an image. +/// +/// A [pending] image is one that has not completed yet. It may also be tracked +/// as [live] because something is listening to it. +/// +/// A [keepAlive] image is being held in the cache, which uses Least Recently +/// Used semantics to determine when to evict an image. These images are subject +/// to eviction based on [ImageCache.maximumSizeBytes] and +/// [ImageCache.maximumSize]. It may be [live], but not [pending]. +/// +/// A [live] image is being held until its [ImageStreamCompleter] has no more +/// listeners. It may also be [pending] or [keepAlive]. +/// +/// An [untracked] image is not being cached. +/// +/// To obtain an [ImageCacheStatus], use [ImageCache.statusForKey] or +/// [ImageProvider.obtainCacheStatus]. +class ImageCacheStatus { + const ImageCacheStatus._({ + this.pending = false, + this.keepAlive = false, + this.live = false, + }) : assert(!pending || !keepAlive); + + /// An image that has been submitted to [ImageCache.putIfAbsent], but + /// not yet completed. + final bool pending; + + /// An image that has been submitted to [ImageCache.putIfAbsent], has + /// completed, fits based on the sizing rules of the cache, and has not been + /// evicted. + /// + /// Such images will be kept alive even if [live] is false, as long + /// as they have not been evicted from the cache based on its sizing rules. + final bool keepAlive; + + /// An image that has been submitted to [ImageCache.putIfAbsent] and has at + /// least one listener on its [ImageStreamCompleter]. + /// + /// Such images may also be [keepAlive] if they fit in the cache based on its + /// sizing rules. They may also be [pending] if they have not yet resolved. + final bool live; + + /// An image that is tracked in some way by the [ImageCache], whether + /// [pending], [keepAlive], or [live]. + bool get tracked => pending || keepAlive || live; + + /// An image that either has not been submitted to + /// [ImageCache.putIfAbsent] or has otherwise been evicted from the + /// [keepAlive] and [live] caches. + bool get untracked => !pending && !keepAlive && !live; + + @override + bool operator ==(Object other) { + if (other.runtimeType != runtimeType) { + return false; + } + return other is ImageCacheStatus + && other.pending == pending + && other.keepAlive == keepAlive + && other.live == live; + } + + @override + int get hashCode => hashValues(pending, keepAlive, live); +} + class _CachedImage { _CachedImage(this.completer, this.sizeBytes); diff --git a/packages/flutter/lib/src/painting/image_provider.dart b/packages/flutter/lib/src/painting/image_provider.dart index dd536d79513..df153166036 100644 --- a/packages/flutter/lib/src/painting/image_provider.dart +++ b/packages/flutter/lib/src/painting/image_provider.dart @@ -17,6 +17,12 @@ import 'binding.dart'; import 'image_cache.dart'; import 'image_stream.dart'; +/// Signature for the callback taken by [_createErrorHandlerAndKey]. +typedef _KeyAndErrorHandlerCallback = void Function(T key, ImageErrorListener handleError); + +/// Signature used for error handling by [_createErrorHandlerAndKey]. +typedef _AsyncKeyErrorHandler = Future Function(T key, dynamic exception, StackTrace stack); + /// Configuration information passed to the [ImageProvider.resolve] method to /// select a specific image. /// @@ -318,7 +324,28 @@ abstract class ImageProvider { final ImageStream stream = createStream(configuration); // Load the key (potentially asynchronously), set up an error handling zone, // and call resolveStreamForKey. - _createErrorHandlerAndKey(configuration, stream); + _createErrorHandlerAndKey( + configuration, + (T key, ImageErrorListener errorHandler) { + resolveStreamForKey(configuration, stream, key, errorHandler); + }, + (T key, dynamic exception, StackTrace stack) async { + await null; // wait an event turn in case a listener has been added to the image stream. + final _ErrorImageCompleter imageCompleter = _ErrorImageCompleter(); + stream.setCompleter(imageCompleter); + imageCompleter.setError( + exception: exception, + stack: stack, + context: ErrorDescription('while resolving an image'), + silent: true, // could be a network error or whatnot + informationCollector: () sync* { + yield DiagnosticsProperty('Image provider', this); + yield DiagnosticsProperty('Image configuration', configuration); + yield DiagnosticsProperty('Image key', key, defaultValue: null); + }, + ); + }, + ); return stream; } @@ -332,30 +359,66 @@ abstract class ImageProvider { return ImageStream(); } - void _createErrorHandlerAndKey(ImageConfiguration configuration, ImageStream stream) { + /// Returns the cache location for the key that this [ImageProvider] creates. + /// + /// The location may be [ImageCacheStatus.untracked], indicating that this + /// image provider's key is not available in the [ImageCache]. + /// + /// The `cache` and `configuration` parameters must not be null. If the + /// `handleError` parameter is null, errors will be reported to + /// [FlutterError.onError], and the method will return null. + /// + /// A completed return value of null indicates that an error has occurred. + Future obtainCacheStatus({ + @required ImageConfiguration configuration, + ImageErrorListener handleError, + }) { assert(configuration != null); - assert(stream != null); + final Completer completer = Completer(); + _createErrorHandlerAndKey( + configuration, + (T key, ImageErrorListener innerHandleError) { + completer.complete(PaintingBinding.instance.imageCache.statusForKey(key)); + }, + (T key, dynamic exception, StackTrace stack) async { + if (handleError != null) { + handleError(exception, stack); + } else { + FlutterError.onError(FlutterErrorDetails( + context: ErrorDescription('while checking the cache location of an image'), + informationCollector: () sync* { + yield DiagnosticsProperty('Image provider', this); + yield DiagnosticsProperty('Image configuration', configuration); + yield DiagnosticsProperty('Image key', key, defaultValue: null); + }, + exception: exception, + stack: stack, + )); + completer.complete(null); + } + }, + ); + return completer.future; + } + + /// This method is used by both [resolve] and [obtainCacheStatus] to ensure + /// that errors thrown during key creation are handled whether synchronous or + /// asynchronous. + void _createErrorHandlerAndKey( + ImageConfiguration configuration, + _KeyAndErrorHandlerCallback successCallback, + _AsyncKeyErrorHandler errorCallback, + ) { T obtainedKey; bool didError = false; Future handleError(dynamic exception, StackTrace stack) async { if (didError) { return; } + if (!didError) { + errorCallback(obtainedKey, exception, stack); + } didError = true; - await null; // wait an event turn in case a listener has been added to the image stream. - final _ErrorImageCompleter imageCompleter = _ErrorImageCompleter(); - stream.setCompleter(imageCompleter); - imageCompleter.setError( - exception: exception, - stack: stack, - context: ErrorDescription('while resolving an image'), - silent: true, // could be a network error or whatnot - informationCollector: () sync* { - yield DiagnosticsProperty('Image provider', this); - yield DiagnosticsProperty('Image configuration', configuration); - yield DiagnosticsProperty('Image key', obtainedKey, defaultValue: null); - }, - ); } // If an error is added to a synchronous completer before a listener has been @@ -384,7 +447,7 @@ abstract class ImageProvider { key.then((T key) { obtainedKey = key; try { - resolveStreamForKey(configuration, stream, key, handleError); + successCallback(key, handleError); } catch (error, stackTrace) { handleError(error, stackTrace); } diff --git a/packages/flutter/lib/src/painting/image_stream.dart b/packages/flutter/lib/src/painting/image_stream.dart index 7b5c263bcd8..5db8bacc05d 100644 --- a/packages/flutter/lib/src/painting/image_stream.dart +++ b/packages/flutter/lib/src/painting/image_stream.dart @@ -203,6 +203,11 @@ class ImageChunkEvent extends Diagnosticable { /// /// ImageStream objects are backed by [ImageStreamCompleter] objects. /// +/// The [ImageCache] will consider an image to be live until the listener count +/// drops to zero after adding at least one listener. The +/// [addOnLastListenerRemovedCallback] method is used for tracking this +/// information. +/// /// See also: /// /// * [ImageProvider], which has an example that includes the use of an @@ -392,6 +397,23 @@ abstract class ImageStreamCompleter extends Diagnosticable { break; } } + if (_listeners.isEmpty) { + for (final VoidCallback callback in _onLastListenerRemovedCallbacks) { + callback(); + } + _onLastListenerRemovedCallbacks.clear(); + } + } + + final List _onLastListenerRemovedCallbacks = []; + + /// Adds a callback to call when [removeListener] results in an empty + /// list of listeners. + /// + /// This callback will never fire if [removeListener] is never called. + void addOnLastListenerRemovedCallback(VoidCallback callback) { + assert(callback != null); + _onLastListenerRemovedCallbacks.add(callback); } /// Calls all the registered listeners to notify them of a new image. diff --git a/packages/flutter/lib/src/widgets/image.dart b/packages/flutter/lib/src/widgets/image.dart index f373af5cca8..12a947e0e36 100644 --- a/packages/flutter/lib/src/widgets/image.dart +++ b/packages/flutter/lib/src/widgets/image.dart @@ -8,6 +8,7 @@ import 'dart:typed_data'; import 'package:flutter/foundation.dart'; import 'package:flutter/painting.dart'; +import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; import 'package:flutter/semantics.dart'; @@ -65,7 +66,30 @@ ImageConfiguration createLocalImageConfiguration(BuildContext context, { Size si /// If the image is later used by an [Image] or [BoxDecoration] or [FadeInImage], /// it will probably be loaded faster. The consumer of the image does not need /// to use the same [ImageProvider] instance. The [ImageCache] will find the image -/// as long as both images share the same key. +/// as long as both images share the same key, and the image is held by the +/// cache. +/// +/// The cache may refuse to hold the image if it is disabled, the image is too +/// large, or some other criteria implemented by a custom [ImageCache] +/// implementation. +/// +/// The [ImageCache] holds a reference to all images passed to [putIfAbsent] as +/// long as their [ImageStreamCompleter] has at least one listener. This method +/// will wait until the end of the frame after its future completes before +/// releasing its own listener. This gives callers a chance to listen to the +/// stream if necessary. A caller can determine if the image ended up in the +/// cache by calling [ImageProvider.obtainCacheStatus]. If it is only held as +/// [ImageCacheStatus.live], and the caller wishes to keep the resolved +/// image in memory, the caller should immediately call `provider.resolve` and +/// add a listener to the returned [ImageStream]. The image will remain pinned +/// in memory at least until the caller removes its listener from the stream, +/// even if it would not otherwise fit into the cache. +/// +/// Callers should be cautious about pinning large images or a large number of +/// images in memory, as this can result in running out of memory and being +/// killed by the operating system. The lower the avilable physical memory, the +/// more susceptible callers will be to running into OOM issues. These issues +/// manifest as immediate process death, sometimes with no other error messages. /// /// The [BuildContext] and [Size] are used to select an image configuration /// (see [createLocalImageConfiguration]). @@ -91,7 +115,12 @@ Future precacheImage( if (!completer.isCompleted) { completer.complete(); } - stream.removeListener(listener); + // Give callers until at least the end of the frame to subscribe to the + // image stream. + // See ImageCache._liveImages + SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) { + stream.removeListener(listener); + }); }, onError: (dynamic exception, StackTrace stackTrace) { if (!completer.isCompleted) { diff --git a/packages/flutter/test/painting/binding_test.dart b/packages/flutter/test/painting/binding_test.dart index 0c0e7dc24ad..f216b1ade81 100644 --- a/packages/flutter/test/painting/binding_test.dart +++ b/packages/flutter/test/painting/binding_test.dart @@ -3,7 +3,10 @@ // found in the LICENSE file. import 'dart:typed_data' show Uint8List; +import 'dart:ui'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter/painting.dart'; @@ -24,4 +27,88 @@ void main() { }); expect(binding.instantiateImageCodecCalledCount, 1); }); + + test('evict clears live references', () async { + final TestPaintingBinding binding = TestPaintingBinding(); + expect(binding.imageCache.clearCount, 0); + expect(binding.imageCache.liveClearCount, 0); + + binding.evict('/path/to/asset.png'); + expect(binding.imageCache.clearCount, 1); + expect(binding.imageCache.liveClearCount, 1); + }); } + +class TestBindingBase implements BindingBase { + @override + void initInstances() {} + + @override + void initServiceExtensions() {} + + @override + Future lockEvents(Future Function() callback) async {} + + @override + bool get locked => throw UnimplementedError(); + + @override + Future performReassemble() { + throw UnimplementedError(); + } + + @override + void postEvent(String eventKind, Map eventData) {} + + @override + Future reassembleApplication() { + throw UnimplementedError(); + } + + @override + void registerBoolServiceExtension({String name, AsyncValueGetter getter, AsyncValueSetter setter}) {} + + @override + void registerNumericServiceExtension({String name, AsyncValueGetter getter, AsyncValueSetter setter}) {} + + @override + void registerServiceExtension({String name, ServiceExtensionCallback callback}) {} + + @override + void registerSignalServiceExtension({String name, AsyncCallback callback}) {} + + @override + void registerStringServiceExtension({String name, AsyncValueGetter getter, AsyncValueSetter setter}) {} + + @override + void unlocked() {} + + @override + Window get window => throw UnimplementedError(); +} + +class TestPaintingBinding extends TestBindingBase with ServicesBinding, PaintingBinding { + + @override + final FakeImageCache imageCache = FakeImageCache(); + + @override + ImageCache createImageCache() => imageCache; +} + +class FakeImageCache extends ImageCache { + int clearCount = 0; + int liveClearCount = 0; + + @override + void clear() { + clearCount += 1; + super.clear(); + } + + @override + void clearLiveImages() { + liveClearCount += 1; + super.clearLiveImages(); + } +} \ No newline at end of file diff --git a/packages/flutter/test/painting/image_cache_test.dart b/packages/flutter/test/painting/image_cache_test.dart index 39f3a2a8bc2..614c1286c2c 100644 --- a/packages/flutter/test/painting/image_cache_test.dart +++ b/packages/flutter/test/painting/image_cache_test.dart @@ -9,13 +9,14 @@ import '../rendering/rendering_tester.dart'; import 'mocks_for_image_cache.dart'; void main() { - group(ImageCache, () { + group('ImageCache', () { setUpAll(() { TestRenderingFlutterBinding(); // initializes the imageCache }); tearDown(() { imageCache.clear(); + imageCache.clearLiveImages(); imageCache.maximumSize = 1000; imageCache.maximumSizeBytes = 10485760; }); @@ -169,7 +170,14 @@ void main() { return completer1; }) as TestImageStreamCompleter; + expect(imageCache.statusForKey(testImage).pending, true); + expect(imageCache.statusForKey(testImage).live, true); imageCache.clear(); + expect(imageCache.statusForKey(testImage).pending, false); + expect(imageCache.statusForKey(testImage).live, true); + imageCache.clearLiveImages(); + expect(imageCache.statusForKey(testImage).pending, false); + expect(imageCache.statusForKey(testImage).live, false); final TestImageStreamCompleter resultingCompleter2 = imageCache.putIfAbsent(testImage, () { return completer2; @@ -240,7 +248,74 @@ void main() { expect(resultingCompleter1, completer1); expect(imageCache.containsKey(testImage), true); + }); + test('putIfAbsent updates LRU properties of a live image', () async { + imageCache.maximumSize = 1; + const TestImage testImage = TestImage(width: 8, height: 8); + const TestImage testImage2 = TestImage(width: 10, height: 10); + + final TestImageStreamCompleter completer1 = TestImageStreamCompleter()..testSetImage(testImage); + final TestImageStreamCompleter completer2 = TestImageStreamCompleter()..testSetImage(testImage2); + + completer1.addListener(ImageStreamListener((ImageInfo info, bool syncCall) {})); + + final TestImageStreamCompleter resultingCompleter1 = imageCache.putIfAbsent(testImage, () { + return completer1; + }) as TestImageStreamCompleter; + + expect(imageCache.statusForKey(testImage).pending, false); + expect(imageCache.statusForKey(testImage).keepAlive, true); + expect(imageCache.statusForKey(testImage).live, true); + expect(imageCache.statusForKey(testImage2).untracked, true); + final TestImageStreamCompleter resultingCompleter2 = imageCache.putIfAbsent(testImage2, () { + return completer2; + }) as TestImageStreamCompleter; + + + expect(imageCache.statusForKey(testImage).pending, false); + expect(imageCache.statusForKey(testImage).keepAlive, false); // evicted + expect(imageCache.statusForKey(testImage).live, true); + expect(imageCache.statusForKey(testImage2).pending, false); + expect(imageCache.statusForKey(testImage2).keepAlive, true); // took the LRU spot. + expect(imageCache.statusForKey(testImage2).live, false); // no listeners + + expect(resultingCompleter1, completer1); + expect(resultingCompleter2, completer2); + }); + + test('Live image cache avoids leaks of unlistened streams', () async { + imageCache.maximumSize = 3; + + const TestImageProvider(1, 1)..resolve(ImageConfiguration.empty); + const TestImageProvider(2, 2)..resolve(ImageConfiguration.empty); + const TestImageProvider(3, 3)..resolve(ImageConfiguration.empty); + const TestImageProvider(4, 4)..resolve(ImageConfiguration.empty); + const TestImageProvider(5, 5)..resolve(ImageConfiguration.empty); + const TestImageProvider(6, 6)..resolve(ImageConfiguration.empty); + + // wait an event loop to let image resolution process. + await null; + + expect(imageCache.currentSize, 3); + expect(imageCache.liveImageCount, 0); + }); + + test('Disabled image cache does not leak live images', () async { + imageCache.maximumSize = 0; + + const TestImageProvider(1, 1)..resolve(ImageConfiguration.empty); + const TestImageProvider(2, 2)..resolve(ImageConfiguration.empty); + const TestImageProvider(3, 3)..resolve(ImageConfiguration.empty); + const TestImageProvider(4, 4)..resolve(ImageConfiguration.empty); + const TestImageProvider(5, 5)..resolve(ImageConfiguration.empty); + const TestImageProvider(6, 6)..resolve(ImageConfiguration.empty); + + // wait an event loop to let image resolution process. + await null; + + expect(imageCache.currentSize, 0); + expect(imageCache.liveImageCount, 0); }); }); } diff --git a/packages/flutter/test/painting/image_provider_test.dart b/packages/flutter/test/painting/image_provider_test.dart index 7da4c208f34..2cc0ade21ea 100644 --- a/packages/flutter/test/painting/image_provider_test.dart +++ b/packages/flutter/test/painting/image_provider_test.dart @@ -111,6 +111,17 @@ void main() { expect(await caughtError.future, true); }); + test('obtainKey errors will be caught - check location', () async { + final ImageProvider imageProvider = ObtainKeyErrorImageProvider(); + final Completer caughtError = Completer(); + FlutterError.onError = (FlutterErrorDetails details) { + caughtError.complete(true); + }; + await imageProvider.obtainCacheStatus(configuration: ImageConfiguration.empty); + + expect(await caughtError.future, true); + }); + test('resolve sync errors will be caught', () async { bool uncaught = false; final Zone testZone = Zone.current.fork(specification: ZoneSpecification( @@ -160,7 +171,6 @@ void main() { test('File image with empty file throws expected error - (image cache)', () async { final Completer error = Completer(); FlutterError.onError = (FlutterErrorDetails details) { - print(details.exception); error.complete(details.exception as StateError); }; final MemoryFileSystem fs = MemoryFileSystem(); @@ -172,7 +182,7 @@ void main() { expect(await error.future, isStateError); }); - group(NetworkImage, () { + group('NetworkImage', () { MockHttpClient httpClient; setUp(() { diff --git a/packages/flutter/test/widgets/image_test.dart b/packages/flutter/test/widgets/image_test.dart index cd4a3421342..7a60f020d70 100644 --- a/packages/flutter/test/widgets/image_test.dart +++ b/packages/flutter/test/widgets/image_test.dart @@ -8,6 +8,7 @@ import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; +import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -23,6 +24,18 @@ Future createTestImage([List bytes = kTransparentImage]) async { } void main() { + int originalCacheSize; + + setUp(() { + originalCacheSize = imageCache.maximumSize; + imageCache.clear(); + imageCache.clearLiveImages(); + }); + + tearDown(() { + imageCache.maximumSize = originalCacheSize; + }); + testWidgets('Verify Image resets its RenderImage when changing providers', (WidgetTester tester) async { final GlobalKey key = GlobalKey(); final TestImageProvider imageProvider1 = TestImageProvider(); @@ -763,7 +776,7 @@ void main() { expect(isSync, isTrue); }); - testWidgets('Precache remove listeners immediately after future completes, does not crash on successive calls #25143', (WidgetTester tester) async { + testWidgets('Precache removes original listener immediately after future completes, does not crash on successive calls #25143', (WidgetTester tester) async { final TestImageStreamCompleter imageStreamCompleter = TestImageStreamCompleter(); final TestImageProvider provider = TestImageProvider(streamCompleter: imageStreamCompleter); @@ -1364,6 +1377,164 @@ void main() { expect(imageProviders.skip(309 - 15).every(loadCalled), true); expect(imageProviders.take(309 - 15).every(loadNotCalled), true); }); + + testWidgets('Same image provider in multiple parts of the tree, no cache room left', (WidgetTester tester) async { + imageCache.maximumSize = 0; + + final ui.Image image = await tester.runAsync(createTestImage); + final TestImageProvider provider1 = TestImageProvider(); + final TestImageProvider provider2 = TestImageProvider(); + + expect(provider1.loadCallCount, 0); + expect(provider2.loadCallCount, 0); + expect(imageCache.liveImageCount, 0); + + await tester.pumpWidget(Column( + children: [ + Image(image: provider1), + Image(image: provider2), + Image(image: provider1), + Image(image: provider1), + Image(image: provider2), + ], + )); + + expect(imageCache.liveImageCount, 2); + expect(imageCache.statusForKey(provider1).live, true); + expect(imageCache.statusForKey(provider1).pending, false); + expect(imageCache.statusForKey(provider1).keepAlive, false); + expect(imageCache.statusForKey(provider2).live, true); + expect(imageCache.statusForKey(provider2).pending, false); + expect(imageCache.statusForKey(provider2).keepAlive, false); + + expect(provider1.loadCallCount, 1); + expect(provider2.loadCallCount, 1); + + provider1.complete(image); + await tester.idle(); + + provider2.complete(image); + await tester.idle(); + + expect(imageCache.liveImageCount, 2); + expect(imageCache.currentSize, 0); + + await tester.pumpWidget(Image(image: provider2)); + await tester.idle(); + expect(imageCache.statusForKey(provider1).untracked, true); + expect(imageCache.statusForKey(provider2).live, true); + expect(imageCache.statusForKey(provider2).pending, false); + expect(imageCache.statusForKey(provider2).keepAlive, false); + expect(imageCache.liveImageCount, 1); + + await tester.pumpWidget(const SizedBox()); + await tester.idle(); + expect(provider1.loadCallCount, 1); + expect(provider2.loadCallCount, 1); + expect(imageCache.liveImageCount, 0); + }); + + testWidgets('precacheImage does not hold weak ref for more than a frame', (WidgetTester tester) async { + imageCache.maximumSize = 0; + final TestImageProvider provider = TestImageProvider(); + Future precache; + await tester.pumpWidget( + Builder( + builder: (BuildContext context) { + precache = precacheImage(provider, context); + return Container(); + } + ) + ); + provider.complete(); + await precache; + + // Should have ended up with only a weak ref, not in cache because cache size is 0 + expect(imageCache.liveImageCount, 1); + expect(imageCache.containsKey(provider), false); + + final ImageCacheStatus providerLocation = await provider.obtainCacheStatus(configuration: ImageConfiguration.empty); + + expect(providerLocation, isNotNull); + expect(providerLocation.live, true); + expect(providerLocation.keepAlive, false); + expect(providerLocation.pending, false); + + // Check that a second resolve of the same image is synchronous. + expect(provider._lastResolvedConfiguration, isNotNull); + final ImageStream stream = provider.resolve(provider._lastResolvedConfiguration); + bool isSync; + final ImageStreamListener listener = ImageStreamListener((ImageInfo image, bool syncCall) { isSync = syncCall; }); + + // Still have live ref because frame has not pumped yet. + await tester.pump(); + expect(imageCache.liveImageCount, 1); + + SchedulerBinding.instance.scheduleFrame(); + await tester.pump(); + // Live ref should be gone - we didn't listen to the stream. + expect(imageCache.liveImageCount, 0); + expect(imageCache.currentSize, 0); + + stream.addListener(listener); + expect(isSync, true); // because the stream still has the image. + + expect(imageCache.liveImageCount, 0); + expect(imageCache.currentSize, 0); + + expect(provider.loadCallCount, 1); + }); + + testWidgets('precacheImage allows time to take over weak refernce', (WidgetTester tester) async { + final TestImageProvider provider = TestImageProvider(); + Future precache; + await tester.pumpWidget( + Builder( + builder: (BuildContext context) { + precache = precacheImage(provider, context); + return Container(); + } + ) + ); + provider.complete(); + await precache; + + // Should have ended up in the cache and have a weak reference. + expect(imageCache.liveImageCount, 1); + expect(imageCache.currentSize, 1); + expect(imageCache.containsKey(provider), true); + + // Check that a second resolve of the same image is synchronous. + expect(provider._lastResolvedConfiguration, isNotNull); + final ImageStream stream = provider.resolve(provider._lastResolvedConfiguration); + bool isSync; + final ImageStreamListener listener = ImageStreamListener((ImageInfo image, bool syncCall) { isSync = syncCall; }); + + // Should have ended up in the cache and still have a weak reference. + expect(imageCache.liveImageCount, 1); + expect(imageCache.currentSize, 1); + expect(imageCache.containsKey(provider), true); + + stream.addListener(listener); + expect(isSync, true); + + expect(imageCache.liveImageCount, 1); + expect(imageCache.currentSize, 1); + expect(imageCache.containsKey(provider), true); + + SchedulerBinding.instance.scheduleFrame(); + await tester.pump(); + + expect(imageCache.liveImageCount, 1); + expect(imageCache.currentSize, 1); + expect(imageCache.containsKey(provider), true); + stream.removeListener(listener); + + expect(imageCache.liveImageCount, 0); + expect(imageCache.currentSize, 1); + expect(imageCache.containsKey(provider), true); + expect(provider.loadCallCount, 1); + }); } class ConfigurationAwareKey { @@ -1405,8 +1576,9 @@ class TestImageProvider extends ImageProvider { ImageStreamCompleter _streamCompleter; ImageConfiguration _lastResolvedConfiguration; - bool get loadCalled => _loadCalled; - bool _loadCalled = false; + bool get loadCalled => _loadCallCount > 0; + int get loadCallCount => _loadCallCount; + int _loadCallCount = 0; @override Future obtainKey(ImageConfiguration configuration) { @@ -1421,12 +1593,13 @@ class TestImageProvider extends ImageProvider { @override ImageStreamCompleter load(Object key, DecoderCallback decode) { - _loadCalled = true; + _loadCallCount += 1; return _streamCompleter; } - void complete() { - _completer.complete(ImageInfo(image: TestImage())); + void complete([ui.Image image]) { + image ??= TestImage(); + _completer.complete(ImageInfo(image: image)); } void fail(dynamic exception, StackTrace stackTrace) {