Closes #4191: Fixed the recommended() tracking category on SystemEngine

components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt

@@ -208,13 +208,18 @@ class SystemEngineView @JvmOverloads constructor(
                     return WebResourceResponse(null, null, null)
                 }
 
-                if (!request.isForMainFrame &&
-                        getOrCreateUrlMatcher(resources, it).matches(resourceUri, Uri.parse(session?.currentUrl))) {
+                val (matches, stringCategory) = getOrCreateUrlMatcher(resources, it).matches(
+                    resourceUri,
+                    Uri.parse(session?.currentUrl)
+                )
+
+                if (!request.isForMainFrame && matches) {
                     session?.internalNotifyObservers {
+                        val matchedCategories = stringCategory.toTrackingProtectionCategories()
                         onTrackerBlocked(
                             Tracker(
                                 resourceUri.toString(),
-                                emptyList()
+                                matchedCategories
                             )
                         )
                     }
@@ -729,9 +734,18 @@ class SystemEngineView @JvmOverloads constructor(
                 UrlMatcher.SOCIAL to TrackingProtectionPolicy.TrackingCategory.SOCIAL
         )
 
+        private fun String?.toTrackingProtectionCategories(): List<TrackingProtectionPolicy.TrackingCategory> {
+            val category = urlMatcherCategoryMap[this]
+            return if (category != null) {
+                listOf(category)
+            } else {
+                emptyList()
+            }
+        }
+
         @Synchronized
         internal fun getOrCreateUrlMatcher(resources: Resources, policy: TrackingProtectionPolicy): UrlMatcher {
-            val categories = urlMatcherCategoryMap.filterValues { policy.trackingCategories.contains(it) }.keys
+            val categories = urlMatcherCategoryMap.filterValues { policy.contains(it) }.keys
 
             URL_MATCHER?.setCategoriesEnabled(categories) ?: run {
                 URL_MATCHER = UrlMatcher.createMatcher(

components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/matcher/UrlMatcher.kt

@@ -91,8 +91,10 @@ class UrlMatcher {
      *
      * @param resourceURI URI of a resource to be loaded by the page
      * @param pageURI URI of the page
+     * @return a [Pair] of <Boolean, String?> the first indicates, if the URI matches and the second
+     * indicates the category of the match if available otherwise null.
      */
-    fun matches(resourceURI: String, pageURI: String): Boolean {
+    fun matches(resourceURI: String, pageURI: String): Pair<Boolean, String?> {
         return matches(Uri.parse(resourceURI), Uri.parse(pageURI))
     }
 
@@ -103,42 +105,45 @@ class UrlMatcher {
      *
      * @param resourceURI URI of a resource to be loaded by the page
      * @param pageURI URI of the page
+     * @return a [Pair] of <Boolean, String?> the first indicates, if the URI matches and the second
+     * indicates the category of the match if available otherwise null.
      */
     @Suppress("ReturnCount", "ComplexMethod")
-    fun matches(resourceURI: Uri, pageURI: Uri): Boolean {
+    fun matches(resourceURI: Uri, pageURI: Uri): Pair<Boolean, String?> {
         val resourceURLString = resourceURI.toString()
         val resourceHost = resourceURI.host
         val pageHost = pageURI.host
+        val notMatchesFound = false to null
 
         if (previouslyUnmatched.contains(resourceURLString)) {
-            return false
+            return notMatchesFound
         }
 
         if (whiteList?.contains(pageURI, resourceURI) == true) {
-            return false
+            return notMatchesFound
         }
 
         if (pageHost != null && pageHost == resourceHost) {
-            return false
+            return notMatchesFound
         }
 
         if (previouslyMatched.contains(resourceURLString)) {
-            return true
+            return true to null
         }
 
         if (resourceHost == null) {
-            return false
+            return notMatchesFound
         }
 
         for ((key, value) in categories) {
             if (enabledCategories.contains(key) && value.findNode(resourceHost.reverse()) != null) {
                 previouslyMatched.add(resourceURLString)
-                return true
+                return true to key
             }
         }
 
         previouslyUnmatched.add(resourceURLString)
-        return false
+        return notMatchesFound
     }
 
     companion object {

components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/SystemEngineViewTest.kt

@@ -69,6 +69,8 @@ import org.robolectric.annotation.Config
 import java.util.Calendar
 import java.util.Date
 import mozilla.components.concept.engine.EngineSession.TrackingProtectionPolicy.TrackingCategory
+import mozilla.components.concept.engine.content.blocking.Tracker
+import java.io.StringReader
 
 @RunWith(AndroidJUnit4::class)
 class SystemEngineViewTest {
@@ -497,11 +499,111 @@ class SystemEngineViewTest {
         val blockedRequest = mock<WebResourceRequest>()
         whenever(blockedRequest.isForMainFrame).thenReturn(false)
         whenever(blockedRequest.url).thenReturn(Uri.parse("http://blocked.random"))
+
+        var trackerBlocked: Tracker? = null
+        engineSession.register(object : EngineSession.Observer {
+            override fun onTrackerBlocked(tracker: Tracker) {
+                trackerBlocked = tracker
+            }
+        })
+
         response = webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest)
         assertNotNull(response)
         assertNull(response!!.data)
         assertNull(response.encoding)
         assertNull(response.mimeType)
+        assertTrue(trackerBlocked!!.trackingCategories.isEmpty())
+    }
+
+    @Test
+    fun `blocked trackers are reported with correct categories`() {
+        val BLOCK_LIST = """{
+      "license": "test-license",
+      "categories": {
+        "Advertising": [
+          {
+            "AdTest1": {
+              "http://www.adtest1.com/": [
+                "adtest1.com"
+              ]
+            }
+          }
+        ],
+        "Analytics": [
+          {
+            "AnalyticsTest": {
+                "http://analyticsTest1.com/": [
+                  "analyticsTest1.com"
+                ]
+            }
+          }
+        ],
+        "Content": [
+          {
+            "ContentTest1": {
+              "http://contenttest1.com/": [
+                "contenttest1.com"
+              ]
+            }
+          }
+        ],
+        "Social": [
+          {
+            "SocialTest1": {
+              "http://www.socialtest1.com/": [
+                "socialtest1.com"
+               ]
+              }
+            }
+        ]
+      }
+        }
+    """
+        SystemEngineView.URL_MATCHER = UrlMatcher.createMatcher(
+            StringReader(BLOCK_LIST),
+            null,
+            StringReader("{}")
+        )
+
+        val engineSession = SystemEngineSession(testContext)
+        val engineView = SystemEngineView(testContext)
+        var trackerBlocked: Tracker? = null
+
+        engineView.render(engineSession)
+        val webViewClient = engineSession.webView.webViewClient
+
+        engineSession.trackingProtectionPolicy = TrackingProtectionPolicy.strict()
+
+        engineSession.register(object : EngineSession.Observer {
+            override fun onTrackerBlocked(tracker: Tracker) {
+                trackerBlocked = tracker
+            }
+        })
+
+        val blockedRequest = mock<WebResourceRequest>()
+        whenever(blockedRequest.isForMainFrame).thenReturn(false)
+
+        whenever(blockedRequest.url).thenReturn(Uri.parse("http://www.adtest1.com/"))
+        webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest)
+
+        assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.AD)
+
+        whenever(blockedRequest.url).thenReturn(Uri.parse("http://analyticsTest1.com/"))
+        webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest)
+
+        assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.ANALYTICS)
+
+        whenever(blockedRequest.url).thenReturn(Uri.parse("http://contenttest1.com/"))
+        webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest)
+
+        assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.CONTENT)
+
+        whenever(blockedRequest.url).thenReturn(Uri.parse("http://www.socialtest1.com/"))
+        webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest)
+
+        assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.SOCIAL)
+
+        SystemEngineView.URL_MATCHER = null
     }
 
     @Test
@@ -923,6 +1025,27 @@ class SystemEngineViewTest {
         assertEquals(setOf(UrlMatcher.ADVERTISING, UrlMatcher.SOCIAL), urlMatcher.enabledCategories)
     }
 
+    @Test
+    fun `URL matcher supports compounded categories`() {
+        val recommendedPolicy = TrackingProtectionPolicy.recommended()
+        val strictPolicy = TrackingProtectionPolicy.strict()
+        val resources = testContext.resources
+        val recommendedCategories = setOf(
+            UrlMatcher.ADVERTISING, UrlMatcher.ANALYTICS, UrlMatcher.SOCIAL
+        )
+        val strictCategories = setOf(
+            UrlMatcher.ADVERTISING, UrlMatcher.ANALYTICS, UrlMatcher.SOCIAL, UrlMatcher.CONTENT
+        )
+
+        var urlMatcher = SystemEngineView.getOrCreateUrlMatcher(resources, recommendedPolicy)
+
+        assertEquals(recommendedCategories, urlMatcher.enabledCategories)
+
+        urlMatcher = SystemEngineView.getOrCreateUrlMatcher(resources, strictPolicy)
+
+        assertEquals(strictCategories, urlMatcher.enabledCategories)
+    }
+
     @Test
     fun `permission requests are forwarded to observers`() {
         val permissionRequest: android.webkit.PermissionRequest = mock()