Commit 0372898e authored by MozLando's avatar MozLando
Browse files

Merge #4788



4788: Abstract away activity/fragment in prompt feature r=Amejia481 a=NotWoods
Co-authored-by: default avatarTiger Oakes <toakes@mozilla.com>
parents 5d387acc a3abde47
...@@ -23,34 +23,18 @@ import mozilla.components.support.ktx.android.content.isPermissionGranted ...@@ -23,34 +23,18 @@ import mozilla.components.support.ktx.android.content.isPermissionGranted
typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
/** /**
* @property activity The [Activity] which hosts the file picker. * @property container The [Activity] or [Fragment] which hosts the file picker.
* @property fragment The [Fragment] which hosts the file picker.
* @property store The [BrowserStore] this feature should subscribe to. * @property store The [BrowserStore] this feature should subscribe to.
* @property onNeedToRequestPermissions a callback invoked when permissions * @property onNeedToRequestPermissions a callback invoked when permissions
* need to be requested before a prompt (e.g. a file picker) can be displayed. * 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. * Once the request is completed, [onPermissionsResult] needs to be invoked.
*/ */
internal class FilePicker private constructor( internal class FilePicker(
private val activity: Activity? = null, private val container: PromptContainer,
private val fragment: Fragment? = null,
private val store: BrowserStore, private val store: BrowserStore,
private var sessionId: String? = null, private var sessionId: String? = null,
override val onNeedToRequestPermissions: OnNeedToRequestPermissions override val onNeedToRequestPermissions: OnNeedToRequestPermissions
) : PermissionsFeature { ) : PermissionsFeature {
constructor(
activity: Activity,
store: BrowserStore,
sessionId: String? = null,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(activity, null, store, sessionId, onNeedToRequestPermissions)
constructor(
fragment: Fragment,
store: BrowserStore,
sessionId: String? = null,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(null, fragment, store, sessionId, onNeedToRequestPermissions)
private val context get() = activity ?: requireNotNull(fragment).requireContext()
/** /**
* The image capture intent doesn't return the URI where the image is saved, * The image capture intent doesn't return the URI where the image is saved,
...@@ -68,20 +52,20 @@ internal class FilePicker private constructor( ...@@ -68,20 +52,20 @@ internal class FilePicker private constructor(
// Compare the accepted values against image/*, video/*, and audio/* // Compare the accepted values against image/*, video/*, and audio/*
for (type in MimeType.values()) { for (type in MimeType.values()) {
val hasPermission = context.isPermissionGranted(type.permission) val hasPermission = container.context.isPermissionGranted(type.permission)
// The captureMode attribute can be used if the accepted types are exactly for // The captureMode attribute can be used if the accepted types are exactly for
// image/*, video/*, or audio/*. // image/*, video/*, or audio/*.
if (hasPermission && type.shouldCapture(promptRequest.mimeTypes, promptRequest.captureMode)) { if (hasPermission && type.shouldCapture(promptRequest.mimeTypes, promptRequest.captureMode)) {
type.buildIntent(context, promptRequest)?.also { type.buildIntent(container.context, promptRequest)?.also {
saveCaptureUriIfPresent(it) saveCaptureUriIfPresent(it)
startActivityForResult(it, FILE_PICKER_ACTIVITY_REQUEST_CODE) container.startActivityForResult(it, R.id.mozac_feature_prompts_file_picker_activity_request_code)
return return
} }
} }
// Otherwise, build the intent and create a chooser later // Otherwise, build the intent and create a chooser later
if (type.matches(promptRequest.mimeTypes)) { if (type.matches(promptRequest.mimeTypes)) {
if (hasPermission) { if (hasPermission) {
type.buildIntent(context, promptRequest)?.also { type.buildIntent(container.context, promptRequest)?.also {
saveCaptureUriIfPresent(it) saveCaptureUriIfPresent(it)
intents.add(it) intents.add(it)
} }
...@@ -100,7 +84,7 @@ internal class FilePicker private constructor( ...@@ -100,7 +84,7 @@ internal class FilePicker private constructor(
putExtra(EXTRA_INITIAL_INTENTS, intents.toTypedArray()) putExtra(EXTRA_INITIAL_INTENTS, intents.toTypedArray())
} }
startActivityForResult(chooser, FILE_PICKER_ACTIVITY_REQUEST_CODE) container.startActivityForResult(chooser, R.id.mozac_feature_prompts_file_picker_activity_request_code)
} else { } else {
onNeedToRequestPermissions(neededPermissions.toTypedArray()) onNeedToRequestPermissions(neededPermissions.toTypedArray())
} }
...@@ -114,7 +98,7 @@ internal class FilePicker private constructor( ...@@ -114,7 +98,7 @@ internal class FilePicker private constructor(
* @param intent The result of the request. * @param intent The result of the request.
*/ */
fun onActivityResult(requestCode: Int, resultCode: Int, intent: Intent?) { fun onActivityResult(requestCode: Int, resultCode: Int, intent: Intent?) {
if (requestCode == FILE_PICKER_ACTIVITY_REQUEST_CODE) { if (requestCode == R.id.mozac_feature_prompts_file_picker_activity_request_code) {
store.consumePromptFrom(sessionId) { store.consumePromptFrom(sessionId) {
val request = it as File val request = it as File
...@@ -174,28 +158,16 @@ internal class FilePicker private constructor( ...@@ -174,28 +158,16 @@ internal class FilePicker private constructor(
if (intent?.clipData != null && request.isMultipleFilesSelection) { if (intent?.clipData != null && request.isMultipleFilesSelection) {
intent.clipData?.run { intent.clipData?.run {
val uris = Array<Uri>(itemCount) { index -> getItemAt(index).uri } val uris = Array<Uri>(itemCount) { index -> getItemAt(index).uri }
request.onMultipleFilesSelected(context, uris) request.onMultipleFilesSelected(container.context, uris)
} }
} else { } else {
val uri = intent?.data ?: captureUri val uri = intent?.data ?: captureUri
uri?.let { uri?.let {
request.onSingleFileSelected(context, it) request.onSingleFileSelected(container.context, it)
} ?: request.onDismiss } ?: request.onDismiss
} }
} }
private fun saveCaptureUriIfPresent(intent: Intent) = private fun saveCaptureUriIfPresent(intent: Intent) =
intent.getParcelableExtra<Uri>(EXTRA_OUTPUT)?.let { captureUri = it } intent.getParcelableExtra<Uri>(EXTRA_OUTPUT)?.let { captureUri = it }
internal fun startActivityForResult(intent: Intent, code: Int) {
if (activity != null) {
activity.startActivityForResult(intent, code)
} else {
requireNotNull(fragment).startActivityForResult(intent, code)
}
}
companion object {
const val FILE_PICKER_ACTIVITY_REQUEST_CODE = 1234
}
} }
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.feature.prompts
import android.content.Context
import android.content.Intent
import androidx.annotation.StringRes
/**
* Wrapper to hold shared functionality between activities and fragments for [PromptFeature].
*/
internal sealed class PromptContainer {
/**
* Getter for [Context].
*/
abstract val context: Context
/**
* Launches an activity for which you would like a result when it finished.
*/
abstract fun startActivityForResult(intent: Intent, code: Int)
/**
* Returns a localized string.
*/
abstract fun getString(@StringRes res: Int): String
internal class Activity(
private val activity: android.app.Activity
) : PromptContainer() {
override val context get() = activity
override fun startActivityForResult(intent: Intent, code: Int) =
activity.startActivityForResult(intent, code)
override fun getString(res: Int) = activity.getString(res)
}
internal class Fragment(
private val fragment: androidx.fragment.app.Fragment
) : PromptContainer() {
override val context get() = fragment.requireContext()
override fun startActivityForResult(intent: Intent, code: Int) =
fragment.startActivityForResult(intent, code)
override fun getString(res: Int) = fragment.getString(res)
}
}
...@@ -55,19 +55,12 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog" ...@@ -55,19 +55,12 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog"
* onActivityResult in your [Activity] or [Fragment] and forward its calls * onActivityResult in your [Activity] or [Fragment] and forward its calls
* to [onActivityResult]. * to [onActivityResult].
* *
* This feature will subscribe to the currently selected [Session] and display * This feature will subscribe to the currently selected session and display
* a suitable native dialog based on [Session.Observer.onPromptRequested] events. * a suitable native dialog based on [Session.Observer.onPromptRequested] events.
* Once the dialog is closed or the user selects an item from the dialog * Once the dialog is closed or the user selects an item from the dialog
* the related [PromptRequest] will be consumed. * the related [PromptRequest] will be consumed.
* *
* @property activity The [Activity] which hosts this feature. If hosted by a * @property container The [Activity] or [Fragment] which hosts this feature.
* [Fragment], this parameter can be ignored. Note that an
* [IllegalStateException] will be thrown if neither an active nor a fragment
* is specified.
* @property fragment The [Fragment] which hosts this feature. If hosted by an
* [Activity], this parameter can be ignored. Note that an
* [IllegalStateException] will be thrown if neither an active nor a fragment
* is specified.
* @property store The [BrowserStore] this feature should subscribe to. * @property store The [BrowserStore] this feature should subscribe to.
* @property customTabId Optional id of a custom tab. Instead of showing context * @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 * menus for the currently selected tab this feature will show only context menus
...@@ -79,9 +72,8 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog" ...@@ -79,9 +72,8 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog"
* Once the request is completed, [onPermissionsResult] needs to be invoked. * Once the request is completed, [onPermissionsResult] needs to be invoked.
*/ */
@Suppress("TooManyFunctions") @Suppress("TooManyFunctions")
class PromptFeature( class PromptFeature internal constructor(
private val activity: Activity? = null, private val container: PromptContainer,
private val fragment: Fragment? = null,
private val store: BrowserStore, private val store: BrowserStore,
private var customTabId: String? = null, private var customTabId: String? = null,
private val fragmentManager: FragmentManager, private val fragmentManager: FragmentManager,
...@@ -95,38 +87,43 @@ class PromptFeature( ...@@ -95,38 +87,43 @@ class PromptFeature(
constructor( constructor(
activity: Activity, activity: Activity,
store: BrowserStore, store: BrowserStore,
sessionId: String? = null, customTabId: String? = null,
fragmentManager: FragmentManager, fragmentManager: FragmentManager,
onNeedToRequestPermissions: OnNeedToRequestPermissions onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this( ) : this(
activity, null, store, sessionId, fragmentManager, onNeedToRequestPermissions PromptContainer.Activity(activity), store, customTabId, fragmentManager, onNeedToRequestPermissions
) )
constructor( constructor(
fragment: Fragment, fragment: Fragment,
store: BrowserStore, store: BrowserStore,
sessionId: String? = null, customTabId: String? = null,
fragmentManager: FragmentManager, fragmentManager: FragmentManager,
onNeedToRequestPermissions: OnNeedToRequestPermissions onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this( ) : this(
null, fragment, store, sessionId, fragmentManager, onNeedToRequestPermissions PromptContainer.Fragment(fragment), store, customTabId, fragmentManager, onNeedToRequestPermissions
) )
@Deprecated("Pass only activity or fragment instead")
init { constructor(
if (activity == null && fragment == null) { activity: Activity? = null,
throw IllegalStateException( fragment: Fragment? = null,
store: BrowserStore,
customTabId: String? = null,
fragmentManager: FragmentManager,
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(
activity?.let { PromptContainer.Activity(it) }
?: fragment?.let { PromptContainer.Fragment(it) }
?: throw IllegalStateException(
"activity and fragment references " + "activity and fragment references " +
"must not be both null, at least one must be initialized." "must not be both null, at least one must be initialized."
) ),
} store,
} customTabId,
fragmentManager,
onNeedToRequestPermissions
)
private val context get() = activity ?: requireNotNull(fragment).requireContext() private val filePicker = FilePicker(container, store, customTabId, onNeedToRequestPermissions)
private val filePicker = if (activity != null) {
FilePicker(activity, store, customTabId, onNeedToRequestPermissions)
} else {
FilePicker(requireNotNull(fragment), store, customTabId, onNeedToRequestPermissions)
}
override val onNeedToRequestPermissions override val onNeedToRequestPermissions
get() = filePicker.onNeedToRequestPermissions get() = filePicker.onNeedToRequestPermissions
...@@ -248,23 +245,21 @@ class PromptFeature( ...@@ -248,23 +245,21 @@ class PromptFeature(
is MultipleChoice -> it.onConfirm(value as Array<Choice>) is MultipleChoice -> it.onConfirm(value as Array<Choice>)
is Authentication -> { is Authentication -> {
val pair = value as Pair<String, String> val (user, password) = value as Pair<String, String>
it.onConfirm(pair.first, pair.second) it.onConfirm(user, password)
} }
is TextPrompt -> { is TextPrompt -> {
val pair = value as Pair<Boolean, String> val (shouldNotShowMoreDialogs, text) = value as Pair<Boolean, String>
val shouldNotShowMoreDialogs = pair.first
promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
it.onConfirm(!shouldNotShowMoreDialogs, pair.second) it.onConfirm(!shouldNotShowMoreDialogs, text)
} }
is PromptRequest.Confirm -> { is PromptRequest.Confirm -> {
val pair = value as Pair<Boolean, MultiButtonDialogFragment.ButtonType> val (isCheckBoxChecked, buttonType) = value as Pair<Boolean, MultiButtonDialogFragment.ButtonType>
val isCheckBoxChecked = pair.first
promptAbuserDetector.userWantsMoreDialogs(!isCheckBoxChecked) promptAbuserDetector.userWantsMoreDialogs(!isCheckBoxChecked)
when (pair.second) { when (buttonType) {
MultiButtonDialogFragment.ButtonType.POSITIVE -> MultiButtonDialogFragment.ButtonType.POSITIVE ->
it.onConfirmPositiveButton(!isCheckBoxChecked) it.onConfirmPositiveButton(!isCheckBoxChecked)
MultiButtonDialogFragment.ButtonType.NEGATIVE -> MultiButtonDialogFragment.ButtonType.NEGATIVE ->
...@@ -392,9 +387,9 @@ class PromptFeature( ...@@ -392,9 +387,9 @@ class PromptFeature(
is PromptRequest.Popup -> { is PromptRequest.Popup -> {
val title = context.getString(R.string.mozac_feature_prompts_popup_dialog_title) val title = container.getString(R.string.mozac_feature_prompts_popup_dialog_title)
val positiveLabel = context.getString(R.string.mozac_feature_prompts_allow) val positiveLabel = container.getString(R.string.mozac_feature_prompts_allow)
val negativeLabel = context.getString(R.string.mozac_feature_prompts_deny) val negativeLabel = container.getString(R.string.mozac_feature_prompts_deny)
ConfirmDialogFragment.newInstance( ConfirmDialogFragment.newInstance(
sessionId = session.id, sessionId = session.id,
...@@ -408,12 +403,12 @@ class PromptFeature( ...@@ -408,12 +403,12 @@ class PromptFeature(
is PromptRequest.Confirm -> { is PromptRequest.Confirm -> {
with(promptRequest) { with(promptRequest) {
val positiveButton = if (positiveButtonTitle.isEmpty()) { val positiveButton = if (positiveButtonTitle.isEmpty()) {
context.getString(R.string.mozac_feature_prompts_ok) container.getString(R.string.mozac_feature_prompts_ok)
} else { } else {
positiveButtonTitle positiveButtonTitle
} }
val negativeButton = if (positiveButtonTitle.isEmpty()) { val negativeButton = if (positiveButtonTitle.isEmpty()) {
context.getString(R.string.mozac_feature_prompts_cancel) container.getString(R.string.mozac_feature_prompts_cancel)
} else { } else {
positiveButtonTitle positiveButtonTitle
} }
...@@ -457,10 +452,6 @@ class PromptFeature( ...@@ -457,10 +452,6 @@ class PromptFeature(
is Alert, is TextPrompt, is PromptRequest.Confirm -> promptAbuserDetector.shouldShowMoreDialogs is Alert, is TextPrompt, is PromptRequest.Confirm -> promptAbuserDetector.shouldShowMoreDialogs
} }
} }
companion object {
const val FILE_PICKER_ACTIVITY_REQUEST_CODE = 1234
}
} }
internal fun BrowserStore.consumePromptFrom( internal fun BrowserStore.consumePromptFrom(
......
<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<!-- This Source Code Form is subject to the terms of the Mozilla Public
- License, v. 2.0. If a copy of the MPL was not distributed with this
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<resources>
<!-- Request code for picking a file from another activity -->
<item name="mozac_feature_prompts_file_picker_activity_request_code" type="id" />
</resources>
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package mozilla.components.feature.prompts package mozilla.components.feature.prompts
import android.Manifest import android.Manifest
import android.app.Activity
import android.app.Activity.RESULT_CANCELED import android.app.Activity.RESULT_CANCELED
import android.app.Activity.RESULT_OK import android.app.Activity.RESULT_OK
import android.content.ClipData import android.content.ClipData
...@@ -14,7 +13,6 @@ import android.content.Intent ...@@ -14,7 +13,6 @@ import android.content.Intent
import android.content.pm.PackageManager.PERMISSION_DENIED import android.content.pm.PackageManager.PERMISSION_DENIED
import android.content.pm.PackageManager.PERMISSION_GRANTED import android.content.pm.PackageManager.PERMISSION_GRANTED
import android.net.Uri import android.net.Uri
import androidx.fragment.app.Fragment
import androidx.test.core.app.ApplicationProvider import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.ext.junit.runners.AndroidJUnit4
import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.ContentAction
...@@ -52,7 +50,7 @@ class FilePickerTest { ...@@ -52,7 +50,7 @@ class FilePickerTest {
onDismiss = {} onDismiss = {}
) )
private lateinit var fragment: Fragment private lateinit var fragment: PromptContainer
private lateinit var store: BrowserStore private lateinit var store: BrowserStore
private lateinit var state: BrowserState private lateinit var state: BrowserState
private lateinit var filePicker: FilePicker private lateinit var filePicker: FilePicker
...@@ -74,31 +72,15 @@ class FilePickerTest { ...@@ -74,31 +72,15 @@ class FilePickerTest {
whenever(state.customTabs).thenReturn(listOf(customTab)) whenever(state.customTabs).thenReturn(listOf(customTab))
filePicker = FilePicker(fragment, store, customTab.id) { } filePicker = FilePicker(fragment, store, customTab.id) { }
filePicker.onActivityResult(FilePicker.FILE_PICKER_ACTIVITY_REQUEST_CODE, 0, null) filePicker.onActivityResult(R.id.mozac_feature_prompts_file_picker_activity_request_code, 0, null)
verify(store).dispatch(ContentAction.ConsumePromptRequestAction(customTab.id)) verify(store).dispatch(ContentAction.ConsumePromptRequestAction(customTab.id))
val selected = prepareSelectedSession(request) val selected = prepareSelectedSession(request)
filePicker = FilePicker(fragment, store) { } filePicker = FilePicker(fragment, store) { }
filePicker.onActivityResult(FilePicker.FILE_PICKER_ACTIVITY_REQUEST_CODE, 0, null) filePicker.onActivityResult(R.id.mozac_feature_prompts_file_picker_activity_request_code, 0, null)
verify(store).dispatch(ContentAction.ConsumePromptRequestAction(selected.id)) verify(store).dispatch(ContentAction.ConsumePromptRequestAction(selected.id))
} }
@Test
fun `startActivityForResult must delegate its calls either to an activity or a fragment`() {
val mockActivity: Activity = mock()
val mockFragment: Fragment = mock()
val intent = Intent()
val code = 1
filePicker = FilePicker(mockActivity, store) { }
filePicker.startActivityForResult(intent, code)
verify(mockActivity).startActivityForResult(intent, code)
filePicker = FilePicker(mockFragment, store) { }
filePicker.startActivityForResult(intent, code)
verify(mockFragment).startActivityForResult(intent, code)
}
@Test @Test
fun `handleFilePickerRequest without the required permission will call onNeedToRequestPermissions`() { fun `handleFilePickerRequest without the required permission will call onNeedToRequestPermissions`() {
var onRequestPermissionWasCalled = false var onRequestPermissionWasCalled = false
...@@ -108,7 +90,7 @@ class FilePickerTest { ...@@ -108,7 +90,7 @@ class FilePickerTest {
onRequestPermissionWasCalled = true onRequestPermissionWasCalled = true
} }
doReturn(context).`when`(fragment).requireContext() doReturn(context).`when`(fragment).context
filePicker.handleFileRequest(request) filePicker.handleFileRequest(request)
...@@ -118,7 +100,7 @@ class FilePickerTest { ...@@ -118,7 +100,7 @@ class FilePickerTest {
@Test @Test
fun `handleFilePickerRequest with the required permission will call startActivityForResult`() { fun `handleFilePickerRequest with the required permission will call startActivityForResult`() {
val mockFragment: Fragment = mock() val mockFragment: PromptContainer = mock()
var onRequestPermissionWasCalled = false var onRequestPermissionWasCalled = false
val context = ApplicationProvider.getApplicationContext<Context>() val context = ApplicationProvider.getApplicationContext<Context>()
...@@ -126,7 +108,7 @@ class FilePickerTest { ...@@ -126,7 +108,7 @@ class FilePickerTest {
onRequestPermissionWasCalled = true onRequestPermissionWasCalled = true
} }
doReturn(context).`when`(mockFragment).requireContext() doReturn(context).`when`(mockFragment).context
grantPermission(Manifest.permission.READ_EXTERNAL_STORAGE) grantPermission(Manifest.permission.READ_EXTERNAL_STORAGE)
...@@ -181,7 +163,7 @@ class FilePickerTest { ...@@ -181,7 +163,7 @@ class FilePickerTest {