Commit 8f1444d7 authored by Christian Sadilek's avatar Christian Sadilek
Browse files

Closes #4289: Migrate feature-prompts to use browser-state

parent 0e774734
......@@ -21,8 +21,14 @@ android {
}
}
tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all {
kotlinOptions.freeCompilerArgs += [
"-Xuse-experimental=kotlinx.coroutines.ExperimentalCoroutinesApi"
]
}
dependencies {
implementation project(':browser-session')
implementation project(':browser-state')
implementation project(':concept-engine')
implementation project(':lib-state')
implementation project(':support-ktx')
......@@ -34,6 +40,7 @@ dependencies {
testImplementation Dependencies.androidx_test_core
testImplementation Dependencies.androidx_test_junit
testImplementation Dependencies.testing_coroutines
testImplementation Dependencies.testing_robolectric
testImplementation Dependencies.testing_mockito
testImplementation project(':support-test')
......
......@@ -14,7 +14,7 @@ import android.provider.MediaStore.EXTRA_OUTPUT
import androidx.annotation.VisibleForTesting
import androidx.annotation.VisibleForTesting.PRIVATE
import androidx.fragment.app.Fragment
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.prompt.PromptRequest
import mozilla.components.concept.engine.prompt.PromptRequest.File
import mozilla.components.support.base.feature.PermissionsFeature
......@@ -25,8 +25,7 @@ typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
/**
* @property activity The [Activity] which hosts the file picker.
* @property fragment The [Fragment] which hosts the file picker.
* @property sessionManager The [SessionManager] instance in order to subscribe
* to the selected [mozilla.components.browser.session.Session].
* @property store The [BrowserStore] this feature should subscribe to.
* @property onNeedToRequestPermissions a callback invoked when permissions
* need to be requested before a prompt (e.g. a file picker) can be displayed.
* Once the request is completed, [onPermissionsResult] needs to be invoked.
......@@ -34,22 +33,22 @@ typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
internal class FilePicker private constructor(
private val activity: Activity? = null,
private val fragment: Fragment? = null,
private val sessionManager: SessionManager,
private val store: BrowserStore,
private var sessionId: String? = null,
override val onNeedToRequestPermissions: OnNeedToRequestPermissions
) : PermissionsFeature {
constructor(
activity: Activity,
sessionManager: SessionManager,
store: BrowserStore,
sessionId: String? = null,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(activity, null, sessionManager, sessionId, onNeedToRequestPermissions)
) : this(activity, null, store, sessionId, onNeedToRequestPermissions)
constructor(
fragment: Fragment,
sessionManager: SessionManager,
store: BrowserStore,
sessionId: String? = null,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(null, fragment, sessionManager, sessionId, onNeedToRequestPermissions)
) : this(null, fragment, store, sessionId, onNeedToRequestPermissions)
private val context get() = activity ?: requireNotNull(fragment).requireContext()
......@@ -116,7 +115,7 @@ internal class FilePicker private constructor(
*/
fun onActivityResult(requestCode: Int, resultCode: Int, intent: Intent?) {
if (requestCode == FILE_PICKER_ACTIVITY_REQUEST_CODE) {
sessionManager.consumePromptFrom(sessionId) {
store.consumePromptFrom(sessionId) {
val request = it as File
if (resultCode == RESULT_OK) {
......@@ -124,8 +123,6 @@ internal class FilePicker private constructor(
} else {
request.onDismiss()
}
true
}
}
}
......@@ -156,9 +153,8 @@ internal class FilePicker private constructor(
*/
@VisibleForTesting(otherwise = PRIVATE)
internal fun onPermissionsGranted() {
sessionManager.consumePromptFrom(sessionId) { promptRequest ->
store.consumePromptFrom(sessionId) { promptRequest ->
handleFileRequest(promptRequest as File, requestPermissions = false)
false
}
}
......@@ -168,9 +164,8 @@ internal class FilePicker private constructor(
*/
@VisibleForTesting(otherwise = PRIVATE)
internal fun onPermissionsDenied() {
sessionManager.consumePromptFrom(sessionId) { request ->
store.consumePromptFrom(sessionId) { request ->
if (request is File) request.onDismiss()
true
}
}
......
......@@ -10,10 +10,16 @@ import androidx.annotation.VisibleForTesting
import androidx.annotation.VisibleForTesting.PRIVATE
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager
import mozilla.components.browser.session.SelectionAwareSessionObserver
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.session.runWithSessionIdOrSelected
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.selector.findCustomTabOrSelectedTab
import mozilla.components.browser.state.selector.findTabOrCustomTab
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.state.SessionState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.prompt.Choice
import mozilla.components.concept.engine.prompt.PromptRequest
import mozilla.components.concept.engine.prompt.PromptRequest.Alert
......@@ -28,9 +34,11 @@ import mozilla.components.concept.engine.prompt.PromptRequest.TimeSelection
import mozilla.components.feature.prompts.ChoiceDialogFragment.Companion.MENU_CHOICE_DIALOG_TYPE
import mozilla.components.feature.prompts.ChoiceDialogFragment.Companion.MULTIPLE_CHOICE_DIALOG_TYPE
import mozilla.components.feature.prompts.ChoiceDialogFragment.Companion.SINGLE_CHOICE_DIALOG_TYPE
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.base.feature.LifecycleAwareFeature
import mozilla.components.support.base.feature.OnNeedToRequestPermissions
import mozilla.components.support.base.feature.PermissionsFeature
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged
import java.security.InvalidParameterException
import java.util.Date
......@@ -60,8 +68,10 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog"
* [Activity], this parameter can be ignored. Note that an
* [IllegalStateException] will be thrown if neither an active nor a fragment
* is specified.
* @property sessionManager The [SessionManager] instance in order to subscribe
* to the selected [Session].
* @property store The [BrowserStore] this feature should subscribe to.
* @property customTabId Optional id of a custom tab. Instead of showing context
* menus for the currently selected tab this feature will show only context menus
* for this custom tab if an id is provided.
* @property fragmentManager The [FragmentManager] to be used when displaying
* a dialog (fragment).
* @property onNeedToRequestPermissions a callback invoked when permissions
......@@ -72,30 +82,33 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog"
class PromptFeature(
private val activity: Activity? = null,
private val fragment: Fragment? = null,
private val sessionManager: SessionManager,
private var sessionId: String? = null,
private val store: BrowserStore,
private var customTabId: String? = null,
private val fragmentManager: FragmentManager,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : LifecycleAwareFeature, PermissionsFeature {
private var scope: CoroutineScope? = null
private var activePromptRequest: PromptRequest? = null
internal val promptAbuserDetector = PromptAbuserDetector()
constructor(
activity: Activity,
sessionManager: SessionManager,
store: BrowserStore,
sessionId: String? = null,
fragmentManager: FragmentManager,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(
activity, null, sessionManager, sessionId, fragmentManager, onNeedToRequestPermissions
activity, null, store, sessionId, fragmentManager, onNeedToRequestPermissions
)
constructor(
fragment: Fragment,
sessionManager: SessionManager,
store: BrowserStore,
sessionId: String? = null,
fragmentManager: FragmentManager,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(
null, fragment, sessionManager, sessionId, fragmentManager, onNeedToRequestPermissions
null, fragment, store, sessionId, fragmentManager, onNeedToRequestPermissions
)
init {
......@@ -107,14 +120,12 @@ class PromptFeature(
}
}
private val observer = PromptRequestObserver(sessionManager, feature = this)
private val context get() = activity ?: requireNotNull(fragment).requireContext()
private val filePicker = if (activity != null) {
FilePicker(activity, sessionManager, sessionId, onNeedToRequestPermissions)
FilePicker(activity, store, customTabId, onNeedToRequestPermissions)
} else {
FilePicker(requireNotNull(fragment), sessionManager, sessionId, onNeedToRequestPermissions)
FilePicker(requireNotNull(fragment), store, customTabId, onNeedToRequestPermissions)
}
override val onNeedToRequestPermissions
......@@ -126,7 +137,23 @@ class PromptFeature(
*/
override fun start() {
promptAbuserDetector.resetJSAlertAbuseState()
observer.observeIdOrSelected(sessionId)
scope = store.flowScoped { flow ->
flow.map { state -> state.findCustomTabOrSelectedTab(customTabId) }
.ifAnyChanged {
arrayOf(it?.content?.promptRequest, it?.content?.loading)
}
.collect { state ->
state?.content?.let {
if (it.promptRequest != activePromptRequest) {
onPromptRequested(state)
} else if (!it.loading) {
promptAbuserDetector.resetJSAlertAbuseState()
}
activePromptRequest = it.promptRequest
}
}
}
fragmentManager.findFragmentByTag(FRAGMENT_TAG)?.let { fragment ->
// There's still a [PromptDialogFragment] visible from the last time. Re-attach this feature so that the
......@@ -140,7 +167,7 @@ class PromptFeature(
* Stops observing the selected session for incoming prompt requests.
*/
override fun stop() {
observer.stop()
scope?.cancel()
}
/**
......@@ -168,33 +195,32 @@ class PromptFeature(
/**
* Invoked when a native dialog needs to be shown.
* Displays a suitable dialog for the pending [promptRequest].
*
* @param session The session which requested the dialog.
* @param promptRequest The session the request the dialog.
*/
@VisibleForTesting(otherwise = PRIVATE)
internal fun onPromptRequested(session: Session, promptRequest: PromptRequest) {
internal fun onPromptRequested(session: SessionState) {
// Some requests are handle with intents
when (promptRequest) {
is File -> filePicker.handleFileRequest(promptRequest)
else -> handleDialogsRequest(promptRequest, session)
session.content.promptRequest?.let { promptRequest ->
when (promptRequest) {
is File -> filePicker.handleFileRequest(promptRequest)
else -> handleDialogsRequest(promptRequest, session)
}
}
}
/**
* Invoked when a dialog is dismissed. This consumes the [PromptFeature]
* value from the [Session] indicated by [sessionId].
* value from the session indicated by [sessionId].
*
* @param sessionId this is the id of the session which requested the prompt.
*/
internal fun onCancel(sessionId: String) {
sessionManager.consumePromptFrom(sessionId) {
store.consumePromptFrom(sessionId) {
when (it) {
is PromptRequest.Dismissible -> it.onDismiss()
is PromptRequest.Popup -> it.onDeny()
}
true
}
}
......@@ -207,7 +233,7 @@ class PromptFeature(
*/
@Suppress("UNCHECKED_CAST", "ComplexMethod")
internal fun onConfirm(sessionId: String, value: Any? = null) {
sessionManager.consumePromptFrom(sessionId) {
store.consumePromptFrom(sessionId) {
when (it) {
is TimeSelection -> it.onConfirm(value as Date)
is Color -> it.onConfirm(value as String)
......@@ -248,7 +274,6 @@ class PromptFeature(
}
}
}
true
}
}
......@@ -259,11 +284,10 @@ class PromptFeature(
* @param sessionId that requested to show the dialog.
*/
internal fun onClear(sessionId: String) {
sessionManager.consumePromptFrom(sessionId) {
store.consumePromptFrom(sessionId) {
when (it) {
is TimeSelection -> it.onClear()
}
true
}
}
......@@ -271,8 +295,8 @@ class PromptFeature(
* Re-attaches a fragment that is still visible but not linked to this feature anymore.
*/
private fun reattachFragment(fragment: PromptDialogFragment) {
val session = sessionManager.findSessionById(fragment.sessionId)
if (session == null || session.promptRequest.isConsumed()) {
val session = store.state.findTabOrCustomTab(fragment.sessionId)
if (session?.content?.promptRequest == null) {
fragmentManager.beginTransaction()
.remove(fragment)
.commitAllowingStateLoss()
......@@ -283,11 +307,11 @@ class PromptFeature(
fragment.feature = this
}
@Suppress("ComplexMethod")
@Suppress("ComplexMethod", "LongMethod")
@VisibleForTesting(otherwise = PRIVATE)
internal fun handleDialogsRequest(
promptRequest: PromptRequest,
session: Session
session: SessionState
) {
// Requests that are handled with dialogs
val dialog = when (promptRequest) {
......@@ -415,6 +439,7 @@ class PromptFeature(
dialog.show(fragmentManager, FRAGMENT_TAG)
} else {
(promptRequest as PromptRequest.Dismissible).onDismiss()
store.dispatch(ContentAction.ConsumePromptRequestAction(session.id))
}
promptAbuserDetector.updateJSDialogAbusedState()
}
......@@ -433,37 +458,23 @@ class PromptFeature(
}
}
/**
* Observes [Session.Observer.onPromptRequested] of the selected session
* and notifies the feature whenever a prompt needs to be shown.
*/
internal class PromptRequestObserver(
sessionManager: SessionManager,
private val feature: PromptFeature
) : SelectionAwareSessionObserver(sessionManager) {
override fun onPromptRequested(session: Session, promptRequest: PromptRequest): Boolean {
feature.onPromptRequested(session, promptRequest)
return false
}
override fun onLoadingStateChanged(session: Session, loading: Boolean) {
if (!loading) {
feature.promptAbuserDetector.resetJSAlertAbuseState()
}
}
}
companion object {
const val FILE_PICKER_ACTIVITY_REQUEST_CODE = 1234
}
}
internal fun SessionManager.consumePromptFrom(
internal fun BrowserStore.consumePromptFrom(
sessionId: String?,
consumer: (PromptRequest) -> Boolean
consume: (PromptRequest) -> Unit
) {
runWithSessionIdOrSelected(sessionId) {
it.promptRequest.consume(consumer)
if (sessionId == null) {
state.selectedTab
} else {
state.findTabOrCustomTab(sessionId)
}?.let { tab ->
tab.content.promptRequest?.let {
consume(it)
dispatch(ContentAction.ConsumePromptRequestAction(tab.id))
}
}
}
......@@ -17,20 +17,23 @@ import android.net.Uri
import androidx.fragment.app.Fragment
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.ContentState
import mozilla.components.browser.state.state.CustomTabSessionState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.prompt.PromptRequest
import mozilla.components.support.base.observer.Consumable
import mozilla.components.support.test.any
import mozilla.components.support.test.eq
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.grantPermission
import mozilla.components.support.test.whenever
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.`when`
import org.mockito.Mockito.anyInt
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.never
......@@ -49,34 +52,35 @@ class FilePickerTest {
onDismiss = {}
)
private lateinit var mockFragment: Fragment
private lateinit var mockSessionManager: SessionManager
private lateinit var fragment: Fragment
private lateinit var store: BrowserStore
private lateinit var state: BrowserState
private lateinit var filePicker: FilePicker
@Before
fun setup() {
mockFragment = mock()
mockSessionManager = spy(SessionManager(mock()))
filePicker = FilePicker(mockFragment, mockSessionManager) { }
fragment = mock()
state = mock()
store = mock()
whenever(store.state).thenReturn(state)
filePicker = FilePicker(fragment, store) { }
}
@Test
fun `FilePicker acts on a given session or the selected session`() {
val session = Session("custom-tab")
`when`(mockSessionManager.findSessionById(session.id)).thenReturn(session)
fun `FilePicker acts on a given (custom tab) session or the selected session`() {
val customTabContent: ContentState = mock()
whenever(customTabContent.promptRequest).thenReturn(request)
val customTab = CustomTabSessionState("custom-tab", customTabContent, mock(), mock())
filePicker = FilePicker(mockFragment, mockSessionManager, session.id) { }
whenever(state.customTabs).thenReturn(listOf(customTab))
filePicker = FilePicker(fragment, store, customTab.id) { }
filePicker.onActivityResult(FilePicker.FILE_PICKER_ACTIVITY_REQUEST_CODE, 0, null)
verify(store).dispatch(ContentAction.ConsumePromptRequestAction(customTab.id))
verify(mockSessionManager).findSessionById(session.id)
val selected = Session("browser-tab")
`when`(mockSessionManager.selectedSession).thenReturn(selected)
filePicker = FilePicker(mockFragment, mockSessionManager) { }
val selected = prepareSelectedSession(request)
filePicker = FilePicker(fragment, store) { }
filePicker.onActivityResult(FilePicker.FILE_PICKER_ACTIVITY_REQUEST_CODE, 0, null)
verify(mockSessionManager).selectedSession
verify(store).dispatch(ContentAction.ConsumePromptRequestAction(selected.id))
}
@Test
......@@ -86,13 +90,11 @@ class FilePickerTest {
val intent = Intent()
val code = 1
filePicker = FilePicker(mockActivity, mockSessionManager) { }
filePicker = FilePicker(mockActivity, store) { }
filePicker.startActivityForResult(intent, code)
verify(mockActivity).startActivityForResult(intent, code)
filePicker = FilePicker(mockFragment, mockSessionManager) { }
filePicker = FilePicker(mockFragment, store) { }
filePicker.startActivityForResult(intent, code)
verify(mockFragment).startActivityForResult(intent, code)
}
......@@ -102,16 +104,16 @@ class FilePickerTest {
var onRequestPermissionWasCalled = false
val context = ApplicationProvider.getApplicationContext<Context>()
filePicker = FilePicker(mockFragment, mockSessionManager) {
filePicker = FilePicker(fragment, store) {
onRequestPermissionWasCalled = true
}
doReturn(context).`when`(mockFragment).requireContext()
doReturn(context).`when`(fragment).requireContext()
filePicker.handleFileRequest(request)
assertTrue(onRequestPermissionWasCalled)
verify(mockFragment, never()).startActivityForResult(Intent(), 1)
verify(fragment, never()).startActivityForResult(Intent(), 1)
}
@Test
......@@ -120,7 +122,7 @@ class FilePickerTest {
var onRequestPermissionWasCalled = false
val context = ApplicationProvider.getApplicationContext<Context>()
filePicker = FilePicker(mockFragment, mockSessionManager) {
filePicker = FilePicker(mockFragment, store) {
onRequestPermissionWasCalled = true
}
......@@ -135,20 +137,14 @@ class FilePickerTest {
}
@Test
fun `onPermissionsGranted will forward its call to filePickerRequest`() {
val session = getSelectedSession()
session.promptRequest = Consumable.from(request)
fun `onPermissionsGranted will forward call to filePickerRequest`() {
val selected = prepareSelectedSession(request)
stubContext()
filePicker = spy(filePicker)
filePicker.onPermissionsGranted()
verify(mockSessionManager).selectedSession
verify(filePicker).handleFileRequest(any(), eq(false))
assertFalse(session.promptRequest.isConsumed())
verify(store).dispatch(ContentAction.ConsumePromptRequestAction(selected.id))
}
@Test
......@@ -158,22 +154,18 @@ class FilePickerTest {
onDismissWasCalled = true
}
val session = getSelectedSession()
val selected = prepareSelectedSession(filePickerRequest)
stubContext()
session.promptRequest = Consumable.from(filePickerRequest)
filePicker.onPermissionsDenied()
verify(mockSessionManager).selectedSession
assertTrue(onDismissWasCalled)
assertTrue(session.promptRequest.isConsumed())
verify(store).dispatch(ContentAction.ConsumePromptRequestAction(selected.id))
}
@Test
fun `onActivityResult with RESULT_OK and isMultipleFilesSelection false will consume PromptRequest of the actual session`() {
var onSingleFileSelectionWasCalled = false
val onSingleFileSelection: (Context, Uri) -> Unit = { _, _ ->
......@@ -182,24 +174,21 @@ class FilePickerTest {
val filePickerRequest = request.copy(onSingleFileSelected = onSingleFileSelection)
val session = getSelectedSession()
val selected = prepareSelectedSession(filePickerRequest)
val intent = Intent()