Issue 616 - Add specificOnly parameter to matchesAny

Issue 616 - Add specificOnly parameter to matchesAny so that adblockpluschrome can enforce $generichide + $genericblock.

[kzar, 2015-03-11 15:36:01]

Don't bother review this yet, it doesn't work. Working on a new patchset now.

[kzar, 2015-03-11 17:06:21]

Patch Set 2 : Make specificOnly parameter for matchesAny actually work...

OK it works now, feel free to review!

[Sebastian Noack, 2015-03-12 11:36:21]

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
File lib/matcher.js (right):

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
lib/matcher.js:174: if (filter.matches(location, contentType, docDomain,
thirdParty, sitekey) &&
Since the filter match is certainly the most expensive operation here, how about
performing the other checks first?

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
lib/matcher.js:174: if (filter.matches(location, contentType, docDomain,
thirdParty, sitekey) &&
This is a very long chain of conditions. How about splitting them in two
if-statements using the continue-statement?

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
lib/matcher.js:176: !(!filter.domains || filter.domains[""])))
Isn't this the same as (filter.domains && !filter.domains[""])?

[kzar, 2015-03-12 17:08:08]

Patch Set 3 : Tidied up _checkEntryMatch logic for finding specificOnly filters.

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
File lib/matcher.js (right):

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
lib/matcher.js:174: if (filter.matches(location, contentType, docDomain,
thirdParty, sitekey) &&
On 2015/03/12 11:36:21, Sebastian Noack wrote:
> Since the filter match is certainly the most expensive operation here, how
about
> performing the other checks first?

Done.

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
lib/matcher.js:174: if (filter.matches(location, contentType, docDomain,
thirdParty, sitekey) &&
On 2015/03/12 11:36:21, Sebastian Noack wrote:
> Since the filter match is certainly the most expensive operation here, how
about
> performing the other checks first?

Done.

http://codereview.adblockplus.org/5991150368325632/diff/5750085036015616/lib/...
lib/matcher.js:176: !(!filter.domains || filter.domains[""])))
On 2015/03/12 11:36:21, Sebastian Noack wrote:
> Isn't this the same as (filter.domains && !filter.domains[""])?

Done.

[Sebastian Noack, 2015-03-12 17:40:06]

LGTM

http://codereview.adblockplus.org/5991150368325632/diff/5718998062727168/lib/...
File lib/matcher.js (left):

http://codereview.adblockplus.org/5991150368325632/diff/5718998062727168/lib/...
lib/matcher.js:171: for (let i = 0; i < list.length; i++)
Nit: This would be a perfect scenario to use a for-of loop. However, since this
is unrelated, I leave it up to you whether to address it here or not.

[kzar, 2015-07-14 16:53:00]

Patch Set 5 : Rebased onto typeMask changes

[kzar, 2015-08-25 17:44:21]

Patch Set 6 : Make use of new isGeneric filter method

[Thomas Greiner, 2015-09-23 17:36:49]

LGTM

[Felix Dahlke, 2015-09-28 08:28:51]

LGTM