Background/resuming video_player on Android sends one initialized…

… event (#7722) Closes flutter/flutter#154602. I also added small clarifications to the error in `video_player` (an assertion/error that are better than a `Completer.complete` default error) and updated the documentation in `video_player_platform` (it might have been obvious, but it definitely is now).
flutter · Oct 2, 2024 · aeecebc · aeecebc
1 parent a566da6
commit aeecebc
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 9 deletions.
diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md
@@ -1,6 +1,8 @@
-## NEXT
+## 2.9.2
 
 * Updates minimum supported SDK version to Flutter 3.19/Dart 3.3.
+* Throws a more descriptive `StateError` in the case where 
+  `VideoPlayerController.initialize` receives more than one `initialized` event.
 
 ## 2.9.1
 

diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart
@@ -465,6 +465,16 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {
             errorDescription: null,
             isCompleted: false,
           );
+          assert(
+            !initializingCompleter.isCompleted,
+            'VideoPlayerController already initialized. This is typically a '
+            'sign that an implementation of the VideoPlayerPlatform '
+            '(${_videoPlayerPlatform.runtimeType}) has a bug and is sending '
+            'more than one initialized event per instance.',
+          );
+          if (initializingCompleter.isCompleted) {
+            throw StateError('VideoPlayerController already initialized');
+          }
           initializingCompleter.complete(null);
           _applyLooping();
           _applyVolume();

diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml
@@ -3,7 +3,7 @@ description: Flutter plugin for displaying inline video with other Flutter
   widgets on Android, iOS, and web.
 repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
-version: 2.9.1
+version: 2.9.2
 
 environment:
   sdk: ^3.3.0

diff --git a/packages/video_player/video_player_android/CHANGELOG.md b/packages/video_player/video_player_android/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 2.7.6
+
+* Fixes a [bug](https://github.com/flutter/flutter/issues/154602) where
+  resuming a video player would cause a `Bad state: Future already completed`.
+
 ## 2.7.5
 
 * Add a deprecation suppression in advance of a new `SurfaceProducer` API.
@@ -18,12 +23,10 @@
 
 * Re-adds Impeller support.
 
-
 ## 2.7.1
 
 * Revert Impeller support.
 
-
 ## 2.7.0
 
 * Re-adds [support for Impeller](https://docs.flutter.dev/release/breaking-changes/android-surface-plugins).

diff --git a/..._android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java b/..._android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java
@@ -14,11 +14,16 @@ final class ExoPlayerEventListener implements Player.Listener {
   private final ExoPlayer exoPlayer;
   private final VideoPlayerCallbacks events;
   private boolean isBuffering = false;
-  private boolean isInitialized = false;
+  private boolean isInitialized;
 
   ExoPlayerEventListener(ExoPlayer exoPlayer, VideoPlayerCallbacks events) {
+    this(exoPlayer, events, false);
+  }
+
+  ExoPlayerEventListener(ExoPlayer exoPlayer, VideoPlayerCallbacks events, boolean initialized) {
     this.exoPlayer = exoPlayer;
     this.events = events;
+    this.isInitialized = initialized;
   }
 
   private void setBuffering(boolean buffering) {

diff --git a/...ideo_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/...ideo_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java
@@ -108,7 +108,9 @@ private ExoPlayer createVideoPlayer() {
     exoPlayer.prepare();
 
     exoPlayer.setVideoSurface(surfaceProducer.getSurface());
-    exoPlayer.addListener(new ExoPlayerEventListener(exoPlayer, videoPlayerEvents));
+
+    boolean wasInitialized = savedStateDuring != null;
+    exoPlayer.addListener(new ExoPlayerEventListener(exoPlayer, videoPlayerEvents, wasInitialized));
     setAudioAttributes(exoPlayer, options.mixWithOthers);
 
     return exoPlayer;

diff --git a/..._player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java b/..._player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java
@@ -13,6 +13,7 @@
 import androidx.media3.common.C;
 import androidx.media3.common.PlaybackParameters;
 import androidx.media3.common.Player;
+import androidx.media3.common.VideoSize;
 import androidx.media3.exoplayer.ExoPlayer;
 import io.flutter.view.TextureRegistry;
 import org.junit.Before;
@@ -48,6 +49,7 @@ public final class VideoPlayerTest {
   @Mock private ExoPlayer mockExoPlayer;
   @Captor private ArgumentCaptor<AudioAttributes> attributesCaptor;
   @Captor private ArgumentCaptor<TextureRegistry.SurfaceProducer.Callback> callbackCaptor;
+  @Captor private ArgumentCaptor<Player.Listener> listenerCaptor;
 
   @Rule public MockitoRule initRule = MockitoJUnit.rule();
 
@@ -197,6 +199,53 @@ public void onSurfaceProducerDestroyedAndRecreatedReleasesAndThenRecreatesAndRes
     videoPlayer.dispose();
   }
 
+  @Test
+  public void onInitializedCalledWhenVideoPlayerInitiallyCreated() {
+    VideoPlayer videoPlayer = createVideoPlayer();
+
+    // Pretend we have a video, and capture the registered event listener.
+    when(mockExoPlayer.getVideoSize()).thenReturn(new VideoSize(300, 200));
+    verify(mockExoPlayer).addListener(listenerCaptor.capture());
+    Player.Listener listener = listenerCaptor.getValue();
+
+    // Trigger an event that would trigger onInitialized.
+    listener.onPlaybackStateChanged(Player.STATE_READY);
+    verify(mockEvents).onInitialized(anyInt(), anyInt(), anyLong(), anyInt());
+
+    videoPlayer.dispose();
+  }
+
+  @Test
+  public void onSurfaceCreatedDoesNotSendInitializeEventAgain() {
+    // The VideoPlayer contract assumes that the event "initialized" is sent exactly once
+    // (duplicate events cause an error to be thrown at the shared Dart layer). This test verifies
+    // that the onInitialized event is sent exactly once per player.
+    //
+    // Regression test for https://github.com/flutter/flutter/issues/154602.
+    VideoPlayer videoPlayer = createVideoPlayer();
+    when(mockExoPlayer.getVideoSize()).thenReturn(new VideoSize(300, 200));
+
+    // Capture the lifecycle events so we can simulate onSurfaceCreated/Destroyed.
+    verify(mockProducer).setCallback(callbackCaptor.capture());
+    TextureRegistry.SurfaceProducer.Callback producerLifecycle = callbackCaptor.getValue();
+
+    // Trigger destroyed/created.
+    producerLifecycle.onSurfaceDestroyed();
+    producerLifecycle.onSurfaceCreated();
+
+    // Initial listener, and the new one from the resume.
+    verify(mockExoPlayer, times(2)).addListener(listenerCaptor.capture());
+    Player.Listener listener = listenerCaptor.getValue();
+
+    // Now trigger that same event, which would happen in the case of a background/resume.
+    listener.onPlaybackStateChanged(Player.STATE_READY);
+
+    // Was not called because it was a result of a background/resume.
+    verify(mockEvents, never()).onInitialized(anyInt(), anyInt(), anyLong(), anyInt());
+
+    videoPlayer.dispose();
+  }
+
   @Test
   public void onSurfaceCreatedWithoutDestroyDoesNotRecreate() {
     // Initially create the video player, which creates the initial surface.

diff --git a/packages/video_player/video_player_android/pubspec.yaml b/packages/video_player/video_player_android/pubspec.yaml
@@ -2,7 +2,7 @@ name: video_player_android
 description: Android implementation of the video_player plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_android
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
-version: 2.7.5
+version: 2.7.6
 
 environment:
   sdk: ^3.5.0

diff --git a/packages/video_player/video_player_platform_interface/CHANGELOG.md b/packages/video_player/video_player_platform_interface/CHANGELOG.md
@@ -1,6 +1,7 @@
-## NEXT
+## 6.2.3
 
 * Updates minimum supported SDK version to Flutter 3.19/Dart 3.3.
+* Clarified that `VideoEventType.initialized` cannot be sent more than once.
 
 ## 6.2.2
 

diff --git a/...ges/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart b/...ges/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart
@@ -278,6 +278,8 @@ class VideoEvent {
 /// completed or to communicate buffering events or play state changed.
 enum VideoEventType {
   /// The video has been initialized.
+  ///
+  /// A maximum of one event of this type may be emitted per instance.
   initialized,
 
   /// The playback has ended.

diff --git a/packages/video_player/video_player_platform_interface/pubspec.yaml b/packages/video_player/video_player_platform_interface/pubspec.yaml
@@ -4,7 +4,7 @@ repository: https://github.com/flutter/packages/tree/main/packages/video_player/
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
 # NOTE: We strongly prefer non-breaking changes, even at the expense of a
 # less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
-version: 6.2.2
+version: 6.2.3
 
 environment:
   sdk: ^3.3.0