Use interface with features that need permissions

components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt

@@ -20,11 +20,11 @@ import mozilla.components.feature.downloads.manager.AndroidDownloadManager
 import mozilla.components.feature.downloads.manager.DownloadManager
 import mozilla.components.feature.downloads.manager.OnDownloadCompleted
 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.base.observer.Consumable
 import mozilla.components.support.ktx.android.content.isPermissionGranted
 
-typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
-
 /**
  * Feature implementation to provide download functionality for the selected
  * session. The feature will subscribe to the selected session and listen
@@ -45,7 +45,7 @@ typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
  */
 class DownloadsFeature(
     private val applicationContext: Context,
-    var onNeedToRequestPermissions: OnNeedToRequestPermissions = { },
+    override var onNeedToRequestPermissions: OnNeedToRequestPermissions = { },
     var onDownloadCompleted: OnDownloadCompleted = { _, _ -> },
     private val downloadManager: DownloadManager = AndroidDownloadManager(applicationContext, onDownloadCompleted),
     sessionManager: SessionManager,
@@ -53,7 +53,7 @@ class DownloadsFeature(
     private val fragmentManager: FragmentManager? = null,
     @VisibleForTesting(otherwise = PRIVATE)
     internal var dialog: DownloadDialogFragment = SimpleDownloadDialogFragment.newInstance()
-) : SelectionAwareSessionObserver(sessionManager), LifecycleAwareFeature {
+) : SelectionAwareSessionObserver(sessionManager), LifecycleAwareFeature, PermissionsFeature {
 
     /**
      * Starts observing downloads on the selected session and sends them to the [DownloadManager]
@@ -98,10 +98,7 @@ class DownloadsFeature(
      * Notifies the feature that the permissions request was completed. It will then
      * either trigger or clear the pending download.
      */
-    fun onPermissionsResult(
-        @Suppress("UNUSED_PARAMETER") permissions: Array<String>,
-        @Suppress("UNUSED_PARAMETER") grantResults: IntArray
-    ) {
+    override fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray) {
         if (applicationContext.isPermissionGranted(INTERNET, WRITE_EXTERNAL_STORAGE)) {
             activeSession?.let { session ->
                 session.download.consume {

components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt

@@ -34,6 +34,8 @@ import mozilla.components.feature.prompts.ChoiceDialogFragment.Companion.MENU_CH
 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.support.base.feature.LifecycleAwareFeature
+import mozilla.components.support.base.feature.OnNeedToRequestPermissions
+import mozilla.components.support.base.feature.PermissionsFeature
 import mozilla.components.support.ktx.android.content.isPermissionGranted
 import java.security.InvalidParameterException
 import java.util.Date
@@ -41,8 +43,6 @@ import java.util.Date
 @VisibleForTesting(otherwise = PRIVATE)
 internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog"
 
-typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
-
 /**
  * Feature for displaying native dialogs for html elements like: input type
  * date, file, time, color, option, menu, authentication, confirmation and alerts.
@@ -82,8 +82,8 @@ class PromptFeature(
     private val sessionManager: SessionManager,
     private var sessionId: String? = null,
     private val fragmentManager: FragmentManager,
-    private val onNeedToRequestPermissions: OnNeedToRequestPermissions
-) : LifecycleAwareFeature {
+    override val onNeedToRequestPermissions: OnNeedToRequestPermissions
+) : LifecycleAwareFeature, PermissionsFeature {
 
     init {
         if (activity == null && fragment == null) {
@@ -154,7 +154,7 @@ class PromptFeature(
      * @see [onNeedToRequestPermissions].
      */
     @Suppress("UNUSED_PARAMETER")
-    fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray) {
+    override fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray) {
         if (grantResults.isNotEmpty() && grantResults.all { it == PackageManager.PERMISSION_GRANTED }) {
             onPermissionsGranted()
         } else {

components/feature/qr/src/main/java/mozilla/components/feature/qr/QrFeature.kt

@@ -11,10 +11,11 @@ import androidx.annotation.VisibleForTesting
 import androidx.fragment.app.FragmentManager
 import mozilla.components.support.base.feature.BackHandler
 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.android.content.isPermissionGranted
 
 typealias OnScanResult = (result: String) -> Unit
-typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
 
 /**
  * Feature implementation that provides QR scanning functionality via the [QrFragment].
@@ -33,8 +34,8 @@ class QrFeature(
     private val context: Context,
     private val fragmentManager: FragmentManager,
     private val onScanResult: OnScanResult = { },
-    private val onNeedToRequestPermissions: OnNeedToRequestPermissions = { }
-) : LifecycleAwareFeature, BackHandler {
+    override val onNeedToRequestPermissions: OnNeedToRequestPermissions = { }
+) : LifecycleAwareFeature, BackHandler, PermissionsFeature {
     private var containerViewId: Int = 0
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@@ -87,10 +88,9 @@ class QrFeature(
      * Notifies the feature that the permission request was completed. If the
      * requested permissions were granted it will open the QR scanner.
      */
-    @Suppress("UNUSED_PARAMETER")
-    fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray) {
+    override fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray) {
         if (context.isPermissionGranted(CAMERA)) {
-            this.scan(containerViewId)
+            scan(containerViewId)
         }
     }
 

components/feature/qr/src/test/java/mozilla/components/feature/qr/QrFeatureTest.kt

@@ -10,6 +10,7 @@ import androidx.fragment.app.FragmentManager
 import androidx.fragment.app.FragmentTransaction
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.feature.qr.QrFeature.Companion.QR_FRAGMENT_TAG
+import mozilla.components.support.base.feature.OnNeedToRequestPermissions
 import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.grantPermission
@@ -64,7 +65,7 @@ class QrFeatureTest {
 
         // Then
         assertFalse(scanResult)
-        verify(permissionsCallback)(arrayOf(CAMERA))
+        verify(permissionsCallback).invoke(arrayOf(CAMERA))
     }
 
     @Test

components/feature/sitepermissions/src/main/java/mozilla/components/feature/sitepermissions/SitePermissionsFeature.kt

@@ -32,13 +32,14 @@ import mozilla.components.concept.engine.permission.Permission.ContentVideoCaptu
 import mozilla.components.concept.engine.permission.PermissionRequest
 import mozilla.components.feature.sitepermissions.SitePermissions.Status.ALLOWED
 import mozilla.components.feature.sitepermissions.SitePermissions.Status.BLOCKED
+import mozilla.components.feature.sitepermissions.SitePermissionsFeature.DialogConfig
 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.android.content.isPermissionGranted
 import mozilla.components.support.ktx.kotlin.toUri
 import java.security.InvalidParameterException
 
-typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
-
 internal const val FRAGMENT_TAG = "mozac_feature_sitepermissions_prompt_dialog"
 
 /**
@@ -70,8 +71,8 @@ class SitePermissionsFeature(
     private val fragmentManager: FragmentManager,
     var promptsStyling: PromptsStyling? = null,
     private val dialogConfig: DialogConfig? = null,
-    private val onNeedToRequestPermissions: OnNeedToRequestPermissions
-) : LifecycleAwareFeature {
+    override val onNeedToRequestPermissions: OnNeedToRequestPermissions
+) : LifecycleAwareFeature, PermissionsFeature {
 
     private val observer = SitePermissionsRequestObserver(sessionManager, feature = this)
     internal val ioCoroutineScope by lazy { coroutineScopeInitializer() }
@@ -101,7 +102,7 @@ class SitePermissionsFeature(
      * @param grantResults the grant results for the corresponding permissions
      * @see [onNeedToRequestPermissions].
      */
-    fun onPermissionsResult(grantResults: IntArray) {
+    override fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray) {
         sessionManager.runWithSessionIdOrSelected(sessionId) { session ->
             session.appPermissionRequest.consume { permissionsRequest ->
 

components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsFeatureTest.kt

@@ -26,6 +26,7 @@ import mozilla.components.concept.engine.permission.PermissionRequest
 import mozilla.components.feature.sitepermissions.SitePermissions.Status.ALLOWED
 import mozilla.components.feature.sitepermissions.SitePermissions.Status.BLOCKED
 import mozilla.components.feature.sitepermissions.SitePermissions.Status.NO_DECISION
+import mozilla.components.support.base.feature.OnNeedToRequestPermissions
 import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
@@ -645,7 +646,7 @@ class SitePermissionsFeatureTest {
 
         session.appPermissionRequest = Consumable.from(mockPermissionRequest)
 
-        sitePermissionFeature.onPermissionsResult(intArrayOf(PERMISSION_GRANTED))
+        sitePermissionFeature.onPermissionsResult(emptyArray(), intArrayOf(PERMISSION_GRANTED))
 
         verify(mockPermissionRequest).grant(emptyList())
         assertTrue(session.appPermissionRequest.isConsumed())
@@ -671,7 +672,7 @@ class SitePermissionsFeatureTest {
 
         session.appPermissionRequest = Consumable.from(mockPermissionRequest)
 
-        sitePermissionFeature.onPermissionsResult(intArrayOf(PERMISSION_GRANTED))
+        sitePermissionFeature.onPermissionsResult(emptyArray(), intArrayOf(PERMISSION_GRANTED))
 
         verify(mockPermissionRequest).grant(emptyList())
         assertTrue(session.appPermissionRequest.isConsumed())
@@ -686,7 +687,7 @@ class SitePermissionsFeatureTest {
 
         session.appPermissionRequest = Consumable.from(mockPermissionRequest)
 
-        sitePermissionFeature.onPermissionsResult(intArrayOf(PERMISSION_DENIED))
+        sitePermissionFeature.onPermissionsResult(emptyArray(), intArrayOf(PERMISSION_DENIED))
 
         verify(mockPermissionRequest).reject()
         assertTrue(session.appPermissionRequest.isConsumed())
@@ -756,4 +757,4 @@ class SitePermissionsFeatureTest {
         mockSessionManager.select(session)
         return session
     }
-}
\ No newline at end of file
+}

components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsRulesTest.kt

@@ -16,6 +16,7 @@ import mozilla.components.concept.engine.permission.Permission.Generic
 import mozilla.components.concept.engine.permission.PermissionRequest
 import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.ASK_TO_ALLOW
 import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.BLOCKED
+import mozilla.components.support.base.feature.OnNeedToRequestPermissions
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals

components/support/base/src/main/java/mozilla/components/support/base/feature/PermissionsFeature.kt

+/* 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.support.base.feature
+
+typealias OnNeedToRequestPermissions = (permissions: Array<String>) -> Unit
+
+/**
+ * Interface for features that need to request permissions from the user.
+ *
+ * Example integration:
+ *
+ * ```
+ * class MyFragment : Fragment {
+ *     val myFeature = MyPermissionsFeature(
+ *         onNeedToRequestPermissions = { permissions ->
+ *             requestPermissions(permissions, REQUEST_CODE_MY_FEATURE)
+ *         }
+ *     )
+ *
+ *     override fun onRequestPermissionsResult(resultCode: Int, permissions: Array<String>, grantResults: IntArray) {
+ *         if (resultCode == REQUEST_CODE_MY_FEATURE) {
+ *             myFeature.onPermissionsResult(permissions, grantResults)
+ *         }
+ *     }
+ *
+ *     companion object {
+ *         private const val REQUEST_CODE_MY_FEATURE = 1
+ *     }
+ * }
+ * ```
+ */
+interface PermissionsFeature {
+
+    /**
+     * A callback invoked when permissions need to be requested by the feature before
+     * it can complete its task. Once the request is completed, [onPermissionsResult]
+     * needs to be invoked.
+     */
+    val onNeedToRequestPermissions: OnNeedToRequestPermissions
+
+    /**
+     * Notifies the feature that a permission request was completed.
+     * The feature should react to this and complete its task.
+     *
+     * @param permissions The permissions that were granted.
+     * @param grantResults The grant results for the corresponding permission
+     */
+    fun onPermissionsResult(permissions: Array<String>, grantResults: IntArray)
+}

samples/browser/src/main/java/org/mozilla/samples/browser/BrowserFragment.kt

@@ -268,7 +268,7 @@ class BrowserFragment : Fragment(), BackHandler {
                 it.onPermissionsResult(permissions, grantResults)
             }
             REQUEST_CODE_APP_PERMISSIONS -> sitePermissionsFeature.withFeature {
-                it.onPermissionsResult(grantResults)
+                it.onPermissionsResult(permissions, grantResults)
             }
         }
     }