Skip to content

Commit

Permalink
[components] Closes mozilla-mobile/android-components#8240: Only dism…
Browse files Browse the repository at this point in the history
…iss prompts that are

not already dismissed
  • Loading branch information
Amejia481 committed Aug 28, 2020
1 parent 21b218b commit 24c42c9
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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 ?: ""

Expand Down Expand Up @@ -219,7 +219,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}

val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}

geckoEngineSession.notifyObservers {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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 ?: ""

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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 ?: ""

Expand Down Expand Up @@ -219,7 +219,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}

val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}

geckoEngineSession.notifyObservers {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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 ?: ""

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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 ?: ""

Expand Down Expand Up @@ -219,7 +219,7 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
}

val onDismiss: () -> Unit = {
geckoResult.complete(prompt.dismiss())
prompt.dismissSafely(geckoResult)
}

geckoEngineSession.notifyObservers {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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 ?: ""

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())
}
}
Loading

0 comments on commit 24c42c9

Please sign in to comment.