Merge #4333

4333: Issue #4244: Add id of tab to intent launched from media notification. r=Amejia481 a=pocmo Co-authored-by: Sebastian Kaspari <s.kaspari@gmail.com>
mozilla-mobile · Sep 9, 2019 · 77fd919 · 77fd919
2 parents 5a03921 + 7093ef0
commit 77fd919
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 16 deletions.
diff --git a/components/feature/media/src/main/java/mozilla/components/feature/media/MediaFeature.kt b/components/feature/media/src/main/java/mozilla/components/feature/media/MediaFeature.kt
@@ -65,4 +65,9 @@ class MediaFeature(
             }
         }
     }
+
+    companion object {
+        const val ACTION_SWITCH_TAB = "mozac.feature.media.SWITCH_TAB"
+        const val EXTRA_TAB_ID = "mozac.feature.media.TAB_ID"
+    }
 }
diff --git a/...re/media/src/main/java/mozilla/components/feature/media/notification/MediaNotification.kt b/...re/media/src/main/java/mozilla/components/feature/media/notification/MediaNotification.kt
@@ -12,6 +12,7 @@ import android.support.v4.media.session.MediaSessionCompat
 import androidx.annotation.DrawableRes
 import androidx.core.app.NotificationCompat
 import mozilla.components.browser.session.Session
+import mozilla.components.feature.media.MediaFeature
 import mozilla.components.feature.media.R
 import mozilla.components.feature.media.service.MediaService
 import mozilla.components.feature.media.state.MediaState
@@ -29,9 +30,6 @@ internal class MediaNotification(
     fun create(state: MediaState, mediaSession: MediaSessionCompat): Notification {
         val channel = MediaNotificationChannel.ensureChannelExists(context)
 
-        val intent = context.packageManager.getLaunchIntentForPackage(context.packageName)
-        val pendingIntent = PendingIntent.getActivity(context, 0, intent, 0)
-
         val data = state.toNotificationData(context)
 
         val builder = NotificationCompat.Builder(context, channel)
@@ -57,14 +55,18 @@ internal class MediaNotification(
             // We only set a content intent if this media notification is not for an "external app"
             // like a custom tab. Currently we can't route the user to that particular activity:
             // https://github.com/mozilla-mobile/android-components/issues/3986
-            builder.setContentIntent(pendingIntent)
+            builder.setContentIntent(data.contentIntent)
         }
 
         return builder.build()
     }
 }
 
 private fun MediaState.toNotificationData(context: Context): NotificationData {
+    val intent = context.packageManager.getLaunchIntentForPackage(context.packageName)?.also {
+        it.action = MediaFeature.ACTION_SWITCH_TAB
+    }
+
     return when (this) {
         is MediaState.Playing -> NotificationData(
             title = session.getTitleOrUrl(context),
@@ -79,7 +81,10 @@ private fun MediaState.toNotificationData(context: Context): NotificationData {
                     0,
                     MediaService.pauseIntent(context),
                     0)
-            ).build()
+            ).build(),
+            contentIntent = PendingIntent.getActivity(context, 0, intent?.apply {
+                putExtra(MediaFeature.EXTRA_TAB_ID, session.id)
+            }, 0)
         )
         is MediaState.Paused -> NotificationData(
             title = session.getTitleOrUrl(context),
@@ -94,7 +99,10 @@ private fun MediaState.toNotificationData(context: Context): NotificationData {
                     0,
                     MediaService.playIntent(context),
                     0)
-            ).build()
+            ).build(),
+            contentIntent = PendingIntent.getActivity(context, 0, intent?.apply {
+                putExtra(MediaFeature.EXTRA_TAB_ID, session.id)
+            }, 0)
         )
         // Dummy notification that is only used to satisfy the requirement to ALWAYS call
         // startForeground with a notification.
@@ -119,7 +127,8 @@ private data class NotificationData(
     val description: String = "",
     @DrawableRes val icon: Int = R.drawable.mozac_feature_media_playing,
     val largeIcon: Bitmap? = null,
-    val action: NotificationCompat.Action? = null
+    val action: NotificationCompat.Action? = null,
+    val contentIntent: PendingIntent? = null
 )
 
 private fun MediaState.isForExternalApp(): Boolean {

diff --git a/...edia/src/test/java/mozilla/components/feature/media/notification/MediaNotificationTest.kt b/...edia/src/test/java/mozilla/components/feature/media/notification/MediaNotificationTest.kt
@@ -5,57 +5,90 @@
 package mozilla.components.feature.media.notification
 
 import android.app.Notification
+import android.content.Context
+import android.content.Intent
+import android.content.pm.PackageManager
 import androidx.core.app.NotificationCompat
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.browser.session.Session
 import mozilla.components.concept.engine.media.Media
+import mozilla.components.feature.media.MediaFeature
 import mozilla.components.feature.media.MockMedia
 import mozilla.components.feature.media.R
 import mozilla.components.feature.media.state.MediaState
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers
+import org.mockito.Mockito.doReturn
+import org.mockito.Mockito.spy
+import org.robolectric.Shadows.shadowOf
 
 @RunWith(AndroidJUnit4::class)
 class MediaNotificationTest {
+    private lateinit var context: Context
+
+    @Before
+    fun setUp() {
+        context = spy(testContext).also {
+            val packageManager: PackageManager = mock()
+            doReturn(Intent()).`when`(packageManager).getLaunchIntentForPackage(ArgumentMatchers.anyString())
+            doReturn(packageManager).`when`(it).packageManager
+        }
+    }
+
     @Test
     fun `media notification for playing state`() {
         val state = MediaState.Playing(
-            Session("https://www.mozilla.org").apply {
+            Session("https://www.mozilla.org", id = "test-tab").apply {
                 title = "Mozilla"
             },
             listOf(
                 MockMedia(Media.PlaybackState.PLAYING)
             ))
 
-        val notification = MediaNotification(testContext)
+        val notification = MediaNotification(context)
             .create(state, mock())
 
         assertEquals("https://www.mozilla.org", notification.text)
         assertEquals("Mozilla", notification.title)
 
         assertEquals(R.drawable.mozac_feature_media_playing, notification.iconResource)
+
+        shadowOf(notification.contentIntent).savedIntent!!.also { intent ->
+            assertEquals(MediaFeature.ACTION_SWITCH_TAB, intent.action)
+            assertTrue(intent.extras!!.containsKey(MediaFeature.EXTRA_TAB_ID))
+            assertEquals("test-tab", intent.getStringExtra(MediaFeature.EXTRA_TAB_ID))
+        }
     }
 
     @Test
     fun `media notification for paused state`() {
         val state = MediaState.Paused(
-            Session("https://www.mozilla.org").apply {
+            Session("https://www.mozilla.org", id = "test-tab").apply {
                 title = "Mozilla"
             },
             listOf(
                 MockMedia(Media.PlaybackState.PAUSE)
             ))
 
-        val notification = MediaNotification(testContext)
+        val notification = MediaNotification(context)
             .create(state, mock())
 
         assertEquals("https://www.mozilla.org", notification.text)
         assertEquals("Mozilla", notification.title)
 
         assertEquals(R.drawable.mozac_feature_media_paused, notification.iconResource)
+
+        shadowOf(notification.contentIntent).savedIntent!!.also { intent ->
+            assertEquals(MediaFeature.ACTION_SWITCH_TAB, intent.action)
+            assertTrue(intent.extras!!.containsKey(MediaFeature.EXTRA_TAB_ID))
+            assertEquals("test-tab", intent.getStringExtra(MediaFeature.EXTRA_TAB_ID))
+        }
     }
 
     fun `media notification for none state`() {
@@ -64,10 +97,10 @@ class MediaNotificationTest {
 
         val state = MediaState.None
 
-        MediaNotification(testContext)
+        MediaNotification(context)
             .create(state, mock())
 
-        val notification = MediaNotification(testContext)
+        val notification = MediaNotification(context)
             .create(state, mock())
 
         assertEquals("", notification.text)
@@ -82,7 +115,7 @@ class MediaNotificationTest {
                 MockMedia(Media.PlaybackState.PLAYING)
             ))
 
-        val notification = MediaNotification(testContext)
+        val notification = MediaNotification(context)
             .create(state, mock())
 
         assertEquals("https://www.mozilla.org", notification.text)
@@ -101,7 +134,7 @@ class MediaNotificationTest {
                     MockMedia(Media.PlaybackState.PLAYING)
                 ))
 
-        val notification = MediaNotification(testContext)
+        val notification = MediaNotification(context)
             .create(state, mock())
 
         assertEquals("", notification.text)
@@ -120,7 +153,7 @@ class MediaNotificationTest {
                         MockMedia(Media.PlaybackState.PAUSE)
                 ))
 
-        val notification = MediaNotification(testContext)
+        val notification = MediaNotification(context)
                 .create(state, mock())
 
         assertEquals("", notification.text)

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -109,6 +109,9 @@ permalink: /changelog/
 * **browser-session**, **feature-intent**
   * ⚠️ **This is a breaking change**: Moved `Intent` related code from `browser-session` to `feature-intent`.
 
+* **feature-media**
+  * The `Intent` launched from the media notification now has its action set to `MediaFeature.ACTION_SWITCH_TAB`. In addition to that the extra `MediaFeature.EXTRA_TAB_ID` contains the id of the tab the media notification is displayed for.
+
 # 10.0.1
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v10.0.0...v10.0.1)