From 9483b4a56c02d491ada4e8349aad69525867625f Mon Sep 17 00:00:00 2001 From: MickeyMoz Date: Tue, 9 Mar 2021 22:35:52 +0000 Subject: [PATCH 1/6] Update GeckoView (Beta) to 87.0.20210309185944. --- buildSrc/src/main/java/Gecko.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/Gecko.kt b/buildSrc/src/main/java/Gecko.kt index 8ff6dfe57cb..84102bb2e93 100644 --- a/buildSrc/src/main/java/Gecko.kt +++ b/buildSrc/src/main/java/Gecko.kt @@ -11,7 +11,7 @@ internal object GeckoVersions { /** * GeckoView Beta Version. */ - const val beta_version = "87.0.20210307185839" + const val beta_version = "87.0.20210309185944" /** * GeckoView Release Version. From 7e3b6102a524f4c84c4c514c4570d808a9feaf47 Mon Sep 17 00:00:00 2001 From: mcarare Date: Wed, 10 Mar 2021 12:07:44 +0200 Subject: [PATCH 2/6] For #9614: Integrate new onTouchEventForDetailResult. --- .../components/browser/engine/gecko/NestedGeckoView.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/NestedGeckoView.kt b/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/NestedGeckoView.kt index 2b92a4e647b..5c7d966fa7b 100644 --- a/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/NestedGeckoView.kt +++ b/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/NestedGeckoView.kt @@ -119,13 +119,11 @@ open class NestedGeckoView(context: Context) : GeckoView(context), NestedScrolli @VisibleForTesting internal fun updateInputResult(event: MotionEvent) { - @Suppress("Deprecation") - // Deprecation to be replaced in https://github.com/mozilla-mobile/android-components/issues/9614 - super.onTouchEventForResult(event) + super.onTouchEventForDetailResult(event) .accept { // This should never be null. // Prefer to crash and investigate after rather than not knowing about problems with this. - inputResult = it!! + inputResult = it?.handledResult()!! startNestedScroll(ViewCompat.SCROLL_AXIS_VERTICAL) } } From dcc78fdfb3a51bcc2609539116ff40d2656ab1d7 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 10 Mar 2021 13:44:10 +0200 Subject: [PATCH 3/6] Closes #9821 - DownloadService will use a default mime type if otherwise empty (#9822) Speculative fix (cannot reproduce the issue) for crashes where based on the stacktrace the download's mime type was empty. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../downloads/AbstractFetchDownloadService.kt | 36 ++-- .../AbstractFetchDownloadServiceTest.kt | 161 ++++++++++++++++-- .../components/support/ktx/kotlin/String.kt | 7 + .../support/ktx/kotlin/StringTest.kt | 24 +++ docs/changelog.md | 3 + 5 files changed, 204 insertions(+), 27 deletions(-) diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt index 8c5075b6ef6..85743dea18a 100644 --- a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt +++ b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt @@ -70,6 +70,7 @@ import mozilla.components.feature.downloads.facts.emitNotificationPauseFact import mozilla.components.feature.downloads.facts.emitNotificationResumeFact import mozilla.components.feature.downloads.facts.emitNotificationTryAgainFact import mozilla.components.support.base.log.logger.Logger +import mozilla.components.support.ktx.kotlin.ifNullOrEmpty import mozilla.components.support.ktx.kotlin.sanitizeURL import mozilla.components.support.ktx.kotlinx.coroutines.throttleLatest import mozilla.components.support.utils.DownloadUtils @@ -461,7 +462,7 @@ abstract class AbstractFetchDownloadService : Service() { title = fileName, description = fileName, isMediaScannerScannable = true, - mimeType = download.contentType ?: "*/*", + mimeType = getSafeContentType(context, download.filePath, download.contentType), path = file.absolutePath, length = download.contentLength ?: file.length(), // Only show notifications if our channel is blocked @@ -885,7 +886,9 @@ abstract class AbstractFetchDownloadService : Service() { internal fun useFileStreamScopedStorage(download: DownloadState, block: (OutputStream) -> Unit) { val values = ContentValues().apply { put(MediaStore.Downloads.DISPLAY_NAME, download.fileName) - put(MediaStore.Downloads.MIME_TYPE, download.contentType ?: "*/*") + put(MediaStore.Downloads.MIME_TYPE, + getSafeContentType(context, download.filePath, download.contentType) + ) put(MediaStore.Downloads.SIZE, download.contentLength) put(MediaStore.Downloads.IS_PENDING, 1) } @@ -968,12 +971,7 @@ abstract class AbstractFetchDownloadService : Service() { fun openFile(context: Context, filePath: String, contentType: String?): Boolean { // Create a new file with the location of the saved file to extract the correct path // `file` has the wrong path, so we must construct it based on the `fileName` and `dir.path`s - val fileLocation = File(filePath) - val constructedFilePath = FileProvider.getUriForFile( - context, - context.packageName + FILE_PROVIDER_EXTENSION, - fileLocation - ) + val constructedFilePath = getFilePathUri(context, filePath) val newIntent = Intent(ACTION_VIEW).apply { setDataAndType(constructedFilePath, getSafeContentType(context, constructedFilePath, contentType)) @@ -994,13 +992,23 @@ abstract class AbstractFetchDownloadService : Service() { val resultContentType = if (!contentTypeFromFile.isNullOrEmpty()) { contentTypeFromFile } else { - if (!contentType.isNullOrEmpty()) { - contentType - } else { - "*/*" - } + contentType.ifNullOrEmpty { "*/*" } } - return (DownloadUtils.sanitizeMimeType(resultContentType) ?: "*/*") + return DownloadUtils.sanitizeMimeType(resultContentType).ifNullOrEmpty { "*/*" } + } + + @VisibleForTesting + internal fun getSafeContentType(context: Context, filePath: String, contentType: String?): String { + return getSafeContentType(context, getFilePathUri(context, filePath), contentType) + } + + @VisibleForTesting + internal fun getFilePathUri(context: Context, filePath: String): Uri { + return FileProvider.getUriForFile( + context, + context.packageName + FILE_PROVIDER_EXTENSION, + File(filePath) + ) } private const val FILE_PROVIDER_EXTENSION = ".feature.downloads.fileprovider" diff --git a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt index c9c053a9c9b..3e3896ad8e1 100644 --- a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt +++ b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt @@ -12,28 +12,31 @@ import android.app.Service import android.content.ContentResolver import android.content.Context import android.content.Intent +import android.net.Uri import android.os.Build import android.os.Environment import androidx.core.app.NotificationManagerCompat +import androidx.core.content.FileProvider import androidx.core.content.getSystemService +import androidx.core.net.toUri import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.delay -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runBlockingTest import kotlinx.coroutines.test.setMain import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.state.content.DownloadState.Status.COMPLETED import mozilla.components.browser.state.state.content.DownloadState.Status.DOWNLOADING import mozilla.components.browser.state.state.content.DownloadState.Status.FAILED import mozilla.components.browser.state.state.content.DownloadState.Status.INITIATED -import mozilla.components.browser.state.state.content.DownloadState.Status.COMPLETED import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.fetch.Client import mozilla.components.concept.fetch.MutableHeaders @@ -63,16 +66,16 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull -import org.junit.Assert.assertTrue import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyBoolean -import org.mockito.ArgumentMatchers.anyString import org.mockito.ArgumentMatchers.anyLong +import org.mockito.ArgumentMatchers.anyString import org.mockito.ArgumentMatchers.isNull import org.mockito.Mock import org.mockito.Mockito.atLeastOnce @@ -89,10 +92,13 @@ import org.mockito.Mockito.verifyZeroInteractions import org.mockito.MockitoAnnotations.initMocks import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config +import org.robolectric.annotation.Implementation +import org.robolectric.annotation.Implements import org.robolectric.shadows.ShadowNotificationManager import java.io.File import java.io.IOException import java.io.InputStream +import java.lang.IllegalArgumentException import kotlin.random.Random @RunWith(AndroidJUnit4::class) @@ -1405,7 +1411,7 @@ class AbstractFetchDownloadServiceTest { } @Test - @Config(sdk = [Build.VERSION_CODES.P]) + @Config(sdk = [Build.VERSION_CODES.P], shadows = [ShadowFileProvider::class]) fun `WHEN a download is completed and the scoped storage is not used it MUST be added manually to the download system database`() = runBlockingTest { val download = DownloadState( url = "http://www.mozilla.org", @@ -1516,7 +1522,7 @@ class AbstractFetchDownloadServiceTest { } @Test - @Config(sdk = [Build.VERSION_CODES.P]) + @Config(sdk = [Build.VERSION_CODES.P], shadows = [ShadowFileProvider::class]) @Suppress("Deprecation") fun `do not pass non-http(s) url to addCompletedDownload`() = runBlockingTest { val download = DownloadState( @@ -1541,7 +1547,7 @@ class AbstractFetchDownloadServiceTest { } @Test - @Config(sdk = [Build.VERSION_CODES.P]) + @Config(sdk = [Build.VERSION_CODES.P], shadows = [ShadowFileProvider::class]) @Suppress("Deprecation") fun `pass http(s) url to addCompletedDownload`() = runBlockingTest { val download = DownloadState( @@ -1565,6 +1571,42 @@ class AbstractFetchDownloadServiceTest { verify(downloadManager).addCompletedDownload(anyString(), anyString(), anyBoolean(), anyString(), anyString(), anyLong(), anyBoolean(), any(), any()) } + @Test + @Config(sdk = [Build.VERSION_CODES.P], shadows = [ShadowFileProvider::class]) + @Suppress("Deprecation") + fun `always call addCompletedDownload with a not empty or null mimeType`() = runBlockingTest { + val service = spy(object : AbstractFetchDownloadService() { + override val httpClient = client + override val store = browserStore + }) + val spyContext = spy(testContext) + var downloadManager: DownloadManager = mock() + doReturn(spyContext).`when`(service).context + doReturn(downloadManager).`when`(spyContext).getSystemService() + val downloadWithNullMimeType = DownloadState( + url = "blob:moz-extension://d5ea9baa-64c9-4c3d-bb38-49308c47997c/", + fileName = "example.apk", + destinationDirectory = folder.root.path, + contentType = null + ) + val downloadWithEmptyMimeType = downloadWithNullMimeType.copy(contentType = "") + val defaultMimeType = "*/*" + + service.addToDownloadSystemDatabaseCompat(downloadWithNullMimeType, this) + verify(downloadManager).addCompletedDownload( + anyString(), anyString(), anyBoolean(), eq(defaultMimeType), + anyString(), anyLong(), anyBoolean(), isNull(), any() + ) + + downloadManager = mock() + doReturn(downloadManager).`when`(spyContext).getSystemService() + service.addToDownloadSystemDatabaseCompat(downloadWithEmptyMimeType, this) + verify(downloadManager).addCompletedDownload( + anyString(), anyString(), anyBoolean(), eq(defaultMimeType), + anyString(), anyLong(), anyBoolean(), isNull(), any() + ) + } + @Test fun `cancelled download does not prevent other notifications`() = runBlocking { val cancelledDownload = DownloadState("https://example.com/file.txt", "file.txt") @@ -1769,7 +1811,7 @@ class AbstractFetchDownloadServiceTest { doReturn(contentTypeFromFile).`when`(contentResolver).getType(any()) doReturn(contentResolver).`when`(spyContext).contentResolver - val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), "any") + val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), "any") assertEquals("application/pdf", result) } @@ -1779,25 +1821,118 @@ class AbstractFetchDownloadServiceTest { val contentType = " application/pdf " val spyContext = spy(testContext) val contentResolver = mock() + doReturn(contentResolver).`when`(spyContext).contentResolver doReturn(null).`when`(contentResolver).getType(any()) + var result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), contentType) + assertEquals("application/pdf", result) + + doReturn("").`when`(contentResolver).getType(any()) + result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), contentType) + assertEquals("application/pdf", result) + } + + @Test + fun `getSafeContentType - WHEN none of the provided content types are available THEN return a generic content type`() { + val spyContext = spy(testContext) + val contentResolver = mock() + doReturn(contentResolver).`when`(spyContext).contentResolver + + doReturn(null).`when`(contentResolver).getType(any()) + var result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), null) + assertEquals("*/*", result) + + doReturn("").`when`(contentResolver).getType(any()) + result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), null) + assertEquals("*/*", result) + } + + // Following 3 tests use the String version of #getSafeContentType while the above 3 tested the Uri version + // The String version just overloads and delegates the Uri one but being in a companion object we cannot + // verify the delegation so we are left to verify the result to prevent any regressions. + @Test + @Config(shadows = [ShadowFileProvider::class]) + fun `getSafeContentType2 - WHEN the file content type is available THEN use it`() { + val contentTypeFromFile = "application/pdf; qs=0.001" + val spyContext = spy(testContext) + val contentResolver = mock() + + doReturn(contentTypeFromFile).`when`(contentResolver).getType(any()) doReturn(contentResolver).`when`(spyContext).contentResolver - val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), contentType) + val result = AbstractFetchDownloadService.getSafeContentType(spyContext, "any", "any") assertEquals("application/pdf", result) } @Test - fun `getSafeContentType - WHEN none of the provided content types are available THEN return a generic content type`() { + @Config(shadows = [ShadowFileProvider::class]) + fun `getSafeContentType2 - WHEN the file content type is not available THEN use the provided content type`() { + val contentType = " application/pdf " val spyContext = spy(testContext) val contentResolver = mock() + doReturn(contentResolver).`when`(spyContext).contentResolver doReturn(null).`when`(contentResolver).getType(any()) + var result = AbstractFetchDownloadService.getSafeContentType(spyContext, "any", contentType) + assertEquals("application/pdf", result) + + doReturn("").`when`(contentResolver).getType(any()) + result = AbstractFetchDownloadService.getSafeContentType(spyContext, "any", contentType) + assertEquals("application/pdf", result) + } + + @Test + @Config(shadows = [ShadowFileProvider::class]) + fun `getSafeContentType2 - WHEN none of the provided content types are available THEN return a generic content type`() { + val spyContext = spy(testContext) + val contentResolver = mock() doReturn(contentResolver).`when`(spyContext).contentResolver - val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), null) + doReturn(null).`when`(contentResolver).getType(any()) + var result = AbstractFetchDownloadService.getSafeContentType(spyContext, "any", null) + assertEquals("*/*", result) + doReturn("").`when`(contentResolver).getType(any()) + result = AbstractFetchDownloadService.getSafeContentType(spyContext, "any", null) assertEquals("*/*", result) } + + // Hard to test #getFilePathUri since it only returns the result of a certain Android api call. + // But let's try. + @Test + fun `getFilePathUri - WHEN called without a registered provider THEN exception is thrown`() { + // There is no app registered provider that could expose a file from the filesystem of the machine running this test. + // Peeking into the exception would indicate whether the code really called "FileProvider.getUriForFile" as expected. + var exception: IllegalArgumentException? = null + try { + AbstractFetchDownloadService.getFilePathUri(testContext, "test.txt") + } catch (e: IllegalArgumentException) { + exception = e + } + + assertTrue(exception!!.stackTrace[0].fileName.contains("FileProvider")) + assertTrue(exception.stackTrace[0].methodName == "getUriForFile") + } + + @Test + @Config(shadows = [ShadowFileProvider::class]) + fun `getFilePathUri - WHEN called THEN return a file provider path for the filePath`() { + // Test that the String filePath is passed to the provider from which we expect a Uri path + val result = AbstractFetchDownloadService.getFilePathUri(testContext, "location/test.txt") + + assertTrue(result.toString().endsWith("location/test.txt")) + } +} + +@Implements(FileProvider::class) +object ShadowFileProvider { + @Implementation + @JvmStatic + @Suppress("UNUSED_PARAMETER") + fun getUriForFile( + context: Context?, + authority: String?, + file: File + ) = "content://authority/random/location/${file.name}".toUri() } diff --git a/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt b/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt index e7e15e45d08..1488e621869 100644 --- a/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt +++ b/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt @@ -191,3 +191,10 @@ fun String.getDataUrlImageExtension(defaultExtension: String = "jpg"): String { return ("data:image\\/([a-zA-Z0-9-.+]+).*").toRegex() .find(this)?.groups?.get(1)?.value ?: defaultExtension } + +/** + * Returns this char sequence if it's not null or empty + * or the result of calling [defaultValue] function if the char sequence is null or empty. + */ +inline fun C?.ifNullOrEmpty(defaultValue: () -> R): C where C : CharSequence, R : C = + if (isNullOrEmpty()) defaultValue() else this diff --git a/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt b/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt index 733a0bed010..3ff5f869345 100644 --- a/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt +++ b/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt @@ -9,6 +9,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull +import org.junit.Assert.assertSame import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith @@ -209,4 +210,27 @@ class StringTest { assertEquals("gif", result) } + + @Test + fun `ifNullOrEmpty returns the same if this CharSequence is not null and not empty`() { + val randomString = "something" + + assertSame(randomString, randomString.ifNullOrEmpty { "something else" }) + } + + @Test + fun `ifNullOrEmpty returns the invocation of the passed in argument if this CharSequence is null`() { + val nullString: String? = null + val validResult = "notNullString" + + assertSame(validResult, nullString.ifNullOrEmpty { validResult }) + } + + @Test + fun `ifNullOrEmpty returns the invocation of the passed in argument if this CharSequence is empty`() { + val nullString = "" + val validResult = "notEmptyString" + + assertSame(validResult, nullString.ifNullOrEmpty { validResult }) + } } diff --git a/docs/changelog.md b/docs/changelog.md index 067c279a145..c76da120dc5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -12,6 +12,9 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/.config.yml) +* **feature-downloads**: + * 🚒 Bug fixed [issue #9821](https://github.com/mozilla-mobile/android-components/issues/9821) - Crash for downloads inferred empty mime types. + * **intent-processing** * 🌟️ Added support for opening links from ACTION_MAIN Intents. This Intents could the result of `Intent.makeMainSelectorActivity(Intent.ACTION_MAIN, Intent.CATEGORY_APP_BROWSER)` calls. From 2706db240ebf538d65b5a51c21af6aa6910c78a4 Mon Sep 17 00:00:00 2001 From: Sebastian Kaspari Date: Wed, 3 Mar 2021 13:34:42 +0100 Subject: [PATCH 4/6] Closes #9777: Android Autofill: Verify app <-> domain relationship and let user confirm. --- components/feature/autofill/build.gradle | 2 + .../autofill/src/main/AndroidManifest.xml | 6 +- .../feature/autofill/AutofillConfiguration.kt | 12 ++ .../feature/autofill/ext/LoginsStorage.kt | 43 ------ .../autofill/handler/FillRequestHandler.kt | 140 ++++++++++++++---- .../autofill/structure/ParsedStructure.kt | 23 +++ .../ui/AbstractAutofillConfirmActivity.kt | 118 +++++++++++++++ .../ui/AbstractAutofillUnlockActivity.kt | 1 + .../verify/CredentialAccessVerifier.kt | 50 +++++++ .../autofill/src/main/res/values/strings.xml | 20 ++- .../service/digitalassetlinks/Relation.kt | 7 +- samples/browser/src/main/AndroidManifest.xml | 4 + .../samples/browser/DefaultComponents.kt | 5 +- .../autofill/AutofillConfirmActivity.kt | 19 +++ 14 files changed, 376 insertions(+), 74 deletions(-) delete mode 100644 components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ext/LoginsStorage.kt create mode 100644 components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillConfirmActivity.kt create mode 100644 components/feature/autofill/src/main/java/mozilla/components/feature/autofill/verify/CredentialAccessVerifier.kt create mode 100644 samples/browser/src/main/java/org/mozilla/samples/browser/autofill/AutofillConfirmActivity.kt diff --git a/components/feature/autofill/build.gradle b/components/feature/autofill/build.gradle index ac59b166c56..8f43d20a67a 100644 --- a/components/feature/autofill/build.gradle +++ b/components/feature/autofill/build.gradle @@ -28,8 +28,10 @@ tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all { } dependencies { + implementation project(':concept-fetch') implementation project(':concept-storage') implementation project(':lib-publicsuffixlist') + implementation project(':service-digitalassetlinks') implementation project(':support-base') implementation project(":support-utils") diff --git a/components/feature/autofill/src/main/AndroidManifest.xml b/components/feature/autofill/src/main/AndroidManifest.xml index 824e5ab7eaf..09cdc0c2a5f 100644 --- a/components/feature/autofill/src/main/AndroidManifest.xml +++ b/components/feature/autofill/src/main/AndroidManifest.xml @@ -2,7 +2,11 @@ - License, v. 2.0. If a copy of the MPL was not distributed with this - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> + xmlns:tools="http://schemas.android.com/tools" + package="mozilla.components.feature.autofill"> + + diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AutofillConfiguration.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AutofillConfiguration.kt index 97efab988da..7b649d609bb 100644 --- a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AutofillConfiguration.kt +++ b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/AutofillConfiguration.kt @@ -4,10 +4,15 @@ package mozilla.components.feature.autofill +import mozilla.components.concept.fetch.Client import mozilla.components.concept.storage.LoginsStorage import mozilla.components.feature.autofill.lock.AutofillLock +import mozilla.components.feature.autofill.ui.AbstractAutofillConfirmActivity import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.feature.autofill.ui.AbstractAutofillUnlockActivity +import mozilla.components.feature.autofill.verify.CredentialAccessVerifier +import mozilla.components.service.digitalassetlinks.local.StatementApi +import mozilla.components.service.digitalassetlinks.local.StatementRelationChecker /** * Configuration for the "Autofill" feature. @@ -15,8 +20,10 @@ import mozilla.components.feature.autofill.ui.AbstractAutofillUnlockActivity * @property storage The [LoginsStorage] used for looking up accounts and passwords to autofill. * @property publicSuffixList Global instance of the public suffix list used for matching domains. * @property unlockActivity Activity class that implements [AbstractAutofillUnlockActivity]. + * @property confirmActivity Activity class that implements [AbstractAutofillConfirmActivity]. * @property applicationName The name of the application that integrates this feature. Used in UI. * @property lock Global [AutofillLock] instance used for unlocking the autofill service. + * @property verifier Helper for verifying the connection between a domain and an application. * @property activityRequestCode The request code used for pending intents that launch an activity * on behalf of the autofill service. */ @@ -24,7 +31,12 @@ data class AutofillConfiguration( val storage: LoginsStorage, val publicSuffixList: PublicSuffixList, val unlockActivity: Class<*>, + val confirmActivity: Class<*>, val applicationName: String, + val httpClient: Client, val lock: AutofillLock = AutofillLock(), + val verifier: CredentialAccessVerifier = CredentialAccessVerifier( + StatementRelationChecker(StatementApi(httpClient)) + ), val activityRequestCode: Int = 1010 ) diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ext/LoginsStorage.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ext/LoginsStorage.kt deleted file mode 100644 index aad56b79b2d..00000000000 --- a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ext/LoginsStorage.kt +++ /dev/null @@ -1,43 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package mozilla.components.feature.autofill.ext - -import mozilla.components.concept.storage.Login -import mozilla.components.concept.storage.LoginsStorage -import mozilla.components.feature.autofill.structure.ParsedStructure -import mozilla.components.lib.publicsuffixlist.PublicSuffixList -import mozilla.components.support.utils.Browsers - -/** - * Returns [Login]s matching the domain or package name found in the [ParsedStructure]. - */ -internal suspend fun LoginsStorage.getByStructure( - publicSuffixList: PublicSuffixList, - structure: ParsedStructure -): List { - val domain = structure.getLookupDomain(publicSuffixList) - return getByBaseDomain(domain) -} - -/** - * Try to find a domain in the [ParsedStructure] for looking up logins. This is either a "web domain" - * for web content the third-party app is displaying (e.g. in a WebView) or the package name of the - * application transformed into a domain. In any case the [publicSuffixList] will be used to turn - * the domain into a "base" domain (public suffix + 1) before returning. - */ -private suspend fun ParsedStructure.getLookupDomain(publicSuffixList: PublicSuffixList): String { - val domain = if (webDomain != null && Browsers.isBrowser(packageName)) { - // If the application we are auto-filling is a known browser and it provided a webDomain - // for the content it is displaying then we try to autofill for that. - webDomain - } else { - // We reverse the package name in the hope that this will resemble a domain name. This is - // of course fragile. So we want to find better mechanisms in the future (e.g. looking up - // what URLs the application registers intent handlers for). - packageName.split('.').asReversed().joinToString(".") - } - - return publicSuffixList.getPublicSuffixPlusOne(domain).await() ?: domain -} diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/handler/FillRequestHandler.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/handler/FillRequestHandler.kt index 946bd654a9a..b20028e59ae 100644 --- a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/handler/FillRequestHandler.kt +++ b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/handler/FillRequestHandler.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.autofill.handler +import android.annotation.SuppressLint import android.app.PendingIntent import android.app.assist.AssistStructure import android.content.Context @@ -19,10 +20,12 @@ import androidx.annotation.RequiresApi import mozilla.components.concept.storage.Login import mozilla.components.feature.autofill.AutofillConfiguration import mozilla.components.feature.autofill.R -import mozilla.components.feature.autofill.ext.getByStructure import mozilla.components.feature.autofill.structure.ParsedStructure +import mozilla.components.feature.autofill.structure.getLookupDomain import mozilla.components.feature.autofill.structure.parseStructure +internal const val EXTRA_LOGIN_ID = "loginId" + /** * Class responsible for handling [FillRequest]s and returning [FillResponse]s. */ @@ -43,15 +46,25 @@ internal class FillRequestHandler( * Handles a fill request for the given [AssistStructure] and returns a matching [FillResponse] * or `null` if the request could not be handled or the passed in [AssistStructure] is `null`. */ + @SuppressLint("InlinedApi") @Suppress("ReturnCount") - suspend fun handle(structure: AssistStructure?, forceUnlock: Boolean = false): FillResponse? { + suspend fun handle( + structure: AssistStructure?, + forceUnlock: Boolean = false + ): FillResponse? { if (structure == null) { return null } val parsedStructure = parseStructure(context, structure) ?: return null - val logins = configuration.storage.getByStructure(configuration.publicSuffixList, parsedStructure) + val lookupDomain = parsedStructure.getLookupDomain(configuration.publicSuffixList) + val needsConfirmation = !configuration.verifier.hasCredentialRelationship( + context, + lookupDomain, + parsedStructure.packageName + ) + val logins = configuration.storage.getByBaseDomain(lookupDomain) if (logins.isEmpty()) { return null } @@ -59,8 +72,44 @@ internal class FillRequestHandler( return if (!configuration.lock.keepUnlocked() && !forceUnlock) { createAuthResponse(context, configuration, parsedStructure) } else { - createLoginsResponse(context, parsedStructure, logins) + createLoginsResponse( + context, + configuration, + parsedStructure, + logins, + needsConfirmation + ) + } + } + + /** + * Handles a fill request for the given [AssistStructure] and returns only a [Dataset] for the + * given [loginId] - or `null` if the request could not be handled or the passed in + * [AssistStructure] is `null` + */ + @Suppress("ReturnCount") + suspend fun handleConfirmation(structure: AssistStructure?, loginId: String): Dataset? { + if (structure == null) { + return null + } + + val parsedStructure = parseStructure(context, structure) ?: return null + val lookupDomain = parsedStructure.getLookupDomain(configuration.publicSuffixList) + + val logins = configuration.storage.getByBaseDomain(lookupDomain) + if (logins.isEmpty()) { + return null } + + val login = logins.firstOrNull { login -> login.guid == loginId } ?: return null + + return createDataSetResponse( + context, + configuration, + login, + parsedStructure, + needsConfirmation = false + ) } } @@ -98,39 +147,76 @@ private fun createAuthResponse( @RequiresApi(Build.VERSION_CODES.O) private fun createLoginsResponse( context: Context, + configuration: AutofillConfiguration, parsedStructure: ParsedStructure, - logins: List + logins: List, + needsConfirmation: Boolean ): FillResponse { val builder = FillResponse.Builder() - logins.forEach { login -> - val usernamePresentation = RemoteViews(context.packageName, android.R.layout.simple_list_item_1) - usernamePresentation.setTextViewText(android.R.id.text1, login.usernamePresentationOrFallback(context)) - val passwordPresentation = RemoteViews(context.packageName, android.R.layout.simple_list_item_1) - passwordPresentation.setTextViewText(android.R.id.text1, login.passwordPresentation(context)) + logins.forEachIndexed { index, login -> + val dataset = createDataSetResponse( + context, + configuration, + login, + parsedStructure, + needsConfirmation, + requestOffset = index + ) + builder.addDataset(dataset) + } - val dataset = Dataset.Builder() + return builder.build() +} - parsedStructure.usernameId?.let { id -> - dataset.setValue( - id, - AutofillValue.forText(login.username), - usernamePresentation - ) - } +@RequiresApi(Build.VERSION_CODES.O) +@Suppress("LongParameterList") +private fun createDataSetResponse( + context: Context, + configuration: AutofillConfiguration, + login: Login, + parsedStructure: ParsedStructure, + needsConfirmation: Boolean, + requestOffset: Int = 0 +): Dataset { + val dataset = Dataset.Builder() + + val usernamePresentation = RemoteViews(context.packageName, android.R.layout.simple_list_item_1) + usernamePresentation.setTextViewText(android.R.id.text1, login.usernamePresentationOrFallback(context)) + val passwordPresentation = RemoteViews(context.packageName, android.R.layout.simple_list_item_1) + passwordPresentation.setTextViewText(android.R.id.text1, login.passwordPresentation(context)) + + parsedStructure.usernameId?.let { id -> + dataset.setValue( + id, + if (needsConfirmation) null else AutofillValue.forText(login.username), + usernamePresentation + ) + } - parsedStructure.passwordId?.let { id -> - dataset.setValue( - id, - AutofillValue.forText(login.password), - passwordPresentation - ) - } + parsedStructure.passwordId?.let { id -> + dataset.setValue( + id, + if (needsConfirmation) null else AutofillValue.forText(login.password), + passwordPresentation + ) + } + + if (needsConfirmation) { + val confirmIntent = Intent(context, configuration.confirmActivity) + confirmIntent.putExtra(EXTRA_LOGIN_ID, login.guid) - builder.addDataset(dataset.build()) + val intentSender: IntentSender = PendingIntent.getActivity( + context, + configuration.activityRequestCode + requestOffset, + confirmIntent, + PendingIntent.FLAG_CANCEL_CURRENT + ).intentSender + + dataset.setAuthentication(intentSender) } - return builder.build() + return dataset.build() } private fun Login.usernamePresentationOrFallback(context: Context): String { diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/structure/ParsedStructure.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/structure/ParsedStructure.kt index 360e1773060..2a72346be31 100644 --- a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/structure/ParsedStructure.kt +++ b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/structure/ParsedStructure.kt @@ -9,6 +9,8 @@ import android.content.Context import android.os.Build import android.view.autofill.AutofillId import androidx.annotation.RequiresApi +import mozilla.components.lib.publicsuffixlist.PublicSuffixList +import mozilla.components.support.utils.Browsers /** * Parsed structure from an autofill request. @@ -24,6 +26,27 @@ internal data class ParsedStructure( val packageName: String ) +/** + * Try to find a domain in the [ParsedStructure] for looking up logins. This is either a "web domain" + * for web content the third-party app is displaying (e.g. in a WebView) or the package name of the + * application transformed into a domain. In any case the [publicSuffixList] will be used to turn + * the domain into a "base" domain (public suffix + 1) before returning. + */ +internal suspend fun ParsedStructure.getLookupDomain(publicSuffixList: PublicSuffixList): String { + val domain = if (webDomain != null && Browsers.isBrowser(packageName)) { + // If the application we are auto-filling is a known browser and it provided a webDomain + // for the content it is displaying then we try to autofill for that. + webDomain + } else { + // We reverse the package name in the hope that this will resemble a domain name. This is + // of course fragile. So we want to find better mechanisms in the future (e.g. looking up + // what URLs the application registers intent handlers for). + packageName.split('.').asReversed().joinToString(".") + } + + return publicSuffixList.getPublicSuffixPlusOne(domain).await() ?: domain +} + @RequiresApi(Build.VERSION_CODES.O) internal fun parseStructure(context: Context, structure: AssistStructure): ParsedStructure? { val activityPackageName = structure.activityComponent.packageName diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillConfirmActivity.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillConfirmActivity.kt new file mode 100644 index 00000000000..4009d941eb0 --- /dev/null +++ b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillConfirmActivity.kt @@ -0,0 +1,118 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.autofill.ui + +import android.app.Dialog +import android.app.assist.AssistStructure +import android.content.Intent +import android.os.Build +import android.os.Bundle +import android.service.autofill.Dataset +import android.view.autofill.AutofillManager +import androidx.annotation.RequiresApi +import androidx.appcompat.app.AlertDialog +import androidx.fragment.app.DialogFragment +import androidx.fragment.app.FragmentActivity +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.runBlocking +import mozilla.components.feature.autofill.AutofillConfiguration +import mozilla.components.feature.autofill.R +import mozilla.components.feature.autofill.handler.EXTRA_LOGIN_ID +import mozilla.components.feature.autofill.handler.FillRequestHandler + +/** + * Activity responsible for asking the user to confirm before autofilling a third-party app. It is + * shown in situations where the authenticity of an application could not be confirmed automatically + * with "Digital Asset Links". + */ +@RequiresApi(Build.VERSION_CODES.O) +abstract class AbstractAutofillConfirmActivity : FragmentActivity() { + abstract val configuration: AutofillConfiguration + + private var dataset: Deferred? = null + private val fillHandler by lazy { FillRequestHandler(context = this, configuration) } + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + val structure: AssistStructure? = intent.getParcelableExtra(AutofillManager.EXTRA_ASSIST_STRUCTURE) + val loginId = intent.getStringExtra(EXTRA_LOGIN_ID) + if (loginId == null) { + cancel() + return + } + + // While the user is asked to confirm, we already try to build the fill response asynchronously. + dataset = lifecycleScope.async(Dispatchers.IO) { + fillHandler.handleConfirmation(structure, loginId) + } + + if (savedInstanceState == null) { + val fragment = AutofillConfirmFragment() + fragment.show(supportFragmentManager, "confirm_fragment") + } + } + + /** + * Confirms the autofill request and returns the credentials to the autofill framework. + */ + internal fun confirm() { + val replyIntent = Intent().apply { + // At this point it should be safe to block since the fill response should be ready once + // the user has authenticated. + runBlocking { putExtra(AutofillManager.EXTRA_AUTHENTICATION_RESULT, dataset?.await()) } + } + + setResult(RESULT_OK, replyIntent) + finish() + } + + /** + * Cancels the autofill request. + */ + internal fun cancel() { + dataset?.cancel() + + setResult(RESULT_CANCELED) + finish() + } +} + +@RequiresApi(Build.VERSION_CODES.O) +internal class AutofillConfirmFragment : DialogFragment() { + private val configuration: AutofillConfiguration + get() = getConfirmActivity().configuration + + override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { + return AlertDialog.Builder(requireContext()) + .setTitle( + getString(R.string.mozac_feature_autofill_confirmation_title) + ) + .setMessage( + getString(R.string.mozac_feature_autofill_confirmation_authenticity, configuration.applicationName) + ) + .setPositiveButton(R.string.mozac_feature_autofill_confirmation_yes) { _, _ -> confirmRequest() } + .setNegativeButton(R.string.mozac_feature_autofill_confirmation_no) { _, _ -> cancelRequest() } + .setOnDismissListener { cancelRequest() } + .create() + } + + private fun confirmRequest() { + getConfirmActivity() + .confirm() + } + + private fun cancelRequest() { + getConfirmActivity() + .cancel() + } + + private fun getConfirmActivity(): AbstractAutofillConfirmActivity { + return requireActivity() as AbstractAutofillConfirmActivity + } +} diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillUnlockActivity.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillUnlockActivity.kt index cd46b7bd0de..2dd93ea0fd0 100644 --- a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillUnlockActivity.kt +++ b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/ui/AbstractAutofillUnlockActivity.kt @@ -60,6 +60,7 @@ abstract class AbstractAutofillUnlockActivity : FragmentActivity() { internal inner class PromptCallback : Authenticator.Callback { override fun onAuthenticationError() { + fillResponse?.cancel() setResult(RESULT_CANCELED) finish() } diff --git a/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/verify/CredentialAccessVerifier.kt b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/verify/CredentialAccessVerifier.kt new file mode 100644 index 00000000000..23abd122e87 --- /dev/null +++ b/components/feature/autofill/src/main/java/mozilla/components/feature/autofill/verify/CredentialAccessVerifier.kt @@ -0,0 +1,50 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.autofill.verify + +import android.content.Context +import mozilla.components.service.digitalassetlinks.AndroidAssetFinder +import mozilla.components.service.digitalassetlinks.AssetDescriptor +import mozilla.components.service.digitalassetlinks.Relation +import mozilla.components.service.digitalassetlinks.local.StatementRelationChecker + +/** + * Helper to verify that a specific application is allowed to receive get the login credentials for + * a specific domain. + * + * The verification is done through Digital Asset Links, which allow a domain to specify associated + * apps and their signatures. + * - https://developers.google.com/digital-asset-links/v1/getting-started + * - https://github.com/google/digitalassetlinks/blob/master/well-known/details.md + */ +class CredentialAccessVerifier( + private val checker: StatementRelationChecker, + private val assetsFinder: AndroidAssetFinder = AndroidAssetFinder() +) { + /** + * Verifies and returns `true` if the application with [packageName] is allowed to receive + * credentials for [domain] according to the hosted Digital Assets Links file. Returns `false` + * otherwise. This method may also return `false` if a verification could not be performed, + * e.g. the device is offline. + */ + fun hasCredentialRelationship( + context: Context, + domain: String, + packageName: String + ): Boolean { + val assets = assetsFinder.getAndroidAppAsset(packageName, context.packageManager).toList() + + // I was expecting us to need to verify all signatures here. But If I understand the usage + // in `OriginVerifier` and the spec (see link in class comment) correctly then verifying one + // certificate is enough to identify an app. + val asset = assets.firstOrNull() ?: return false + + return checker.checkRelationship( + AssetDescriptor.Web("https://$domain"), + Relation.GET_LOGIN_CREDS, + asset + ) + } +} diff --git a/components/feature/autofill/src/main/res/values/strings.xml b/components/feature/autofill/src/main/res/values/strings.xml index decf15c303a..3be9c25bf3c 100644 --- a/components/feature/autofill/src/main/res/values/strings.xml +++ b/components/feature/autofill/src/main/res/values/strings.xml @@ -16,4 +16,22 @@ Password for %1$s - \ No newline at end of file + + + Verification failed + + + %1$s could not verify the authenticity of the application. Do you want to proceed with autofilling the selected credentials? + + + Yes + + + No + diff --git a/components/service/digitalassetlinks/src/main/java/mozilla/components/service/digitalassetlinks/Relation.kt b/components/service/digitalassetlinks/src/main/java/mozilla/components/service/digitalassetlinks/Relation.kt index 55a0bd531f8..a2470d1e146 100644 --- a/components/service/digitalassetlinks/src/main/java/mozilla/components/service/digitalassetlinks/Relation.kt +++ b/components/service/digitalassetlinks/src/main/java/mozilla/components/service/digitalassetlinks/Relation.kt @@ -20,5 +20,10 @@ enum class Relation(val kindAndDetail: String) { /** * Requests the ability to handle all URLs from a given origin. */ - HANDLE_ALL_URLS("delegate_permission/common.handle_all_urls") + HANDLE_ALL_URLS("delegate_permission/common.handle_all_urls"), + + /** + * Grants the target permission to retrieve sign-in credentials stored for the source. + */ + GET_LOGIN_CREDS("delegate_permission/common.get_login_creds") } diff --git a/samples/browser/src/main/AndroidManifest.xml b/samples/browser/src/main/AndroidManifest.xml index fe3e8b3d8e1..06eaf88378f 100644 --- a/samples/browser/src/main/AndroidManifest.xml +++ b/samples/browser/src/main/AndroidManifest.xml @@ -124,6 +124,10 @@ android:exported="false" android:theme="@android:style/Theme.Translucent.NoTitleBar" /> + + Date: Wed, 10 Mar 2021 12:42:40 +0000 Subject: [PATCH 5/6] Update GeckoView (Nightly) to 88.0.20210310093927. --- buildSrc/src/main/java/Gecko.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/Gecko.kt b/buildSrc/src/main/java/Gecko.kt index 84102bb2e93..b2ed8c1e3a8 100644 --- a/buildSrc/src/main/java/Gecko.kt +++ b/buildSrc/src/main/java/Gecko.kt @@ -6,7 +6,7 @@ internal object GeckoVersions { /** * GeckoView Nightly Version. */ - const val nightly_version = "88.0.20210309094921" + const val nightly_version = "88.0.20210310093927" /** * GeckoView Beta Version. From 4155fff604166170c4b2b3139cdc1f369ace1b8c Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Wed, 10 Mar 2021 12:04:43 -0500 Subject: [PATCH 6/6] Closes #9859: Detect media notification icon size is incorrect --- .../gecko/mediasession/GeckoMediaSessionDelegate.kt | 11 ++++++++++- .../gecko/mediasession/GeckoMediaSessionDelegate.kt | 11 ++++++++++- .../gecko/mediasession/GeckoMediaSessionDelegate.kt | 11 ++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) 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 index ed6a4864f61..5c66f956c64 100644 --- 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 @@ -15,6 +15,7 @@ import org.mozilla.geckoview.MediaSession as GeckoViewMediaSession private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L private const val ARTWORK_IMAGE_SIZE = 48 +private const val ARTWORK_ERROR_SIZE = 1 internal class GeckoMediaSessionDelegate( private val engineSession: GeckoEngineSession @@ -40,9 +41,17 @@ internal class GeckoMediaSessionDelegate( val getArtwork: (suspend () -> Bitmap?)? = metaData.artwork?.let { { try { - withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) { + var bitmap = withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) { it.getBitmap(ARTWORK_IMAGE_SIZE).await() } + + /* workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1697255 */ + if (bitmap != null && + (bitmap.height <= ARTWORK_ERROR_SIZE || bitmap.width <= ARTWORK_ERROR_SIZE)) { + bitmap = null + } + + bitmap } catch (e: ImageProcessingException) { null } 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 index ed6a4864f61..5c66f956c64 100644 --- 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 @@ -15,6 +15,7 @@ import org.mozilla.geckoview.MediaSession as GeckoViewMediaSession private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L private const val ARTWORK_IMAGE_SIZE = 48 +private const val ARTWORK_ERROR_SIZE = 1 internal class GeckoMediaSessionDelegate( private val engineSession: GeckoEngineSession @@ -40,9 +41,17 @@ internal class GeckoMediaSessionDelegate( val getArtwork: (suspend () -> Bitmap?)? = metaData.artwork?.let { { try { - withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) { + var bitmap = withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) { it.getBitmap(ARTWORK_IMAGE_SIZE).await() } + + /* workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1697255 */ + if (bitmap != null && + (bitmap.height <= ARTWORK_ERROR_SIZE || bitmap.width <= ARTWORK_ERROR_SIZE)) { + bitmap = null + } + + bitmap } catch (e: ImageProcessingException) { null } 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 index 4ef5f44d1c6..c233b5595e2 100644 --- 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 @@ -14,6 +14,7 @@ import org.mozilla.geckoview.MediaSession as GeckoViewMediaSession private const val ARTWORK_RETRIEVE_TIMEOUT = 1000L private const val ARTWORK_IMAGE_SIZE = 48 +private const val ARTWORK_ERROR_SIZE = 1 internal class GeckoMediaSessionDelegate( private val engineSession: GeckoEngineSession @@ -39,9 +40,17 @@ internal class GeckoMediaSessionDelegate( val getArtwork: (suspend () -> Bitmap?)? = metaData.artwork?.let { { try { - withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) { + var bitmap = withTimeoutOrNull(ARTWORK_RETRIEVE_TIMEOUT) { it.getBitmap(ARTWORK_IMAGE_SIZE).await() } + + /* workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1697255 */ + if (bitmap != null && + (bitmap.height <= ARTWORK_ERROR_SIZE || bitmap.width <= ARTWORK_ERROR_SIZE)) { + bitmap = null + } + + bitmap } catch (e: IllegalArgumentException) { null }