Skip to content

GitLab

Commit 0b8c6320 authored by Arturo Mejia's avatar Arturo Mejia
Browse files

Closes #8240: Only dismiss prompts that are 

not already dismissed
parent 622ca64e
Hide whitespace changes
Inline Side-by-side
Showing with 142 additions and 28 deletions
components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt
View file @ 0b8c6320
......@@ -85,7 +85,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(Autocomplete.LoginSelectOption(login.toLoginEntry())))
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
geckoEngineSession.notifyObservers {
......@@ -110,7 +110,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(Autocomplete.LoginSelectOption(login.toLoginEntry())))
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
// Exactly one of `httpRealm` and `formSubmitURL` must be present to be a valid login entry.
......@@ -174,7 +174,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
prompt: PromptDelegate.AlertPrompt
): GeckoResult<PromptResponse> {
val geckoResult = GeckoResult<PromptResponse>()
val onConfirm: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onConfirm: () -> Unit = { prompt.dismissSafely(geckoResult) }
val title = prompt.title ?: ""
val message = prompt.message ?: ""
......@@ -219,7 +219,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
geckoEngineSession.notifyObservers {
......@@ -298,7 +298,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}
}
val onDismiss: () -> Unit = { geckoResult.complete(geckoPrompt.dismiss()) }
val onDismiss: () -> Unit = { geckoPrompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -328,7 +328,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
val title = prompt.title ?: ""
val inputLabel = prompt.message ?: ""
val inputValue = prompt.defaultValue ?: ""
val onDismiss: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: () -> Unit = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -352,7 +352,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
): GeckoResult<PromptResponse>? {
val geckoResult = GeckoResult<PromptResponse>()
val onConfirm: (String) -> Unit = { geckoResult.complete(prompt.confirm(it)) }
val onDismiss: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: () -> Unit = { prompt.dismissSafely(geckoResult) }
val defaultColor = prompt.defaultValue ?: ""
......@@ -403,7 +403,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
val geckoResult = GeckoResult<PromptResponse>()
val onSuccess = { geckoResult.complete(prompt.confirm(GECKO_PROMPT_SHARE_RESULT.SUCCESS)) }
val onFailure = { geckoResult.complete(prompt.confirm(GECKO_PROMPT_SHARE_RESULT.FAILURE)) }
val onDismiss = { geckoResult.complete(prompt.dismiss()) }
val onDismiss = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -437,7 +437,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(PromptDelegate.ButtonPrompt.Type.NEGATIVE))
}
val onDismiss: (Boolean) -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: (Boolean) -> Unit = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -573,3 +573,13 @@ internal fun Date.toString(format: String): String {
val formatter = SimpleDateFormat(format, Locale.ROOT)
return formatter.format(this) ?: ""
}
/**
* Only dismiss if the prompt is not already dismissed.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun PromptDelegate.BasePrompt.dismissSafely(geckoResult: GeckoResult<PromptResponse>) {
if (!this.isComplete) {
geckoResult.complete(dismiss())
}
}
components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
View file @ 0b8c6320
......@@ -26,10 +26,12 @@ import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.spy
import org.mockito.Mockito
import org.mockito.Mockito.doReturn
import org.mozilla.gecko.util.GeckoBundle
import org.mozilla.geckoview.Autocomplete
import org.mozilla.geckoview.GeckoResult
import org.mozilla.geckoview.GeckoRuntime
import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.DATE
......@@ -1080,6 +1082,30 @@ class GeckoPromptDelegateTest {
assertTrue(dismissWasCalled)
}
@Test
fun `dismissSafely only dismiss if the prompt is NOT already dismissed`() {
val prompt = spy(GeckoAlertPrompt())
val geckoResult = mock<GeckoResult<GeckoSession.PromptDelegate.PromptResponse>>()
doReturn(false).`when`(prompt).isComplete
prompt.dismissSafely(geckoResult)
Mockito.verify(geckoResult).complete(any())
}
@Test
fun `dismissSafely do nothing if the prompt is already dismissed`() {
val prompt = spy(GeckoAlertPrompt())
val geckoResult = mock<GeckoResult<GeckoSession.PromptDelegate.PromptResponse>>()
doReturn(true).`when`(prompt).isComplete
prompt.dismissSafely(geckoResult)
Mockito.verify(geckoResult, Mockito.never()).complete(any())
}
class GeckoChoicePrompt(
title: String,
message: String,
......
components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt
View file @ 0b8c6320
......@@ -85,7 +85,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(Autocomplete.LoginSelectOption(login.toLoginEntry())))
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
geckoEngineSession.notifyObservers {
......@@ -110,7 +110,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(Autocomplete.LoginSelectOption(login.toLoginEntry())))
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
// Exactly one of `httpRealm` and `formSubmitURL` must be present to be a valid login entry.
......@@ -174,7 +174,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
prompt: PromptDelegate.AlertPrompt
): GeckoResult<PromptResponse> {
val geckoResult = GeckoResult<PromptResponse>()
val onConfirm: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onConfirm: () -> Unit = { prompt.dismissSafely(geckoResult) }
val title = prompt.title ?: ""
val message = prompt.message ?: ""
......@@ -219,7 +219,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
geckoEngineSession.notifyObservers {
......@@ -298,7 +298,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}
}
val onDismiss: () -> Unit = { geckoResult.complete(geckoPrompt.dismiss()) }
val onDismiss: () -> Unit = { geckoPrompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -328,7 +328,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
val title = prompt.title ?: ""
val inputLabel = prompt.message ?: ""
val inputValue = prompt.defaultValue ?: ""
val onDismiss: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: () -> Unit = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -352,7 +352,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
): GeckoResult<PromptResponse>? {
val geckoResult = GeckoResult<PromptResponse>()
val onConfirm: (String) -> Unit = { geckoResult.complete(prompt.confirm(it)) }
val onDismiss: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: () -> Unit = { prompt.dismissSafely(geckoResult) }
val defaultColor = prompt.defaultValue ?: ""
......@@ -403,7 +403,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
val geckoResult = GeckoResult<PromptResponse>()
val onSuccess = { geckoResult.complete(prompt.confirm(GECKO_PROMPT_SHARE_RESULT.SUCCESS)) }
val onFailure = { geckoResult.complete(prompt.confirm(GECKO_PROMPT_SHARE_RESULT.FAILURE)) }
val onDismiss = { geckoResult.complete(prompt.dismiss()) }
val onDismiss = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -437,7 +437,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(PromptDelegate.ButtonPrompt.Type.NEGATIVE))
}
val onDismiss: (Boolean) -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: (Boolean) -> Unit = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -573,3 +573,14 @@ internal fun Date.toString(format: String): String {
val formatter = SimpleDateFormat(format, Locale.ROOT)
return formatter.format(this) ?: ""
}
/**
* Only dismiss if the prompt is not already dismissed.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun PromptDelegate.BasePrompt.dismissSafely(geckoResult: GeckoResult<PromptResponse>) {
if (!this.isComplete) {
geckoResult.complete(dismiss())
}
}
components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
View file @ 0b8c6320
......@@ -28,8 +28,11 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.spy
import org.mockito.Mockito.verify
import org.mockito.Mockito.never
import org.mozilla.gecko.util.GeckoBundle
import org.mozilla.geckoview.Autocomplete
import org.mozilla.geckoview.GeckoResult
import org.mozilla.geckoview.GeckoRuntime
import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.DATE
......@@ -1080,6 +1083,30 @@ class GeckoPromptDelegateTest {
assertTrue(dismissWasCalled)
}
@Test
fun `dismissSafely only dismiss if the prompt is NOT already dismissed`() {
val prompt = spy(GeckoAlertPrompt())
val geckoResult = mock<GeckoResult<GeckoSession.PromptDelegate.PromptResponse>>()
doReturn(false).`when`(prompt).isComplete
prompt.dismissSafely(geckoResult)
verify(geckoResult).complete(any())
}
@Test
fun `dismissSafely do nothing if the prompt is already dismissed`() {
val prompt = spy(GeckoAlertPrompt())
val geckoResult = mock<GeckoResult<GeckoSession.PromptDelegate.PromptResponse>>()
doReturn(true).`when`(prompt).isComplete
prompt.dismissSafely(geckoResult)
verify(geckoResult, never()).complete(any())
}
class GeckoChoicePrompt(
title: String,
message: String,
......
components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt
View file @ 0b8c6320
......@@ -85,7 +85,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(Autocomplete.LoginSelectOption(login.toLoginEntry())))
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
geckoEngineSession.notifyObservers {
......@@ -110,7 +110,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(Autocomplete.LoginSelectOption(login.toLoginEntry())))
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
// Exactly one of `httpRealm` and `formSubmitURL` must be present to be a valid login entry.
......@@ -174,7 +174,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
prompt: PromptDelegate.AlertPrompt
): GeckoResult<PromptResponse> {
val geckoResult = GeckoResult<PromptResponse>()
val onConfirm: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onConfirm: () -> Unit = { prompt.dismissSafely(geckoResult) }
val title = prompt.title ?: ""
val message = prompt.message ?: ""
......@@ -219,7 +219,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}
val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}
geckoEngineSession.notifyObservers {
......@@ -298,7 +298,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}
}
val onDismiss: () -> Unit = { geckoResult.complete(geckoPrompt.dismiss()) }
val onDismiss: () -> Unit = { geckoPrompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -328,7 +328,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
val title = prompt.title ?: ""
val inputLabel = prompt.message ?: ""
val inputValue = prompt.defaultValue ?: ""
val onDismiss: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: () -> Unit = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -352,7 +352,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
): GeckoResult<PromptResponse>? {
val geckoResult = GeckoResult<PromptResponse>()
val onConfirm: (String) -> Unit = { geckoResult.complete(prompt.confirm(it)) }
val onDismiss: () -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: () -> Unit = { prompt.dismissSafely(geckoResult) }
val defaultColor = prompt.defaultValue ?: ""
......@@ -403,7 +403,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
val geckoResult = GeckoResult<PromptResponse>()
val onSuccess = { geckoResult.complete(prompt.confirm(GECKO_PROMPT_SHARE_RESULT.SUCCESS)) }
val onFailure = { geckoResult.complete(prompt.confirm(GECKO_PROMPT_SHARE_RESULT.FAILURE)) }
val onDismiss = { geckoResult.complete(prompt.dismiss()) }
val onDismiss = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -437,7 +437,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
geckoResult.complete(prompt.confirm(PromptDelegate.ButtonPrompt.Type.NEGATIVE))
}
val onDismiss: (Boolean) -> Unit = { geckoResult.complete(prompt.dismiss()) }
val onDismiss: (Boolean) -> Unit = { prompt.dismissSafely(geckoResult) }
geckoEngineSession.notifyObservers {
onPromptRequest(
......@@ -573,3 +573,13 @@ internal fun Date.toString(format: String): String {
val formatter = SimpleDateFormat(format, Locale.ROOT)
return formatter.format(this) ?: ""
}
/**
* Only dismiss if the prompt is not already dismissed.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun PromptDelegate.BasePrompt.dismissSafely(geckoResult: GeckoResult<PromptResponse>) {
if (!this.isComplete) {
geckoResult.complete(dismiss())
}
}
components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
View file @ 0b8c6320
......@@ -27,7 +27,10 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.spy
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.verify
import org.mockito.Mockito.never
import org.mozilla.gecko.util.GeckoBundle
import org.mozilla.geckoview.GeckoResult
import org.mozilla.geckoview.GeckoRuntime
import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.DATE
......@@ -957,6 +960,30 @@ class GeckoPromptDelegateTest {
assertTrue(dismissWasCalled)
}
@Test
fun `dismissSafely only dismiss if the prompt is NOT already dismissed`() {
val prompt = spy(GeckoAlertPrompt())
val geckoResult = mock<GeckoResult<GeckoSession.PromptDelegate.PromptResponse>>()
doReturn(false).`when`(prompt).isComplete
prompt.dismissSafely(geckoResult)
verify(geckoResult).complete(any())
}
@Test
fun `dismissSafely do nothing if the prompt is already dismissed`() {
val prompt = spy(GeckoAlertPrompt())
val geckoResult = mock<GeckoResult<GeckoSession.PromptDelegate.PromptResponse>>()
doReturn(true).`when`(prompt).isComplete
prompt.dismissSafely(geckoResult)
verify(geckoResult, never()).complete(any())
}
class GeckoChoicePrompt(
title: String,
message: String,
......
docs/changelog.md
View file @ 0b8c6320
......@@ -34,6 +34,9 @@ permalink: /changelog/
* **feature-prompts**
* Replaced generic icon in `LoginDialogFragment` with site icon (keep the generic one as fallback)
* **browser-engine-gecko**, **browser-engine-gecko-beta**, **browser-engine-gecko-nightly**
* 🚒 Bug fixed [issue #8240](https://github.com/mozilla-mobile/android-components/issues/8240) Crash when dismissing Share dialog.
* **feature-downloads**
* ⚠️ **This is a breaking change**: `AndroidDownloadManager.download` returns a `Strings`, `AndroidDownloadManager.tryAgain` requires a `Strings` `id` parameter.
* ⚠️ **This is a breaking change**: `ConsumeDownloadAction` requires a `Strings` `id` parameter.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment