Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Closes #8240: Only dismiss prompts that are not already dismissed #8244

Merged
merged 1 commit into from
Aug 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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