Closes mozilla-mobile#9243: Shutdown correctly when Media Session Ser…

…vice is killed
rocketsroger · Dec 17, 2020 · f3fa133 · f3fa133
1 parent 3320a79
commit f3fa133
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 10 deletions.
diff --git a/...nents/feature/media/src/main/java/mozilla/components/feature/media/MediaSessionFeature.kt b/...nents/feature/media/src/main/java/mozilla/components/feature/media/MediaSessionFeature.kt
@@ -40,7 +40,7 @@ class MediaSessionFeature(
             }.map { tab ->
                 tab.none {
                     it.mediaSessionState != null &&
-                        it.mediaSessionState!!.playbackState != MediaSession.PlaybackState.UNKNOWN
+                        it.mediaSessionState!!.playbackState == MediaSession.PlaybackState.PLAYING
                 }
             }.ifChanged().collect { isEmpty ->
                 process(isEmpty)

diff --git a/...dia/src/main/java/mozilla/components/feature/media/service/MediaSessionServiceDelegate.kt b/...dia/src/main/java/mozilla/components/feature/media/service/MediaSessionServiceDelegate.kt
@@ -9,6 +9,7 @@ import android.content.Intent
 import android.media.AudioManager
 import android.support.v4.media.MediaMetadataCompat
 import android.support.v4.media.session.MediaSessionCompat
+import androidx.annotation.VisibleForTesting
 import androidx.core.app.NotificationManagerCompat
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.cancel
@@ -65,8 +66,7 @@ internal class MediaSessionServiceDelegate(
     }
 
     fun onDestroy() {
-        scope?.cancel()
-
+        destroy()
         logger.debug("Service destroyed")
     }
 
@@ -163,7 +163,14 @@ internal class MediaSessionServiceDelegate(
         }
     }
 
-    private fun shutdown() {
+    @VisibleForTesting
+    internal fun destroy() {
+        scope?.cancel()
+        audioFocus.abandon()
+    }
+
+    @VisibleForTesting
+    internal fun shutdown() {
         audioFocus.abandon()
         mediaSession.release()
         service.stopSelf()

diff --git a/...s/feature/media/src/test/java/mozilla/components/feature/media/MediaSessionFeatureTest.kt b/...s/feature/media/src/test/java/mozilla/components/feature/media/MediaSessionFeatureTest.kt
@@ -108,7 +108,7 @@ class MediaSessionFeatureTest {
     }
 
     @Test
-    fun `feature only starts foreground service when there were no previous media session`() {
+    fun `feature only starts foreground service when there were no previous playing media session`() {
         val mockApplicationContext: Context = mock()
         val initialState = BrowserState(
             tabs = listOf(createTab(
@@ -156,6 +156,12 @@ class MediaSessionFeatureTest {
         dispatcher.advanceUntilIdle()
         verify(mockApplicationContext, times(1)).startForegroundService(any())
 
+        store.dispatch(MediaSessionAction.UpdateMediaPlaybackStateAction(store.state.tabs[0].id,
+            MediaSession.PlaybackState.PAUSED))
+        store.waitUntilIdle()
+        dispatcher.advanceUntilIdle()
+        verify(mockApplicationContext, times(1)).startForegroundService(any())
+
         store.dispatch(MediaSessionAction.UpdateMediaPlaybackStateAction(store.state.tabs[0].id,
             MediaSession.PlaybackState.PLAYING))
         store.waitUntilIdle()

diff --git a/...src/test/java/mozilla/components/feature/media/service/MediaSessionServiceDelegateTest.kt b/...src/test/java/mozilla/components/feature/media/service/MediaSessionServiceDelegateTest.kt
@@ -21,6 +21,7 @@ import mozilla.components.support.test.robolectric.testContext import mozilla.co
 import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers
 import org.mockito.Mockito.never
+import org.mockito.Mockito.spy
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 
@@ -57,7 +58,7 @@ class MediaSessionServiceDelegateTest {
         )
         val store = BrowserStore(initialState)
         val service: AbstractMediaSessionService = mock()
-        val delegate = MediaSessionServiceDelegate(testContext, service, store)
+        val delegate = spy(MediaSessionServiceDelegate(testContext, service, store))
 
         delegate.onCreate()
 
@@ -69,6 +70,7 @@ class MediaSessionServiceDelegateTest {
         store.waitUntilIdle()
 
         verify(service).stopSelf()
+        verify(delegate).shutdown()
 
         delegate.onDestroy()
     }
@@ -83,19 +85,21 @@ class MediaSessionServiceDelegateTest {
         )
         val store = BrowserStore(initialState)
         val service: AbstractMediaSessionService = mock()
-        val delegate = MediaSessionServiceDelegate(testContext, service, store)
+        val delegate = spy(MediaSessionServiceDelegate(testContext, service, store))
 
         delegate.onCreate()
 
         verify(service).startForeground(ArgumentMatchers.anyInt(), any())
         verify(service, never()).stopSelf()
+        verify(delegate, never()).shutdown()
 
         store.dispatch(MediaSessionAction.DeactivatedMediaSessionAction(store.state.customTabs[0].id))
 
         store.waitUntilIdle()
         dispatcher.advanceUntilIdle()
 
         verify(service).stopSelf()
+        verify(delegate).shutdown()
 
         delegate.onDestroy()
     }
@@ -114,12 +118,13 @@ class MediaSessionServiceDelegateTest {
         )
         val store = BrowserStore(initialState)
         val service: AbstractMediaSessionService = mock()
-        val delegate = MediaSessionServiceDelegate(testContext, service, store)
+        val delegate = spy(MediaSessionServiceDelegate(testContext, service, store))
 
         delegate.onCreate()
 
         verify(service).startForeground(ArgumentMatchers.anyInt(), any())
         verify(service, never()).stopSelf()
+        verify(delegate, never()).shutdown()
         verify(controller, never()).pause()
 
         delegate.onStartCommand(AbstractMediaSessionService.pauseIntent(
@@ -144,12 +149,13 @@ class MediaSessionServiceDelegateTest {
         )
         val store = BrowserStore(initialState)
         val service: AbstractMediaSessionService = mock()
-        val delegate = MediaSessionServiceDelegate(testContext, service, store)
+        val delegate = spy(MediaSessionServiceDelegate(testContext, service, store))
 
         delegate.onCreate()
 
         verify(service).startForeground(ArgumentMatchers.anyInt(), any())
         verify(service, never()).stopSelf()
+        verify(delegate, never()).shutdown()
         verify(controller, never()).pause()
 
         delegate.onStartCommand(AbstractMediaSessionService.playIntent(
@@ -222,13 +228,14 @@ class MediaSessionServiceDelegateTest {
         )
         val store = BrowserStore(initialState)
         val service: AbstractMediaSessionService = mock()
-        val delegate = MediaSessionServiceDelegate(testContext, service, store)
+        val delegate = spy(MediaSessionServiceDelegate(testContext, service, store))
         val mediaSessionCallback = MediaSessionCallback(store)
 
         delegate.onCreate()
 
         verify(service).startForeground(ArgumentMatchers.anyInt(), any())
         verify(service, never()).stopSelf()
+        verify(delegate, never()).shutdown()
         verify(controller, never()).pause()
         verify(controller, never()).play()
 
@@ -238,4 +245,27 @@ class MediaSessionServiceDelegateTest {
         mediaSessionCallback.onPlay()
         verify(controller).play()
     }
+
+    @Test
+    fun `destroying service will stop all playback sessions`() {
+        val controller: MediaSession.Controller = mock()
+        val initialState = BrowserState(
+            tabs = listOf(createTab(
+                "https://www.mozilla.org",
+                mediaSessionState = MediaSessionState(
+                    controller,
+                    playbackState = MediaSession.PlaybackState.PLAYING
+                )
+            ))
+        )
+        val store = BrowserStore(initialState)
+        val service: AbstractMediaSessionService = mock()
+        val delegate = spy(MediaSessionServiceDelegate(testContext, service, store))
+
+        delegate.onCreate()
+
+        delegate.onDestroy()
+        verify(service, never()).stopSelf()
+        verify(delegate).destroy()
+    }
 }