diff --git a/packages/vector_graphics/CHANGELOG.md b/packages/vector_graphics/CHANGELOG.md index 8c2dce6bb06..b9fc41f50d5 100644 --- a/packages/vector_graphics/CHANGELOG.md +++ b/packages/vector_graphics/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.2.5 + +* Fixes a leak of the decoded picture when a `VectorGraphic` is removed before + loading finishes. + ## 1.2.4 * Fixes an unhandled exception reported for every asynchronous loading failure, diff --git a/packages/vector_graphics/lib/src/vector_graphics.dart b/packages/vector_graphics/lib/src/vector_graphics.dart index 23f36c44a5a..1c0646b2eed 100644 --- a/packages/vector_graphics/lib/src/vector_graphics.dart +++ b/packages/vector_graphics/lib/src/vector_graphics.dart @@ -421,18 +421,20 @@ class _VectorGraphicWidgetState extends State { try { final _PictureData data = await _loadPicture(context, key, loader); data.count += 1; + if (data.count == 1) { + _livePictureCache[key] = data; + } // The widget may have changed, requesting a new vector graphic before // this operation could complete. if (!mounted || loader != widget.loader) { - _maybeReleasePicture(data); + // Other widgets awaiting the same load resume after this one, all in the + // same microtask. Defer the release to a later microtask so they can claim + // the picture first; otherwise it could be disposed while they still need it. + scheduleMicrotask(() => _maybeReleasePicture(data)); return; } - if (data.count == 1) { - _livePictureCache[key] = data; - } - setState(() { _maybeReleasePicture(_pictureData); _pictureData = data; diff --git a/packages/vector_graphics/pubspec.yaml b/packages/vector_graphics/pubspec.yaml index 22e8eed23c7..73c5831f219 100644 --- a/packages/vector_graphics/pubspec.yaml +++ b/packages/vector_graphics/pubspec.yaml @@ -2,7 +2,7 @@ name: vector_graphics description: A vector graphics rendering package for Flutter using a binary encoding. repository: https://github.com/flutter/packages/tree/main/packages/vector_graphics issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_graphics%22 -version: 1.2.4 +version: 1.2.5 environment: sdk: ^3.11.0 diff --git a/packages/vector_graphics/test/caching_test.dart b/packages/vector_graphics/test/caching_test.dart index 33b9a81dc36..81244306fe9 100644 --- a/packages/vector_graphics/test/caching_test.dart +++ b/packages/vector_graphics/test/caching_test.dart @@ -3,10 +3,12 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:typed_data'; +import 'dart:ui' as ui; +import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:vector_graphics/src/render_vector_graphic.dart'; import 'package:vector_graphics/src/vector_graphics.dart'; import 'package:vector_graphics_codec/vector_graphics_codec.dart'; @@ -249,6 +251,105 @@ void main() { expect(testBundle.loadKeys, ['bar.svg', 'foo.svg', 'foo.svg']); }); + + group('Pictures decoded after their VectorGraphic is removed', () { + // Collects the pictures created during a test. + Set trackPictures() { + final created = {}; + void listener(ObjectEvent event) { + final Object object = event.object; + if (event is ObjectCreated && object is ui.Picture) { + created.add(object); + } + } + + FlutterMemoryAllocations.instance.addListener(listener); + addTearDown(() => FlutterMemoryAllocations.instance.removeListener(listener)); + return created; + } + + testWidgets('are disposed', (WidgetTester tester) async { + final loader = CompleterBytesLoader(); + final Set created = trackPictures(); + + await tester.pumpWidget(VectorGraphic(loader: loader)); + await tester.pumpWidget(const SizedBox()); + loader.complete(); + await tester.pumpAndSettle(); + + expect(created, hasLength(1)); + expect(created.single.debugDisposed, isTrue); + }); + + testWidgets('are disposed once when every widget sharing the load is removed', ( + WidgetTester tester, + ) async { + final loader = CompleterBytesLoader(); + final Set created = trackPictures(); + + await tester.pumpWidget( + Column( + children: [ + VectorGraphic(loader: loader), + VectorGraphic(loader: loader), + ], + ), + ); + await tester.pumpWidget(const SizedBox()); + loader.complete(); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(created, hasLength(1)); + expect(created.single.debugDisposed, isTrue); + }); + + testWidgets('stay alive while another widget sharing the load shows them', ( + WidgetTester tester, + ) async { + final loader = CompleterBytesLoader(); + + await tester.pumpWidget( + Column( + children: [ + VectorGraphic(key: GlobalKey(), loader: loader), + VectorGraphic(loader: loader), + ], + ), + ); + await tester.pumpWidget(Column(children: [VectorGraphic(loader: loader)])); + loader.complete(); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + final ui.Picture picture = tester.allRenderObjects + .whereType() + .single + .pictureInfo + .picture; + expect(picture.debugDisposed, isFalse); + + await tester.pumpWidget(const SizedBox()); + expect(picture.debugDisposed, isTrue); + }); + + testWidgets('are disposed when the loader changes before decoding finishes', ( + WidgetTester tester, + ) async { + final first = CompleterBytesLoader(); + final second = CompleterBytesLoader(); + + final Set created = trackPictures(); + await tester.pumpWidget(VectorGraphic(loader: first)); + await tester.pumpWidget(VectorGraphic(loader: second)); + first.complete(); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(created, hasLength(1)); + expect(created.single.debugDisposed, isTrue); + }); + }); } class TestAssetBundle extends Fake implements AssetBundle { @@ -296,3 +397,17 @@ class TestWidgetsLocalizations extends DefaultWidgetsLocalizations { @override TextDirection get textDirection => TextDirection.ltr; } + +class CompleterBytesLoader extends BytesLoader { + final Completer _completer = Completer(); + + /// Finishes loading with a small, empty vector graphic. + void complete() { + final buffer = VectorGraphicsBuffer(); + codec.writeSize(buffer, 10, 10); + _completer.complete(buffer.done()); + } + + @override + Future loadBytes(BuildContext? context) => _completer.future; +}