Noissue - Remove unnecessary checks for specific-only matches

For specific-only lookups, there's no need to check if the filter is a whitelist filter. CombinedMatcher already should make sure that the specific-only flag is not passed for whitelist filters.

The Matcher class should not be used directly.

[Manish Jethani, 2019-02-06 11:18:20]

Patch Set 1

Looking for a quick review.

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js
File lib/matcher.js (left):

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js#oldc...
lib/matcher.js:361: !(filter instanceof WhitelistFilter))
It's up to the caller to not pass the specific-only flag for whitelist filters.

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js
File lib/matcher.js (right):

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js#newc...
lib/matcher.js:315: if (!specificOnly && (typeMask & DEFAULT_TYPES) != 0)
These are all generic filters so the code should not enter here if specificOnly
is true

[hub, 2019-02-06 15:46:45]

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js
File lib/matcher.js (left):

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js#oldc...
lib/matcher.js:361: !(filter instanceof WhitelistFilter))
On 2019/02/06 11:18:20, Manish Jethani wrote:
> It's up to the caller to not pass the specific-only flag for whitelist
filters.

I tend to think we should protect ourselves from the caller.

[Sebastian Noack, 2019-02-06 15:57:54]

LGTM

[Manish Jethani, 2019-02-06 16:16:01]

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js
File lib/matcher.js (left):

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js#oldc...
lib/matcher.js:361: !(filter instanceof WhitelistFilter))
On 2019/02/06 15:46:44, hub wrote:
> On 2019/02/06 11:18:20, Manish Jethani wrote:
> > It's up to the caller to not pass the specific-only flag for whitelist
> filters.
> 
> I tend to think we should protect ourselves from the caller.

Yeah, but my point was that we should not expose the Matcher class directly. If
the caller wants the correct ad blocking behavior, they should use
CombinedMatcher. If they want "raw" filter matching, then they can use Matcher
but then it's just a dumb object and follows a fixed script. The nuance is in
CombinedMatcher.

[hub, 2019-02-06 17:47:56]

LGTM

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js
File lib/matcher.js (left):

https://codereview.adblockplus.org/30000574/diff/30000575/lib/matcher.js#oldc...
lib/matcher.js:361: !(filter instanceof WhitelistFilter))
On 2019/02/06 16:16:00, Manish Jethani wrote:
> On 2019/02/06 15:46:44, hub wrote:
> > On 2019/02/06 11:18:20, Manish Jethani wrote:
> > > It's up to the caller to not pass the specific-only flag for whitelist
> > filters.
> > 
> > I tend to think we should protect ourselves from the caller.
> 
> Yeah, but my point was that we should not expose the Matcher class directly.
If
> the caller wants the correct ad blocking behavior, they should use
> CombinedMatcher. If they want "raw" filter matching, then they can use Matcher
> but then it's just a dumb object and follows a fixed script. The nuance is in
> CombinedMatcher.

Fair enough.