From 824787e4fdba35691c5285e58c3c0e58666db7bd Mon Sep 17 00:00:00 2001 From: Ben Hagen Date: Fri, 20 Dec 2019 14:13:50 +0100 Subject: [PATCH 1/5] Make sure the plugin is correctly initialized --- .../video_player/video_player/CHANGELOG.md | 4 +++ .../video_player/lib/video_player.dart | 29 +++++++++---------- .../video_player/video_player/pubspec.yaml | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 013e20f93118..a9f8cde2d792 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.10.5+1 + +* Make sure the plugin is correctly initialized + ## 0.10.5 * Support `web` by default. diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index cb24ac8cb100..2033f0b04609 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -14,10 +14,10 @@ import 'package:video_player_platform_interface/video_player_platform_interface. export 'package:video_player_platform_interface/video_player_platform_interface.dart' show DurationRange, DataSourceType, VideoFormat; -// This will clear all open videos on the platform when a full restart is -// performed. -// ignore: unused_element -final VideoPlayerPlatform _ = VideoPlayerPlatform.instance..init(); +final VideoPlayerPlatform _videoPlayerPlatform = VideoPlayerPlatform.instance + // This will clear all open videos on the platform when a full restart is + // performed. + ..init(); /// The duration, current position, buffering state, error state and settings /// of a [VideoPlayerController]. @@ -229,8 +229,7 @@ class VideoPlayerController extends ValueNotifier { ); break; } - _textureId = - await VideoPlayerPlatform.instance.create(dataSourceDescription); + _textureId = await _videoPlayerPlatform.create(dataSourceDescription); _creatingCompleter.complete(null); final Completer initializingCompleter = Completer(); @@ -274,7 +273,7 @@ class VideoPlayerController extends ValueNotifier { _timer?.cancel(); } - _eventSubscription = VideoPlayerPlatform.instance + _eventSubscription = _videoPlayerPlatform .videoEventsFor(_textureId) .listen(eventListener, onError: errorListener); return initializingCompleter.future; @@ -288,7 +287,7 @@ class VideoPlayerController extends ValueNotifier { _isDisposed = true; _timer?.cancel(); await _eventSubscription?.cancel(); - await VideoPlayerPlatform.instance.dispose(_textureId); + await _videoPlayerPlatform.dispose(_textureId); } _lifeCycleObserver.dispose(); } @@ -323,7 +322,7 @@ class VideoPlayerController extends ValueNotifier { if (!value.initialized || _isDisposed) { return; } - await VideoPlayerPlatform.instance.setLooping(_textureId, value.isLooping); + await _videoPlayerPlatform.setLooping(_textureId, value.isLooping); } Future _applyPlayPause() async { @@ -331,7 +330,7 @@ class VideoPlayerController extends ValueNotifier { return; } if (value.isPlaying) { - await VideoPlayerPlatform.instance.play(_textureId); + await _videoPlayerPlatform.play(_textureId); _timer = Timer.periodic( const Duration(milliseconds: 500), (Timer timer) async { @@ -347,7 +346,7 @@ class VideoPlayerController extends ValueNotifier { ); } else { _timer?.cancel(); - await VideoPlayerPlatform.instance.pause(_textureId); + await _videoPlayerPlatform.pause(_textureId); } } @@ -355,7 +354,7 @@ class VideoPlayerController extends ValueNotifier { if (!value.initialized || _isDisposed) { return; } - await VideoPlayerPlatform.instance.setVolume(_textureId, value.volume); + await _videoPlayerPlatform.setVolume(_textureId, value.volume); } /// The position in the current video. @@ -363,7 +362,7 @@ class VideoPlayerController extends ValueNotifier { if (_isDisposed) { return null; } - return await VideoPlayerPlatform.instance.getPosition(_textureId); + return await _videoPlayerPlatform.getPosition(_textureId); } /// Sets the video's current timestamp to be at [moment]. The next @@ -380,7 +379,7 @@ class VideoPlayerController extends ValueNotifier { } else if (position < const Duration()) { position = const Duration(); } - await VideoPlayerPlatform.instance.seekTo(_textureId, position); + await _videoPlayerPlatform.seekTo(_textureId, position); value = value.copyWith(position: position); } @@ -480,7 +479,7 @@ class _VideoPlayerState extends State { Widget build(BuildContext context) { return _textureId == null ? Container() - : VideoPlayerPlatform.instance.buildView(_textureId); + : _videoPlayerPlatform.buildView(_textureId); } } diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 43c9ead07d25..33af2872ff14 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -1,7 +1,7 @@ name: video_player description: Flutter plugin for displaying inline video with other Flutter widgets on Android and iOS. -version: 0.10.5 +version: 0.10.5+1 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player flutter: From 726ec8c8077f749dcdecd5855374c9ae2fd158a5 Mon Sep 17 00:00:00 2001 From: Ben Hagen Date: Fri, 20 Dec 2019 18:48:07 +0100 Subject: [PATCH 2/5] Test if init has been called --- .../video_player/video_player/test/video_player_test.dart | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index d2a90b7182c9..9464ce6897e5 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -91,6 +91,14 @@ void main() { fakeVideoPlayerPlatform = FakeVideoPlayerPlatform(); }); + test('plugin initialized', () async { + final VideoPlayerController controller = VideoPlayerController.network( + 'https://127.0.0.1', + ); + await controller.initialize(); + expect(fakeVideoPlayerPlatform.calls.first.method, 'init'); + }); + group('initialize', () { test('asset', () async { final VideoPlayerController controller = VideoPlayerController.asset( From 75cbe0decf4207b78416960f54f72c3bafd6949b Mon Sep 17 00:00:00 2001 From: Ben Hagen Date: Fri, 10 Jan 2020 23:37:13 +0100 Subject: [PATCH 3/5] Split initialization test case into its own file --- .../video_player_initialization_test.dart | 22 +++++++++++++++++++ .../video_player/test/video_player_test.dart | 8 ------- 2 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 packages/video_player/video_player/test/video_player_initialization_test.dart diff --git a/packages/video_player/video_player/test/video_player_initialization_test.dart b/packages/video_player/video_player/test/video_player_initialization_test.dart new file mode 100644 index 000000000000..f4327e784459 --- /dev/null +++ b/packages/video_player/video_player/test/video_player_initialization_test.dart @@ -0,0 +1,22 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:video_player/video_player.dart'; + +import 'video_player_test.dart' show FakeVideoPlayerPlatform; + +void main() { + test('plugin initialized', () async { + WidgetsFlutterBinding.ensureInitialized(); + FakeVideoPlayerPlatform fakeVideoPlayerPlatform = FakeVideoPlayerPlatform(); + + final VideoPlayerController controller = VideoPlayerController.network( + 'https://127.0.0.1', + ); + await controller.initialize(); + expect(fakeVideoPlayerPlatform.calls.first.method, 'init'); + }); +} diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index 9464ce6897e5..d2a90b7182c9 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -91,14 +91,6 @@ void main() { fakeVideoPlayerPlatform = FakeVideoPlayerPlatform(); }); - test('plugin initialized', () async { - final VideoPlayerController controller = VideoPlayerController.network( - 'https://127.0.0.1', - ); - await controller.initialize(); - expect(fakeVideoPlayerPlatform.calls.first.method, 'init'); - }); - group('initialize', () { test('asset', () async { final VideoPlayerController controller = VideoPlayerController.asset( From 5647dbd67a44d4bc18b27383722bd7f55e875157 Mon Sep 17 00:00:00 2001 From: Ben Hagen Date: Fri, 10 Jan 2020 23:48:15 +0100 Subject: [PATCH 4/5] Update version --- packages/video_player/video_player/CHANGELOG.md | 7 ++++++- packages/video_player/video_player/pubspec.yaml | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index a9f8cde2d792..1c5bc5049993 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,7 +1,12 @@ -## 0.10.5+1 +## 0.10.5+2 * Make sure the plugin is correctly initialized +## 0.10.5+1 + +* Fixes issue where `initialize()` `Future` stalls when failing to load source + data and does not throw an error. + ## 0.10.5 * Support `web` by default. diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 33af2872ff14..691acc12c184 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -1,7 +1,7 @@ name: video_player description: Flutter plugin for displaying inline video with other Flutter widgets on Android and iOS. -version: 0.10.5+1 +version: 0.10.5+2 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player flutter: From 3ead6994e3f9d3fde789edf1f986f4fafc7acd58 Mon Sep 17 00:00:00 2001 From: Ben Hagen Date: Sat, 11 Jan 2020 00:10:20 +0100 Subject: [PATCH 5/5] Explain why initialization test should be only test --- .../video_player/test/video_player_initialization_test.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/video_player/video_player/test/video_player_initialization_test.dart b/packages/video_player/video_player/test/video_player_initialization_test.dart index f4327e784459..61d28070e948 100644 --- a/packages/video_player/video_player/test/video_player_initialization_test.dart +++ b/packages/video_player/video_player/test/video_player_initialization_test.dart @@ -9,6 +9,8 @@ import 'package:video_player/video_player.dart'; import 'video_player_test.dart' show FakeVideoPlayerPlatform; void main() { + // This test needs to run first and therefore needs to be the only test + // in this file. test('plugin initialized', () async { WidgetsFlutterBinding.ensureInitialized(); FakeVideoPlayerPlatform fakeVideoPlayerPlatform = FakeVideoPlayerPlatform();