Unverified Commit 37200a0b authored by Dan Field's avatar Dan Field Committed by GitHub

Remove chunk event subscription when disposing a MultiFrameImageStreamCompleter (#94902)

* Remove chunk event subscription when disposing a MultiFrameImageStreamCompleter

* Add impl..

* stray prints
parent 22dd0f42
...@@ -571,6 +571,8 @@ abstract class ImageStreamCompleter with Diagnosticable { ...@@ -571,6 +571,8 @@ abstract class ImageStreamCompleter with Diagnosticable {
} }
bool _disposed = false; bool _disposed = false;
@mustCallSuper
void _maybeDispose() { void _maybeDispose() {
if (!_hadAtLeastOneListener || _disposed || _listeners.isNotEmpty || _keepAliveHandles != 0) { if (!_hadAtLeastOneListener || _disposed || _listeners.isNotEmpty || _keepAliveHandles != 0) {
return; return;
...@@ -851,7 +853,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { ...@@ -851,7 +853,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
); );
}); });
if (chunkEvents != null) { if (chunkEvents != null) {
chunkEvents.listen(reportImageChunkEvent, _chunkSubscription = chunkEvents.listen(reportImageChunkEvent,
onError: (Object error, StackTrace stack) { onError: (Object error, StackTrace stack) {
reportError( reportError(
context: ErrorDescription('loading an image'), context: ErrorDescription('loading an image'),
...@@ -865,6 +867,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { ...@@ -865,6 +867,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
} }
} }
StreamSubscription<ImageChunkEvent>? _chunkSubscription;
ui.Codec? _codec; ui.Codec? _codec;
final double _scale; final double _scale;
final InformationCollector? _informationCollector; final InformationCollector? _informationCollector;
...@@ -990,4 +993,14 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { ...@@ -990,4 +993,14 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
_timer = null; _timer = null;
} }
} }
@override
void _maybeDispose() {
super._maybeDispose();
if (_disposed) {
_chunkSubscription?.onData(null);
_chunkSubscription?.cancel();
_chunkSubscription = null;
}
}
} }
...@@ -811,44 +811,31 @@ void main() { ...@@ -811,44 +811,31 @@ void main() {
expect(oneFrameCodec.numFramesAsked, 1); expect(oneFrameCodec.numFramesAsked, 1);
}); // https://github.com/flutter/flutter/issues/82532 }); // https://github.com/flutter/flutter/issues/82532
// TODO(amirh): enable this once WidgetTester supports flushTimers. test('Multi-frame complete unsubscribes to chunk events when disposed', () async {
// https://github.com/flutter/flutter/issues/30344 final FakeCodec codec = await FakeCodec.fromData(Uint8List.fromList(kTransparentImage));
// testWidgets('remove and add listener before a delayed frame is scheduled', (WidgetTester tester) async { final StreamController<ImageChunkEvent> chunkStream = StreamController<ImageChunkEvent>();
// final MockCodec mockCodec = MockCodec();
// mockCodec.frameCount = 3; final MultiFrameImageStreamCompleter completer = MultiFrameImageStreamCompleter(
// mockCodec.repetitionCount = 0; codec: Future<Codec>.value(codec),
// final Completer<Codec> codecCompleter = Completer<Codec>(); scale: 1.0,
// chunkEvents: chunkStream.stream,
// final ImageStreamCompleter imageStream = MultiFrameImageStreamCompleter( );
// codec: codecCompleter.future,
// scale: 1.0, expect(chunkStream.hasListener, true);
// );
// chunkStream.add(const ImageChunkEvent(cumulativeBytesLoaded: 1, expectedTotalBytes: 3));
// final ImageListener listener = (ImageInfo image, bool synchronousCall) { };
// imageStream.addListener(ImageLoadingListener(listener)); final ImageStreamListener listener = ImageStreamListener((ImageInfo info, bool syncCall) {});
// // Cause the completer to dispose.
// codecCompleter.complete(mockCodec); completer.addListener(listener);
// await tester.idle(); completer.removeListener(listener);
//
// final FrameInfo frame1 = FakeFrameInfo(20, 10, const Duration(milliseconds: 200)); expect(chunkStream.hasListener, false);
// final FrameInfo frame2 = FakeFrameInfo(200, 100, const Duration(milliseconds: 400));
// final FrameInfo frame3 = FakeFrameInfo(200, 100, Duration.zero); // The above expectation should cover this, but the point of this test is to
// // make sure the completer does not assert that it's disposed and still
// mockCodec.completeNextFrame(frame1); // receiving chunk events. Streams from the network can keep sending data
// await tester.idle(); // let nextFrameFuture complete // even after evicting an image from the cache, for example.
// await tester.pump(); // first animation frame shows on first app frame. chunkStream.add(const ImageChunkEvent(cumulativeBytesLoaded: 2, expectedTotalBytes: 3));
// });
// mockCodec.completeNextFrame(frame2);
// await tester.pump(const Duration(milliseconds: 100)); // emit 2nd frame.
//
// tester.flushTimers();
//
// imageStream.removeListener(listener);
// imageStream.addListener(ImageLoadingListener(listener));
//
// mockCodec.completeNextFrame(frame3);
// await tester.idle(); // let nextFrameFuture complete
//
// await tester.pump();
// });
} }
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