From 7e49209e89a6559aaba16a324a58cca3a5a0af2b Mon Sep 17 00:00:00 2001 From: creativecreatorormaybenot Date: Wed, 7 Oct 2020 04:02:44 +0000 Subject: [PATCH 1/4] Fix timer leak --- .../video_player/video_player/CHANGELOG.md | 5 ++++ .../integration_test/video_player_test.dart | 27 ++++++++++++++++++- .../video_player/lib/video_player.dart | 3 +++ .../video_player/video_player/pubspec.yaml | 2 +- 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index f7fad2648b3e..8dce28ed5c63 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.11.0+1 + +* Fixed uncanceled timers when calling `play` on the controller multiple times causing value + listeners to be called indefinitely (after `pause`) and more often than needed. + ## 0.11.0 * Added option to set the video playback speed on the video controller. diff --git a/packages/video_player/video_player/example/integration_test/video_player_test.dart b/packages/video_player/video_player/example/integration_test/video_player_test.dart index 0953c8feb6c0..c8bc49d7b9b9 100644 --- a/packages/video_player/video_player/example/integration_test/video_player_test.dart +++ b/packages/video_player/video_player/example/integration_test/video_player_test.dart @@ -3,8 +3,9 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:io'; -import 'package:integration_test/integration_test.dart'; + import 'package:flutter_test/flutter_test.dart'; +import 'package:integration_test/integration_test.dart'; import 'package:video_player/video_player.dart'; const Duration _playDuration = Duration(seconds: 1); @@ -62,5 +63,29 @@ void main() { expect(_controller.value.isPlaying, false); expect(_controller.value.position, pausedPosition); }, skip: Platform.isIOS); + + testWidgets('cancels timers', (WidgetTester tester) async { + await _controller.initialize(); + + var eventCount = 0; + final listener = () => eventCount++; + _controller.addListener(listener); + + // Play for a second, then pause, and then wait a second. + await _controller.play(); + // Call play twice to test if timers leak. + await _controller.play(); + await tester.pumpAndSettle(_playDuration); + + final prePauseEventCount = eventCount; + await _controller.pause(); + await tester.pumpAndSettle(_playDuration); + + // If timers were properly canceled, there should only be one new event + // (from pausing). If timers were not canceled, there will be more events. + expect(eventCount, prePauseEventCount + 1); + + _controller.removeListener(listener); + }, skip: Platform.isIOS); }); } diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index bf98096af45d..ac1645085e36 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -391,6 +391,9 @@ class VideoPlayerController extends ValueNotifier { } if (value.isPlaying) { await _videoPlayerPlatform.play(_textureId); + + // Cancel previous timer. + _timer?.cancel(); _timer = Timer.periodic( const Duration(milliseconds: 500), (Timer timer) async { diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 973c0bc82589..30272c4c6b1f 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -4,7 +4,7 @@ description: Flutter plugin for displaying inline video with other Flutter # 0.10.y+z is compatible with 1.0.0, if you land a breaking change bump # the version to 2.0.0. # See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0 -version: 0.11.0 +version: 0.11.0+1 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player flutter: From b55a1d4c48803a363722788f247cfcc83cff6084 Mon Sep 17 00:00:00 2001 From: creativecreatorormaybenot Date: Wed, 7 Oct 2020 04:06:07 +0000 Subject: [PATCH 2/4] Remove integration test --- .../integration_test/video_player_test.dart | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/packages/video_player/video_player/example/integration_test/video_player_test.dart b/packages/video_player/video_player/example/integration_test/video_player_test.dart index c8bc49d7b9b9..3314d4bfd212 100644 --- a/packages/video_player/video_player/example/integration_test/video_player_test.dart +++ b/packages/video_player/video_player/example/integration_test/video_player_test.dart @@ -63,29 +63,5 @@ void main() { expect(_controller.value.isPlaying, false); expect(_controller.value.position, pausedPosition); }, skip: Platform.isIOS); - - testWidgets('cancels timers', (WidgetTester tester) async { - await _controller.initialize(); - - var eventCount = 0; - final listener = () => eventCount++; - _controller.addListener(listener); - - // Play for a second, then pause, and then wait a second. - await _controller.play(); - // Call play twice to test if timers leak. - await _controller.play(); - await tester.pumpAndSettle(_playDuration); - - final prePauseEventCount = eventCount; - await _controller.pause(); - await tester.pumpAndSettle(_playDuration); - - // If timers were properly canceled, there should only be one new event - // (from pausing). If timers were not canceled, there will be more events. - expect(eventCount, prePauseEventCount + 1); - - _controller.removeListener(listener); - }, skip: Platform.isIOS); }); } From 39496c2a83a232c4f53e10361e294c66be4bbf20 Mon Sep 17 00:00:00 2001 From: creativecreatorormaybenot Date: Wed, 7 Oct 2020 04:10:06 +0000 Subject: [PATCH 3/4] rev --- .../example/integration_test/video_player_test.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/video_player/video_player/example/integration_test/video_player_test.dart b/packages/video_player/video_player/example/integration_test/video_player_test.dart index 3314d4bfd212..0953c8feb6c0 100644 --- a/packages/video_player/video_player/example/integration_test/video_player_test.dart +++ b/packages/video_player/video_player/example/integration_test/video_player_test.dart @@ -3,9 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:io'; - -import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; +import 'package:flutter_test/flutter_test.dart'; import 'package:video_player/video_player.dart'; const Duration _playDuration = Duration(seconds: 1); From cdcb9419a013e6a68dce1941d507a801020d65b9 Mon Sep 17 00:00:00 2001 From: creativecreatorormaybenot Date: Wed, 7 Oct 2020 04:20:06 +0000 Subject: [PATCH 4/4] changelog --- packages/video_player/video_player/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 8dce28ed5c63..30736bd3861b 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,7 +1,7 @@ ## 0.11.0+1 -* Fixed uncanceled timers when calling `play` on the controller multiple times causing value - listeners to be called indefinitely (after `pause`) and more often than needed. +* Fixed uncanceled timers when calling `play` on the controller multiple times before `pause`, which + caused value listeners to be called indefinitely (after `pause`) and more often than needed. ## 0.11.0