From 15fa68ab1dea1a5c20419e611f7881b32b88e9e1 Mon Sep 17 00:00:00 2001 From: Polina Cherkasova Date: Thu, 25 Jan 2024 17:02:17 -0800 Subject: [PATCH] Instrument ImageInfo. (#141411) --- .../lib/src/painting/image_stream.dart | 48 +++++++++++++------ .../test/painting/image_stream_test.dart | 13 +++++ .../test/painting/mocks_for_image_cache.dart | 2 +- 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/flutter/lib/src/painting/image_stream.dart b/packages/flutter/lib/src/painting/image_stream.dart index ffdcf45452..ab3d1db828 100644 --- a/packages/flutter/lib/src/painting/image_stream.dart +++ b/packages/flutter/lib/src/painting/image_stream.dart @@ -8,38 +8,55 @@ import 'dart:ui' as ui show Codec, FrameInfo, Image; import 'package:flutter/foundation.dart'; import 'package:flutter/scheduler.dart'; -const String _flutterWidgetsLibrary = 'package:flutter/widgets.dart'; +const String _flutterPaintingLibrary = 'package:flutter/painting.dart'; /// A [dart:ui.Image] object with its corresponding scale. /// /// 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. +/// The disposing contract for [ImageInfo] (as well as for [ui.Image]) +/// is different from traditional one, where +/// an object should dispose a member if the object created the member. +/// Instead, the disposal contract is as follows: +/// +/// * [ImageInfo] disposes [image], even if it is received as a constructor argument. +/// * [ImageInfo] is expected to be disposed not by the object, that created it, +/// but by the object that owns reference to it. +/// * It is expected that only one object owns reference to [ImageInfo] object. +/// +/// Safety tips: +/// +/// * To share the [ImageInfo] or [ui.Image] between objects, use the [clone] method, +/// which will not clone the entire underlying image, but only reference to it and information about it. +/// * After passing a [ui.Image] or [ImageInfo] reference to another object, +/// release the reference. @immutable class ImageInfo { /// Creates an [ImageInfo] object for the given [image] and [scale]. /// /// The [debugLabel] may be used to identify the source of this image. - const ImageInfo({ required this.image, this.scale = 1.0, this.debugLabel }); + /// + /// See details for disposing contract in the class description. + ImageInfo({ required this.image, this.scale = 1.0, this.debugLabel }) { + if (kFlutterMemoryAllocationsEnabled) { + MemoryAllocations.instance.dispatchObjectCreated( + library: _flutterPaintingLibrary, + className: '$ImageInfo', + object: this, + ); + } + } /// 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 details for disposing contract in the class description. + /// /// See also: /// /// * [Image.clone], which describes how and why to clone images. @@ -125,6 +142,9 @@ class ImageInfo { /// and no clones of it or the image it contains can be made. void dispose() { assert((image.debugGetOpenHandleStackTraces()?.length ?? 1) > 0); + if (kFlutterMemoryAllocationsEnabled) { + MemoryAllocations.instance.dispatchObjectDisposed(object: this); + } image.dispose(); } @@ -443,7 +463,7 @@ class ImageStreamCompleterHandle { // https://github.com/flutter/flutter/issues/137435 if (kFlutterMemoryAllocationsEnabled) { FlutterMemoryAllocations.instance.dispatchObjectCreated( - library: _flutterWidgetsLibrary, + library: _flutterPaintingLibrary, className: '$ImageStreamCompleterHandle', object: this, ); diff --git a/packages/flutter/test/painting/image_stream_test.dart b/packages/flutter/test/painting/image_stream_test.dart index 6f2e28caa2..615596e4b8 100644 --- a/packages/flutter/test/painting/image_stream_test.dart +++ b/packages/flutter/test/painting/image_stream_test.dart @@ -891,4 +891,17 @@ void main() { areCreateAndDispose, ); }); + + testWidgets('ImageInfo dispatches memory events', (WidgetTester tester) async { + await expectLater( + await memoryEvents( + () async { + final ImageInfo info = ImageInfo(image: image20x10); + info.dispose(); + }, + ImageInfo, + ), + areCreateAndDispose, + ); + }); } diff --git a/packages/flutter/test/painting/mocks_for_image_cache.dart b/packages/flutter/test/painting/mocks_for_image_cache.dart index bc80315747..59c36b17e7 100644 --- a/packages/flutter/test/painting/mocks_for_image_cache.dart +++ b/packages/flutter/test/painting/mocks_for_image_cache.dart @@ -9,7 +9,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/painting.dart'; class TestImageInfo extends ImageInfo { - const TestImageInfo(this.value, { + TestImageInfo(this.value, { required super.image, super.scale, super.debugLabel,