Skip to content

Commit 8023527

Browse files
authored
fix: Dispose codec after completing frame creation (#159945)
ref flutter/flutter#93404, [comment](flutter/flutter#93404 (comment)) and [comment](flutter/flutter#93404 (comment)). Added a process to call `ui.Codec`'s `dispose`. With this change, [HtmlBlobCodec](https://github.com/flutter/flutter/blob/8e0993eda8b0bbf0e11162cf18df1effd134db09/engine/src/flutter/lib/web_ui/lib/src/engine/html_image_element_codec.dart#L100)'s dispose will be called in Safari when using CanvasKit, and `revokeObjectURL` will be executed as expected. * https://api.flutter.dev/flutter/dart-ui/Codec/dispose.html * https://bugs.webkit.org/show_bug.cgi?id=31253 * https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL_static * https://developer.mozilla.org/en-US/docs/Web/API/URL/revokeObjectURL_static If this fix looks good, I will open the flutter/flutter#161481 to remove the `AlearmClock` from the `BrowserImageDecoder`. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent b37e7aa commit 8023527

File tree

5 files changed

+73
-5
lines changed

5 files changed

+73
-5
lines changed

engine/src/flutter/lib/ui/painting.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2630,7 +2630,12 @@ void decodeImageFromList(Uint8List list, ImageDecoderCallback callback) {
26302630

26312631
Future<void> _decodeImageFromListAsync(Uint8List list, ImageDecoderCallback callback) async {
26322632
final Codec codec = await instantiateImageCodec(list);
2633-
final FrameInfo frameInfo = await codec.getNextFrame();
2633+
final FrameInfo frameInfo;
2634+
try {
2635+
frameInfo = await codec.getNextFrame();
2636+
} finally {
2637+
codec.dispose();
2638+
}
26342639
callback(frameInfo.image);
26352640
}
26362641

engine/src/flutter/lib/web_ui/lib/painting.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,12 @@ void decodeImageFromList(Uint8List list, ImageDecoderCallback callback) {
695695

696696
Future<void> _decodeImageFromListAsync(Uint8List list, ImageDecoderCallback callback) async {
697697
final Codec codec = await instantiateImageCodec(list);
698-
final FrameInfo frameInfo = await codec.getNextFrame();
698+
final FrameInfo frameInfo;
699+
try {
700+
frameInfo = await codec.getNextFrame();
701+
} finally {
702+
codec.dispose();
703+
}
699704
callback(frameInfo.image);
700705
}
701706

packages/flutter/lib/src/painting/image_decoder.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ import 'binding.dart';
2525
Future<ui.Image> decodeImageFromList(Uint8List bytes) async {
2626
final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromUint8List(bytes);
2727
final ui.Codec codec = await PaintingBinding.instance.instantiateImageCodecWithSize(buffer);
28-
final ui.FrameInfo frameInfo = await codec.getNextFrame();
28+
final ui.FrameInfo frameInfo;
29+
try {
30+
frameInfo = await codec.getNextFrame();
31+
} finally {
32+
codec.dispose();
33+
}
2934
return frameInfo.image;
3035
}

packages/flutter/lib/src/painting/image_stream.dart

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -980,7 +980,8 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
980980
/// Immediately starts decoding the first image frame when the codec is ready.
981981
///
982982
/// The `codec` parameter is a future for an initialized [ui.Codec] that will
983-
/// be used to decode the image.
983+
/// be used to decode the image. This completer takes ownership of the passed
984+
/// `codec` and will dispose it once it is no longer needed.
984985
///
985986
/// The `scale` parameter is the linear scale factor for drawing this frames
986987
/// of this image at their intended size.
@@ -1071,7 +1072,11 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
10711072
final int completedCycles = _framesEmitted ~/ _codec!.frameCount;
10721073
if (_codec!.repetitionCount == -1 || completedCycles <= _codec!.repetitionCount) {
10731074
_decodeNextFrameAndSchedule();
1075+
return;
10741076
}
1077+
1078+
_codec!.dispose();
1079+
_codec = null;
10751080
return;
10761081
}
10771082
final Duration delay = _frameDuration! - (timestamp - _shownTimestamp);
@@ -1105,6 +1110,11 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
11051110
);
11061111
return;
11071112
}
1113+
if (_codec == null) {
1114+
// codec was disposed during getNextFrame
1115+
return;
1116+
}
1117+
11081118
if (_codec!.frameCount == 1) {
11091119
// ImageStreamCompleter listeners removed while waiting for next frame to
11101120
// be decoded.
@@ -1119,6 +1129,9 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
11191129
);
11201130
_nextFrame!.image.dispose();
11211131
_nextFrame = null;
1132+
1133+
_codec!.dispose();
1134+
_codec = null;
11221135
return;
11231136
}
11241137
_scheduleAppFrame();
@@ -1161,6 +1174,9 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
11611174
_chunkSubscription?.onData(null);
11621175
_chunkSubscription?.cancel();
11631176
_chunkSubscription = null;
1177+
1178+
_codec?.dispose();
1179+
_codec = null;
11641180
}
11651181
}
11661182
}

packages/flutter/test/painting/image_stream_test.dart

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,16 @@ class MockCodec implements Codec {
4141

4242
int numFramesAsked = 0;
4343

44+
bool disposed = false;
45+
4446
Completer<FrameInfo> _nextFrameCompleter = Completer<FrameInfo>();
4547

4648
@override
4749
Future<FrameInfo> getNextFrame() {
50+
if (disposed) {
51+
throw StateError('Codec is disposed');
52+
}
53+
4854
numFramesAsked += 1;
4955
return _nextFrameCompleter.future;
5056
}
@@ -59,7 +65,13 @@ class MockCodec implements Codec {
5965
}
6066

6167
@override
62-
void dispose() {}
68+
void dispose() {
69+
if (disposed) {
70+
throw StateError('Codec is already disposed');
71+
}
72+
73+
disposed = true;
74+
}
6375
}
6476

6577
class FakeEventReportingImageStreamCompleter extends ImageStreamCompleter {
@@ -106,6 +118,7 @@ void main() {
106118
imageStream.addListener(ImageStreamListener(listener));
107119
await tester.idle();
108120
expect(mockCodec.numFramesAsked, 1);
121+
expect(mockCodec.disposed, false);
109122
});
110123

111124
testWidgets('Decoding starts when a codec is ready after a listener is added', (
@@ -130,6 +143,7 @@ void main() {
130143
completer.complete(mockCodec);
131144
await tester.idle();
132145
expect(mockCodec.numFramesAsked, 1);
146+
expect(mockCodec.disposed, false);
133147
});
134148

135149
testWidgets('Decoding does not crash when disposed', (WidgetTester tester) async {
@@ -156,7 +170,9 @@ void main() {
156170

157171
final FrameInfo frame = FakeFrameInfo(const Duration(milliseconds: 200), image20x10);
158172
mockCodec.completeNextFrame(frame);
173+
expect(mockCodec.disposed, false);
159174
imageStream.removeListener(streamListener);
175+
expect(mockCodec.disposed, true);
160176
await tester.idle();
161177
});
162178

@@ -334,6 +350,7 @@ void main() {
334350
await tester.idle();
335351

336352
expect(tester.takeException(), 'frame completion error');
353+
expect(mockCodec.disposed, false);
337354
});
338355

339356
testWidgets('ImageStream emits frame (static image)', (WidgetTester tester) async {
@@ -362,7 +379,9 @@ void main() {
362379

363380
final FrameInfo frame = FakeFrameInfo(const Duration(milliseconds: 200), image20x10);
364381
mockCodec.completeNextFrame(frame);
382+
expect(mockCodec.disposed, false);
365383
await tester.idle();
384+
expect(mockCodec.disposed, true);
366385

367386
expect(emittedImages.every((ImageInfo info) => info.image.isCloneOf(frame.image)), true);
368387

@@ -420,7 +439,9 @@ void main() {
420439
// quit the test without pending timers.
421440
await tester.pump(const Duration(milliseconds: 400));
422441

442+
expect(mockCodec.disposed, false);
423443
imageStream.removeListener(listener);
444+
expect(mockCodec.disposed, true);
424445
imageCache.clear();
425446
});
426447

@@ -469,7 +490,9 @@ void main() {
469490
// quit the test without pending timers.
470491
await tester.pump(const Duration(milliseconds: 200));
471492

493+
expect(mockCodec.disposed, false);
472494
imageStream.removeListener(listener);
495+
expect(mockCodec.disposed, true);
473496
imageCache.clear();
474497
});
475498

@@ -505,7 +528,9 @@ void main() {
505528
await tester.pump(); // first animation frame shows on first app frame.
506529
mockCodec.completeNextFrame(frame2);
507530
await tester.idle(); // let nextFrameFuture complete
531+
expect(mockCodec.disposed, false);
508532
await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame.
533+
expect(mockCodec.disposed, true);
509534
mockCodec.completeNextFrame(frame1);
510535
// allow another frame to complete (but we shouldn't be asking for it as
511536
// this animation should not repeat.
@@ -562,7 +587,9 @@ void main() {
562587
expect(mockCodec.numFramesAsked, 3);
563588

564589
handle.dispose();
590+
expect(mockCodec.disposed, false);
565591
imageStream.removeListener(listener);
592+
expect(mockCodec.disposed, true);
566593
imageCache.clear();
567594
});
568595

@@ -619,7 +646,9 @@ void main() {
619646
expect(emittedImages2[0].image.isCloneOf(frame1.image), true);
620647
expect(emittedImages2[1].image.isCloneOf(frame2.image), true);
621648

649+
expect(mockCodec.disposed, false);
622650
imageStream.removeListener(listener2);
651+
expect(mockCodec.disposed, true);
623652
});
624653

625654
testWidgets('timer is canceled when listeners are removed', (WidgetTester tester) async {
@@ -653,7 +682,9 @@ void main() {
653682
await tester.idle(); // let nextFrameFuture complete
654683
await tester.pump();
655684

685+
expect(mockCodec.disposed, false);
656686
imageStream.removeListener(ImageStreamListener(listener));
687+
expect(mockCodec.disposed, true);
657688
// The test framework will fail this if there are pending timers at this
658689
// point.
659690
});
@@ -699,7 +730,9 @@ void main() {
699730
expect(mockCodec.numFramesAsked, 3);
700731
timeDilation = 1.0; // restore time dilation, or it will affect other tests
701732

733+
expect(mockCodec.disposed, false);
702734
imageStream.removeListener(listener);
735+
expect(mockCodec.disposed, true);
703736
});
704737

705738
testWidgets('error handlers can intercept errors', (WidgetTester tester) async {
@@ -734,6 +767,7 @@ void main() {
734767
// No exception is passed up.
735768
expect(tester.takeException(), isNull);
736769
expect(capturedException, 'frame completion error');
770+
expect(mockCodec.disposed, false);
737771
});
738772

739773
testWidgets(
@@ -772,6 +806,7 @@ void main() {
772806
await tester.pump(); // first animation frame shows on first app frame.
773807

774808
await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame.
809+
expect(mockCodec.disposed, false);
775810
},
776811
);
777812

@@ -918,7 +953,9 @@ void main() {
918953

919954
expect(onImageCount, 1);
920955

956+
expect(mockCodec.disposed, false);
921957
handle.dispose();
958+
expect(mockCodec.disposed, true);
922959
});
923960

924961
test('MultiFrameImageStreamCompleter - one frame image should only be decoded once', () async {

0 commit comments

Comments
 (0)