From 8b84d3002be280d63587e95f7b32009ddb4cac7d Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Oct 2024 11:05:34 -0700 Subject: [PATCH 1/5] Fix order of calling release. --- .../video_player/video_player_android/CHANGELOG.md | 6 ++++++ .../flutter/plugins/videoplayer/VideoPlayer.java | 6 +++++- .../plugins/videoplayer/VideoPlayerTest.java | 14 +++++++++++--- .../video_player/video_player_android/pubspec.yaml | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/video_player/video_player_android/CHANGELOG.md b/packages/video_player/video_player_android/CHANGELOG.md index 053247bf09f..51babe13a6f 100644 --- a/packages/video_player/video_player_android/CHANGELOG.md +++ b/packages/video_player/video_player_android/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.7.10 + +* Fixes a [bug](https://github.com/flutter/flutter/issues/156158) where + disposing a video player (including implicitly by switching tabs or views + in a running app) would cause native stack traces. + ## 2.7.9 * Updates Java compatibility version to 11. diff --git a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index c8afd2ebc72..7d8920f8048 100644 --- a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -17,6 +17,8 @@ import androidx.media3.common.MediaItem; import androidx.media3.common.PlaybackParameters; import androidx.media3.exoplayer.ExoPlayer; + +import io.flutter.Log; import io.flutter.view.TextureRegistry; final class VideoPlayer implements TextureRegistry.SurfaceProducer.Callback { @@ -97,6 +99,7 @@ public void onSurfaceCreated() { @RestrictTo(RestrictTo.Scope.LIBRARY) public void onSurfaceDestroyed() { + Log.e("=================================VideoPlayer", "onSurfaceDestroyed()"); exoPlayer.stop(); savedStateDuring = ExoPlayerState.save(exoPlayer); exoPlayer.release(); @@ -160,7 +163,8 @@ long getPosition() { } void dispose() { - surfaceProducer.release(); + Log.e("=================================VideoPlayer", "dispose()"); exoPlayer.release(); + surfaceProducer.release(); } } diff --git a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java index b5dde43b9e0..9300840d2a9 100644 --- a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java +++ b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java @@ -5,6 +5,7 @@ package io.flutter.plugins.videoplayer; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -22,11 +23,16 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; import org.robolectric.RobolectricTestRunner; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + /** * Unit tests for {@link VideoPlayer}. * @@ -264,12 +270,14 @@ public void onSurfaceCreatedWithoutDestroyDoesNotRecreate() { } @Test - public void disposeReleasesTextureAndPlayer() { + public void disposeReleasesExoPlayerBeforeTexture() { VideoPlayer videoPlayer = createVideoPlayer(); + videoPlayer.dispose(); - verify(mockProducer).release(); - verify(mockExoPlayer).release(); + InOrder inOrder = inOrder(mockExoPlayer, mockProducer); + inOrder.verify(mockExoPlayer).release(); + inOrder.verify(mockProducer).release(); } // TODO(matanlurey): Replace with inline calls to onSurfaceAvailable once diff --git a/packages/video_player/video_player_android/pubspec.yaml b/packages/video_player/video_player_android/pubspec.yaml index 46c7db289db..26367b06fc3 100644 --- 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.9 +version: 2.7.10 environment: sdk: ^3.5.0 From f71f502884befbf0df9341ff37bfaa9c2d8ee80c Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Oct 2024 11:20:49 -0700 Subject: [PATCH 2/5] Update test. --- .../java/io/flutter/plugins/videoplayer/VideoPlayerTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java index 9300840d2a9..643b28454f5 100644 --- a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java +++ b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java @@ -275,6 +275,8 @@ public void disposeReleasesExoPlayerBeforeTexture() { videoPlayer.dispose(); + // Regression test for https://github.com/flutter/flutter/issues/156158. + // The player must be destroyed before the surface it is writing to. InOrder inOrder = inOrder(mockExoPlayer, mockProducer); inOrder.verify(mockExoPlayer).release(); inOrder.verify(mockProducer).release(); From 9da2cfe31117675eaebefbc3ab5116401b5cf172 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Oct 2024 11:21:47 -0700 Subject: [PATCH 3/5] Remove logs. --- .../main/java/io/flutter/plugins/videoplayer/VideoPlayer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index 7d8920f8048..144ce70df05 100644 --- a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -99,7 +99,6 @@ public void onSurfaceCreated() { @RestrictTo(RestrictTo.Scope.LIBRARY) public void onSurfaceDestroyed() { - Log.e("=================================VideoPlayer", "onSurfaceDestroyed()"); exoPlayer.stop(); savedStateDuring = ExoPlayerState.save(exoPlayer); exoPlayer.release(); @@ -163,7 +162,6 @@ long getPosition() { } void dispose() { - Log.e("=================================VideoPlayer", "dispose()"); exoPlayer.release(); surfaceProducer.release(); } From 61b3ef403f4d933ae8a04cd454da427aec1da7b9 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Oct 2024 12:26:57 -0700 Subject: [PATCH 4/5] Format. --- .../main/java/io/flutter/plugins/videoplayer/VideoPlayer.java | 2 -- .../java/io/flutter/plugins/videoplayer/VideoPlayerTest.java | 4 ---- 2 files changed, 6 deletions(-) diff --git a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index 144ce70df05..12f7c9264f3 100644 --- a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -17,8 +17,6 @@ import androidx.media3.common.MediaItem; import androidx.media3.common.PlaybackParameters; import androidx.media3.exoplayer.ExoPlayer; - -import io.flutter.Log; import io.flutter.view.TextureRegistry; final class VideoPlayer implements TextureRegistry.SurfaceProducer.Callback { diff --git a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java index 643b28454f5..7edf0e89e8f 100644 --- a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java +++ b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java @@ -5,7 +5,6 @@ package io.flutter.plugins.videoplayer; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -29,9 +28,6 @@ import org.mockito.junit.MockitoRule; import org.robolectric.RobolectricTestRunner; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; /** * Unit tests for {@link VideoPlayer}. From 4fa424abd92151c5db66b294433bc31cea74565a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Oct 2024 12:43:05 -0700 Subject: [PATCH 5/5] ++ --- .../java/io/flutter/plugins/videoplayer/VideoPlayerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java index 7edf0e89e8f..b741ab2f269 100644 --- a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java +++ b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java @@ -28,7 +28,6 @@ import org.mockito.junit.MockitoRule; import org.robolectric.RobolectricTestRunner; - /** * Unit tests for {@link VideoPlayer}. *