Unverified Commit e66ec8e0 authored by Tong Mu's avatar Tong Mu Committed by GitHub

Dispose AnimationSheetRecorder to avoid leaks (#133365)

This PR adds `AnimationSheetRecorder.dispose`, which disposes all the images generated by the recorder, eliminating leaks.

Fixes https://github.com/flutter/flutter/issues/133071.
parent a7dbec31
...@@ -7,8 +7,6 @@ ...@@ -7,8 +7,6 @@
@Tags(<String>['reduced-test-set']) @Tags(<String>['reduced-test-set'])
library; library;
import 'dart:ui' as ui;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
...@@ -23,6 +21,7 @@ void main() { ...@@ -23,6 +21,7 @@ void main() {
testWidgetsWithLeakTracking('recording disposes images', testWidgetsWithLeakTracking('recording disposes images',
(WidgetTester tester) async { (WidgetTester tester) async {
final AnimationSheetBuilder builder = AnimationSheetBuilder(frameSize: _DecuplePixels.size); final AnimationSheetBuilder builder = AnimationSheetBuilder(frameSize: _DecuplePixels.size);
addTearDown(builder.dispose);
await tester.pumpFrames( await tester.pumpFrames(
builder.record( builder.record(
...@@ -33,13 +32,12 @@ void main() { ...@@ -33,13 +32,12 @@ void main() {
); );
}, },
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001 skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
leakTrackingTestConfig: const LeakTrackingTestConfig(allowAllNotDisposed: true),
); );
testWidgetsWithLeakTracking('correctly records frames using collate', testWidgetsWithLeakTracking('correctly records frames using collate',
(WidgetTester tester) async { (WidgetTester tester) async {
final AnimationSheetBuilder builder = AnimationSheetBuilder(frameSize: _DecuplePixels.size); final AnimationSheetBuilder builder = AnimationSheetBuilder(frameSize: _DecuplePixels.size);
addTearDown(builder.dispose);
await tester.pumpFrames( await tester.pumpFrames(
builder.record( builder.record(
...@@ -66,18 +64,12 @@ void main() { ...@@ -66,18 +64,12 @@ void main() {
const Duration(milliseconds: 100), const Duration(milliseconds: 100),
); );
final ui.Image image = await builder.collate(5);
await expectLater( await expectLater(
image, builder.collate(5),
matchesGoldenFile('test.animation_sheet_builder.collate.png'), matchesGoldenFile('test.animation_sheet_builder.collate.png'),
); );
image.dispose();
}, },
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001 skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
leakTrackingTestConfig: const LeakTrackingTestConfig(allowAllNotDisposed: true),
); // https://github.com/flutter/flutter/issues/56001 ); // https://github.com/flutter/flutter/issues/56001
testWidgetsWithLeakTracking('use allLayers to record out-of-subtree contents', (WidgetTester tester) async { testWidgetsWithLeakTracking('use allLayers to record out-of-subtree contents', (WidgetTester tester) async {
...@@ -85,6 +77,7 @@ void main() { ...@@ -85,6 +77,7 @@ void main() {
frameSize: const Size(8, 2), frameSize: const Size(8, 2),
allLayers: true, allLayers: true,
); );
addTearDown(builder.dispose);
// The `record` (sized 8, 2) is placed on top of `_DecuplePixels` // The `record` (sized 8, 2) is placed on top of `_DecuplePixels`
// (sized 12, 3), aligned at its top left. // (sized 12, 3), aligned at its top left.
...@@ -105,17 +98,12 @@ void main() { ...@@ -105,17 +98,12 @@ void main() {
const Duration(milliseconds: 100), const Duration(milliseconds: 100),
); );
final ui.Image image = await builder.collate(5);
await expectLater( await expectLater(
image, builder.collate(5),
matchesGoldenFile('test.animation_sheet_builder.out_of_tree.png'), matchesGoldenFile('test.animation_sheet_builder.out_of_tree.png'),
); );
image.dispose();
}, },
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001 skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
leakTrackingTestConfig: const LeakTrackingTestConfig(allowAllNotDisposed: true),
); );
} }
......
...@@ -7,8 +7,6 @@ ...@@ -7,8 +7,6 @@
@Tags(<String>['reduced-test-set']) @Tags(<String>['reduced-test-set'])
library; library;
import 'dart:ui' as ui;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
...@@ -23,6 +21,7 @@ void main() { ...@@ -23,6 +21,7 @@ void main() {
testWidgetsWithLeakTracking('Should show event indicator for pointer events', (WidgetTester tester) async { testWidgetsWithLeakTracking('Should show event indicator for pointer events', (WidgetTester tester) async {
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(200, 200), allLayers: true); final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(200, 200), allLayers: true);
addTearDown(animationSheet.dispose);
final List<Offset> taps = <Offset>[]; final List<Offset> taps = <Offset>[];
Widget target({bool recording = true}) => Container( Widget target({bool recording = true}) => Container(
padding: const EdgeInsets.fromLTRB(20, 10, 25, 20), padding: const EdgeInsets.fromLTRB(20, 10, 25, 20),
...@@ -81,6 +80,7 @@ void main() { ...@@ -81,6 +80,7 @@ void main() {
testWidgetsWithLeakTracking('Should show event indicator for pointer events with setSurfaceSize', (WidgetTester tester) async { testWidgetsWithLeakTracking('Should show event indicator for pointer events with setSurfaceSize', (WidgetTester tester) async {
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(200, 200), allLayers: true); final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(200, 200), allLayers: true);
addTearDown(animationSheet.dispose);
final List<Offset> taps = <Offset>[]; final List<Offset> taps = <Offset>[];
Widget target({bool recording = true}) => Container( Widget target({bool recording = true}) => Container(
padding: const EdgeInsets.fromLTRB(20, 10, 25, 20), padding: const EdgeInsets.fromLTRB(20, 10, 25, 20),
...@@ -131,13 +131,10 @@ void main() { ...@@ -131,13 +131,10 @@ void main() {
await tester.pumpFrames(target(), const Duration(milliseconds: 50)); await tester.pumpFrames(target(), const Duration(milliseconds: 50));
expect(taps, isEmpty); expect(taps, isEmpty);
final ui.Image image = await animationSheet.collate(6);
await expectLater( await expectLater(
image, animationSheet.collate(6),
matchesGoldenFile('LiveBinding.press.animation.2.png'), matchesGoldenFile('LiveBinding.press.animation.2.png'),
); );
image.dispose();
}, },
skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001 skip: isBrowser, // [intended] https://github.com/flutter/flutter/issues/56001
// TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071 // TODO(polina-c): remove after fixing https://github.com/flutter/flutter/issues/133071
......
...@@ -720,6 +720,7 @@ void main() { ...@@ -720,6 +720,7 @@ void main() {
testWidgets('Material2 - RefreshProgressIndicator uses expected animation', (WidgetTester tester) async { testWidgets('Material2 - RefreshProgressIndicator uses expected animation', (WidgetTester tester) async {
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(50, 50)); final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(50, 50));
addTearDown(animationSheet.dispose);
await tester.pumpFrames(animationSheet.record( await tester.pumpFrames(animationSheet.record(
Theme( Theme(
...@@ -729,13 +730,14 @@ void main() { ...@@ -729,13 +730,14 @@ void main() {
), const Duration(seconds: 3)); ), const Duration(seconds: 3));
await expectLater( await expectLater(
await animationSheet.collate(20), animationSheet.collate(20),
matchesGoldenFile('m2_material.refresh_progress_indicator.png'), matchesGoldenFile('m2_material.refresh_progress_indicator.png'),
); );
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
testWidgets('Material3 - RefreshProgressIndicator uses expected animation', (WidgetTester tester) async { testWidgets('Material3 - RefreshProgressIndicator uses expected animation', (WidgetTester tester) async {
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(50, 50)); final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(50, 50));
addTearDown(animationSheet.dispose);
await tester.pumpFrames(animationSheet.record( await tester.pumpFrames(animationSheet.record(
Theme( Theme(
...@@ -745,7 +747,7 @@ void main() { ...@@ -745,7 +747,7 @@ void main() {
), const Duration(seconds: 3)); ), const Duration(seconds: 3));
await expectLater( await expectLater(
await animationSheet.collate(20), animationSheet.collate(20),
matchesGoldenFile('m3_material.refresh_progress_indicator.png'), matchesGoldenFile('m3_material.refresh_progress_indicator.png'),
); );
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
...@@ -1017,6 +1019,7 @@ void main() { ...@@ -1017,6 +1019,7 @@ void main() {
testWidgets('Material2 - Indeterminate CircularProgressIndicator uses expected animation', (WidgetTester tester) async { testWidgets('Material2 - Indeterminate CircularProgressIndicator uses expected animation', (WidgetTester tester) async {
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(40, 40)); final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(40, 40));
addTearDown(animationSheet.dispose);
await tester.pumpFrames(animationSheet.record( await tester.pumpFrames(animationSheet.record(
Theme( Theme(
...@@ -1032,13 +1035,14 @@ void main() { ...@@ -1032,13 +1035,14 @@ void main() {
), const Duration(seconds: 2)); ), const Duration(seconds: 2));
await expectLater( await expectLater(
await animationSheet.collate(20), animationSheet.collate(20),
matchesGoldenFile('m2_material.circular_progress_indicator.indeterminate.png'), matchesGoldenFile('m2_material.circular_progress_indicator.indeterminate.png'),
); );
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
testWidgets('Material3 - Indeterminate CircularProgressIndicator uses expected animation', (WidgetTester tester) async { testWidgets('Material3 - Indeterminate CircularProgressIndicator uses expected animation', (WidgetTester tester) async {
final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(40, 40)); final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(40, 40));
addTearDown(animationSheet.dispose);
await tester.pumpFrames(animationSheet.record( await tester.pumpFrames(animationSheet.record(
Theme( Theme(
...@@ -1054,7 +1058,7 @@ void main() { ...@@ -1054,7 +1058,7 @@ void main() {
), const Duration(seconds: 2)); ), const Duration(seconds: 2));
await expectLater( await expectLater(
await animationSheet.collate(20), animationSheet.collate(20),
matchesGoldenFile('m3_material.circular_progress_indicator.indeterminate.png'), matchesGoldenFile('m3_material.circular_progress_indicator.indeterminate.png'),
); );
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001
......
...@@ -9,6 +9,36 @@ import 'package:flutter/rendering.dart'; ...@@ -9,6 +9,36 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart'; import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
// A Future<ui.Image> that stores the resolved result.
class _AsyncImage {
_AsyncImage(Future<ui.Image> task) {
_task = task.then((ui.Image image) {
_result = image;
});
}
// Returns the resolved image.
Future<ui.Image> result() async {
if (_result != null) {
return _result!;
}
await _task;
assert(_result != null);
return _result!;
}
late final Future<void> _task;
ui.Image? _result;
// Wait for a list of `_AsyncImage` and returns the list of its resolved
// images.
static Future<List<ui.Image>> resolveList(List<_AsyncImage> targets) {
final Iterable<Future<ui.Image>> images = targets.map<Future<ui.Image>>(
(_AsyncImage target) => target.result());
return Future.wait<ui.Image>(images);
}
}
/// Records the frames of an animating widget, and later displays the frames as a /// Records the frames of an animating widget, and later displays the frames as a
/// grid in an animation sheet. /// grid in an animation sheet.
/// ///
...@@ -20,6 +50,7 @@ import 'package:flutter/widgets.dart'; ...@@ -20,6 +50,7 @@ import 'package:flutter/widgets.dart';
/// Using this class includes the following steps: /// Using this class includes the following steps:
/// ///
/// * Create an instance of this class. /// * Create an instance of this class.
/// * Register [dispose] to the test's tear down callbacks.
/// * Pump frames that render the target widget wrapped in [record]. Every frame /// * Pump frames that render the target widget wrapped in [record]. Every frame
/// that has `recording` being true will be recorded. /// that has `recording` being true will be recorded.
/// * Acquire the output image with [collate] and compare against the golden /// * Acquire the output image with [collate] and compare against the golden
...@@ -33,6 +64,7 @@ import 'package:flutter/widgets.dart'; ...@@ -33,6 +64,7 @@ import 'package:flutter/widgets.dart';
/// testWidgets('Inkwell animation sheet', (WidgetTester tester) async { /// testWidgets('Inkwell animation sheet', (WidgetTester tester) async {
/// // Create instance /// // Create instance
/// final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(48, 24)); /// final AnimationSheetBuilder animationSheet = AnimationSheetBuilder(frameSize: const Size(48, 24));
/// addTearDown(animationSheet.dispose);
/// ///
/// final Widget target = Material( /// final Widget target = Material(
/// child: Directionality( /// child: Directionality(
...@@ -90,6 +122,24 @@ class AnimationSheetBuilder { ...@@ -90,6 +122,24 @@ class AnimationSheetBuilder {
this.allLayers = false, this.allLayers = false,
}) : assert(!kIsWeb); }) : assert(!kIsWeb);
/// Dispose all recorded frames and result images.
///
/// This method must be called before the test case ends (usually as a tear
/// down callback) to properly deallocate the images.
///
/// After this method is called, there will be no frames to [collate].
Future<void> dispose() async {
final List<_AsyncImage> targets = <_AsyncImage>[
..._recordedFrames,
..._results,
];
_recordedFrames.clear();
_results.clear();
for (final ui.Image image in await _AsyncImage.resolveList(targets)) {
image.dispose();
}
}
/// The size of the child to be recorded. /// The size of the child to be recorded.
/// ///
/// This size is applied as a tight layout constraint for the child, and is /// This size is applied as a tight layout constraint for the child, and is
...@@ -112,20 +162,7 @@ class AnimationSheetBuilder { ...@@ -112,20 +162,7 @@ class AnimationSheetBuilder {
/// Defaults to false. /// Defaults to false.
final bool allLayers; final bool allLayers;
final List<Future<ui.Image>> _recordedFrames = <Future<ui.Image>>[]; final List<_AsyncImage> _recordedFrames = <_AsyncImage>[];
Future<List<ui.Image>> get _frames async {
final List<ui.Image> frames = await Future.wait<ui.Image>(_recordedFrames, eagerError: true);
assert(() {
for (final ui.Image frame in frames) {
assert(frame.width == frameSize.width && frame.height == frameSize.height,
'Unexpected size mismatch: frame has (${frame.width}, ${frame.height}) '
'while `frameSize` is $frameSize.'
);
}
return true;
}());
return frames;
}
/// Returns a widget that renders a widget in a box that can be recorded. /// Returns a widget that renders a widget in a box that can be recorded.
/// ///
...@@ -152,22 +189,41 @@ class AnimationSheetBuilder { ...@@ -152,22 +189,41 @@ class AnimationSheetBuilder {
key: key, key: key,
size: frameSize, size: frameSize,
allLayers: allLayers, allLayers: allLayers,
handleRecorded: recording ? _recordedFrames.add : null, handleRecorded: !recording ? null : (Future<ui.Image> futureImage) {
_recordedFrames.add(_AsyncImage(() async {
final ui.Image image = await futureImage;
assert(image.width == frameSize.width && image.height == frameSize.height,
'Unexpected size mismatch: frame has (${image.width}, ${image.height}) '
'while `frameSize` is $frameSize.'
);
return image;
}()));
},
child: child, child: child,
); );
} }
// The result images generated by `collate`.
//
// They're stored here to be disposed by [dispose].
final List<_AsyncImage> _results = <_AsyncImage>[];
/// Returns an result image by putting all frames together in a table. /// Returns an result image by putting all frames together in a table.
/// ///
/// This method returns a table of captured frames, `cellsPerRow` images /// This method returns an image that arranges the captured frames in a table,
/// per row, from left to right, top to bottom. /// which has `cellsPerRow` images per row with the order from left to right,
/// top to bottom.
///
/// The result image of this method is managed by [AnimationSheetBuilder],
/// and should not be disposed by the caller.
/// ///
/// An example of using this method can be found at [AnimationSheetBuilder]. /// An example of using this method can be found at [AnimationSheetBuilder].
Future<ui.Image> collate(int cellsPerRow) async { Future<ui.Image> collate(int cellsPerRow) async {
final List<ui.Image> frames = await _frames; assert(_recordedFrames.isNotEmpty,
assert(frames.isNotEmpty,
'No frames are collected. Have you forgot to set `recording` to true?'); 'No frames are collected. Have you forgot to set `recording` to true?');
return _collateFrames(frames, frameSize, cellsPerRow); final _AsyncImage result = _AsyncImage(_collateFrames(_recordedFrames, frameSize, cellsPerRow));
_results.add(result);
return result.result();
} }
} }
...@@ -281,7 +337,8 @@ class _RenderPostFrameCallbacker extends RenderProxyBox { ...@@ -281,7 +337,8 @@ class _RenderPostFrameCallbacker extends RenderProxyBox {
} }
} }
Future<ui.Image> _collateFrames(List<ui.Image> frames, Size frameSize, int cellsPerRow) async { Future<ui.Image> _collateFrames(List<_AsyncImage> futureFrames, Size frameSize, int cellsPerRow) async {
final List<ui.Image> frames = await _AsyncImage.resolveList(futureFrames);
final int rowNum = (frames.length / cellsPerRow).ceil(); final int rowNum = (frames.length / cellsPerRow).ceil();
final ui.PictureRecorder recorder = ui.PictureRecorder(); final ui.PictureRecorder recorder = ui.PictureRecorder();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment