From 78e044f5ec185b213b6dc8a361073dc5a41345bc Mon Sep 17 00:00:00 2001 From: amirh Date: Wed, 22 Nov 2017 15:27:26 -0800 Subject: [PATCH] Cancel the animated image stream timer if all listeners were removed. (#13158) This is a bug in my previous CL: instead of cancelling the timer if there are no more listeners, I canceled it if there were listeners (I can claim I just missed a not :) ). Not cancelling the timer when removing the last listener was not that bad, as the timer callback is guarded by a check to see if there are listeners. So the animation will not continue. But in the case there were multiple listeners on the same stream, and one of them is removed, this bug will stop the animation for all other listeners. I added a test case for this scenario. --- .../lib/src/services/image_stream.dart | 2 +- .../test/services/image_stream_test.dart | 80 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/services/image_stream.dart b/packages/flutter/lib/src/services/image_stream.dart index a50c9a76fd..d5343d2b61 100644 --- a/packages/flutter/lib/src/services/image_stream.dart +++ b/packages/flutter/lib/src/services/image_stream.dart @@ -440,7 +440,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { @override void removeListener(ImageListener listener) { super.removeListener(listener); - if (_hasActiveListeners) { + if (!_hasActiveListeners) { _timer?.cancel(); _timer = null; } diff --git a/packages/flutter/test/services/image_stream_test.dart b/packages/flutter/test/services/image_stream_test.dart index f86ae793be..331d78b073 100644 --- a/packages/flutter/test/services/image_stream_test.dart +++ b/packages/flutter/test/services/image_stream_test.dart @@ -312,6 +312,86 @@ void main() { expect(mockCodec.numFramesAsked, 3); }); + testWidgets('multiple stream listeners', (WidgetTester tester) async { + final MockCodec mockCodec = new MockCodec(); + mockCodec.frameCount = 2; + mockCodec.repetitionCount = -1; + final Completer codecCompleter = new Completer(); + + final ImageStreamCompleter imageStream = new MultiFrameImageStreamCompleter( + codec: codecCompleter.future, + scale: 1.0, + ); + + final List emittedImages1 = []; + final ImageListener listener1 = (ImageInfo image, bool synchronousCall) { + emittedImages1.add(image); + }; + final List emittedImages2 = []; + final ImageListener listener2 = (ImageInfo image, bool synchronousCall) { + emittedImages2.add(image); + }; + imageStream.addListener(listener1); + imageStream.addListener(listener2); + + codecCompleter.complete(mockCodec); + await tester.idle(); + + final FrameInfo frame1 = new FakeFrameInfo(20, 10, const Duration(milliseconds: 200)); + final FrameInfo frame2 = new FakeFrameInfo(200, 100, const Duration(milliseconds: 400)); + + mockCodec.completeNextFrame(frame1); + await tester.idle(); // let nextFrameFuture complete + await tester.pump(); // first animation frame shows on first app frame. + expect(emittedImages1, equals([new ImageInfo(image: frame1.image)])); + expect(emittedImages2, equals([new ImageInfo(image: frame1.image)])); + + mockCodec.completeNextFrame(frame2); + await tester.idle(); // let nextFrameFuture complete + await tester.pump(); // next app frame will schedule a timer. + imageStream.removeListener(listener1); + + await tester.pump(const Duration(milliseconds: 400)); // emit 2nd frame. + expect(emittedImages1, equals([new ImageInfo(image: frame1.image)])); + expect(emittedImages2, equals([ + new ImageInfo(image: frame1.image), + new ImageInfo(image: frame2.image), + ])); + }); + + testWidgets('timer is canceled when listeners are removed', (WidgetTester tester) async { + final MockCodec mockCodec = new MockCodec(); + mockCodec.frameCount = 2; + mockCodec.repetitionCount = -1; + final Completer codecCompleter = new Completer(); + + final ImageStreamCompleter imageStream = new MultiFrameImageStreamCompleter( + codec: codecCompleter.future, + scale: 1.0, + ); + + final ImageListener listener = (ImageInfo image, bool synchronousCall) {}; + imageStream.addListener(listener); + + codecCompleter.complete(mockCodec); + await tester.idle(); + + final FrameInfo frame1 = new FakeFrameInfo(20, 10, const Duration(milliseconds: 200)); + final FrameInfo frame2 = new FakeFrameInfo(200, 100, const Duration(milliseconds: 400)); + + mockCodec.completeNextFrame(frame1); + await tester.idle(); // let nextFrameFuture complete + await tester.pump(); // first animation frame shows on first app frame. + + mockCodec.completeNextFrame(frame2); + await tester.idle(); // let nextFrameFuture complete + await tester.pump(); + + imageStream.removeListener(listener); + // The test framework will fail this if there are pending timers at this + // point. + }); + testWidgets('timeDilation affects animation frame timers', (WidgetTester tester) async { final MockCodec mockCodec = new MockCodec(); mockCodec.frameCount = 2;