Closes mozilla-mobile#9531: Return content icon if get artwork fails

rocketsroger · mergify[bot] · commit df1cf70e1453 · 2021-01-28T16:06:09.000Z
diff --git a/components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/mediasession/GeckoMediaSessionDelegate.kt b/components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/mediasession/GeckoMediaSessionDelegate.kt
@@ -5,12 +5,16 @@
 package mozilla.components.browser.engine.gecko.mediasession
 
 import android.graphics.Bitmap
+import kotlinx.coroutines.withTimeoutOrNull
 import mozilla.components.browser.engine.gecko.GeckoEngineSession
 import mozilla.components.browser.engine.gecko.await
 import mozilla.components.concept.engine.mediasession.MediaSession
 import org.mozilla.geckoview.GeckoSession
 import org.mozilla.geckoview.MediaSession as GeckoViewMediaSession
 
+private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L
+private const val ARTWORK_IMAGE_SIZE = 48
+
 internal class GeckoMediaSessionDelegate(
     private val engineSession: GeckoEngineSession
 ) : GeckoViewMediaSession.Delegate {
@@ -32,8 +36,16 @@ internal class GeckoMediaSessionDelegate(
         mediaSession: GeckoViewMediaSession,
         metaData: GeckoViewMediaSession.Metadata
     ) {
-        val getArtwork: (suspend (Int) -> Bitmap?)? = metaData.artwork?.let {
-            { size -> it.getBitmap(size).await() }
+        val getArtwork: (suspend () -> Bitmap?)? = metaData.artwork?.let {
+            {
+                try {
+                    withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) {
+                        it.getBitmap(ARTWORK_IMAGE_SIZE).await()
+                    }
+                } catch (e: IllegalArgumentException) {
+                    null
+                }
+            }
         }
 
         engineSession.notifyObservers {
diff --git a/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/mediasession/GeckoMediaSessionDelegate.kt b/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/mediasession/GeckoMediaSessionDelegate.kt
@@ -5,12 +5,16 @@
 package mozilla.components.browser.engine.gecko.mediasession
 
 import android.graphics.Bitmap
+import kotlinx.coroutines.withTimeoutOrNull
 import mozilla.components.browser.engine.gecko.GeckoEngineSession
 import mozilla.components.browser.engine.gecko.await
 import mozilla.components.concept.engine.mediasession.MediaSession
 import org.mozilla.geckoview.GeckoSession
 import org.mozilla.geckoview.MediaSession as GeckoViewMediaSession
 
+private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L
+private const val ARTWORK_IMAGE_SIZE = 48
+
 internal class GeckoMediaSessionDelegate(
     private val engineSession: GeckoEngineSession
 ) : GeckoViewMediaSession.Delegate {
@@ -32,8 +36,16 @@ internal class GeckoMediaSessionDelegate(
         mediaSession: GeckoViewMediaSession,
         metaData: GeckoViewMediaSession.Metadata
     ) {
-        val getArtwork: (suspend (Int) -> Bitmap?)? = metaData.artwork?.let {
-            { size -> it.getBitmap(size).await() }
+        val getArtwork: (suspend () -> Bitmap?)? = metaData.artwork?.let {
+            {
+                try {
+                    withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) {
+                        it.getBitmap(ARTWORK_IMAGE_SIZE).await()
+                    }
+                } catch (e: IllegalArgumentException) {
+                    null
+                }
+            }
         }
 
         engineSession.notifyObservers {
diff --git a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/mediasession/GeckoMediaSessionDelegate.kt b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/mediasession/GeckoMediaSessionDelegate.kt
@@ -5,12 +5,16 @@
 package mozilla.components.browser.engine.gecko.mediasession
 
 import android.graphics.Bitmap
+import kotlinx.coroutines.withTimeoutOrNull
 import mozilla.components.browser.engine.gecko.GeckoEngineSession
 import mozilla.components.browser.engine.gecko.await
 import mozilla.components.concept.engine.mediasession.MediaSession
 import org.mozilla.geckoview.GeckoSession
 import org.mozilla.geckoview.MediaSession as GeckoViewMediaSession
 
+private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L
+private const val ARTWORK_IMAGE_SIZE = 48
+
 internal class GeckoMediaSessionDelegate(
     private val engineSession: GeckoEngineSession
 ) : GeckoViewMediaSession.Delegate {
@@ -32,8 +36,16 @@ internal class GeckoMediaSessionDelegate(
         mediaSession: GeckoViewMediaSession,
         metaData: GeckoViewMediaSession.Metadata
     ) {
-        val getArtwork: (suspend (Int) -> Bitmap?)? = metaData.artwork?.let {
-            { size -> it.getBitmap(size).await() }
+        val getArtwork: (suspend () -> Bitmap?)? = metaData.artwork?.let {
+            {
+                try {
+                    withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) {
+                        it.getBitmap(ARTWORK_IMAGE_SIZE).await()
+                    }
+                } catch (e: IllegalArgumentException) {
+                    null
+                }
+            }
         }
 
         engineSession.notifyObservers {
diff --git a/components/concept/engine/src/main/java/mozilla/components/concept/engine/mediasession/MediaSession.kt b/components/concept/engine/src/main/java/mozilla/components/concept/engine/mediasession/MediaSession.kt
@@ -45,7 +45,7 @@ class MediaSession {
         val title: String? = null,
         val artist: String? = null,
         val album: String? = null,
-        val getArtwork: (suspend (Int) -> Bitmap?)?
+        val getArtwork: (suspend () -> Bitmap?)?
     )
 
     /**
diff --git a/components/feature/media/src/main/java/mozilla/components/feature/media/ext/SessionState.kt b/components/feature/media/src/main/java/mozilla/components/feature/media/ext/SessionState.kt
@@ -6,15 +6,11 @@ package mozilla.components.feature.media.ext
 
 import android.content.Context
 import android.graphics.Bitmap
-import kotlinx.coroutines.withTimeoutOrNull
 import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.SessionState
 import mozilla.components.concept.engine.mediasession.MediaSession
 import mozilla.components.feature.media.R
 
-private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L
-private const val ARTWORK_IMAGE_SIZE = 48
-
 internal fun SessionState?.getTitleOrUrl(context: Context, title: String? = null): String = when {
     this == null -> context.getString(R.string.mozac_feature_media_notification_private_mode)
     content.private -> context.getString(R.string.mozac_feature_media_notification_private_mode)
@@ -23,13 +19,13 @@ internal fun SessionState?.getTitleOrUrl(context: Context, title: String? = null
     else -> content.url
 }
 
-internal suspend fun SessionState?.getNonPrivateIcon(getArtwork: (suspend (Int) -> Bitmap?)?): Bitmap? = when {
+@Suppress("TooGenericExceptionCaught")
+internal suspend fun SessionState?.getNonPrivateIcon(
+    getArtwork: (suspend () -> Bitmap?)?
+): Bitmap? = when {
     this == null -> null
     content.private -> null
-    getArtwork != null ->
-        withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) {
-            getArtwork(ARTWORK_IMAGE_SIZE)
-        }
+    getArtwork != null -> getArtwork() ?: content.icon
     else -> content.icon
 }
 
diff --git a/components/feature/media/src/test/java/mozilla/components/feature/media/ext/SessionStateKtTest.kt b/components/feature/media/src/test/java/mozilla/components/feature/media/ext/SessionStateKtTest.kt
@@ -15,7 +15,8 @@ import org.junit.runner.RunWith
 @RunWith(AndroidJUnit4::class)
 class SessionStateKtTest {
     private val bitmap: Bitmap = mock()
-    private val getArtwork: (suspend (Int) -> Bitmap?) = { bitmap }
+    private val getArtwork: (suspend () -> Bitmap?) = { bitmap }
+    private val getArtworkNull: (suspend () -> Bitmap?) = { null }
 
     @Test
     fun `getNonPrivateIcon returns null when in private mode`() {
@@ -67,6 +68,24 @@ class SessionStateKtTest {
         assertEquals(result, icon)
     }
 
+    @Test
+    fun `getNonPrivateIcon returns content icon when getArtwork return null`() {
+        val sessionState: SessionState = mock()
+        val contentState: ContentState = mock()
+        val icon: Bitmap = mock()
+
+        whenever(sessionState.content).thenReturn(contentState)
+        whenever(contentState.private).thenReturn(false)
+        whenever(contentState.icon).thenReturn(icon)
+
+        var result: Bitmap?
+        runBlocking {
+            result = sessionState.getNonPrivateIcon(getArtworkNull)
+        }
+
+        assertEquals(result, icon)
+    }
+
     @Test
     fun `getTitleOrUrl returns null when in private mode`() {
         val sessionState: SessionState = mock()

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ class MediaSession {`
`45`	`45`	`val title: String? = null,`
`46`	`46`	`val artist: String? = null,`
`47`	`47`	`val album: String? = null,`
`48`		`- val getArtwork: (suspend (Int) -> Bitmap?)?`
	`48`	`+ val getArtwork: (suspend () -> Bitmap?)?`
`49`	`49`	`)`
`50`	`50`
`51`	`51`	`/**`