diff --git a/packages/flutter/lib/src/painting/decoration_image.dart b/packages/flutter/lib/src/painting/decoration_image.dart index f257e532db..60e151ee28 100644 --- a/packages/flutter/lib/src/painting/decoration_image.dart +++ b/packages/flutter/lib/src/painting/decoration_image.dart @@ -295,6 +295,11 @@ class DecorationImagePainter { void _handleImage(ImageInfo value, bool synchronousCall) { if (_image == value) return; + if (_image != null && _image!.isCloneOf(value)) { + value.dispose(); + return; + } + _image?.dispose(); _image = value; assert(_onChanged != null); if (!synchronousCall) @@ -312,6 +317,8 @@ class DecorationImagePainter { _handleImage, onError: _details.onError, )); + _image?.dispose(); + _image = null; } @override @@ -429,6 +436,12 @@ void paintImage({ assert(repeat != null); assert(flipHorizontally != null); assert(isAntiAlias != null); + assert( + image.debugGetOpenHandleStackTraces()?.isNotEmpty ?? true, + 'Cannot paint an image that is disposed.\n' + 'The caller of paintImage is expected to wait to dispose the image until ' + 'after painting has completed.' + ); if (rect.isEmpty) return; Size outputSize = rect.size; diff --git a/packages/flutter/lib/src/painting/image_cache.dart b/packages/flutter/lib/src/painting/image_cache.dart index 28d68fa9da..2288415d28 100644 --- a/packages/flutter/lib/src/painting/image_cache.dart +++ b/packages/flutter/lib/src/painting/image_cache.dart @@ -6,6 +6,7 @@ import 'dart:developer'; import 'dart:ui' show hashValues; import 'package:flutter/foundation.dart'; +import 'package:flutter/scheduler.dart'; import 'image_stream.dart'; @@ -238,7 +239,7 @@ class ImageCache { // In such a case, we need to make sure subsequent calls to // putIfAbsent don't return this image that may never complete. final _LiveImage? image = _liveImages.remove(key); - image?.removeListener(); + image?.dispose(); } final _PendingImage? pendingImage = _pendingImages.remove(key); if (pendingImage != null) { @@ -259,6 +260,7 @@ class ImageCache { }); } _currentSizeBytes -= image.sizeBytes!; + image.dispose(); return true; } if (!kReleaseMode) { @@ -276,23 +278,30 @@ class ImageCache { /// [maximumSize] and [maximumSizeBytes]. void _touch(Object key, _CachedImage image, TimelineTask? timelineTask) { assert(timelineTask != null); - if (image.sizeBytes != null && image.sizeBytes! <= maximumSizeBytes) { + if (image.sizeBytes != null && image.sizeBytes! <= maximumSizeBytes && maximumSize > 0) { _currentSizeBytes += image.sizeBytes!; _cache[key] = image; _checkCacheSize(timelineTask); + } else { + image.dispose(); } } - void _trackLiveImage(Object key, _LiveImage image) { + void _trackLiveImage(Object key, ImageStreamCompleter completer, int? sizeBytes) { // Avoid adding unnecessary callbacks to the completer. _liveImages.putIfAbsent(key, () { // 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(image.handleRemove); - return image; - }).sizeBytes ??= image.sizeBytes; + // Even if the cache size is 0, we still add this tracker, which will add + // a keep alive handle to the stream. + return _LiveImage( + completer, + () { + _liveImages.remove(key); + }, + ); + }).sizeBytes ??= sizeBytes; } /// Returns the previously cached [ImageStream] for the given key, if available; @@ -337,14 +346,25 @@ class ImageCache { } // The image might have been keptAlive but had no listeners (so not live). // Make sure the cache starts tracking it as live again. - _trackLiveImage(key, _LiveImage(image.completer, image.sizeBytes, () => _liveImages.remove(key))); + _trackLiveImage( + key, + image.completer, + image.sizeBytes, + ); _cache[key] = image; return image.completer; } - final _CachedImage? liveImage = _liveImages[key]; + final _LiveImage? liveImage = _liveImages[key]; if (liveImage != null) { - _touch(key, liveImage, timelineTask); + _touch( + key, + _CachedImage( + liveImage.completer, + sizeBytes: liveImage.sizeBytes, + ), + timelineTask, + ); if (!kReleaseMode) { timelineTask!.finish(arguments: {'result': 'keepAlive'}); } @@ -353,7 +373,7 @@ class ImageCache { try { result = loader(); - _trackLiveImage(key, _LiveImage(result, null, () => _liveImages.remove(key))); + _trackLiveImage(key, result, null); } catch (error, stackTrace) { if (!kReleaseMode) { timelineTask!.finish(arguments: { @@ -384,33 +404,33 @@ class ImageCache { // 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 == null || info.image == null ? 0 : info.image.height * info.image.width * 4; - - final _CachedImage image = _CachedImage(result!, imageSize); - - _trackLiveImage( - key, - _LiveImage( - result, - imageSize, - () => _liveImages.remove(key), - ), + int? sizeBytes; + if (info != null) { + sizeBytes = info.image.height * info.image.width * 4; + info.dispose(); + } + final _CachedImage image = _CachedImage( + result!, + sizeBytes: sizeBytes, ); + _trackLiveImage(key, result, sizeBytes); + + // Only touch if the cache was enabled when resolve was initially called. + if (untrackedPendingImage == null) { + _touch(key, image, listenerTask); + } else { + image.dispose(); + } + final _PendingImage? pendingImage = untrackedPendingImage ?? _pendingImages.remove(key); if (pendingImage != null) { pendingImage.removeListener(); } - // 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, - 'sizeInBytes': imageSize, + 'sizeInBytes': sizeBytes, }); timelineTask!.finish(arguments: { 'currentSizeBytes': currentSizeBytes, @@ -469,7 +489,7 @@ class ImageCache { /// that are also being held by at least one other object. void clearLiveImages() { for (final _LiveImage image in _liveImages.values) { - image.removeListener(); + image.dispose(); } _liveImages.clear(); } @@ -489,6 +509,7 @@ class ImageCache { final Object key = _cache.keys.first; final _CachedImage image = _cache[key]!; _currentSizeBytes -= image.sizeBytes!; + image.dispose(); _cache.remove(key); if (!kReleaseMode) { finishArgs['evictedKeys'].add(key.toString()); @@ -576,22 +597,59 @@ class ImageCacheStatus { String toString() => '${objectRuntimeType(this, 'ImageCacheStatus')}(pending: $pending, live: $live, keepAlive: $keepAlive)'; } -class _CachedImage { - _CachedImage(this.completer, this.sizeBytes); +/// Base class for [_CachedImage] and [_LiveImage]. +/// +/// Exists primarily so that a [_LiveImage] cannot be added to the +/// [ImageCache._cache]. +abstract class _CachedImageBase { + _CachedImageBase( + this.completer, { + this.sizeBytes, + }) : assert(completer != null), + handle = completer.keepAlive(); final ImageStreamCompleter completer; int? sizeBytes; + ImageStreamCompleterHandle? handle; + + @mustCallSuper + void dispose() { + assert(handle != null); + // Give any interested parties a chance to listen to the stream before we + // potentially dispose it. + SchedulerBinding.instance!.addPostFrameCallback((Duration timeStamp) { + assert(handle != null); + handle?.dispose(); + handle = null; + }); + } } -class _LiveImage extends _CachedImage { - _LiveImage(ImageStreamCompleter completer, int? sizeBytes, this.handleRemove) - : super(completer, sizeBytes); +class _CachedImage extends _CachedImageBase { + _CachedImage(ImageStreamCompleter completer, {int? sizeBytes}) + : super(completer, sizeBytes: sizeBytes); +} - final VoidCallback handleRemove; - - void removeListener() { - completer.removeOnLastListenerRemovedCallback(handleRemove); +class _LiveImage extends _CachedImageBase { + _LiveImage(ImageStreamCompleter completer, VoidCallback handleRemove, {int? sizeBytes}) + : super(completer, sizeBytes: sizeBytes) { + _handleRemove = () { + handleRemove(); + dispose(); + }; + completer.addOnLastListenerRemovedCallback(_handleRemove); } + + late VoidCallback _handleRemove; + + @override + void dispose() { + completer.removeOnLastListenerRemovedCallback(_handleRemove); + super.dispose(); + } + + @override + String toString() => describeIdentity(this); } class _PendingImage { diff --git a/packages/flutter/lib/src/painting/image_provider.dart b/packages/flutter/lib/src/painting/image_provider.dart index 33167df42e..b135784fcb 100644 --- a/packages/flutter/lib/src/painting/image_provider.dart +++ b/packages/flutter/lib/src/painting/image_provider.dart @@ -284,6 +284,7 @@ typedef DecoderCallback = Future Function(Uint8List bytes, {int? cache /// void _updateImage(ImageInfo imageInfo, bool synchronousCall) { /// setState(() { /// // Trigger a build whenever the image changes. +/// _imageInfo?.image?.dispose(); /// _imageInfo = imageInfo; /// }); /// } @@ -291,6 +292,8 @@ typedef DecoderCallback = Future Function(Uint8List bytes, {int? cache /// @override /// void dispose() { /// _imageStream.removeListener(ImageStreamListener(_updateImage)); +/// _imageInfo?.image?.dispose(); +/// _imageInfo = null; /// super.dispose(); /// } /// diff --git a/packages/flutter/lib/src/painting/image_stream.dart b/packages/flutter/lib/src/painting/image_stream.dart index 670b677e45..23a481a24c 100644 --- a/packages/flutter/lib/src/painting/image_stream.dart +++ b/packages/flutter/lib/src/painting/image_stream.dart @@ -2,7 +2,6 @@ // 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:ui' as ui show Image, Codec, FrameInfo; import 'dart:ui' show hashValues; @@ -14,17 +13,80 @@ import 'package:flutter/scheduler.dart'; /// /// ImageInfo objects are used by [ImageStream] objects to represent the /// actual data of the image once it has been obtained. +/// +/// The receiver of an [ImageInfo] object must call [dispose]. To safely share +/// the object with other clients, use the [clone] method before calling +/// dispose. @immutable class ImageInfo { /// Creates an [ImageInfo] object for the given [image] and [scale]. /// - /// Both the image and the scale must not be null. + /// Both the [image] and the [scale] must not be null. /// - /// The tag may be used to identify the source of this image. + /// The [debugLabel] may be used to identify the source of this image. const ImageInfo({ required this.image, this.scale = 1.0, this.debugLabel }) : assert(image != null), assert(scale != null); + /// Creates an [ImageInfo] with a cloned [image]. + /// + /// Once all outstanding references to the [image] are disposed, it is no + /// longer safe to access properties of it or attempt to draw it. Clones serve + /// to create new references to the underlying image data that can safely be + /// disposed without knowledge of whether some other reference holder will + /// still need access to the underlying image. Once a client disposes of its + /// own image reference, it can no longer access the image, but other clients + /// will be able to access their own references. + /// + /// This method must be used in cases where a client holding an [ImageInfo] + /// needs to share the image info object with another client and will still + /// need to access the underlying image data at some later point, e.g. to + /// share it again with another client. + /// + /// See also: + /// + /// * [Image.clone] + ImageInfo clone() { + return ImageInfo( + image: image.clone(), + scale: scale, + debugLabel: debugLabel, + ); + } + + /// Whether this [ImageInfo] is a [clone] of the `other`. + /// + /// This method is a convenience wrapper for [Image.isCloneOf], and is useful + /// for clients that are trying to determine whether new layout or painting + /// logic is required when recieving a new image reference. + /// + /// {@tool snippet} + /// + /// The following sample shows how to appropriately check whether the + /// [ImageInfo] reference refers to new image data or not. + /// + /// ```dart + /// ImageInfo _imageInfo; + /// set imageInfo (ImageInfo value) { + /// // If the image reference is exactly the same, or its a clone of the + /// // current reference, we can immediately dispose of it and avoid + /// // recalculating anything. + /// if (value == _imageInfo || value != null && _imageInfo != null && value.isCloneOf(_imageInfo)) { + /// value?.dispose(); + /// return; + /// } + /// _imageInfo?.dispose(); + /// _imageInfo = value; + /// // Perform work to determine size, or paint the image. + /// } + /// ``` + /// {@end-tool} + bool isCloneOf(ImageInfo other) { + return other.image.isCloneOf(image) + && scale == scale + && other.debugLabel == debugLabel; + } + /// The raw image pixels. /// /// This is the object to pass to the [Canvas.drawImage], @@ -46,6 +108,15 @@ class ImageInfo { /// A string used for debugging purpopses to identify the source of this image. final String? debugLabel; + /// Disposes of this object. + /// + /// Once this method has been called, the object should not be used anymore, + /// and no clones of it or the image it contains can be made. + void dispose() { + assert((image.debugGetOpenHandleStackTraces()?.length ?? 1) > 0); + image.dispose(); + } + @override String toString() => '${debugLabel != null ? '$debugLabel ' : ''}$image @ ${debugFormatDouble(scale)}x'; @@ -137,6 +208,10 @@ class ImageStreamListener { /// /// Used in [ImageStreamListener]. /// +/// The `image` argument contains information about the image to be rendered. +/// The implementer of [ImageStreamListener.onImage] is expected to call dispose +/// on the [ui.Image] it receives. +/// /// The `synchronousCall` argument is true if the listener is being invoked /// during the call to `addListener`. This can be useful if, for example, /// [ImageStream.addListener] is invoked during a frame, so that a new rendering @@ -268,6 +343,9 @@ class ImageStream with Diagnosticable { /// times when the image stream completes (whether because a new image is /// available or because an error occurs). Likewise, to remove all instances /// of the listener, [removeListener] would need to called N times as well. + /// + /// When a `listener` receives an [ImageInfo] object, the `listener` is + /// responsible for disposing of the [ImageInfo.image]. /// {@endtemplate} void addListener(ImageStreamListener listener) { if (_completer != null) @@ -325,6 +403,37 @@ class ImageStream with Diagnosticable { } } +/// An opaque handle that keeps an [ImageStreamCompleter] alive even if it has +/// lost its last listener. +/// +/// To create a handle, use [ImageStreamCompleter.keepAlive]. +/// +/// Such handles are useful when an image cache needs to keep a completer alive +/// but does not actually have a listener subscribed, or when a widget that +/// displays an image needs to temporarily unsubscribe from the completer but +/// may re-subscribe in the future, for example when the [TickerMode] changes. +class ImageStreamCompleterHandle { + ImageStreamCompleterHandle._(ImageStreamCompleter this._completer) { + _completer!._keepAliveHandles += 1; + } + + ImageStreamCompleter? _completer; + + /// Call this method to signal the [ImageStreamCompleter] that it can now be + /// disposed when its last listener drops. + /// + /// This method must only be called once per object. + void dispose() { + assert(_completer != null); + assert(_completer!._keepAliveHandles > 0); + assert(!_completer!._disposed); + + _completer!._keepAliveHandles -= 1; + _completer!._maybeDispose(); + _completer = null; + } +} + /// Base class for those that manage the loading of [dart:ui.Image] objects for /// [ImageStream]s. /// @@ -358,6 +467,10 @@ abstract class ImageStreamCompleter with Diagnosticable { @visibleForTesting bool get hasListeners => _listeners.isNotEmpty; + /// We should avoid disposing a completer if it has never had a listener, even + /// if all [keepAlive] handles get disposed. + bool _hadAtLeastOneListener = false; + /// Adds a listener callback that is called whenever a new concrete [ImageInfo] /// object is available or an error is reported. If a concrete image is /// already available, or if an error has been already reported, this object @@ -368,10 +481,12 @@ abstract class ImageStreamCompleter with Diagnosticable { /// /// {@macro flutter.painting.imageStream.addListener} void addListener(ImageStreamListener listener) { + _checkDisposed(); + _hadAtLeastOneListener = true; _listeners.add(listener); if (_currentImage != null) { try { - listener.onImage(_currentImage!, true); + listener.onImage(_currentImage!.clone(), true); } catch (exception, stack) { reportError( context: ErrorDescription('by a synchronously-called image listener'), @@ -396,11 +511,29 @@ abstract class ImageStreamCompleter with Diagnosticable { } } + int _keepAliveHandles = 0; + /// Creates an [ImageStreamCompleterHandle] that will prevent this stream from + /// being disposed at least until the handle is disposed. + /// + /// Such handles are useful when an image cache needs to keep a completer + /// alive but does not itself have a listener subscribed, or when a widget + /// that displays an image needs to temporarily unsubscribe from the completer + /// but may re-subscribe in the future, for example when the [TickerMode] + /// changes. + ImageStreamCompleterHandle keepAlive() { + _checkDisposed(); + return ImageStreamCompleterHandle._(this); + } + /// Stops the specified [listener] from receiving image stream events. /// /// If [listener] has been added multiple times, this removes the _first_ /// instance of the listener. + /// + /// Once all listeners have been removed and all [keepAlive] handles have been + /// disposed, this image stream is no longer usable. void removeListener(ImageStreamListener listener) { + _checkDisposed(); for (int i = 0; i < _listeners.length; i += 1) { if (_listeners[i] == listener) { _listeners.removeAt(i); @@ -408,21 +541,49 @@ abstract class ImageStreamCompleter with Diagnosticable { } } if (_listeners.isEmpty) { - for (final VoidCallback callback in _onLastListenerRemovedCallbacks) { + final List callbacks = _onLastListenerRemovedCallbacks.toList(); + for (final VoidCallback callback in callbacks) { callback(); } _onLastListenerRemovedCallbacks.clear(); + _maybeDispose(); + } + } + + bool _disposed = false; + void _maybeDispose() { + if (!_hadAtLeastOneListener || _disposed || _listeners.isNotEmpty || _keepAliveHandles != 0) { + return; + } + + _currentImage?.dispose(); + _currentImage = null; + _disposed = true; + } + + void _checkDisposed() { + if (_disposed) { + throw StateError( + 'Stream has been disposed.\n' + 'An ImageStream is considered disposed once at least one listener has ' + 'been added and subsequently all listeners have been removed and no ' + 'handles are outstanding from the keepAlive method.\n' + 'To resolve this error, maintain at least one listener on the stream, ' + 'or create an ImageStreamCompleterHandle from the keepAlive ' + 'method, or create a new stream for the image.', + ); } } final List _onLastListenerRemovedCallbacks = []; /// Adds a callback to call when [removeListener] results in an empty - /// list of listeners. + /// list of listeners and there are no [keepAlive] handles outstanding. /// /// This callback will never fire if [removeListener] is never called. void addOnLastListenerRemovedCallback(VoidCallback callback) { assert(callback != null); + _checkDisposed(); _onLastListenerRemovedCallbacks.add(callback); } @@ -430,13 +591,17 @@ abstract class ImageStreamCompleter with Diagnosticable { /// [addOnLastListenerRemovedCallback]. void removeOnLastListenerRemovedCallback(VoidCallback callback) { assert(callback != null); + _checkDisposed(); _onLastListenerRemovedCallbacks.remove(callback); } /// Calls all the registered listeners to notify them of a new image. @protected void setImage(ImageInfo image) { + _checkDisposed(); + _currentImage?.dispose(); _currentImage = image; + if (_listeners.isEmpty) return; // Make a copy to allow for concurrent modification. @@ -444,7 +609,7 @@ abstract class ImageStreamCompleter with Diagnosticable { List.from(_listeners); for (final ImageStreamListener listener in localListeners) { try { - listener.onImage(image, false); + listener.onImage(image.clone(), false); } catch (exception, stack) { reportError( context: ErrorDescription('by an image listener'), @@ -531,6 +696,7 @@ abstract class ImageStreamCompleter with Diagnosticable { /// [ImageChunkEvent]. @protected void reportImageChunkEvent(ImageChunkEvent event){ + _checkDisposed(); if (hasListeners) { // Make a copy to allow for concurrent modification. final List localListeners = _listeners @@ -554,6 +720,7 @@ abstract class ImageStreamCompleter with Diagnosticable { _listeners, ifPresent: '${_listeners.length} listener${_listeners.length == 1 ? "" : "s" }', )); + description.add(FlagProperty('disposed', value: _disposed, ifTrue: '')); } } @@ -700,10 +867,16 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { _frameCallbackScheduled = false; if (!hasListeners) return; + assert(_nextFrame != null); if (_isFirstFrame() || _hasFrameDurationPassed(timestamp)) { - _emitFrame(ImageInfo(image: _nextFrame!.image, scale: _scale, debugLabel: debugLabel)); + _emitFrame(ImageInfo( + image: _nextFrame!.image.clone(), + scale: _scale, + debugLabel: debugLabel, + )); _shownTimestamp = timestamp; _frameDuration = _nextFrame!.duration; + _nextFrame!.image.dispose(); _nextFrame = null; final int completedCycles = _framesEmitted ~/ _codec!.frameCount; if (_codec!.repetitionCount == -1 || completedCycles <= _codec!.repetitionCount) { @@ -726,6 +899,10 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { } Future _decodeNextFrameAndSchedule() async { + // This will be null if we gave it away. If not, it's still ours and it + // must be disposed of. + _nextFrame?.image.dispose(); + _nextFrame = null; try { _nextFrame = await _codec!.getNextFrame(); } catch (exception, stack) { @@ -741,7 +918,13 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { if (_codec!.frameCount == 1) { // This is not an animated image, just return it and don't schedule more // frames. - _emitFrame(ImageInfo(image: _nextFrame!.image, scale: _scale, debugLabel: debugLabel)); + _emitFrame(ImageInfo( + image: _nextFrame!.image.clone(), + scale: _scale, + debugLabel: debugLabel, + )); + _nextFrame!.image.dispose(); + _nextFrame = null; return; } _scheduleAppFrame(); diff --git a/packages/flutter/lib/src/rendering/image.dart b/packages/flutter/lib/src/rendering/image.dart index 2659f69ca3..7832f61911 100644 --- a/packages/flutter/lib/src/rendering/image.dart +++ b/packages/flutter/lib/src/rendering/image.dart @@ -85,8 +85,16 @@ class RenderImage extends RenderBox { ui.Image? get image => _image; ui.Image? _image; set image(ui.Image? value) { - if (value == _image) + if (value == _image) { return; + } + // If we get a clone of our image, it's the same underlying native data - + // dispose of the new clone and return early. + if (value != null && _image != null && value.isCloneOf(_image!)) { + value.dispose(); + return; + } + _image?.dispose(); _image = value; markNeedsPaint(); if (_width == null || _height == null) diff --git a/packages/flutter/lib/src/widgets/basic.dart b/packages/flutter/lib/src/widgets/basic.dart index 5c3d80b57d..2958280acb 100644 --- a/packages/flutter/lib/src/widgets/basic.dart +++ b/packages/flutter/lib/src/widgets/basic.dart @@ -5363,6 +5363,10 @@ class RichText extends MultiChildRenderObjectWidget { /// The image is painted using [paintImage], which describes the meanings of the /// various fields on this class in more detail. /// +/// The [image] is not disposed of by this widget. Creators of the widget are +/// expected to call [Image.dispose] on the [image] once the [RawImage] is no +/// longer buildable. +/// /// This widget is rarely used directly. Instead, consider using [Image]. class RawImage extends LeafRenderObjectWidget { /// Creates a widget that displays an image. @@ -5394,6 +5398,10 @@ class RawImage extends LeafRenderObjectWidget { super(key: key); /// The image to display. + /// + /// Since a [RawImage] is stateless, it does not ever dispose this image. + /// Creators of a [RawImage] are expected to call [Image.dispose] on this + /// image handle when the [RawImage] will no longer be needed. final ui.Image? image; /// A string identifying the source of the image. @@ -5516,8 +5524,13 @@ class RawImage extends LeafRenderObjectWidget { @override RenderImage createRenderObject(BuildContext context) { assert((!matchTextDirection && alignment is Alignment) || debugCheckHasDirectionality(context)); + assert( + image?.debugGetOpenHandleStackTraces()?.isNotEmpty ?? true, + 'Creator of a RawImage disposed of the image when the RawImage still ' + 'needed it.' + ); return RenderImage( - image: image, + image: image?.clone(), debugImageLabel: debugImageLabel, width: width, height: height, @@ -5538,8 +5551,13 @@ class RawImage extends LeafRenderObjectWidget { @override void updateRenderObject(BuildContext context, RenderImage renderObject) { + assert( + image?.debugGetOpenHandleStackTraces()?.isNotEmpty ?? true, + 'Creator of a RawImage disposed of the image when the RawImage still ' + 'needed it.' + ); renderObject - ..image = image + ..image = image?.clone() ..debugImageLabel = debugImageLabel ..width = width ..height = height @@ -5556,6 +5574,12 @@ class RawImage extends LeafRenderObjectWidget { ..filterQuality = filterQuality; } + @override + void didUnmountRenderObject(RenderImage renderObject) { + // Have the render object dispose its image handle. + renderObject.image = null; + } + @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); diff --git a/packages/flutter/lib/src/widgets/image.dart b/packages/flutter/lib/src/widgets/image.dart index 0011bf5765..068fcf634b 100644 --- a/packages/flutter/lib/src/widgets/image.dart +++ b/packages/flutter/lib/src/widgets/image.dart @@ -1084,6 +1084,7 @@ class _ImageState extends State with WidgetsBindingObserver { late DisposableBuildContext> _scrollAwareContext; Object? _lastException; StackTrace? _lastStack; + ImageStreamCompleterHandle? _completerHandle; @override void initState() { @@ -1097,7 +1098,9 @@ class _ImageState extends State with WidgetsBindingObserver { assert(_imageStream != null); WidgetsBinding.instance!.removeObserver(this); _stopListeningToStream(); + _completerHandle?.dispose(); _scrollAwareContext.dispose(); + _replaceImage(info: null); super.dispose(); } @@ -1109,7 +1112,7 @@ class _ImageState extends State with WidgetsBindingObserver { if (TickerMode.of(context)) _listenToStream(); else - _stopListeningToStream(); + _stopListeningToStream(keepStreamAlive: true); super.didChangeDependencies(); } @@ -1119,8 +1122,9 @@ class _ImageState extends State with WidgetsBindingObserver { super.didUpdateWidget(oldWidget); if (_isListeningToStream && (widget.loadingBuilder == null) != (oldWidget.loadingBuilder == null)) { - _imageStream!.removeListener(_getListener()); + final ImageStreamListener oldListener = _getListener(); _imageStream!.addListener(_getListener(recreateListener: true)); + _imageStream!.removeListener(oldListener); } if (widget.image != oldWidget.image) _resolveImage(); @@ -1182,7 +1186,7 @@ class _ImageState extends State with WidgetsBindingObserver { void _handleImageFrame(ImageInfo imageInfo, bool synchronousCall) { setState(() { - _imageInfo = imageInfo; + _replaceImage(info: imageInfo); _loadingProgress = null; _lastException = null; _lastStack = null; @@ -1200,6 +1204,11 @@ class _ImageState extends State with WidgetsBindingObserver { }); } + void _replaceImage({required ImageInfo? info}) { + _imageInfo?.dispose(); + _imageInfo = info; + } + // Updates _imageStream to newStream, and moves the stream listener // registration from the old stream to the new stream (if a listener was // registered). @@ -1211,7 +1220,7 @@ class _ImageState extends State with WidgetsBindingObserver { _imageStream!.removeListener(_getListener()); if (!widget.gaplessPlayback) - setState(() { _imageInfo = null; }); + setState(() { _replaceImage(info: null); }); setState(() { _loadingProgress = null; @@ -1227,13 +1236,29 @@ class _ImageState extends State with WidgetsBindingObserver { void _listenToStream() { if (_isListeningToStream) return; + _imageStream!.addListener(_getListener()); + _completerHandle?.dispose(); + _completerHandle = null; + _isListeningToStream = true; } - void _stopListeningToStream() { + /// Stops listening to the image stream, if this state object has attached a + /// listener. + /// + /// If the listener from this state is the last listener on the stream, the + /// stream will be disposed. To keep the stream alive, set `keepStreamAlive` + /// to true, which will add a passive listener on the stream and is compatible + /// with the [TickerMode] being off. + void _stopListeningToStream({bool keepStreamAlive = false}) { if (!_isListeningToStream) return; + + if (keepStreamAlive && _completerHandle == null && _imageStream?.completer != null) { + _completerHandle = _imageStream!.completer!.keepAlive(); + } + _imageStream!.removeListener(_getListener()); _isListeningToStream = false; } @@ -1246,6 +1271,10 @@ class _ImageState extends State with WidgetsBindingObserver { } Widget result = RawImage( + // Do not clone the image, because RawImage is a stateless wrapper. + // The image will be disposed by this state object when it is not needed + // anymore, such as when it is unmounted or when the image stream pushes + // a new image. image: _imageInfo?.image, debugImageLabel: _imageInfo?.debugLabel, width: widget.width, diff --git a/packages/flutter/test/painting/decoration_test.dart b/packages/flutter/test/painting/decoration_test.dart index a4161f0ba1..060fc68324 100644 --- a/packages/flutter/test/painting/decoration_test.dart +++ b/packages/flutter/test/painting/decoration_test.dart @@ -6,6 +6,7 @@ @TestOn('!chrome') import 'dart:async'; +import 'dart:typed_data'; import 'dart:ui' as ui show Image, ColorFilter; import 'package:flutter/foundation.dart'; @@ -13,6 +14,7 @@ import 'package:flutter/painting.dart'; import 'package:fake_async/fake_async.dart'; import '../flutter_test_alternative.dart'; +import '../image_data.dart'; import '../painting/mocks_for_image_cache.dart'; import '../rendering/rendering_tester.dart'; @@ -62,6 +64,10 @@ class SynchronousErrorTestImageProvider extends ImageProvider { } class AsyncTestImageProvider extends ImageProvider { + AsyncTestImageProvider(this.image); + + final ui.Image image; + @override Future obtainKey(ImageConfiguration configuration) { return Future.value(2); @@ -70,7 +76,7 @@ class AsyncTestImageProvider extends ImageProvider { @override ImageStreamCompleter load(int key, DecoderCallback decode) { return OneFrameImageStreamCompleter( - Future.value(TestImageInfo(key)) + Future.value(TestImageInfo(key, image: image)) ); } } @@ -100,6 +106,31 @@ class DelayedImageProvider extends ImageProvider { String toString() => '${describeIdentity(this)}()'; } +class MultiFrameImageProvider extends ImageProvider { + MultiFrameImageProvider(this.completer); + + final MultiImageCompleter completer; + + @override + Future obtainKey(ImageConfiguration configuration) { + return SynchronousFuture(this); + } + + @override + ImageStreamCompleter load(MultiFrameImageProvider key, DecoderCallback decode) { + return completer; + } + + @override + String toString() => '${describeIdentity(this)}()'; +} + +class MultiImageCompleter extends ImageStreamCompleter { + void testSetImage(ImageInfo info) { + setImage(info); + } +} + void main() { TestRenderingFlutterBinding(); // initializes the imageCache @@ -151,9 +182,10 @@ void main() { expect(onChangedCalled, equals(false)); }); - test('BoxDecorationImageListenerAsync', () { + test('BoxDecorationImageListenerAsync', () async { + final ui.Image image = await createTestImage(width: 10, height: 10); FakeAsync().run((FakeAsync async) { - final ImageProvider imageProvider = AsyncTestImageProvider(); + final ImageProvider imageProvider = AsyncTestImageProvider(image); final DecorationImage backgroundImage = DecorationImage(image: imageProvider); final BoxDecoration boxDecoration = BoxDecoration(image: backgroundImage); @@ -173,6 +205,42 @@ void main() { }); }); + test('BoxDecorationImageListener does not change when image is clone', () async { + final ui.Image image1 = await createTestImage(width: 10, height: 10, cache: false); + final ui.Image image2 = await createTestImage(width: 10, height: 10, cache: false); + final MultiImageCompleter completer = MultiImageCompleter(); + final MultiFrameImageProvider imageProvider = MultiFrameImageProvider(completer); + final DecorationImage backgroundImage = DecorationImage(image: imageProvider); + + final BoxDecoration boxDecoration = BoxDecoration(image: backgroundImage); + bool onChangedCalled = false; + final BoxPainter boxPainter = boxDecoration.createBoxPainter(() { + onChangedCalled = true; + }); + + final TestCanvas canvas = TestCanvas(); + const ImageConfiguration imageConfiguration = ImageConfiguration(size: Size.zero); + boxPainter.paint(canvas, Offset.zero, imageConfiguration); + + // The onChanged callback should be invoked asynchronously. + expect(onChangedCalled, equals(false)); + + completer.testSetImage(ImageInfo(image: image1.clone())); + await null; + + expect(onChangedCalled, equals(true)); + onChangedCalled = false; + completer.testSetImage(ImageInfo(image: image1.clone())); + await null; + + expect(onChangedCalled, equals(false)); + + completer.testSetImage(ImageInfo(image: image2.clone())); + await null; + + expect(onChangedCalled, equals(true)); + }); + // Regression test for https://github.com/flutter/flutter/issues/7289. // A reference test would be better. test('BoxDecoration backgroundImage clip', () async { @@ -626,4 +694,30 @@ void main() { expect(call.positionalArguments[2].size, const Size(25.0, 25.0)); expect(call.positionalArguments[2], const Rect.fromLTRB(0.0, 0.0, 25.0, 25.0)); }); + + test('DecorationImagePainter disposes of image when disposed', () async { + final ImageProvider provider = MemoryImage(Uint8List.fromList(kTransparentImage)); + + final ImageStream stream = provider.resolve(ImageConfiguration.empty); + + final Completer infoCompleter = Completer(); + void _listener(ImageInfo image, bool syncCall) { + assert(!infoCompleter.isCompleted); + infoCompleter.complete(image); + } + stream.addListener(ImageStreamListener(_listener)); + + final ImageInfo info = await infoCompleter.future; + final int baselineRefCount = info.image.debugGetOpenHandleStackTraces().length; + + final DecorationImagePainter painter = DecorationImage(image: provider).createPainter(() {}); + final Canvas canvas = TestCanvas(); + painter.paint(canvas, Rect.zero, Path(), ImageConfiguration.empty); + + expect(info.image.debugGetOpenHandleStackTraces().length, baselineRefCount + 1); + painter.dispose(); + expect(info.image.debugGetOpenHandleStackTraces().length, baselineRefCount); + + info.dispose(); + }); } diff --git a/packages/flutter/test/painting/image_cache_test.dart b/packages/flutter/test/painting/image_cache_test.dart index d5af602726..718642f8bf 100644 --- a/packages/flutter/test/painting/image_cache_test.dart +++ b/packages/flutter/test/painting/image_cache_test.dart @@ -6,9 +6,11 @@ import 'dart:ui' as ui; +import 'package:flutter/foundation.dart'; import 'package:flutter/painting.dart'; -import '../flutter_test_alternative.dart'; +import 'package:flutter/scheduler.dart'; +import '../flutter_test_alternative.dart'; import '../rendering/rendering_tester.dart'; import 'mocks_for_image_cache.dart'; @@ -456,7 +458,6 @@ void main() { final TestImageStreamCompleter completer1 = TestImageStreamCompleter() ..addListener(listener); - imageCache.putIfAbsent(testImage, () => completer1); expect(imageCache.statusForKey(testImage).pending, true); expect(imageCache.statusForKey(testImage).live, true); @@ -484,4 +485,88 @@ void main() { expect(imageCache.statusForKey(testImage).keepAlive, true); expect(imageCache.currentSizeBytes, testImageSize); }); + + test('Image is obtained and disposed of properly for cache', () async { + const int key = 1; + final ui.Image testImage = await createTestImage(width: 8, height: 8, cache: false); + expect(testImage.debugGetOpenHandleStackTraces().length, 1); + + ImageInfo imageInfo; + final ImageStreamListener listener = ImageStreamListener((ImageInfo info, bool syncCall) { + imageInfo = info; + }); + + final TestImageStreamCompleter completer = TestImageStreamCompleter(); + + completer.addListener(listener); + imageCache.putIfAbsent(key, () => completer); + + expect(testImage.debugGetOpenHandleStackTraces().length, 1); + + // This should cause keepAlive to be set to true. + completer.testSetImage(testImage); + expect(imageInfo, isNotNull); + // +1 ImageStreamCompleter + expect(testImage.debugGetOpenHandleStackTraces().length, 2); + + completer.removeListener(listener); + + // Force us to the end of the frame. + SchedulerBinding.instance.scheduleFrame(); + await SchedulerBinding.instance.endOfFrame; + + expect(testImage.debugGetOpenHandleStackTraces().length, 2); + + expect(imageCache.evict(key), true); + + // Force us to the end of the frame. + SchedulerBinding.instance.scheduleFrame(); + await SchedulerBinding.instance.endOfFrame; + + // -1 _CachedImage + // -1 ImageStreamCompleter + expect(testImage.debugGetOpenHandleStackTraces().length, 1); + + imageInfo.dispose(); + expect(testImage.debugGetOpenHandleStackTraces().length, 0); + }, skip: kIsWeb); // Web does not care about image handles. + + test('Image is obtained and disposed of properly for cache when listener is still active', () async { + const int key = 1; + final ui.Image testImage = await createTestImage(width: 8, height: 8, cache: false); + expect(testImage.debugGetOpenHandleStackTraces().length, 1); + + ImageInfo imageInfo; + final ImageStreamListener listener = ImageStreamListener((ImageInfo info, bool syncCall) { + imageInfo = info; + }); + + final TestImageStreamCompleter completer = TestImageStreamCompleter(); + + completer.addListener(listener); + imageCache.putIfAbsent(key, () => completer); + + expect(testImage.debugGetOpenHandleStackTraces().length, 1); + + // This should cause keepAlive to be set to true. + completer.testSetImage(testImage); + expect(imageInfo, isNotNull); + // Just our imageInfo and the completer. + expect(testImage.debugGetOpenHandleStackTraces().length, 2); + + expect(imageCache.evict(key), true); + + // Force us to the end of the frame. + SchedulerBinding.instance.scheduleFrame(); + await SchedulerBinding.instance.endOfFrame; + + // Live image still around since there's still a listener, and the listener + // should be holding a handle. + expect(testImage.debugGetOpenHandleStackTraces().length, 2); + completer.removeListener(listener); + + expect(testImage.debugGetOpenHandleStackTraces().length, 1); + imageInfo.dispose(); + expect(testImage.debugGetOpenHandleStackTraces().length, 0); + }, skip: kIsWeb); // Web does not care about open image handles. } diff --git a/packages/flutter/test/painting/image_stream_test.dart b/packages/flutter/test/painting/image_stream_test.dart index 2f9aa75b21..3f156d9973 100644 --- a/packages/flutter/test/painting/image_stream_test.dart +++ b/packages/flutter/test/painting/image_stream_test.dart @@ -8,13 +8,12 @@ import 'dart:async'; import 'dart:ui'; import 'package:flutter/painting.dart'; -import 'package:flutter/scheduler.dart' show timeDilation; +import 'package:flutter/scheduler.dart' show timeDilation, SchedulerBinding; import 'package:flutter_test/flutter_test.dart'; import 'package:meta/meta.dart'; class FakeFrameInfo implements FrameInfo { - FakeFrameInfo(this._duration, this._image); - + const FakeFrameInfo(this._duration, this._image); final Duration _duration; final Image _image; @@ -24,6 +23,15 @@ class FakeFrameInfo implements FrameInfo { @override Image get image => _image; + + int get imageHandleCount => image.debugGetOpenHandleStackTraces().length; + + FakeFrameInfo clone() { + return FakeFrameInfo( + _duration, + _image.clone(), + ); + } } class MockCodec implements Codec { @@ -72,7 +80,7 @@ class FakeEventReportingImageStreamCompleter extends ImageStreamCompleter { void main() { Image image20x10; Image image200x100; - setUpAll(() async { + setUp(() async { image20x10 = await createTestImage(width: 20, height: 10); image200x100 = await createTestImage(width: 200, height: 100); }); @@ -299,7 +307,7 @@ void main() { mockCodec.completeNextFrame(frame); await tester.idle(); - expect(emittedImages, equals([ImageInfo(image: frame.image)])); + expect(emittedImages.every((ImageInfo info) => info.image.isCloneOf(frame.image)), true); }); testWidgets('ImageStream emits frames (animated images)', (WidgetTester tester) async { @@ -329,7 +337,7 @@ void main() { expect(emittedImages.length, 0); await tester.pump(); - expect(emittedImages, equals([ImageInfo(image: frame1.image)])); + expect(emittedImages.single.image.isCloneOf(frame1.image), true); final FrameInfo frame2 = FakeFrameInfo(const Duration(milliseconds: 400), image200x100); mockCodec.completeNextFrame(frame2); @@ -340,10 +348,8 @@ void main() { expect(emittedImages.length, 1); await tester.pump(const Duration(milliseconds: 100)); - expect(emittedImages, equals([ - ImageInfo(image: frame1.image), - ImageInfo(image: frame2.image), - ])); + expect(emittedImages[0].image.isCloneOf(frame1.image), true); + expect(emittedImages[1].image.isCloneOf(frame2.image), true); // Let the pending timer for the next frame to complete so we can cleanly // quit the test without pending timers. @@ -369,24 +375,22 @@ void main() { codecCompleter.complete(mockCodec); await tester.idle(); - final FrameInfo frame1 = FakeFrameInfo(const Duration(milliseconds: 200), image20x10); - final FrameInfo frame2 = FakeFrameInfo(const Duration(milliseconds: 400), image200x100); + final FakeFrameInfo frame1 = FakeFrameInfo(const Duration(milliseconds: 200), image20x10); + final FakeFrameInfo frame2 = FakeFrameInfo(const Duration(milliseconds: 400), image200x100); - mockCodec.completeNextFrame(frame1); + mockCodec.completeNextFrame(frame1.clone()); await tester.idle(); // let nextFrameFuture complete await tester.pump(); // first animation frame shows on first app frame. - mockCodec.completeNextFrame(frame2); + mockCodec.completeNextFrame(frame2.clone()); await tester.idle(); // let nextFrameFuture complete await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame. - mockCodec.completeNextFrame(frame1); + mockCodec.completeNextFrame(frame1.clone()); await tester.idle(); // let nextFrameFuture complete await tester.pump(const Duration(milliseconds: 400)); // emit 3rd frame - expect(emittedImages, equals([ - ImageInfo(image: frame1.image), - ImageInfo(image: frame2.image), - ImageInfo(image: frame1.image), - ])); + expect(emittedImages[0].image.isCloneOf(frame1.image), true); + expect(emittedImages[1].image.isCloneOf(frame2.image), true); + expect(emittedImages[2].image.isCloneOf(frame1.image), true); // Let the pending timer for the next frame to complete so we can cleanly // quit the test without pending timers. @@ -427,13 +431,11 @@ void main() { await tester.idle(); await tester.pump(const Duration(milliseconds: 400)); - expect(emittedImages, equals([ - ImageInfo(image: frame1.image), - ImageInfo(image: frame2.image), - ])); + expect(emittedImages[0].image.isCloneOf(frame1.image), true); + expect(emittedImages[1].image.isCloneOf(frame2.image), true); }); - testWidgets('frames are only decoded when there are active listeners', (WidgetTester tester) async { + testWidgets('frames are only decoded when there are listeners', (WidgetTester tester) async { final MockCodec mockCodec = MockCodec(); mockCodec.frameCount = 2; mockCodec.repetitionCount = -1; @@ -446,6 +448,7 @@ void main() { final ImageListener listener = (ImageInfo image, bool synchronousCall) { }; imageStream.addListener(ImageStreamListener(listener)); + final ImageStreamCompleterHandle handle = imageStream.keepAlive(); codecCompleter.complete(mockCodec); await tester.idle(); @@ -468,6 +471,8 @@ void main() { imageStream.addListener(ImageStreamListener(listener)); await tester.idle(); // let nextFrameFuture complete expect(mockCodec.numFramesAsked, 3); + + handle.dispose(); }); testWidgets('multiple stream listeners', (WidgetTester tester) async { @@ -501,8 +506,9 @@ void main() { mockCodec.completeNextFrame(frame1); await tester.idle(); // let nextFrameFuture complete await tester.pump(); // first animation frame shows on first app frame. - expect(emittedImages1, equals([ImageInfo(image: frame1.image)])); - expect(emittedImages2, equals([ImageInfo(image: frame1.image)])); + + expect(emittedImages1.single.image.isCloneOf(frame1.image), true); + expect(emittedImages2.single.image.isCloneOf(frame1.image), true); mockCodec.completeNextFrame(frame2); await tester.idle(); // let nextFrameFuture complete @@ -510,11 +516,10 @@ void main() { imageStream.removeListener(ImageStreamListener(listener1)); await tester.pump(const Duration(milliseconds: 400)); // emit 2nd frame. - expect(emittedImages1, equals([ImageInfo(image: frame1.image)])); - expect(emittedImages2, equals([ - ImageInfo(image: frame1.image), - ImageInfo(image: frame2.image), - ])); + expect(emittedImages1.single.image.isCloneOf(frame1.image), true); + expect(emittedImages2[0].image.isCloneOf(frame1.image), true); + expect(emittedImages2[1].image.isCloneOf(frame2.image), true); + }); testWidgets('timer is canceled when listeners are removed', (WidgetTester tester) async { @@ -639,8 +644,8 @@ void main() { await tester.idle(); // let nextFrameFuture complete - imageStream.removeListener(ImageStreamListener(listener)); imageStream.addListener(ImageStreamListener(listener)); + imageStream.removeListener(ImageStreamListener(listener)); final FrameInfo frame1 = FakeFrameInfo(const Duration(milliseconds: 200), image20x10); @@ -684,6 +689,76 @@ void main() { compare(onImage1: handleImage, onChunk1: handleChunk, onError1: handleError, onImage2: handleImage, onError2: handleError, areEqual: false); }); + testWidgets('Keep alive handles do not drive frames or prevent last listener callbacks', (WidgetTester tester) async { + final Image image10x10 = await tester.runAsync(() => createTestImage(width: 10, height: 10)); + final MockCodec mockCodec = MockCodec(); + mockCodec.frameCount = 2; + mockCodec.repetitionCount = -1; + final Completer codecCompleter = Completer(); + + final ImageStreamCompleter imageStream = MultiFrameImageStreamCompleter( + codec: codecCompleter.future, + scale: 1.0, + ); + + int onImageCount = 0; + final ImageListener activeListener = (ImageInfo image, bool synchronousCall) { + onImageCount += 1; + }; + bool lastListenerDropped = false; + imageStream.addOnLastListenerRemovedCallback(() { + lastListenerDropped = true; + }); + + expect(lastListenerDropped, false); + final ImageStreamCompleterHandle handle = imageStream.keepAlive(); + expect(lastListenerDropped, false); + SchedulerBinding.instance.debugAssertNoTransientCallbacks('Only passive listeners'); + + codecCompleter.complete(mockCodec); + await tester.idle(); + + expect(onImageCount, 0); + + final FakeFrameInfo frame1 = FakeFrameInfo(Duration.zero, image20x10); + mockCodec.completeNextFrame(frame1); + await tester.idle(); + SchedulerBinding.instance.debugAssertNoTransientCallbacks('Only passive listeners'); + await tester.pump(); + expect(onImageCount, 0); + + imageStream.addListener(ImageStreamListener(activeListener)); + + final FakeFrameInfo frame2 = FakeFrameInfo(Duration.zero, image10x10); + mockCodec.completeNextFrame(frame2); + await tester.idle(); + expect(SchedulerBinding.instance.transientCallbackCount, 1); + await tester.pump(); + + expect(onImageCount, 1); + + imageStream.removeListener(ImageStreamListener(activeListener)); + expect(lastListenerDropped, true); + + mockCodec.completeNextFrame(frame1); + await tester.idle(); + expect(SchedulerBinding.instance.transientCallbackCount, 1); + await tester.pump(); + + expect(onImageCount, 1); + + SchedulerBinding.instance.debugAssertNoTransientCallbacks('Only passive listeners'); + + mockCodec.completeNextFrame(frame2); + await tester.idle(); + SchedulerBinding.instance.debugAssertNoTransientCallbacks('Only passive listeners'); + await tester.pump(); + + expect(onImageCount, 1); + + handle.dispose(); + }); + // TODO(amirh): enable this once WidgetTester supports flushTimers. // https://github.com/flutter/flutter/issues/30344 // testWidgets('remove and add listener before a delayed frame is scheduled', (WidgetTester tester) async { diff --git a/packages/flutter/test/painting/mocks_for_image_cache.dart b/packages/flutter/test/painting/mocks_for_image_cache.dart index 899254cf58..c8ca57feec 100644 --- a/packages/flutter/test/painting/mocks_for_image_cache.dart +++ b/packages/flutter/test/painting/mocks_for_image_cache.dart @@ -10,8 +10,10 @@ import 'dart:ui' as ui show Image; import 'package:flutter/foundation.dart'; import 'package:flutter/painting.dart'; +// ignore: must_be_immutable class TestImageInfo implements ImageInfo { - const TestImageInfo(this.value, { this.image, this.scale = 1.0, this.debugLabel }); + TestImageInfo(this.value, { @required this.image, this.scale = 1.0, this.debugLabel }) + : assert(image != null); @override final ui.Image image; @@ -26,6 +28,39 @@ class TestImageInfo implements ImageInfo { @override String toString() => '$runtimeType($value)'; + + @override + TestImageInfo clone() { + return TestImageInfo(value, image: image.clone(), scale: scale, debugLabel: debugLabel); + } + + @override + bool isCloneOf(ImageInfo other) { + assert(other != null); + return other.image.isCloneOf(image) + && scale == scale + && other.debugLabel == debugLabel; + } + + @override + void dispose() { + image.dispose(); + } + + @override + int get hashCode => hashValues(value, image, scale, debugLabel); + + @override + bool operator ==(Object other) { + if (other.runtimeType != runtimeType) + return false; + return other is TestImageInfo + && other.value == value + && other.image.isCloneOf(image) + && other.scale == scale + && other.debugLabel == debugLabel; + + } } class TestImageProvider extends ImageProvider { @@ -44,7 +79,7 @@ class TestImageProvider extends ImageProvider { @override ImageStreamCompleter load(int key, DecoderCallback decode) { return OneFrameImageStreamCompleter( - SynchronousFuture(TestImageInfo(imageValue, image: image)) + SynchronousFuture(TestImageInfo(imageValue, image: image.clone())) ); } diff --git a/packages/flutter/test/rendering/image_test.dart b/packages/flutter/test/rendering/image_test.dart index 504d44b468..f06377b95f 100644 --- a/packages/flutter/test/rendering/image_test.dart +++ b/packages/flutter/test/rendering/image_test.dart @@ -176,4 +176,38 @@ Future main() async { image.colorBlendMode = BlendMode.color; expect(image.colorBlendMode, BlendMode.color); }); + + test('Render image disposes its image', () async { + final ui.Image image = await createTestImage(width: 10, height: 10, cache: false); + expect(image.debugGetOpenHandleStackTraces().length, 1); + + final RenderImage renderImage = RenderImage(image: image.clone()); + expect(image.debugGetOpenHandleStackTraces().length, 2); + + renderImage.image = image.clone(); + expect(image.debugGetOpenHandleStackTraces().length, 2); + + renderImage.image = null; + expect(image.debugGetOpenHandleStackTraces().length, 1); + + image.dispose(); + expect(image.debugGetOpenHandleStackTraces().length, 0); + }, skip: kIsWeb); // Web doesn't track open image handles. + + test('Render image does not dispose its image if setting the same image twice', () async { + final ui.Image image = await createTestImage(width: 10, height: 10, cache: false); + expect(image.debugGetOpenHandleStackTraces().length, 1); + + final RenderImage renderImage = RenderImage(image: image.clone()); + expect(image.debugGetOpenHandleStackTraces().length, 2); + + renderImage.image = renderImage.image; + expect(image.debugGetOpenHandleStackTraces().length, 2); + + renderImage.image = null; + expect(image.debugGetOpenHandleStackTraces().length, 1); + + image.dispose(); + expect(image.debugGetOpenHandleStackTraces().length, 0); + }, skip: kIsWeb); // Web doesn't track open image handles. } diff --git a/packages/flutter/test/rendering/mock_canvas.dart b/packages/flutter/test/rendering/mock_canvas.dart index 9aa9c590b6..934d79ff54 100644 --- a/packages/flutter/test/rendering/mock_canvas.dart +++ b/packages/flutter/test/rendering/mock_canvas.dart @@ -1315,7 +1315,7 @@ class _DrawImagePaintPredicate extends _DrawCommandPaintPredicate { void verifyArguments(List arguments) { super.verifyArguments(arguments); final ui.Image imageArgument = arguments[0] as ui.Image; - if (image != null && imageArgument != image) + if (image != null && !image.isCloneOf(imageArgument)) throw 'It called $methodName with an image, $imageArgument, which was not exactly the expected image ($image).'; final Offset pointArgument = arguments[0] as Offset; if (x != null && y != null) { @@ -1359,7 +1359,7 @@ class _DrawImageRectPaintPredicate extends _DrawCommandPaintPredicate { void verifyArguments(List arguments) { super.verifyArguments(arguments); final ui.Image imageArgument = arguments[0] as ui.Image; - if (image != null && imageArgument != image) + if (image != null && !image.isCloneOf(imageArgument)) throw 'It called $methodName with an image, $imageArgument, which was not exactly the expected image ($image).'; final Rect sourceArgument = arguments[1] as Rect; if (source != null && sourceArgument != source) diff --git a/packages/flutter/test/widgets/fade_in_image_test.dart b/packages/flutter/test/widgets/fade_in_image_test.dart index 427793857a..371879e78d 100644 --- a/packages/flutter/test/widgets/fade_in_image_test.dart +++ b/packages/flutter/test/widgets/fade_in_image_test.dart @@ -132,15 +132,15 @@ Future main() async { placeholderProvider.complete(); await tester.pump(); - expect(findFadeInImage(tester).placeholder.rawImage.image, same(placeholderImage)); + expect(findFadeInImage(tester).placeholder.rawImage.image.isCloneOf(placeholderImage), true); expect(findFadeInImage(tester).target.rawImage.image, null); imageProvider.complete(); await tester.pump(); for (int i = 0; i < 5; i += 1) { final FadeInImageParts parts = findFadeInImage(tester); - expect(parts.placeholder.rawImage.image, same(placeholderImage)); - expect(parts.target.rawImage.image, same(targetImage)); + expect(parts.placeholder.rawImage.image.isCloneOf(placeholderImage), true); + expect(parts.target.rawImage.image.isCloneOf(targetImage), true); expect(parts.placeholder.opacity, moreOrLessEquals(1 - i / 5)); expect(parts.target.opacity, 0); await tester.pump(const Duration(milliseconds: 10)); @@ -148,8 +148,8 @@ Future main() async { for (int i = 0; i < 5; i += 1) { final FadeInImageParts parts = findFadeInImage(tester); - expect(parts.placeholder.rawImage.image, same(placeholderImage)); - expect(parts.target.rawImage.image, same(targetImage)); + expect(parts.placeholder.rawImage.image.isCloneOf(placeholderImage), true); + expect(parts.target.rawImage.image.isCloneOf(targetImage), true); expect(parts.placeholder.opacity, 0); expect(parts.target.opacity, moreOrLessEquals(i / 5)); await tester.pump(const Duration(milliseconds: 10)); @@ -159,7 +159,7 @@ Future main() async { placeholder: placeholderProvider, image: imageProvider, )); - expect(findFadeInImage(tester).target.rawImage.image, same(targetImage)); + expect(findFadeInImage(tester).target.rawImage.image.isCloneOf(targetImage), true); expect(findFadeInImage(tester).target.opacity, 1); }); @@ -174,7 +174,7 @@ Future main() async { image: imageProvider, )); - expect(findFadeInImage(tester).target.rawImage.image, same(targetImage)); + expect(findFadeInImage(tester).target.rawImage.image.isCloneOf(targetImage), true); expect(findFadeInImage(tester).placeholder, isNull); expect(findFadeInImage(tester).target.opacity, 1); }); @@ -195,7 +195,7 @@ Future main() async { final State state = findFadeInImage(tester).state; placeholderProvider.complete(); await tester.pump(); - expect(findFadeInImage(tester).placeholder.rawImage.image, same(placeholderImage)); + expect(findFadeInImage(tester).placeholder.rawImage.image.isCloneOf(placeholderImage), true); await tester.pumpWidget(FadeInImage( placeholder: secondPlaceholderProvider, @@ -207,7 +207,7 @@ Future main() async { secondPlaceholderProvider.complete(); await tester.pump(); - expect(findFadeInImage(tester).placeholder.rawImage.image, same(replacementImage)); + expect(findFadeInImage(tester).placeholder.rawImage.image.isCloneOf(replacementImage), true); expect(findFadeInImage(tester).state, same(state)); }); @@ -263,7 +263,7 @@ Future main() async { secondImageProvider.complete(); await tester.pump(); - expect(findFadeInImage(tester).target.rawImage.image, same(replacementImage)); + expect(findFadeInImage(tester).target.rawImage.image.isCloneOf(replacementImage), true); expect(findFadeInImage(tester).state, same(state)); expect(findFadeInImage(tester).placeholder.opacity, moreOrLessEquals(1)); expect(findFadeInImage(tester).target.opacity, moreOrLessEquals(0)); diff --git a/packages/flutter/test/widgets/image_test.dart b/packages/flutter/test/widgets/image_test.dart index 6cb3ccef75..7cb2507b46 100644 --- a/packages/flutter/test/widgets/image_test.dart +++ b/packages/flutter/test/widgets/image_test.dart @@ -472,7 +472,7 @@ void main() { 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)')); 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: $imageString @ 1.0x, 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), pixels: null, loadingProgress: null, frameNumber: 0, wasSynchronouslyLoaded: false)')); }); testWidgets('Stream completer errors can be listened to by attaching before resolving', (WidgetTester tester) async { @@ -904,8 +904,8 @@ void main() { }); testWidgets('Image State can be reconfigured to use another image', (WidgetTester tester) async { - final Image image1 = Image(image: TestImageProvider()..complete(image10x10), width: 10.0, excludeFromSemantics: true); - final Image image2 = Image(image: TestImageProvider()..complete(image10x10), width: 20.0, excludeFromSemantics: true); + final Image image1 = Image(image: TestImageProvider()..complete(image10x10.clone()), width: 10.0, excludeFromSemantics: true); + final Image image2 = Image(image: TestImageProvider()..complete(image10x10.clone()), width: 20.0, excludeFromSemantics: true); final Column column = Column(children: [image1, image2]); await tester.pumpWidget(column, null, EnginePhase.layout); @@ -1039,7 +1039,7 @@ void main() { }); testWidgets('Image invokes frameBuilder with correct wasSynchronouslyLoaded=true', (WidgetTester tester) async { - final TestImageStreamCompleter streamCompleter = TestImageStreamCompleter(ImageInfo(image: image10x10)); + final TestImageStreamCompleter streamCompleter = TestImageStreamCompleter(ImageInfo(image: image10x10.clone())); final TestImageProvider imageProvider = TestImageProvider(streamCompleter: streamCompleter); int lastFrame; bool lastFrameWasSync; @@ -1058,7 +1058,7 @@ void main() { expect(lastFrame, 0); expect(lastFrameWasSync, isTrue); expect(find.byType(RawImage), findsOneWidget); - streamCompleter.setData(imageInfo: ImageInfo(image: image10x10)); + streamCompleter.setData(imageInfo: ImageInfo(image: image10x10.clone())); await tester.pump(); expect(lastFrame, 1); expect(lastFrameWasSync, isTrue); @@ -1474,10 +1474,10 @@ void main() { expect(provider1.loadCallCount, 1); expect(provider2.loadCallCount, 1); - provider1.complete(image10x10); + provider1.complete(image10x10.clone()); await tester.idle(); - provider2.complete(image10x10); + provider2.complete(image10x10.clone()); await tester.idle(); expect(imageCache.liveImageCount, 2); @@ -1763,6 +1763,76 @@ void main() { debugOnPaintImage = null; }); + testWidgets('Disposes image handle when disposed', (WidgetTester tester) async { + final ui.Image image = await tester.runAsync(() => createTestImage(width: 1, height: 1, cache: false)); + + expect(image.debugGetOpenHandleStackTraces().length, 1); + + final ImageProvider provider = TestImageProvider( + streamCompleter: OneFrameImageStreamCompleter( + Future.value( + ImageInfo( + image: image, + scale: 1.0, + debugLabel: 'TestImage', + ), + ), + ), + ); + + // creating the provider should not have changed anything, and the provider + // now owns the handle. + expect(image.debugGetOpenHandleStackTraces().length, 1); + + await tester.pumpWidget(Image(image: provider)); + + // Image widget + 1, render object + 1 + expect(image.debugGetOpenHandleStackTraces().length, 3); + + await tester.pumpWidget(const SizedBox()); + + // Image widget and render object go away + expect(image.debugGetOpenHandleStackTraces().length, 1); + + await provider.evict(); + + tester.binding.scheduleFrame(); + await tester.pump(); + + // Image cache listener go away and Image stream listeners go away. + // Image is now at zero. + expect(image.debugGetOpenHandleStackTraces().length, 0); + }, skip: kIsWeb); // Web does not care about image handle/disposal. + + testWidgets('Keeps stream alive when ticker mode is disabled', (WidgetTester tester) async { + imageCache.maximumSize = 0; + final ui.Image image = await tester.runAsync(() => createTestImage(width: 1, height: 1, cache: false)); + final TestImageProvider provider = TestImageProvider(); + provider.complete(image); + + await tester.pumpWidget( + TickerMode( + enabled: true, + child: Image(image: provider), + ), + ); + expect(find.byType(Image), findsOneWidget); + + await tester.pumpWidget(TickerMode( + enabled: false, + child: Image(image: provider), + ), + ); + expect(find.byType(Image), findsOneWidget); + + await tester.pumpWidget(TickerMode( + enabled: true, + child: Image(image: provider), + ), + ); + expect(find.byType(Image), findsOneWidget); + }); + testWidgets('Load a good image after a bad image was loaded should not call errorBuilder', (WidgetTester tester) async { final UniqueKey errorKey = UniqueKey(); final ui.Image image = await tester.runAsync(() => createTestImage()); @@ -1899,6 +1969,12 @@ class TestImageProvider extends ImageProvider { String toString() => '${describeIdentity(this)}()'; } +class SimpleTestImageStreamCompleter extends ImageStreamCompleter { + void testSetImage(ui.Image image) { + setImage(ImageInfo(image: image, scale: 1.0)); + } +} + class TestImageStreamCompleter extends ImageStreamCompleter { TestImageStreamCompleter([this._currentImage]); @@ -1909,7 +1985,7 @@ class TestImageStreamCompleter extends ImageStreamCompleter { void addListener(ImageStreamListener listener) { listeners.add(listener); if (_currentImage != null) { - listener.onImage(_currentImage, true); + listener.onImage(_currentImage.clone(), true); } } @@ -1923,12 +1999,13 @@ class TestImageStreamCompleter extends ImageStreamCompleter { ImageChunkEvent chunkEvent, }) { if (imageInfo != null) { + _currentImage?.dispose(); _currentImage = imageInfo; } final List localListeners = listeners.toList(); for (final ImageStreamListener listener in localListeners) { if (imageInfo != null) { - listener.onImage(imageInfo, false); + listener.onImage(imageInfo.clone(), false); } if (chunkEvent != null && listener.onChunk != null) { listener.onChunk(chunkEvent); diff --git a/packages/flutter_test/lib/src/animation_sheet.dart b/packages/flutter_test/lib/src/animation_sheet.dart index 4dbb82f9fe..274c2b6eb3 100644 --- a/packages/flutter_test/lib/src/animation_sheet.dart +++ b/packages/flutter_test/lib/src/animation_sheet.dart @@ -166,7 +166,7 @@ class AnimationSheetBuilder { key: key, cellSize: frameSize, children: frames.map((ui.Image image) => RawImage( - image: image, + image: image.clone(), width: frameSize.width, height: frameSize.height, )).toList(), diff --git a/packages/flutter_test/lib/src/image.dart b/packages/flutter_test/lib/src/image.dart index 3bfcc1e301..038ed0f985 100644 --- a/packages/flutter_test/lib/src/image.dart +++ b/packages/flutter_test/lib/src/image.dart @@ -38,12 +38,12 @@ Future createTestImage({ final int cacheKey = hashValues(width, height); if (cache && _cache.containsKey(cacheKey)) { - return _cache[cacheKey]!; + return _cache[cacheKey]!.clone(); } final ui.Image image = await _createImage(width, height); if (cache) { - _cache[cacheKey] = image; + _cache[cacheKey] = image.clone(); } return image; });