Issue 7052 - Use string-based matching for literal patterns

[Manish Jethani, 2018-10-17 09:05:04]

Patch Set 1

https://codereview.adblockplus.org/29912636/diff/29913555/lib/filterClasses.js
File lib/filterClasses.js (right):

https://codereview.adblockplus.org/29912636/diff/29913555/lib/filterClasses.j...
lib/filterClasses.js:716: if (!this.matchCase && isLiteralPattern(regexpSource))
Lowercase the pattern once here if it is literal and case-insensitive.

https://codereview.adblockplus.org/29912636/diff/29913555/lib/filterClasses.j...
lib/filterClasses.js:849: return index != -1 && location[index] != "/" &&
If the index is not -1 then it's highly unlikely that it'll fail the other two
tests; this is why it makes sense to get the index first.

https://codereview.adblockplus.org/29912636/diff/29913555/test/filterClasses.js
File test/filterClasses.js (right):

https://codereview.adblockplus.org/29912636/diff/29913555/test/filterClasses....
test/filterClasses.js:302: compareFilter(test, "dd[]{}$%<>&()*d",
["type=filterlist", "text=dd[]{}$%<>&()*d",
"regexp=dd\\[\\]\\{\\}\\$\\%\\<\\>\\&\\(\\).*d"]);
Line 302 and line 310 are the only cases where I didn't just remove the
"regexp=" part, instead I made it a non-literal pattern, because the point of
these tests is to check the escaping of special characters in the generated
regular expression.

[hub, 2018-10-19 16:37:04]

LGTM

[Manish Jethani, 2018-10-19 21:59:14]

Patch Sets 5-7

I figured it was better to keep the logic for matching "||" and such within the
same file as the one that generates the regular expression for non-literal
patterns, i.e. lib/common.js

https://codereview.adblockplus.org/29912636/diff/29916555/lib/common.js
File lib/common.js (right):

https://codereview.adblockplus.org/29912636/diff/29916555/lib/common.js#newco...
lib/common.js:58: // expression, but since we're faking it here with
string-based matching we
Since it's all in the same file now I don't think this comment adds much value
(it's potentially misleading actually because "normally" implies this is not
normal, but half the filters are actually going to go through this path).

[Manish Jethani, 2018-10-21 11:53:44]

Patch Set 8

PLEASE IGNORE 5-7. I realize that lib/common.js exists only so the code can be
shared with lib/content/elemHideEmulation.js

Please just compare this with Patch Set 4 now. I've renamed tripleAnchorRegExp
to doubleAnchorRegExp and made the initialization a bit more explicit, as it
wasn't obvious what "|||" (three pipes) was supposed to mean.

[hub, 2018-10-22 13:40:50]

LGTM