Merge pull request #7924 from darkdh/7916

Only check third party url when not in main frame

app/adBlock.js

@@ -42,8 +42,7 @@ const startAdBlocking = (adblock, resourceName, shouldCheckMainFrame) => {
     }
     const firstPartyUrl = urlParse(mainFrameUrl)
     const url = urlParse(details.url)
-    const cancel = (details.resourceType !== 'mainFrame' || shouldCheckMainFrame) &&
-      shouldDoAdBlockCheck(details.resourceType, firstPartyUrl, url) &&
+    const cancel = shouldDoAdBlockCheck(details.resourceType, firstPartyUrl, url, shouldCheckMainFrame) &&
       adblock.matches(details.url, mapFilterType[details.resourceType], firstPartyUrl.host)
 
     return {

app/browser/ads/adBlockUtil.js

@@ -26,12 +26,15 @@ const mapFilterType = {
  * @param resourceType {string} The resource type from the web request API
  * @param firstPartyUrl {Url} The parsed URL of the main frame URL loading the url
  * @param url {Url} The parsed URL of the resource for consideration
+ * @param shouldCheckMainFrame {boolean} Whether check main frame
  */
-const shouldDoAdBlockCheck = (resourceType, firstPartyUrl, url) =>
+const shouldDoAdBlockCheck = (resourceType, firstPartyUrl, url, shouldCheckMainFrame) =>
   firstPartyUrl.protocol &&
     // By default first party hosts are allowed, but enable the check if a flag is specified in siteHacks
-    (isThirdPartyHost(firstPartyUrl.hostname || '', url.hostname) ||
-      siteHacks[firstPartyUrl.hostname] && siteHacks[firstPartyUrl.hostname].allowFirstPartyAdblockChecks) &&
+    shouldCheckMainFrame ||
+    (resourceType !== 'mainFrame' &&
+      isThirdPartyHost(firstPartyUrl.hostname || '', url.hostname) ||
+       siteHacks[firstPartyUrl.hostname] && siteHacks[firstPartyUrl.hostname].allowFirstPartyAdblockChecks) &&
     // Only check http and https for now
     firstPartyUrl.protocol.startsWith('http') &&
     // Only do adblock if the host isn't in the whitelist

test/unit/app/browser/ads/adBlockUtilTest.js

@@ -12,32 +12,35 @@ const {shouldDoAdBlockCheck} = require('../../../../../app/browser/ads/adBlockUt
 describe('adBlockUtil test', function () {
   describe('shouldDoAdBlockCheck', function () {
     it('http protocol allows ad block checks', function () {
-      assert.ok(shouldDoAdBlockCheck('script', new URL('https://www.brave.com'), thirdPartyResource))
+      assert.ok(shouldDoAdBlockCheck('script', new URL('https://www.brave.com'), thirdPartyResource, false))
     })
     it('https protocol allows ad block checks', function () {
-      assert.ok(shouldDoAdBlockCheck('script', new URL('https://www.brave.com'), thirdPartyResource))
+      assert.ok(shouldDoAdBlockCheck('script', new URL('https://www.brave.com'), thirdPartyResource, false))
     })
     it('ftp protocol does not allow ad block checks', function () {
-      assert.ok(!shouldDoAdBlockCheck('script', new URL('ftp://www.brave.com'), thirdPartyResource))
+      assert.ok(!shouldDoAdBlockCheck('script', new URL('ftp://www.brave.com'), thirdPartyResource, false))
     })
     it('should check third party urls', function () {
-      assert.ok(shouldDoAdBlockCheck('script', site, thirdPartyResource))
+      assert.ok(shouldDoAdBlockCheck('script', site, thirdPartyResource, false))
     })
     it('should NOT check first party urls', function () {
-      assert.ok(!shouldDoAdBlockCheck('script', site, firstPartyResource))
+      assert.ok(!shouldDoAdBlockCheck('script', site, firstPartyResource, false))
     })
     it('Avoid checks with unknown resource types', function () {
       // This test is valid just as long as we don't start handling beefaroni resource types in the ad block lib!!!
-      assert.ok(!shouldDoAdBlockCheck('beefaroni', site, new URL('https://disqus.com/test')))
+      assert.ok(!shouldDoAdBlockCheck('beefaroni', site, new URL('https://disqus.com/test'), false))
     })
     it('should check first party hosts on youtube', function () {
-      assert.ok(shouldDoAdBlockCheck('script', new URL('https://www.youtube.com'), new URL('https://www.youtube.com/script.js')))
+      assert.ok(shouldDoAdBlockCheck('script', new URL('https://www.youtube.com'), new URL('https://www.youtube.com/script.js'), false))
     })
     it('diqus is allowed as third party, for now', function () {
-      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://disqus.com/test')))
-      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://hello.disqus.com/test')))
-      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://a.disquscdn.com/test')))
-      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://b.a.disquscdn.com/test')))
+      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://disqus.com/test'), false))
+      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://hello.disqus.com/test'), false))
+      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://a.disquscdn.com/test'), false))
+      assert.ok(!shouldDoAdBlockCheck('script', site, new URL('https://b.a.disquscdn.com/test'), false))
+    })
+    it('should NOT check third party urls for main frame', function () {
+      assert.ok(shouldDoAdBlockCheck('mainFrame', site, site, true))
     })
   })
 })