Internalize third-party request check in matcher

[mjethani]

Background

We moved the third-party request checking function into adblockpluscore recently (#7232). But there's no reason for external code to call this function directly: the matcher can check if a request is third-party based on the URL and the document's domain. Because most requests hit the cache anyway, of which we recently increased the capacity (#7254), the check wouldn't even have to be done in most cases.

What to change

See ​patch.

Integration notes

There are two changes in the APIs of Matcher and CombinedMatcher (both from lib/matcher.js):

1. Where a method formerly expected the thirdParty argument, now it does not
2. Where a method formerly expected a location argument (type string), it now expects a url argument (type URL or URLInfo (as returned by parseURL() in lib/url.js))

The following methods are affected:

• Matcher.matchesAny()
• CombinedMatcher.matchesAny()
• CombinedMatcher.search()
• CombinedMatcher.isWhitelisted()

This will require at least the following changes to other repos:

adblockpluschrome files:

• lib/csp.js
• lib/devtools.js
• lib/filterComposer.js
• lib/popupBlocker.js
• lib/requestBlocker.js
• lib/whitelisting.js

adblockplusui files:

• background.js
• messageResponder.js

Hints for testes

After the related to changes to adblockpluschrome, check that the $third-party filter option is working. If there's a filter like /foo/*$third-party then it should block a request to https://example.com/foo/bar/ only if the request is coming from a document not loaded from example.com. If the flag is flipped instead so that the filter looks like /foo/*$~third-party, then it should not block the request unless it is coming from example.com.

[mjethani]

I am not seeing any performance benefit after the changes. In fact, maybe it even hurts the performance. In any case, this is no longer about performance. As such I am setting this to a lower priority for now. Let's look into this for a future release.

[sebastian]

Even if there is no performance benefit per-se, I think from an API point of view it doesn't make any sense to provide an isThirdParty() function just so that the calling code can pass the result into the matcher. The ​simplifications in adblockpluschrome seem well worth it. After all, this was the reason we moved the public suffix list and related logic into core in the first place.

[mjethani]

Replying to sebastian:

Even if there is no performance benefit per-se, I think from an API point of view it doesn't make any sense to provide an isThirdParty() function just so that the calling code can pass the result into the matcher.

I agree.

[abpbot]

A commit referencing this issue has landed:
​Issue 7260 - Internalize third-party request check in matcher

[Ross]

Done. Working as described.

ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2