From 7e6c7660a95324c7f9d66c0c762a209089eb6324 Mon Sep 17 00:00:00 2001 From: Jelle Foks Date: Tue, 2 Apr 2024 13:28:39 -0700 Subject: [PATCH] Possibly avoid ANRs during MediaSession updates and actions. (#2808) Always post the task for the MediaSession action, and ensure the Android MediaSession is stopped when the JS MediaSession is destroyed. b/332368140 --- cobalt/media_session/media_session_client.cc | 32 ++++++++++++++----- cobalt/media_session/media_session_client.h | 13 ++------ cobalt/media_session/media_session_test.cc | 8 +++++ .../java/dev/cobalt/coat/StarboardBridge.java | 6 ++++ .../dev/cobalt/media/CobaltMediaSession.java | 11 ++++++- .../shared/android_media_session_client.cc | 8 ++--- 6 files changed, 53 insertions(+), 25 deletions(-) diff --git a/cobalt/media_session/media_session_client.cc b/cobalt/media_session/media_session_client.cc index 8c8bad0ea2cc..21c76c7b7cba 100644 --- a/cobalt/media_session/media_session_client.cc +++ b/cobalt/media_session/media_session_client.cc @@ -212,6 +212,28 @@ void MediaSessionClient::UpdatePlatformPlaybackState( PostDelayedTaskForMaybeFreezeCallback(); } +void MediaSessionClient::InvokeAction( + const CobaltExtensionMediaSessionAction& action) { + std::unique_ptr details( + new CobaltExtensionMediaSessionActionDetails()); + CobaltExtensionMediaSessionActionDetailsInit(details.get(), action); + DCHECK(media_session_->task_runner_); + media_session_->task_runner_->PostTask( + FROM_HERE, base::BindOnce(&MediaSessionClient::InvokeActionInternal, + AsWeakPtr(), base::Passed(&details))); +} + +void MediaSessionClient::InvokeAction( + const CobaltExtensionMediaSessionActionDetails& details) { + std::unique_ptr details_ptr( + new CobaltExtensionMediaSessionActionDetails(details)); + DCHECK(media_session_->task_runner_); + media_session_->task_runner_->PostTask( + FROM_HERE, base::Bind(&MediaSessionClient::InvokeActionInternal, + AsWeakPtr(), base::Passed(&details_ptr))); +} + + void MediaSessionClient::RunMaybeFreezeCallback(int sequence_number) { if (sequence_number != sequence_number_) return; @@ -233,14 +255,8 @@ void MediaSessionClient::InvokeActionInternal( details->action == kCobaltExtensionMediaSessionActionSeekto); DCHECK(!details->fast_seek || details->action == kCobaltExtensionMediaSessionActionSeekto); - - DCHECK(media_session_->task_runner_); - if (!media_session_->task_runner_->BelongsToCurrentThread()) { - media_session_->task_runner_->PostTask( - FROM_HERE, base::Bind(&MediaSessionClient::InvokeActionInternal, - AsWeakPtr(), base::Passed(&details))); - return; - } + CHECK(media_session_->task_runner_); + CHECK(media_session_->task_runner_->BelongsToCurrentThread()); MediaSession::ActionMap::iterator it = media_session_->action_map_.find( ConvertMediaSessionAction(details->action)); diff --git a/cobalt/media_session/media_session_client.h b/cobalt/media_session/media_session_client.h index 3c174982fcb3..55c734c99526 100644 --- a/cobalt/media_session/media_session_client.h +++ b/cobalt/media_session/media_session_client.h @@ -61,19 +61,10 @@ class MediaSessionClient : public base::SupportsWeakPtr { // Invokes a given media session action // https://wicg.github.io/mediasession/#actions-model // Can be invoked from any thread. - void InvokeAction(CobaltExtensionMediaSessionAction action) { - std::unique_ptr details( - new CobaltExtensionMediaSessionActionDetails()); - CobaltExtensionMediaSessionActionDetailsInit(details.get(), action); - InvokeActionInternal(std::move(details)); - } + void InvokeAction(const CobaltExtensionMediaSessionAction& action); // Invokes a given media session action that takes additional details. - void InvokeAction(CobaltExtensionMediaSessionActionDetails details) { - std::unique_ptr details_ptr( - new CobaltExtensionMediaSessionActionDetails(details)); - InvokeActionInternal(std::move(details_ptr)); - } + void InvokeAction(const CobaltExtensionMediaSessionActionDetails& details); // Invoked on the browser thread when any metadata, position state, playback // state, or supported session actions change. diff --git a/cobalt/media_session/media_session_test.cc b/cobalt/media_session/media_session_test.cc index 5cfdfee5962b..7b3e9a6b3966 100644 --- a/cobalt/media_session/media_session_test.cc +++ b/cobalt/media_session/media_session_test.cc @@ -222,6 +222,7 @@ TEST(MediaSessionTest, NullActionClears) { .to_ulong()); session->mock_session_client()->InvokeAction( kCobaltExtensionMediaSessionActionPlay); + base::RunLoop().RunUntilIdle(); session->SetActionHandler(kMediaSessionActionPlay, null_holder); session->mock_session_client()->WaitForSessionStateChange(); @@ -231,6 +232,7 @@ TEST(MediaSessionTest, NullActionClears) { .to_ulong()); session->mock_session_client()->InvokeAction( kCobaltExtensionMediaSessionActionPlay); + base::RunLoop().RunUntilIdle(); EXPECT_GE(session->mock_session_client()->GetMediaSessionChangeCount(), 3); } @@ -360,6 +362,7 @@ TEST(MediaSessionTest, InvokeAction) { &details, kCobaltExtensionMediaSessionActionSeekto); details.seek_time = 1.2; session->mock_session_client()->InvokeAction(details); + base::RunLoop().RunUntilIdle(); } TEST(MediaSessionTest, SeekDetails) { @@ -381,17 +384,20 @@ TEST(MediaSessionTest, SeekDetails) { .WillOnce(Return(CallbackResult())); session->mock_session_client()->InvokeAction( kCobaltExtensionMediaSessionActionSeekforward); + base::RunLoop().RunUntilIdle(); EXPECT_CALL(cf, Run(SeekNoOffset(kMediaSessionActionSeekbackward))) .WillOnce(Return(CallbackResult())); session->mock_session_client()->InvokeAction( kCobaltExtensionMediaSessionActionSeekbackward); + base::RunLoop().RunUntilIdle(); EXPECT_CALL(cf, Run(SeekTime(1.2))).WillOnce(Return(CallbackResult())); CobaltExtensionMediaSessionActionDetailsInit( &details, kCobaltExtensionMediaSessionActionSeekto); details.seek_time = 1.2; session->mock_session_client()->InvokeAction(details); + base::RunLoop().RunUntilIdle(); EXPECT_CALL(cf, Run(SeekOffset(kMediaSessionActionSeekforward, 3.4))) .WillOnce(Return(CallbackResult())); @@ -399,6 +405,7 @@ TEST(MediaSessionTest, SeekDetails) { &details, kCobaltExtensionMediaSessionActionSeekforward); details.seek_offset = 3.4; session->mock_session_client()->InvokeAction(details); + base::RunLoop().RunUntilIdle(); EXPECT_CALL(cf, Run(SeekOffset(kMediaSessionActionSeekbackward, 5.6))) .WillOnce(Return(CallbackResult())); @@ -406,6 +413,7 @@ TEST(MediaSessionTest, SeekDetails) { &details, kCobaltExtensionMediaSessionActionSeekbackward); details.seek_offset = 5.6; session->mock_session_client()->InvokeAction(details); + base::RunLoop().RunUntilIdle(); session->mock_session_client()->WaitForSessionStateChange(); EXPECT_GE(session->mock_session_client()->GetMediaSessionChangeCount(), 0); diff --git a/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java b/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java index 2fc034304525..d730b15d52a7 100644 --- a/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java +++ b/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java @@ -647,6 +647,12 @@ void updateMediaSession( playbackState, actions, positionMs, speed, title, artist, album, artwork, duration); } + @SuppressWarnings("unused") + @UsedByNative + public void deactivateMediaSession() { + cobaltMediaSession.deactivateMediaSession(); + } + /** Returns string for kSbSystemPropertyUserAgentAuxField */ @SuppressWarnings("unused") @UsedByNative diff --git a/starboard/android/apk/app/src/main/java/dev/cobalt/media/CobaltMediaSession.java b/starboard/android/apk/app/src/main/java/dev/cobalt/media/CobaltMediaSession.java index 0b28e088079f..2dc78d99f665 100644 --- a/starboard/android/apk/app/src/main/java/dev/cobalt/media/CobaltMediaSession.java +++ b/starboard/android/apk/app/src/main/java/dev/cobalt/media/CobaltMediaSession.java @@ -261,7 +261,16 @@ private void configureMediaFocus(int playbackState, long positionMs, float speed } if (deactivating) { // Suspending lands here. - Log.i(TAG, "MediaSession release"); + deactivateMediaSession(); + } + } + + public void deactivateMediaSession() { + Log.i(TAG, "MediaSession release"); + if (mediaSession != null) { + if (mediaSession.isActive()) { + mediaSession.setActive(false); + } mediaSession.release(); mediaSession = null; } diff --git a/starboard/android/shared/android_media_session_client.cc b/starboard/android/shared/android_media_session_client.cc index ddea11679f16..ce5feb3abab8 100644 --- a/starboard/android/shared/android_media_session_client.cc +++ b/starboard/android/shared/android_media_session_client.cc @@ -172,11 +172,6 @@ void OnMediaSessionStateChanged( jint playback_state = CobaltExtensionPlaybackStateToPlaybackState( session_state.actual_playback_state); - SbOnce(&once_flag, OnceInit); - SbMutexAcquire(&mutex); - - SbMutexRelease(&mutex); - jlong playback_state_actions = MediaSessionActionsToPlaybackStateActions( session_state.available_actions); @@ -271,6 +266,9 @@ void DestroyMediaSessionClientCallback() { g_update_platform_playback_state_callback = NULL; SbMutexRelease(&mutex); + + JniEnvExt* env = JniEnvExt::Get(); + env->CallStarboardVoidMethodOrAbort("deactivateMediaSession", "()V"); } } // namespace