Closes mozilla-mobile#2706: Do not save new site permissions in priva…

…te sessions.
csadilek · Apr 30, 2019 · 7e576fc · 7e576fc
1 parent 1818fb5
commit 7e576fc
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 20 deletions.
diff --git a/...ssions/src/main/java/mozilla/components/feature/sitepermissions/SitePermissionsFeature.kt b/...ssions/src/main/java/mozilla/components/feature/sitepermissions/SitePermissionsFeature.kt
@@ -68,7 +68,11 @@ class SitePermissionsFeature(
 ) : LifecycleAwareFeature {
 
     private val observer = SitePermissionsRequestObserver(sessionManager, feature = this)
-    internal val ioCoroutineScope by lazy { CoroutineScope(Dispatchers.IO) }
+    internal val ioCoroutineScope by lazy { coroutineScopeInitializer() }
+
+    internal var coroutineScopeInitializer = {
+        CoroutineScope(Dispatchers.IO)
+    }
 
     override fun start() {
         observer.observeSelected()
@@ -119,30 +123,35 @@ class SitePermissionsFeature(
             request.grant()
 
             if (shouldStore) {
-                ioCoroutineScope.launch {
-                    storeSitePermissions(request, grantedPermissions, ALLOWED)
-                }
+                storeSitePermissions(session, request, grantedPermissions, ALLOWED)
             }
             true
         }
     }
 
     @Synchronized internal fun storeSitePermissions(
+        session: Session,
         request: PermissionRequest,
         permissions: List<Permission> = request.permissions,
         status: SitePermissions.Status
     ) {
-        var sitePermissions = storage.findSitePermissionsBy(request.host)
-
-        if (sitePermissions == null) {
-            sitePermissions = request.toSitePermissions(
-                status = status,
-                permissions = permissions
-            )
-            storage.save(sitePermissions)
-        } else {
-            sitePermissions = request.toSitePermissions(status, sitePermissions)
-            storage.update(sitePermissions)
+        if (session.private) {
+            return
+        }
+
+        ioCoroutineScope.launch {
+            var sitePermissions = storage.findSitePermissionsBy(request.host)
+
+            if (sitePermissions == null) {
+                sitePermissions = request.toSitePermissions(
+                    status = status,
+                    permissions = permissions
+                )
+                storage.save(sitePermissions)
+            } else {
+                sitePermissions = request.toSitePermissions(status, sitePermissions)
+                storage.update(sitePermissions)
+            }
         }
     }
 
@@ -157,9 +166,7 @@ class SitePermissionsFeature(
             request.reject()
 
             if (shouldStore) {
-                ioCoroutineScope.launch {
-                    storeSitePermissions(request = request, status = BLOCKED)
-                }
+                storeSitePermissions(session, request = request, status = BLOCKED)
             }
             true
         }

diff --git a/...ns/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsFeatureTest.kt b/...ns/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsFeatureTest.kt
@@ -257,11 +257,34 @@ class SitePermissionsFeatureTest {
         val sitePermissionsList = listOf(ContentGeoLocation())
         val request: PermissionRequest = mock()
 
-        sitePermissionFeature.storeSitePermissions(request, sitePermissionsList, ALLOWED)
+        runBlocking {
+            sitePermissionFeature.coroutineScopeInitializer = {
+                this
+            }
+            sitePermissionFeature.storeSitePermissions(mock(), request, sitePermissionsList, ALLOWED)
+        }
         verify(mockStorage).findSitePermissionsBy(anyString())
         verify(mockStorage).save(any())
     }
 
+    @Test
+    fun `storing a SitePermissions with a private session must NOT call save on the store`() {
+        val sitePermissionsList = listOf(ContentGeoLocation())
+        val request: PermissionRequest = mock()
+        val session: Session = mock()
+
+        doReturn(true).`when`(session).private
+
+        runBlocking {
+            sitePermissionFeature.coroutineScopeInitializer = {
+                this
+            }
+            sitePermissionFeature.storeSitePermissions(session, request, sitePermissionsList, ALLOWED)
+        }
+
+        verify(mockStorage, times(0)).save(any())
+    }
+
     @Test
     fun `storing an already existing SitePermissions must call update on the store`() {
         val sitePermissionsList = listOf(
@@ -281,8 +304,18 @@ class SitePermissionsFeatureTest {
             doReturn(sitePermissionFromStorage).`when`(mockStorage).findSitePermissionsBy(anyString())
             doReturn(permissions).`when`(mockRequest).permissions
 
+            val feature = SitePermissionsFeature(
+                anchorView = anchorView,
+                sessionManager = mockSessionManager,
+                onNeedToRequestPermissions = mockOnNeedToRequestPermissions,
+                storage = mockStorage
+            )
+
             try {
-                sitePermissionFeature.storeSitePermissions(mockRequest, permissions, ALLOWED)
+                runBlocking {
+                    feature.coroutineScopeInitializer = { this }
+                    feature.storeSitePermissions(mock(), mockRequest, permissions, ALLOWED)
+                }
                 verify(mockStorage, times(index + 1)).findSitePermissionsBy(anyString())
                 verify(mockStorage, times(index + 1)).update(any())
             } catch (e: InvalidParameterException) {

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -12,6 +12,9 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt)
 
+* **feature-sitepermissions**
+  * Do not save new site permissions in private sessions.
+
 # 0.51.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v0.50.0...v0.51.0)