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
youtube · Apr 2, 2024 · 7e6c766 · 7e6c766
1 parent 30dacbc
commit 7e6c766
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 25 deletions.
diff --git 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<CobaltExtensionMediaSessionActionDetails> 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<CobaltExtensionMediaSessionActionDetails> 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
@@ -61,19 +61,10 @@ class MediaSessionClient : public base::SupportsWeakPtr<MediaSessionClient> {
   // 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<CobaltExtensionMediaSessionActionDetails> 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<CobaltExtensionMediaSessionActionDetails> 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
@@ -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,31 +384,36 @@ TEST(MediaSessionTest, SeekDetails) {
       .WillOnce(Return(CallbackResult<void>()));
   session->mock_session_client()->InvokeAction(
       kCobaltExtensionMediaSessionActionSeekforward);
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(cf, Run(SeekNoOffset(kMediaSessionActionSeekbackward)))
       .WillOnce(Return(CallbackResult<void>()));
   session->mock_session_client()->InvokeAction(
       kCobaltExtensionMediaSessionActionSeekbackward);
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(cf, Run(SeekTime(1.2))).WillOnce(Return(CallbackResult<void>()));
   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<void>()));
   CobaltExtensionMediaSessionActionDetailsInit(
       &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<void>()));
   CobaltExtensionMediaSessionActionDetailsInit(
       &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
@@ -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
@@ -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
@@ -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