For #8554 - Migrate permissionFeature to KotlinFlow

mozilla-mobile · Oct 28, 2020 · 1befbad · 1befbad
1 parent e0d106e
commit 1befbad
Show file tree

Hide file tree

Showing 22 changed files with 1,029 additions and 1,062 deletions.
diff --git a/...rc/main/java/mozilla/components/browser/engine/gecko/permission/GeckoPermissionRequest.kt b/...rc/main/java/mozilla/components/browser/engine/gecko/permission/GeckoPermissionRequest.kt
@@ -22,16 +22,19 @@ import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_AUTOPLAY
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_DESKTOP_NOTIFICATION
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_GEOLOCATION
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_PERSISTENT_STORAGE
+import java.util.UUID
 
 /**
  * Gecko-based implementation of [PermissionRequest].
  *
  * @property permissions the list of requested permissions.
  * @property callback the callback to grant/reject the requested permissions.
+ * @property id a unique identifier for the request.
  */
 sealed class GeckoPermissionRequest constructor(
     override val permissions: List<Permission>,
-    private val callback: PermissionDelegate.Callback? = null
+    private val callback: PermissionDelegate.Callback? = null,
+    override val id: String = UUID.randomUUID().toString()
 ) : PermissionRequest {
 
     /**

diff --git a/...rc/main/java/mozilla/components/browser/engine/gecko/permission/GeckoPermissionRequest.kt b/...rc/main/java/mozilla/components/browser/engine/gecko/permission/GeckoPermissionRequest.kt
@@ -22,16 +22,19 @@ import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_AUTOPLAY
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_DESKTOP_NOTIFICATION
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_GEOLOCATION
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_PERSISTENT_STORAGE
+import java.util.UUID
 
 /**
  * Gecko-based implementation of [PermissionRequest].
  *
  * @property permissions the list of requested permissions.
  * @property callback the callback to grant/reject the requested permissions.
+ * @property id a unique identifier for the request.
  */
 sealed class GeckoPermissionRequest constructor(
     override val permissions: List<Permission>,
-    private val callback: PermissionDelegate.Callback? = null
+    private val callback: PermissionDelegate.Callback? = null,
+    override val id: String = UUID.randomUUID().toString()
 ) : PermissionRequest {
 
     /**

diff --git a/...rc/main/java/mozilla/components/browser/engine/gecko/permission/GeckoPermissionRequest.kt b/...rc/main/java/mozilla/components/browser/engine/gecko/permission/GeckoPermissionRequest.kt
@@ -22,16 +22,19 @@ import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_AUTOPLAY
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_DESKTOP_NOTIFICATION
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_GEOLOCATION
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.PERMISSION_PERSISTENT_STORAGE
+import java.util.UUID
 
 /**
  * Gecko-based implementation of [PermissionRequest].
  *
  * @property permissions the list of requested permissions.
  * @property callback the callback to grant/reject the requested permissions.
+ * @property id a unique identifier for the request.
  */
 sealed class GeckoPermissionRequest constructor(
     override val permissions: List<Permission>,
-    private val callback: PermissionDelegate.Callback? = null
+    private val callback: PermissionDelegate.Callback? = null,
+    override val id: String = UUID.randomUUID().toString()
 ) : PermissionRequest {
 
     /**

diff --git a/.../main/java/mozilla/components/browser/engine/system/permission/SystemPermissionRequest.kt b/.../main/java/mozilla/components/browser/engine/system/permission/SystemPermissionRequest.kt
@@ -17,6 +17,7 @@ import android.webkit.PermissionRequest.RESOURCE_PROTECTED_MEDIA_ID
  */
 class SystemPermissionRequest(private val nativeRequest: android.webkit.PermissionRequest) : PermissionRequest {
     override val uri: String = nativeRequest.origin.toString()
+    override val id: String = java.util.UUID.randomUUID().toString()
 
     override val permissions = nativeRequest.resources.map { resource ->
         permissionsMap.getOrElse(resource) { Permission.Generic(resource) }

diff --git a/components/browser/session/src/main/java/mozilla/components/browser/session/Session.kt b/components/browser/session/src/main/java/mozilla/components/browser/session/Session.kt
@@ -29,7 +29,6 @@ import mozilla.components.concept.engine.content.blocking.Tracker
 import mozilla.components.concept.engine.manifest.WebAppManifest
 import mozilla.components.concept.engine.media.RecordingDevice
 import mozilla.components.concept.engine.permission.PermissionRequest
-import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
 import java.util.UUID
@@ -276,29 +275,6 @@ class Session(
         // we try to lookup the icon from an attached BrowserStore if possible.
         get() = store?.state?.findTabOrCustomTab(id)?.content?.icon
 
-    /**
-     * [Consumable] permission request from web content. A [PermissionRequest]
-     * must be consumed i.e. either [PermissionRequest.grant] or
-     * [PermissionRequest.reject] must be called. A content permission request
-     * can also be cancelled, which will result in a new empty [Consumable].
-     */
-    var contentPermissionRequest: Consumable<PermissionRequest> by Delegates.vetoable(Consumable.empty()) {
-        _, _, request ->
-            val consumers = wrapConsumers<PermissionRequest> { onContentPermissionRequested(this@Session, it) }
-            !request.consumeBy(consumers)
-    }
-
-    /**
-     * [Consumable] permission request for the app. A [PermissionRequest]
-     * must be consumed i.e. either [PermissionRequest.grant] or
-     * [PermissionRequest.reject] must be called.
-     */
-    var appPermissionRequest: Consumable<PermissionRequest> by Delegates.vetoable(Consumable.empty()) {
-        _, _, request ->
-            val consumers = wrapConsumers<PermissionRequest> { onAppPermissionRequested(this@Session, it) }
-            !request.consumeBy(consumers)
-    }
-
     /**
      * List of recording devices (e.g. camera or microphone) currently in use by web content.
      */

diff --git a/...browser/session/src/main/java/mozilla/components/browser/session/engine/EngineObserver.kt b/...browser/session/src/main/java/mozilla/components/browser/session/engine/EngineObserver.kt
@@ -38,7 +38,6 @@ import mozilla.components.concept.engine.prompt.PromptRequest
 import mozilla.components.concept.engine.window.WindowRequest
 import mozilla.components.concept.fetch.Response
 import mozilla.components.lib.state.Store
-import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.ktx.android.net.isInScope
 import mozilla.components.support.ktx.kotlin.isSameOriginAs
 
@@ -75,10 +74,7 @@ internal class EngineObserver(
         }
 
         if (!session.url.isSameOriginAs(url)) {
-            session.contentPermissionRequest.consume {
-                it.reject()
-                true
-            }
+            store?.dispatch(ContentAction.ClearPermissionRequests(session.id))
         }
         session.url = url
 
@@ -255,15 +251,30 @@ internal class EngineObserver(
     }
 
     override fun onContentPermissionRequest(permissionRequest: PermissionRequest) {
-        session.contentPermissionRequest = Consumable.from(permissionRequest)
+        store?.dispatch(
+            ContentAction.UpdatePermissionsRequest(
+                session.id,
+                permissionRequest
+            )
+        )
     }
 
     override fun onCancelContentPermissionRequest(permissionRequest: PermissionRequest) {
-        session.contentPermissionRequest = Consumable.empty()
+        store?.dispatch(
+            ContentAction.ConsumePermissionsRequest(
+                session.id,
+                permissionRequest
+            )
+        )
     }
 
     override fun onAppPermissionRequest(permissionRequest: PermissionRequest) {
-        session.appPermissionRequest = Consumable.from(permissionRequest)
+        store?.dispatch(
+            ContentAction.UpdateAppPermissionsRequest(
+                session.id,
+                permissionRequest
+            )
+        )
     }
 
     override fun onPromptRequest(promptRequest: PromptRequest) {

diff --git a/components/browser/session/src/test/java/mozilla/components/browser/session/SessionTest.kt b/components/browser/session/src/test/java/mozilla/components/browser/session/SessionTest.kt
@@ -22,7 +22,6 @@ import mozilla.components.concept.engine.manifest.Size
 import mozilla.components.concept.engine.manifest.WebAppManifest
 import mozilla.components.concept.engine.media.RecordingDevice
 import mozilla.components.concept.engine.permission.PermissionRequest
-import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.test.any
 import mozilla.components.support.test.argumentCaptor
 import mozilla.components.support.test.eq
@@ -580,61 +579,6 @@ class SessionTest {
         defaultObserver.onRecordingDevicesChanged(session, emptyList())
     }
 
-    @Test
-    fun `permission requests will be set on session if no observer consumes them`() {
-        val contentPermissionRequest: PermissionRequest = mock()
-        val appPermissionRequest: PermissionRequest = mock()
-
-        val session = Session("https://www.mozilla.org")
-        session.contentPermissionRequest = Consumable.from(contentPermissionRequest)
-        session.appPermissionRequest = Consumable.from(appPermissionRequest)
-        assertFalse(session.contentPermissionRequest.isConsumed())
-
-        var contentPermissionRequestIsSet = false
-        var appPermissionRequestIsSet = false
-        session.contentPermissionRequest.consume {
-            contentPermissionRequestIsSet = true
-            true
-        }
-        session.appPermissionRequest.consume {
-            appPermissionRequestIsSet = true
-            true
-        }
-        assertTrue(contentPermissionRequestIsSet)
-        assertTrue(appPermissionRequestIsSet)
-    }
-
-    @Test
-    fun `permission requests will not be set on session if consumed by observer`() {
-        var contentPermissionCallbackExecuted = false
-        var appPermissionCallbackExecuted = false
-
-        val session = Session("https://www.mozilla.org")
-        session.register(object : Session.Observer {
-            override fun onContentPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean {
-                contentPermissionCallbackExecuted = true
-                return true
-            }
-
-            override fun onAppPermissionRequested(session: Session, permissionRequest: PermissionRequest): Boolean {
-                appPermissionCallbackExecuted = true
-                return true
-            }
-        })
-
-        val contentPermissionRequest: PermissionRequest = mock()
-        session.contentPermissionRequest = Consumable.from(contentPermissionRequest)
-
-        val appPermissionRequestIsSet: PermissionRequest = mock()
-        session.appPermissionRequest = Consumable.from(appPermissionRequestIsSet)
-
-        assertTrue(contentPermissionCallbackExecuted)
-        assertTrue(session.contentPermissionRequest.isConsumed())
-
-        assertTrue(appPermissionCallbackExecuted)
-        assertTrue(session.appPermissionRequest.isConsumed())
-    }
-
     @Test
     fun `handle empty blocked trackers list race conditions`() {
         val observer = mock(Session.Observer::class.java)

diff --git a/...ser/session/src/test/java/mozilla/components/browser/session/engine/EngineObserverTest.kt b/...ser/session/src/test/java/mozilla/components/browser/session/engine/EngineObserverTest.kt
@@ -8,6 +8,8 @@ import android.content.Intent
 import android.graphics.Bitmap
 import android.view.WindowManager
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.test.runBlockingTest
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
 import mozilla.components.browser.state.action.ContentAction
@@ -29,7 +31,6 @@ import mozilla.components.concept.engine.permission.PermissionRequest
 import mozilla.components.concept.engine.prompt.PromptRequest
 import mozilla.components.concept.engine.window.WindowRequest
 import mozilla.components.concept.fetch.Response
-import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.test.any
 import mozilla.components.support.test.eq
 import mozilla.components.support.test.libstate.ext.waitUntilIdle
@@ -42,6 +43,7 @@ import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito.doReturn
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.never
 import org.mockito.Mockito.reset
@@ -308,30 +310,30 @@ class EngineObserverTest {
 
     @Test
     fun engineObserverClearsContentPermissionRequestIfNewPageStartsLoading() {
-        val session = Session("https://www.mozilla.org")
-        val permissionRequest: PermissionRequest = mock()
-        val observer = EngineObserver(session, mock())
-
-        observer.onContentPermissionRequest(permissionRequest)
-
-        observer.onLocationChange("https://getpocket.com")
+        val session = Session("https://www.mozilla.org", id = "sessionId")
+        val store: BrowserStore = mock()
+        val observer = EngineObserver(session, store)
+        val action = ContentAction.ClearPermissionRequests("sessionId")
+        doReturn(Job()).`when`(store).dispatch(action)
 
-        assertTrue(session.contentPermissionRequest.isConsumed())
-        verify(permissionRequest).reject()
+        runBlockingTest {
+            observer.onLocationChange("https://getpocket.com")
+            verify(store).dispatch(action)
+        }
     }
 
     @Test
     fun engineObserverDoesNotClearContentPermissionRequestIfSamePageStartsLoading() {
         val session = Session("https://www.mozilla.org")
-        val permissionRequest: PermissionRequest = mock()
-        val observer = EngineObserver(session, mock())
-
-        observer.onContentPermissionRequest(permissionRequest)
-
-        observer.onLocationChange("https://www.mozilla.org/hello.html")
+        val store: BrowserStore = mock()
+        val observer = EngineObserver(session, store)
+        val action = ContentAction.ClearPermissionRequests("sessionId")
+        doReturn(Job()).`when`(store).dispatch(action)
 
-        assertFalse(session.contentPermissionRequest.isConsumed())
-        verify(permissionRequest, never()).reject()
+        runBlockingTest {
+            observer.onLocationChange("https://www.mozilla.org/hello.html")
+            verify(store, never()).dispatch(action)
+        }
     }
 
     @Test
@@ -509,39 +511,36 @@ class EngineObserverTest {
     @Test
     fun engineSessionObserverWithContentPermissionRequests() {
         val permissionRequest = mock(PermissionRequest::class.java)
-        val session = Session("")
-        val observer = EngineObserver(session, mock())
-
-        assertTrue(session.contentPermissionRequest.isConsumed())
-        observer.onContentPermissionRequest(permissionRequest)
-        assertFalse(session.contentPermissionRequest.isConsumed())
+        val session = Session("url", id = "id")
+        val store: BrowserStore = mock()
+        val observer = EngineObserver(session, store)
+        val action = ContentAction.UpdatePermissionsRequest(
+            session.id,
+            permissionRequest
+        )
+        doReturn(Job()).`when`(store).dispatch(action)
 
-        observer.onCancelContentPermissionRequest(permissionRequest)
-        assertTrue(session.contentPermissionRequest.isConsumed())
+        runBlockingTest {
+            observer.onContentPermissionRequest(permissionRequest)
+            verify(store).dispatch(action)
+        }
     }
 
     @Test
     fun engineSessionObserverWithAppPermissionRequests() {
         val permissionRequest = mock(PermissionRequest::class.java)
         val session = Session("")
-        val observer = EngineObserver(session, mock())
-
-        assertTrue(session.appPermissionRequest.isConsumed())
-        observer.onAppPermissionRequest(permissionRequest)
-        assertFalse(session.appPermissionRequest.isConsumed())
-    }
-
-    @Test
-    fun engineObserverConsumesContentPermissionRequestIfNewPageStartsLoading() {
-        val permissionRequest = mock(PermissionRequest::class.java)
-        val session = Session("https://www.mozilla.org")
-        session.contentPermissionRequest = Consumable.from(permissionRequest)
-
-        val observer = EngineObserver(session, mock())
-        observer.onLocationChange("https://getpocket.com")
+        val store: BrowserStore = mock()
+        val observer = EngineObserver(session, store)
+        val action = ContentAction.UpdateAppPermissionsRequest(
+            session.id,
+            permissionRequest
+        )
 
-        verify(permissionRequest).reject()
-        assertTrue(session.contentPermissionRequest.isConsumed())
+        runBlockingTest {
+            observer.onAppPermissionRequest(permissionRequest)
+            verify(store).dispatch(action)
+        }
     }
 
     @Test