Closes mozilla-mobile#8240: Only dismiss prompts that are

not already dismissed
Amejia481 · Aug 26, 2020 · d463184 · d463184
1 parent e47c5f0
commit d463184
Show file tree

Hide file tree

Showing 7 changed files with 149 additions and 36 deletions.
diff --git a/...-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/...-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt
@@ -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())
+    }
+}
diff --git a/...a/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/...a/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
@@ -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
@@ -145,11 +147,11 @@ class GeckoPromptDelegateTest {
         )
 
         mockSession.register(
-            object : EngineSession.Observer {
-                override fun onPromptRequest(promptRequest: PromptRequest) {
-                    promptRequestSingleChoice = promptRequest
-                }
-            })
+                object : EngineSession.Observer {
+                    override fun onPromptRequest(promptRequest: PromptRequest) {
+                        promptRequestSingleChoice = promptRequest
+                    }
+                })
 
         val geckoResult = gecko.onChoicePrompt(mock(), geckoPrompt)
         geckoResult!!.accept {
@@ -478,8 +480,7 @@ class GeckoPromptDelegateTest {
                 dateRequest = promptRequest
             }
         })
-        val geckoResult =
-            promptDelegate.onDateTimePrompt(mock(), GeckoDateTimePrompt(type = DATETIME_LOCAL))
+        val geckoResult = promptDelegate.onDateTimePrompt(mock(), GeckoDateTimePrompt(type = DATETIME_LOCAL))
         geckoResult!!.accept {
             confirmCalled = true
         }
@@ -1080,6 +1081,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,
@@ -1102,7 +1127,7 @@ class GeckoPromptDelegateTest {
         title: String = "title",
         type: Int,
         capture: Int = 0,
-        mimeTypes: Array<out String> = emptyArray()
+        mimeTypes: Array<out String > = emptyArray()
     ) : GeckoSession.PromptDelegate.FilePrompt(title, type, capture, mimeTypes)
 
     class GeckoAuthPrompt(

diff --git a/...ghtly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/...ghtly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt
@@ -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())
+    }
+}
diff --git a/...y/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/...y/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
@@ -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,