Fix leaks in some image tests and explain hackyness of opted-out. (#154481)

This commit is contained in:
Polina Cherkasova 2024-10-22 10:22:01 -07:00 committed by GitHub
parent 3e9901dac9
commit 9edd1ae008
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 70 additions and 65 deletions

View File

@ -7,40 +7,34 @@ import 'dart:ui' as ui show Image;
import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
class TestImageProvider extends ImageProvider<TestImageProvider> {
const TestImageProvider(this.image);
class _TestImageProvider extends ImageProvider<_TestImageProvider> {
_TestImageProvider(ui.Image image) {
_completer = OneFrameImageStreamCompleter(
SynchronousFuture<ImageInfo>(ImageInfo(image: image)),
);
}
final ui.Image image;
late final OneFrameImageStreamCompleter _completer;
@override
Future<TestImageProvider> obtainKey(ImageConfiguration configuration) {
return SynchronousFuture<TestImageProvider>(this);
Future<_TestImageProvider> obtainKey(ImageConfiguration configuration) {
return SynchronousFuture<_TestImageProvider>(this);
}
@override
ImageStreamCompleter loadImage(TestImageProvider key, ImageDecoderCallback decode) {
return OneFrameImageStreamCompleter(
SynchronousFuture<ImageInfo>(ImageInfo(image: image)),
);
ImageStreamCompleter loadImage(_TestImageProvider key, ImageDecoderCallback decode) {
return _completer;
}
}
void main() {
// TODO(polina-c): dispose ImageStreamCompleterHandle, https://github.com/flutter/flutter/issues/145599 [leaks-to-clean]
LeakTesting.settings = LeakTesting.settings.withIgnoredAll();
late ui.Image testImage;
setUpAll(() async {
setUp(() async {
testImage = await createTestImage(width: 16, height: 9);
});
tearDownAll(() {
testImage.dispose();
});
testWidgets('DecorationImage RTL with alignment topEnd and match', (WidgetTester tester) async {
await tester.pumpWidget(
Directionality(
@ -51,7 +45,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
matchTextDirection: true,
@ -90,7 +84,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
matchTextDirection: true,
@ -126,7 +120,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
),
@ -161,7 +155,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
),
@ -196,7 +190,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
matchTextDirection: true,
),
@ -228,7 +222,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
),
),
@ -255,7 +249,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
matchTextDirection: true,
),
@ -283,7 +277,7 @@ void main() {
height: 50.0,
decoration: BoxDecoration(
image: DecorationImage(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
matchTextDirection: true,
),
@ -310,7 +304,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
matchTextDirection: true,
@ -336,6 +330,8 @@ void main() {
..restore(),
);
expect(find.byType(SizedBox), isNot(paints..scale()..scale()));
imageCache.clear();
});
testWidgets('Image LTR with alignment topEnd (and pointless match)', (WidgetTester tester) async {
@ -347,7 +343,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
matchTextDirection: true,
@ -381,7 +377,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
),
@ -414,7 +410,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: AlignmentDirectional.topEnd,
repeat: ImageRepeat.repeatX,
),
@ -447,7 +443,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
matchTextDirection: true,
),
@ -475,7 +471,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
),
),
@ -500,7 +496,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
matchTextDirection: true,
),
@ -526,7 +522,7 @@ void main() {
width: 100.0,
height: 50.0,
child: Image(
image: TestImageProvider(testImage),
image: _TestImageProvider(testImage),
alignment: Alignment.centerRight,
matchTextDirection: true,
),
@ -544,22 +540,25 @@ void main() {
});
testWidgets('Image - Switch needing direction', (WidgetTester tester) async {
final _TestImageProvider provider = _TestImageProvider(testImage);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Image(
image: TestImageProvider(testImage),
image: provider,
alignment: Alignment.centerRight,
),
),
duration: Duration.zero,
phase: EnginePhase.layout, // so that we don't try to paint the fake images
);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Image(
image: TestImageProvider(testImage),
image: provider,
alignment: AlignmentDirectional.centerEnd,
matchTextDirection: true,
),
@ -567,16 +566,19 @@ void main() {
duration: Duration.zero,
phase: EnginePhase.layout, // so that we don't try to paint the fake images
);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Image(
image: TestImageProvider(testImage),
image: provider,
alignment: Alignment.centerRight,
),
),
duration: Duration.zero,
phase: EnginePhase.layout, // so that we don't try to paint the fake images
);
imageCache.clear();
});
}

View File

@ -824,8 +824,7 @@ void main() {
});
testWidgets('Precache removes original listener immediately after future completes, does not crash on successive calls #25143',
// TODO(polina-c): dispose ImageStreamCompleterHandle, https://github.com/flutter/flutter/issues/145599 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final _TestImageStreamCompleter imageStreamCompleter = _TestImageStreamCompleter();
final _TestImageProvider provider = _TestImageProvider(streamCompleter: imageStreamCompleter);
@ -845,12 +844,17 @@ void main() {
// Make sure the first listener can be called re-entrantly
final ImageInfo imageInfo = ImageInfo(image: image10x10);
listeners[1].onImage(imageInfo.clone(), false);
listeners[1].onImage(imageInfo.clone(), false);
// Make sure the second listener can be called re-entrantly.
listeners[0].onImage(imageInfo.clone(), false);
listeners[0].onImage(imageInfo.clone(), false);
imageInfo.dispose();
imageStreamCompleter.dispose();
imageCache.clear();
});
testWidgets('Precache completes with onError on error', (WidgetTester tester) async {
@ -1022,8 +1026,7 @@ void main() {
});
testWidgets('Image invokes frameBuilder with correct frameNumber argument',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final ui.Codec codec = (await tester.runAsync(() {
return ui.instantiateImageCodec(Uint8List.fromList(kAnimatedGif));
@ -1094,8 +1097,7 @@ void main() {
});
testWidgets('Image invokes frameBuilder with correct wasSynchronouslyLoaded=true',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final _TestImageStreamCompleter streamCompleter = _TestImageStreamCompleter(ImageInfo(image: image10x10.clone()));
final _TestImageProvider imageProvider = _TestImageProvider(streamCompleter: streamCompleter);
@ -1155,8 +1157,7 @@ void main() {
});
testWidgets('Image state handles enabling and disabling of tickers',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final ui.Codec codec = (await tester.runAsync(() {
return ui.instantiateImageCodec(Uint8List.fromList(kAnimatedGif));
@ -1621,8 +1622,7 @@ void main() {
});
testWidgets('precacheImage allows time to take over weak reference',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final _TestImageProvider provider = _TestImageProvider();
late Future<void> precache;
@ -1674,10 +1674,7 @@ void main() {
expect(provider.loadCallCount, 1);
});
testWidgets('evict an image during precache',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
(WidgetTester tester) async {
testWidgets('evict an image during precache', (WidgetTester tester) async {
// This test checks that the live image tracking does not hold on to a
// pending image that will never complete because it has been evicted from
// the cache.
@ -1707,6 +1704,8 @@ void main() {
await tester.pump();
expect(imageCache.statusForKey(provider).keepAlive, true);
expect(imageCache.statusForKey(provider).live, false);
imageCache.clear();
});
});
@ -1791,10 +1790,7 @@ void main() {
}
testWidgets(
'Rotated images',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
(WidgetTester tester) async {
'Rotated images', (WidgetTester tester) async {
await testRotatedImage(tester, true);
await testRotatedImage(tester, false);
},
@ -1802,10 +1798,7 @@ void main() {
);
testWidgets(
'Image opacity',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
(WidgetTester tester) async {
'Image opacity', (WidgetTester tester) async {
final Key key = UniqueKey();
await tester.pumpWidget(RepaintBoundary(
key: key,
@ -1854,8 +1847,7 @@ void main() {
);
testWidgets('Reports image size when painted',
// TODO(polina-c): make sure images are disposed, https://github.com/flutter/flutter/issues/141388 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
late ImageSizeInfo imageSizeInfo;
int count = 0;
@ -1966,8 +1958,7 @@ void main() {
});
testWidgets('Load a good image after a bad image was loaded should not call errorBuilder',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final UniqueKey errorKey = UniqueKey();
final ui.Image image = (await tester.runAsync(() => createTestImage()))!;
@ -2067,8 +2058,7 @@ void main() {
});
testWidgets('Animated GIFs do not require layout for subsequent frames',
// TODO(polina-c): clean up leaks, https://github.com/flutter/flutter/issues/134787 [leaks-to-clean]
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(),
experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), // The test leaks by design, see [_TestImageStreamCompleter].
(WidgetTester tester) async {
final ui.Codec codec = (await tester.runAsync(() {
return ui.instantiateImageCodec(Uint8List.fromList(kAnimatedGif));
@ -2196,6 +2186,14 @@ class _TestImageProvider extends ImageProvider<Object> {
String toString() => '${describeIdentity(this)}()';
}
/// An [ImageStreamCompleter] that gives access to the added listeners.
///
/// Such an access to listeners is hacky,
/// because it breaks encapsulation by allowing to invoke listeners without
/// taking care about lifecycle of the created images, that may result in not disposed images.
///
/// That's why some tests that use it
/// are opted out from leak tracking.
class _TestImageStreamCompleter extends ImageStreamCompleter {
_TestImageStreamCompleter([this._currentImage]);
@ -2243,6 +2241,11 @@ class _TestImageStreamCompleter extends ImageStreamCompleter {
listener.onError?.call(exception, stackTrace);
}
}
void dispose() {
final List<ImageStreamListener> listenersCopy = listeners.toList();
listenersCopy.forEach(removeListener);
}
}
class _DebouncingImageProvider extends ImageProvider<Object> {