Skip to content

Commit

Permalink
For mozilla-mobile#8989 - Support multiple prompts in the same Session
Browse files Browse the repository at this point in the history
Following crash reports it was seen that it is possible for multiple prompts to
be shown at the same time with an edgecase being that one prompt request comes
after the user interacted with a previous prompt but before the consume call
completing in AC / GV time at which this code will try to use the new prompt,
not the one the user interacted with.

Having support for multiple prompt requests in ContentState and tightly
coupling a PromptDialogFragment with it's PromptRequest ensures any action
consuming a PromptDialogFragment will always consume the PromptRequest for
which that dialog was shown irrespective of the number of prompts or which is
currently shown on top.
  • Loading branch information
Mugurell committed May 26, 2021
1 parent 54bcefa commit ebfc8bf
Show file tree
Hide file tree
Showing 35 changed files with 523 additions and 289 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,8 @@ sealed class ContentAction : BrowserAction() {
/**
* Removes the [PromptRequest] of the [ContentState] with the given [sessionId].
*/
data class ConsumePromptRequestAction(val sessionId: String) : ContentAction()
data class ConsumePromptRequestAction(val sessionId: String, val promptRequest: PromptRequest) :
ContentAction()

/**
* Adds a [FindResultState] to the [ContentState] with the given [sessionId].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ internal object ContentStateReducer {
it.copy(hitResult = null)
}
is ContentAction.UpdatePromptRequestAction -> updateContentState(state, action.sessionId) {
it.copy(promptRequest = action.promptRequest)
it.copy(promptRequests = it.promptRequests + action.promptRequest)
}
is ContentAction.ConsumePromptRequestAction -> updateContentState(state, action.sessionId) {
it.copy(promptRequest = null)
it.copy(promptRequests = it.promptRequests - action.promptRequest)
}
is ContentAction.AddFindResultAction -> updateContentState(state, action.sessionId) {
it.copy(findResults = it.findResults + action.findResult)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import mozilla.components.concept.engine.window.WindowRequest
* @property download Last unhandled download request.
* @property share Last unhandled request to share an internet resource that first needs to be downloaded.
* @property hitResult the target of the latest long click operation.
* @property promptRequest the last received [PromptRequest].
* @property promptRequests current[PromptRequest]s.
* @property findResults the list of results of the latest "find in page" operation.
* @property windowRequest the last received [WindowRequest].
* @property searchRequest the last received [SearchRequest]
Expand Down Expand Up @@ -74,7 +74,7 @@ data class ContentState(
val download: DownloadState? = null,
val share: ShareInternetResourceState? = null,
val hitResult: HitResult? = null,
val promptRequest: PromptRequest? = null,
val promptRequests: List<PromptRequest> = emptyList(),
val findResults: List<FindResultState> = emptyList(),
val windowRequest: WindowRequest? = null,
val searchRequest: SearchRequest? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,24 +470,27 @@ class ContentActionTest {
}

@Test
fun `UpdatePromptRequestAction updates request`() {
assertNull(tab.content.promptRequest)
fun `UpdatePromptRequestAction updates requests`() {
assertTrue(tab.content.promptRequests.isEmpty())

val promptRequest1: PromptRequest = mock()

store.dispatch(
ContentAction.UpdatePromptRequestAction(tab.id, promptRequest1)
).joinBlocking()

assertEquals(promptRequest1, tab.content.promptRequest)
assertEquals(1, tab.content.promptRequests.size)
assertEquals(promptRequest1, tab.content.promptRequests[0])

val promptRequest2: PromptRequest = mock()

store.dispatch(
ContentAction.UpdatePromptRequestAction(tab.id, promptRequest2)
).joinBlocking()

assertEquals(promptRequest2, tab.content.promptRequest)
assertEquals(2, tab.content.promptRequests.size)
assertEquals(promptRequest1, tab.content.promptRequests[0])
assertEquals(promptRequest2, tab.content.promptRequests[1])
}

@Test
Expand All @@ -498,13 +501,14 @@ class ContentActionTest {
ContentAction.UpdatePromptRequestAction(tab.id, promptRequest)
).joinBlocking()

assertEquals(promptRequest, tab.content.promptRequest)
assertEquals(1, tab.content.promptRequests.size)
assertEquals(promptRequest, tab.content.promptRequests[0])

store.dispatch(
ContentAction.ConsumePromptRequestAction(tab.id)
ContentAction.ConsumePromptRequestAction(tab.id, promptRequest)
).joinBlocking()

assertNull(tab.content.promptRequest)
assertTrue(tab.content.promptRequests.isEmpty())
}

@Test
Expand Down Expand Up @@ -722,7 +726,7 @@ class ContentActionTest {

@Test
fun `UpdateAppIntentAction updates request`() {
assertNull(tab.content.promptRequest)
assertTrue(tab.content.promptRequests.isEmpty())

val appIntent1: AppIntentState = mock()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,24 @@ package mozilla.components.concept.engine.prompt

import android.content.Context
import android.net.Uri
import mozilla.components.concept.storage.Login
import mozilla.components.concept.engine.prompt.PromptRequest.Authentication.Level
import mozilla.components.concept.engine.prompt.PromptRequest.Authentication.Method
import mozilla.components.concept.engine.prompt.PromptRequest.TimeSelection.Type
import mozilla.components.concept.storage.Login
import java.util.UUID

/**
* Value type that represents a request for showing a native dialog for prompt web content.
*
* @param shouldDismissOnLoad Whether or not the dialog should automatically be dismissed when a new page is loaded.
* Defaults to `true`.
* @param uid [PromptRequest] unique identifier. Defaults to a random UUID.
* (This two parameters, though present in all subclasses are not evaluated in subclasses equals() calls)
*/
sealed class PromptRequest {
sealed class PromptRequest(
val shouldDismissOnLoad: Boolean = true,
val uid: String = UUID.randomUUID().toString()
) {
/**
* Value type that represents a request for a single choice prompt.
* @property choices All the possible options.
Expand Down
Loading

0 comments on commit ebfc8bf

Please sign in to comment.