Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/matcher.js

Issue 29926557: Issue 6994 - Use shortcut matching for location-only filters (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Fix comment Created Oct. 25, 2018, 9:27 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/filterClasses.js ('k') | test/filterListener.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/matcher.js
===================================================================
--- a/lib/matcher.js
+++ b/lib/matcher.js
@@ -61,77 +61,91 @@
/**
* Blacklist/whitelist filter matching
*/
class Matcher
{
constructor()
{
/**
- * Lookup table for filters by their associated keyword
- * @type {Map.<string,(Filter|Set.<Filter>)>}
+ * Lookup table for simple filters by their associated keyword
+ * @type {Map.<string,(RegExpFilter|Set.<RegExpFilter>)>}
* @private
*/
- this._filterByKeyword = new Map();
+ this._simpleFiltersByKeyword = new Map();
+
+ /**
+ * Lookup table for complex filters by their associated keyword
+ * @type {Map.<string,(RegExpFilter|Set.<RegExpFilter>)>}
+ * @private
+ */
+ this._complexFiltersByKeyword = new Map();
}
/**
* Removes all known filters
*/
clear()
{
- this._filterByKeyword.clear();
+ this._simpleFiltersByKeyword.clear();
+ this._complexFiltersByKeyword.clear();
}
/**
* Adds a filter to the matcher
* @param {RegExpFilter} filter
*/
add(filter)
{
+ let filtersByKeyword = filter.isLocationOnly() ?
+ this._simpleFiltersByKeyword :
+ this._complexFiltersByKeyword;
// Look for a suitable keyword
let keyword = this.findKeyword(filter);
- let set = this._filterByKeyword.get(keyword);
+ let set = filtersByKeyword.get(keyword);
if (typeof set == "undefined")
{
- this._filterByKeyword.set(keyword, filter);
+ filtersByKeyword.set(keyword, filter);
}
else if (set.size == 1)
{
if (filter != set)
- this._filterByKeyword.set(keyword, new Set([set, filter]));
+ filtersByKeyword.set(keyword, new Set([set, filter]));
}
else
{
set.add(filter);
}
}
/**
* Removes a filter from the matcher
* @param {RegExpFilter} filter
*/
remove(filter)
{
+ let filtersByKeyword = filter.isLocationOnly() ?
+ this._simpleFiltersByKeyword :
+ this._complexFiltersByKeyword;
let keyword = this.findKeyword(filter);
- let set = this._filterByKeyword.get(keyword);
+ let set = filtersByKeyword.get(keyword);
if (typeof set == "undefined")
return;
if (set.size == 1)
{
if (filter == set)
- this._filterByKeyword.delete(keyword);
+ filtersByKeyword.delete(keyword);
}
else
{
set.delete(filter);
if (set.size == 1)
- this._filterByKeyword.set(keyword, [...set][0]);
+ filtersByKeyword.set(keyword, [...set][0]);
}
}
/**
* Chooses a keyword to be associated with the filter
* @param {Filter} filter
* @returns {string} keyword or an empty string if no keyword could be found
* @protected
@@ -142,24 +156,27 @@
let {pattern} = filter;
if (pattern == null)
return result;
let candidates = pattern.toLowerCase().match(allKeywordsRegExp);
if (!candidates)
return result;
- let hash = this._filterByKeyword;
let resultCount = 0xFFFFFF;
let resultLength = 0;
for (let i = 0, l = candidates.length; i < l; i++)
{
let candidate = candidates[i].substr(1);
- let filters = hash.get(candidate);
- let count = typeof filters != "undefined" ? filters.size : 0;
+ let simpleFilters = this._simpleFiltersByKeyword.get(candidate);
+ let complexFilters = this._complexFiltersByKeyword.get(candidate);
+ let count = (typeof simpleFilters != "undefined" ?
+ simpleFilters.size : 0) +
+ (typeof complexFilters != "undefined" ?
+ complexFilters.size : 0);
if (count < resultCount ||
(count == resultCount && candidate.length > resultLength))
{
result = candidate;
resultCount = count;
resultLength = candidate.length;
}
}
@@ -176,29 +193,48 @@
* @param {string} [sitekey]
* @param {boolean} [specificOnly]
* @returns {?Filter}
* @protected
*/
checkEntryMatch(keyword, location, typeMask, docDomain, thirdParty, sitekey,
specificOnly)
{
- let set = this._filterByKeyword.get(keyword);
- if (typeof set == "undefined")
- return null;
-
- for (let filter of set)
+ // We need to skip the simple (location-only) filters if the type mask does
+ // not contain any default content types.
Manish Jethani 2018/10/25 21:28:40 Fixed the comment, it's actually the opposite.
+ if ((typeMask & RegExpFilter.prototype.contentType) != 0)
{
- if (specificOnly && filter.isGeneric() &&
- !(filter instanceof WhitelistFilter))
- continue;
+ let simpleSet = this._simpleFiltersByKeyword.get(keyword);
+ if (simpleSet)
+ {
+ for (let filter of simpleSet)
+ {
+ if (specificOnly && !(filter instanceof WhitelistFilter))
+ continue;
+
+ if (filter.matchesLocation(location))
+ return filter;
+ }
+ }
+ }
- if (filter.matches(location, typeMask, docDomain, thirdParty, sitekey))
- return filter;
+ let complexSet = this._complexFiltersByKeyword.get(keyword);
+ if (complexSet)
+ {
+ for (let filter of complexSet)
+ {
+ if (specificOnly && filter.isGeneric() &&
+ !(filter instanceof WhitelistFilter))
+ continue;
+
+ if (filter.matches(location, typeMask, docDomain, thirdParty, sitekey))
+ return filter;
+ }
}
+
return null;
}
/**
* Tests whether the URL matches any of the known filters
* @param {string} location
* URL to be tested
* @param {number} typeMask
« no previous file with comments | « lib/filterClasses.js ('k') | test/filterListener.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld