Skip to content

Commit e66ec8e

Browse files
authored
Dispose AnimationSheetRecorder to avoid leaks (#133365)
This PR adds `AnimationSheetRecorder.dispose`, which disposes all the images generated by the recorder, eliminating leaks. Fixes flutter/flutter#133071.
1 parent a7dbec3 commit e66ec8e

File tree

4 files changed

+94
-48
lines changed

4 files changed

+94
-48
lines changed

packages/flutter/test/animation/animation_sheet_test.dart

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
@Tags(<String>['reduced-test-set'])
88
library;
99

10-
import 'dart:ui' as ui;
11-
1210
import 'package:flutter/material.dart';
1311
import 'package:flutter_test/flutter_test.dart';
1412
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
@@ -23,6 +21,7 @@ void main() {
2321
testWidgetsWithLeakTracking('recording disposes images',
2422
(WidgetTester tester) async {
2523
final AnimationSheetBuilder builder = AnimationSheetBuilder(frameSize: _DecuplePixels.size);
24+
addTearDown(builder.dispose);
2625

2726
await tester.pumpFrames(
2827
builder.record(
@@ -33,13 +32,12 @@ void main() {
3332
);
3433
},
3534
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
36-
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
37-
leakTrackingTestConfig: const LeakTrackingTestConfig(allowAllNotDisposed: true),
3835
);
3936

4037
testWidgetsWithLeakTracking('correctly records frames using collate',
4138
(WidgetTester tester) async {
4239
final AnimationSheetBuilder builder = AnimationSheetBuilder(frameSize: _DecuplePixels.size);
40+
addTearDown(builder.dispose);
4341

4442
await tester.pumpFrames(
4543
builder.record(
@@ -66,25 +64,20 @@ void main() {
6664
const Duration(milliseconds: 100),
6765
);
6866

69-
final ui.Image image = await builder.collate(5);
70-
7167
await expectLater(
72-
image,
68+
builder.collate(5),
7369
matchesGoldenFile('test.animation_sheet_builder.collate.png'),
7470
);
75-
76-
image.dispose();
7771
},
7872
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
79-
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
80-
leakTrackingTestConfig: const LeakTrackingTestConfig(allowAllNotDisposed: true),
8173
); // https://github.com/flutter/flutter/issues/56001
8274

8375
testWidgetsWithLeakTracking('use allLayers to record out-of-subtree contents', (WidgetTester tester) async {
8476
final AnimationSheetBuilder builder = AnimationSheetBuilder(
8577
frameSize: const Size(8, 2),
8678
allLayers: true,
8779
);
80+
addTearDown(builder.dispose);
8881

8982
// The `record` (sized 8, 2) is placed on top of `_DecuplePixels`
9083
// (sized 12, 3), aligned at its top left.
@@ -105,17 +98,12 @@ void main() {
10598
const Duration(milliseconds: 100),
10699
);
107100

108-
final ui.Image image = await builder.collate(5);
109-
110101
await expectLater(
111-
image,
102+
builder.collate(5),
112103
matchesGoldenFile('test.animation_sheet_builder.out_of_tree.png'),
113104
);
114-
image.dispose();
115105
},
116106
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
117-
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
118-
leakTrackingTestConfig: const LeakTrackingTestConfig(allowAllNotDisposed: true),
119107
);
120108
}
121109

packages/flutter/test/animation/live_binding_test.dart

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
@Tags(<String>['reduced-test-set'])
88
library;
99

10-
import 'dart:ui' as ui;
11-
1210
import 'package:flutter/material.dart';
1311
import 'package:flutter_test/flutter_test.dart';
1412
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
@@ -23,6 +21,7 @@ void main() {
2321

2422
testWidgetsWithLeakTracking('Should show event indicator for pointer events', (WidgetTester tester) async {
2523
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(200, 200), allLayers: true);
24+
addTearDown(animationSheet.dispose);
2625
final List<Offset> taps = <Offset>[];
2726
Widget target({bool recording = true}) => Container(
2827
padding: const EdgeInsets.fromLTRB(20, 10, 25, 20),
@@ -81,6 +80,7 @@ void main() {
8180

8281
testWidgetsWithLeakTracking('Should show event indicator for pointer events with setSurfaceSize', (WidgetTester tester) async {
8382
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(200, 200), allLayers: true);
83+
addTearDown(animationSheet.dispose);
8484
final List<Offset> taps = <Offset>[];
8585
Widget target({bool recording = true}) => Container(
8686
padding: const EdgeInsets.fromLTRB(20, 10, 25, 20),
@@ -131,13 +131,10 @@ void main() {
131131
await tester.pumpFrames(target(), const Duration(milliseconds: 50));
132132
expect(taps, isEmpty);
133133

134-
final ui.Image image = await animationSheet.collate(6);
135-
136134
await expectLater(
137-
image,
135+
animationSheet.collate(6),
138136
matchesGoldenFile('LiveBinding.press.animation.2.png'),
139137
);
140-
image.dispose();
141138
},
142139
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
143140
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071

packages/flutter/test/material/progress_indicator_test.dart

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,7 @@ void main() {
720720

721721
testWidgets('Material2 - RefreshProgressIndicator uses expected animation', (WidgetTester tester) async {
722722
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(50, 50));
723+
addTearDown(animationSheet.dispose);
723724

724725
await tester.pumpFrames(animationSheet.record(
725726
Theme(
@@ -729,13 +730,14 @@ void main() {
729730
), const Duration(seconds: 3));
730731

731732
await expectLater(
732-
await animationSheet.collate(20),
733+
animationSheet.collate(20),
733734
matchesGoldenFile('m2_material.refresh_progress_indicator.png'),
734735
);
735736
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
736737

737738
testWidgets('Material3 - RefreshProgressIndicator uses expected animation', (WidgetTester tester) async {
738739
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(50, 50));
740+
addTearDown(animationSheet.dispose);
739741

740742
await tester.pumpFrames(animationSheet.record(
741743
Theme(
@@ -745,7 +747,7 @@ void main() {
745747
), const Duration(seconds: 3));
746748

747749
await expectLater(
748-
await animationSheet.collate(20),
750+
animationSheet.collate(20),
749751
matchesGoldenFile('m3_material.refresh_progress_indicator.png'),
750752
);
751753
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
@@ -1017,6 +1019,7 @@ void main() {
10171019

10181020
testWidgets('Material2 - Indeterminate CircularProgressIndicator uses expected animation', (WidgetTester tester) async {
10191021
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(40, 40));
1022+
addTearDown(animationSheet.dispose);
10201023

10211024
await tester.pumpFrames(animationSheet.record(
10221025
Theme(
@@ -1032,13 +1035,14 @@ void main() {
10321035
), const Duration(seconds: 2));
10331036

10341037
await expectLater(
1035-
await animationSheet.collate(20),
1038+
animationSheet.collate(20),
10361039
matchesGoldenFile('m2_material.circular_progress_indicator.indeterminate.png'),
10371040
);
10381041
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
10391042

10401043
testWidgets('Material3 - Indeterminate CircularProgressIndicator uses expected animation', (WidgetTester tester) async {
10411044
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(40, 40));
1045+
addTearDown(animationSheet.dispose);
10421046

10431047
await tester.pumpFrames(animationSheet.record(
10441048
Theme(
@@ -1054,7 +1058,7 @@ void main() {
10541058
), const Duration(seconds: 2));
10551059

10561060
await expectLater(
1057-
await animationSheet.collate(20),
1061+
animationSheet.collate(20),
10581062
matchesGoldenFile('m3_material.circular_progress_indicator.indeterminate.png'),
10591063
);
10601064
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001

packages/flutter_test/lib/src/animation_sheet.dart

Lines changed: 78 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,36 @@ import 'package:flutter/rendering.dart';
99
import 'package:flutter/scheduler.dart';
1010
import 'package:flutter/widgets.dart';
1111

12+
// A Future<ui.Image> that stores the resolved result.
13+
class _AsyncImage {
14+
_AsyncImage(Future<ui.Image> task) {
15+
_task = task.then((ui.Image image) {
16+
_result = image;
17+
});
18+
}
19+
20+
// Returns the resolved image.
21+
Future<ui.Image> result() async {
22+
if (_result != null) {
23+
return _result!;
24+
}
25+
await _task;
26+
assert(_result != null);
27+
return _result!;
28+
}
29+
30+
late final Future<void> _task;
31+
ui.Image? _result;
32+
33+
// Wait for a list of `_AsyncImage` and returns the list of its resolved
34+
// images.
35+
static Future<List<ui.Image>> resolveList(List<_AsyncImage> targets) {
36+
final Iterable<Future<ui.Image>> images = targets.map<Future<ui.Image>>(
37+
(_AsyncImage target) => target.result());
38+
return Future.wait<ui.Image>(images);
39+
}
40+
}
41+
1242
/// Records the frames of an animating widget, and later displays the frames as a
1343
/// grid in an animation sheet.
1444
///
@@ -20,6 +50,7 @@ import 'package:flutter/widgets.dart';
2050
/// Using this class includes the following steps:
2151
///
2252
/// * Create an instance of this class.
53+
/// * Register [dispose] to the test's tear down callbacks.
2354
/// * Pump frames that render the target widget wrapped in [record]. Every frame
2455
/// that has `recording` being true will be recorded.
2556
/// * Acquire the output image with [collate] and compare against the golden
@@ -33,6 +64,7 @@ import 'package:flutter/widgets.dart';
3364
/// testWidgets('Inkwell animation sheet', (WidgetTester tester) async {
3465
/// // Create instance
3566
/// final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(48, 24));
67+
/// addTearDown(animationSheet.dispose);
3668
///
3769
/// final Widget target = Material(
3870
/// child: Directionality(
@@ -90,6 +122,24 @@ class AnimationSheetBuilder {
90122
this.allLayers = false,
91123
}) : assert(!kIsWeb);
92124

125+
/// Dispose all recorded frames and result images.
126+
///
127+
/// This method must be called before the test case ends (usually as a tear
128+
/// down callback) to properly deallocate the images.
129+
///
130+
/// After this method is called, there will be no frames to [collate].
131+
Future<void> dispose() async {
132+
final List<_AsyncImage> targets = <_AsyncImage>[
133+
..._recordedFrames,
134+
..._results,
135+
];
136+
_recordedFrames.clear();
137+
_results.clear();
138+
for (final ui.Image image in await _AsyncImage.resolveList(targets)) {
139+
image.dispose();
140+
}
141+
}
142+
93143
/// The size of the child to be recorded.
94144
///
95145
/// This size is applied as a tight layout constraint for the child, and is
@@ -112,20 +162,7 @@ class AnimationSheetBuilder {
112162
/// Defaults to false.
113163
final bool allLayers;
114164

115-
final List<Future<ui.Image>> _recordedFrames = <Future<ui.Image>>[];
116-
Future<List<ui.Image>> get _frames async {
117-
final List<ui.Image> frames = await Future.wait<ui.Image>(_recordedFrames, eagerError: true);
118-
assert(() {
119-
for (final ui.Image frame in frames) {
120-
assert(frame.width == frameSize.width && frame.height == frameSize.height,
121-
'Unexpected size mismatch: frame has (${frame.width}, ${frame.height}) '
122-
'while `frameSize` is $frameSize.'
123-
);
124-
}
125-
return true;
126-
}());
127-
return frames;
128-
}
165+
final List<_AsyncImage> _recordedFrames = <_AsyncImage>[];
129166

130167
/// Returns a widget that renders a widget in a box that can be recorded.
131168
///
@@ -152,22 +189,41 @@ class AnimationSheetBuilder {
152189
key: key,
153190
size: frameSize,
154191
allLayers: allLayers,
155-
handleRecorded: recording ? _recordedFrames.add : null,
192+
handleRecorded: !recording ? null : (Future<ui.Image> futureImage) {
193+
_recordedFrames.add(_AsyncImage(() async {
194+
final ui.Image image = await futureImage;
195+
assert(image.width == frameSize.width && image.height == frameSize.height,
196+
'Unexpected size mismatch: frame has (${image.width}, ${image.height}) '
197+
'while `frameSize` is $frameSize.'
198+
);
199+
return image;
200+
}()));
201+
},
156202
child: child,
157203
);
158204
}
159205

206+
// The result images generated by `collate`.
207+
//
208+
// They're stored here to be disposed by [dispose].
209+
final List<_AsyncImage> _results = <_AsyncImage>[];
210+
160211
/// Returns an result image by putting all frames together in a table.
161212
///
162-
/// This method returns a table of captured frames, `cellsPerRow` images
163-
/// per row, from left to right, top to bottom.
213+
/// This method returns an image that arranges the captured frames in a table,
214+
/// which has `cellsPerRow` images per row with the order from left to right,
215+
/// top to bottom.
216+
///
217+
/// The result image of this method is managed by [AnimationSheetBuilder],
218+
/// and should not be disposed by the caller.
164219
///
165220
/// An example of using this method can be found at [AnimationSheetBuilder].
166221
Future<ui.Image> collate(int cellsPerRow) async {
167-
final List<ui.Image> frames = await _frames;
168-
assert(frames.isNotEmpty,
222+
assert(_recordedFrames.isNotEmpty,
169223
'No frames are collected. Have you forgot to set `recording` to true?');
170-
return _collateFrames(frames, frameSize, cellsPerRow);
224+
final _AsyncImage result = _AsyncImage(_collateFrames(_recordedFrames, frameSize, cellsPerRow));
225+
_results.add(result);
226+
return result.result();
171227
}
172228
}
173229

@@ -281,7 +337,8 @@ class _RenderPostFrameCallbacker extends RenderProxyBox {
281337
}
282338
}
283339

284-
Future<ui.Image> _collateFrames(List<ui.Image> frames, Size frameSize, int cellsPerRow) async {
340+
Future<ui.Image> _collateFrames(List<_AsyncImage> futureFrames, Size frameSize, int cellsPerRow) async {
341+
final List<ui.Image> frames = await _AsyncImage.resolveList(futureFrames);
285342
final int rowNum = (frames.length / cellsPerRow).ceil();
286343

287344
final ui.PictureRecorder recorder = ui.PictureRecorder();

0 commit comments

Comments
 (0)