fix: Dispose codec after completing frame creation (#159945)

ref https://github.com/flutter/flutter/issues/93404,
[comment](https://github.com/flutter/flutter/issues/93404#issuecomment-2525453113)
and
[comment](https://github.com/flutter/flutter/issues/93404#issuecomment-2547622795).

Added a process to call `ui.Codec`'s `dispose`.
With this change,
[HtmlBlobCodec](8e0993eda8/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
https://github.com/flutter/flutter/pull/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
This commit is contained in:
Koji Wakamiya 2025-02-12 08:20:06 +09:00 committed by GitHub
parent b37e7aaaab
commit 80235272c9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 73 additions and 5 deletions

View File

@ -2630,7 +2630,12 @@ void decodeImageFromList(Uint8List list, ImageDecoderCallback callback) {
Future<void> _decodeImageFromListAsync(Uint8List list, ImageDecoderCallback callback) async { Future<void> _decodeImageFromListAsync(Uint8List list, ImageDecoderCallback callback) async {
final Codec codec = await instantiateImageCodec(list); final Codec codec = await instantiateImageCodec(list);
final FrameInfo frameInfo = await codec.getNextFrame(); final FrameInfo frameInfo;
try {
frameInfo = await codec.getNextFrame();
} finally {
codec.dispose();
}
callback(frameInfo.image); callback(frameInfo.image);
} }

View File

@ -695,7 +695,12 @@ void decodeImageFromList(Uint8List list, ImageDecoderCallback callback) {
Future<void> _decodeImageFromListAsync(Uint8List list, ImageDecoderCallback callback) async { Future<void> _decodeImageFromListAsync(Uint8List list, ImageDecoderCallback callback) async {
final Codec codec = await instantiateImageCodec(list); final Codec codec = await instantiateImageCodec(list);
final FrameInfo frameInfo = await codec.getNextFrame(); final FrameInfo frameInfo;
try {
frameInfo = await codec.getNextFrame();
} finally {
codec.dispose();
}
callback(frameInfo.image); callback(frameInfo.image);
} }

View File

@ -25,6 +25,11 @@ import 'binding.dart';
Future<ui.Image> decodeImageFromList(Uint8List bytes) async { Future<ui.Image> decodeImageFromList(Uint8List bytes) async {
final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromUint8List(bytes); final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromUint8List(bytes);
final ui.Codec codec = await PaintingBinding.instance.instantiateImageCodecWithSize(buffer); final ui.Codec codec = await PaintingBinding.instance.instantiateImageCodecWithSize(buffer);
final ui.FrameInfo frameInfo = await codec.getNextFrame(); final ui.FrameInfo frameInfo;
try {
frameInfo = await codec.getNextFrame();
} finally {
codec.dispose();
}
return frameInfo.image; return frameInfo.image;
} }

View File

@ -980,7 +980,8 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
/// Immediately starts decoding the first image frame when the codec is ready. /// Immediately starts decoding the first image frame when the codec is ready.
/// ///
/// The `codec` parameter is a future for an initialized [ui.Codec] that will /// The `codec` parameter is a future for an initialized [ui.Codec] that will
/// be used to decode the image. /// be used to decode the image. This completer takes ownership of the passed
/// `codec` and will dispose it once it is no longer needed.
/// ///
/// The `scale` parameter is the linear scale factor for drawing this frames /// The `scale` parameter is the linear scale factor for drawing this frames
/// of this image at their intended size. /// of this image at their intended size.
@ -1071,7 +1072,11 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
final int completedCycles = _framesEmitted ~/ _codec!.frameCount; final int completedCycles = _framesEmitted ~/ _codec!.frameCount;
if (_codec!.repetitionCount == -1 || completedCycles <= _codec!.repetitionCount) { if (_codec!.repetitionCount == -1 || completedCycles <= _codec!.repetitionCount) {
_decodeNextFrameAndSchedule(); _decodeNextFrameAndSchedule();
return;
} }
_codec!.dispose();
_codec = null;
return; return;
} }
final Duration delay = _frameDuration! - (timestamp - _shownTimestamp); final Duration delay = _frameDuration! - (timestamp - _shownTimestamp);
@ -1105,6 +1110,11 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
); );
return; return;
} }
if (_codec == null) {
// codec was disposed during getNextFrame
return;
}
if (_codec!.frameCount == 1) { if (_codec!.frameCount == 1) {
// ImageStreamCompleter listeners removed while waiting for next frame to // ImageStreamCompleter listeners removed while waiting for next frame to
// be decoded. // be decoded.
@ -1119,6 +1129,9 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
); );
_nextFrame!.image.dispose(); _nextFrame!.image.dispose();
_nextFrame = null; _nextFrame = null;
_codec!.dispose();
_codec = null;
return; return;
} }
_scheduleAppFrame(); _scheduleAppFrame();
@ -1161,6 +1174,9 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
_chunkSubscription?.onData(null); _chunkSubscription?.onData(null);
_chunkSubscription?.cancel(); _chunkSubscription?.cancel();
_chunkSubscription = null; _chunkSubscription = null;
_codec?.dispose();
_codec = null;
} }
} }
} }

View File

@ -41,10 +41,16 @@ class MockCodec implements Codec {
int numFramesAsked = 0; int numFramesAsked = 0;
bool disposed = false;
Completer<FrameInfo> _nextFrameCompleter = Completer<FrameInfo>(); Completer<FrameInfo> _nextFrameCompleter = Completer<FrameInfo>();
@override @override
Future<FrameInfo> getNextFrame() { Future<FrameInfo> getNextFrame() {
if (disposed) {
throw StateError('Codec is disposed');
}
numFramesAsked += 1; numFramesAsked += 1;
return _nextFrameCompleter.future; return _nextFrameCompleter.future;
} }
@ -59,7 +65,13 @@ class MockCodec implements Codec {
} }
@override @override
void dispose() {} void dispose() {
if (disposed) {
throw StateError('Codec is already disposed');
}
disposed = true;
}
} }
class FakeEventReportingImageStreamCompleter extends ImageStreamCompleter { class FakeEventReportingImageStreamCompleter extends ImageStreamCompleter {
@ -106,6 +118,7 @@ void main() {
imageStream.addListener(ImageStreamListener(listener)); imageStream.addListener(ImageStreamListener(listener));
await tester.idle(); await tester.idle();
expect(mockCodec.numFramesAsked, 1); expect(mockCodec.numFramesAsked, 1);
expect(mockCodec.disposed, false);
}); });
testWidgets('Decoding starts when a codec is ready after a listener is added', ( testWidgets('Decoding starts when a codec is ready after a listener is added', (
@ -130,6 +143,7 @@ void main() {
completer.complete(mockCodec); completer.complete(mockCodec);
await tester.idle(); await tester.idle();
expect(mockCodec.numFramesAsked, 1); expect(mockCodec.numFramesAsked, 1);
expect(mockCodec.disposed, false);
}); });
testWidgets('Decoding does not crash when disposed', (WidgetTester tester) async { testWidgets('Decoding does not crash when disposed', (WidgetTester tester) async {
@ -156,7 +170,9 @@ void main() {
final FrameInfo frame = FakeFrameInfo(const Duration(milliseconds: 200), image20x10); final FrameInfo frame = FakeFrameInfo(const Duration(milliseconds: 200), image20x10);
mockCodec.completeNextFrame(frame); mockCodec.completeNextFrame(frame);
expect(mockCodec.disposed, false);
imageStream.removeListener(streamListener); imageStream.removeListener(streamListener);
expect(mockCodec.disposed, true);
await tester.idle(); await tester.idle();
}); });
@ -334,6 +350,7 @@ void main() {
await tester.idle(); await tester.idle();
expect(tester.takeException(), 'frame completion error'); expect(tester.takeException(), 'frame completion error');
expect(mockCodec.disposed, false);
}); });
testWidgets('ImageStream emits frame (static image)', (WidgetTester tester) async { testWidgets('ImageStream emits frame (static image)', (WidgetTester tester) async {
@ -362,7 +379,9 @@ void main() {
final FrameInfo frame = FakeFrameInfo(const Duration(milliseconds: 200), image20x10); final FrameInfo frame = FakeFrameInfo(const Duration(milliseconds: 200), image20x10);
mockCodec.completeNextFrame(frame); mockCodec.completeNextFrame(frame);
expect(mockCodec.disposed, false);
await tester.idle(); await tester.idle();
expect(mockCodec.disposed, true);
expect(emittedImages.every((ImageInfo info) => info.image.isCloneOf(frame.image)), true); expect(emittedImages.every((ImageInfo info) => info.image.isCloneOf(frame.image)), true);
@ -420,7 +439,9 @@ void main() {
// quit the test without pending timers. // quit the test without pending timers.
await tester.pump(const Duration(milliseconds: 400)); await tester.pump(const Duration(milliseconds: 400));
expect(mockCodec.disposed, false);
imageStream.removeListener(listener); imageStream.removeListener(listener);
expect(mockCodec.disposed, true);
imageCache.clear(); imageCache.clear();
}); });
@ -469,7 +490,9 @@ void main() {
// quit the test without pending timers. // quit the test without pending timers.
await tester.pump(const Duration(milliseconds: 200)); await tester.pump(const Duration(milliseconds: 200));
expect(mockCodec.disposed, false);
imageStream.removeListener(listener); imageStream.removeListener(listener);
expect(mockCodec.disposed, true);
imageCache.clear(); imageCache.clear();
}); });
@ -505,7 +528,9 @@ void main() {
await tester.pump(); // first animation frame shows on first app frame. await tester.pump(); // first animation frame shows on first app frame.
mockCodec.completeNextFrame(frame2); mockCodec.completeNextFrame(frame2);
await tester.idle(); // let nextFrameFuture complete await tester.idle(); // let nextFrameFuture complete
expect(mockCodec.disposed, false);
await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame. await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame.
expect(mockCodec.disposed, true);
mockCodec.completeNextFrame(frame1); mockCodec.completeNextFrame(frame1);
// allow another frame to complete (but we shouldn't be asking for it as // allow another frame to complete (but we shouldn't be asking for it as
// this animation should not repeat. // this animation should not repeat.
@ -562,7 +587,9 @@ void main() {
expect(mockCodec.numFramesAsked, 3); expect(mockCodec.numFramesAsked, 3);
handle.dispose(); handle.dispose();
expect(mockCodec.disposed, false);
imageStream.removeListener(listener); imageStream.removeListener(listener);
expect(mockCodec.disposed, true);
imageCache.clear(); imageCache.clear();
}); });
@ -619,7 +646,9 @@ void main() {
expect(emittedImages2[0].image.isCloneOf(frame1.image), true); expect(emittedImages2[0].image.isCloneOf(frame1.image), true);
expect(emittedImages2[1].image.isCloneOf(frame2.image), true); expect(emittedImages2[1].image.isCloneOf(frame2.image), true);
expect(mockCodec.disposed, false);
imageStream.removeListener(listener2); imageStream.removeListener(listener2);
expect(mockCodec.disposed, true);
}); });
testWidgets('timer is canceled when listeners are removed', (WidgetTester tester) async { testWidgets('timer is canceled when listeners are removed', (WidgetTester tester) async {
@ -653,7 +682,9 @@ void main() {
await tester.idle(); // let nextFrameFuture complete await tester.idle(); // let nextFrameFuture complete
await tester.pump(); await tester.pump();
expect(mockCodec.disposed, false);
imageStream.removeListener(ImageStreamListener(listener)); imageStream.removeListener(ImageStreamListener(listener));
expect(mockCodec.disposed, true);
// The test framework will fail this if there are pending timers at this // The test framework will fail this if there are pending timers at this
// point. // point.
}); });
@ -699,7 +730,9 @@ void main() {
expect(mockCodec.numFramesAsked, 3); expect(mockCodec.numFramesAsked, 3);
timeDilation = 1.0; // restore time dilation, or it will affect other tests timeDilation = 1.0; // restore time dilation, or it will affect other tests
expect(mockCodec.disposed, false);
imageStream.removeListener(listener); imageStream.removeListener(listener);
expect(mockCodec.disposed, true);
}); });
testWidgets('error handlers can intercept errors', (WidgetTester tester) async { testWidgets('error handlers can intercept errors', (WidgetTester tester) async {
@ -734,6 +767,7 @@ void main() {
// No exception is passed up. // No exception is passed up.
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
expect(capturedException, 'frame completion error'); expect(capturedException, 'frame completion error');
expect(mockCodec.disposed, false);
}); });
testWidgets( testWidgets(
@ -772,6 +806,7 @@ void main() {
await tester.pump(); // first animation frame shows on first app frame. await tester.pump(); // first animation frame shows on first app frame.
await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame. await tester.pump(const Duration(milliseconds: 200)); // emit 2nd frame.
expect(mockCodec.disposed, false);
}, },
); );
@ -918,7 +953,9 @@ void main() {
expect(onImageCount, 1); expect(onImageCount, 1);
expect(mockCodec.disposed, false);
handle.dispose(); handle.dispose();
expect(mockCodec.disposed, true);
}); });
test('MultiFrameImageStreamCompleter - one frame image should only be decoded once', () async { test('MultiFrameImageStreamCompleter - one frame image should only be decoded once', () async {