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

Unified Diff: lib/filterClasses.js

Issue 29935564: Issue 7452 - Do not cache element hiding filter objects by default Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Rebase on GitLab MR #24 Created April 7, 2019, 6:08 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
« lib/elemHide.js ('K') | « lib/elemHide.js ('k') | lib/filterListener.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -126,16 +126,27 @@
if (domains)
domains.set("", !hasIncludes);
}
return domains;
}
/**
+ * Checks whether the given filter object should be cached in
+ * <code>{@link Filter.knownFilters}</code> based on properties of the object.
+ * @param {Filter} filter
+ * @returns {boolean}
+ */
+function shouldCacheFilter(filter)
+{
+ return !(filter instanceof ElemHideFilter);
Manish Jethani 2019/04/07 18:14:11 This is a separate function because I think we'll
hub 2019/04/08 20:44:53 Acknowledged.
+}
+
+/**
* Abstract base class for filters
*
* @param {string} text string representation of the filter
* @constructor
*/
function Filter(text)
{
this.text = text;
@@ -206,19 +217,21 @@
*/
Filter.invalidCSPRegExp = /(;|^) ?(base-uri|referrer|report-to|report-uri|upgrade-insecure-requests)\b/i;
/**
* Creates a filter of correct type from its text representation - does the
* basic parsing and calls the right constructor then.
*
* @param {string} text as in Filter()
+ * @param {boolean} [forceCache] Whether to force-cache the filter object.
Manish Jethani 2019/04/07 18:14:11 The idea is that the second parameter is for inter
+ * <em>For internal use only.<em>
* @return {Filter}
*/
-Filter.fromText = function(text)
+Filter.fromText = function(text, forceCache = true)
{
let filter = Filter.knownFilters.get(text);
if (filter)
return filter;
if (text[0] == "!")
{
filter = new CommentFilter(text);
@@ -227,17 +240,19 @@
{
let match = text.includes("#") ? Filter.contentRegExp.exec(text) : null;
if (match)
filter = ContentFilter.fromText(text, match[1], match[2], match[3]);
else
filter = RegExpFilter.fromText(text);
}
- Filter.knownFilters.set(filter.text, filter);
+ if (forceCache || shouldCacheFilter(filter))
+ Filter.knownFilters.set(filter.text, filter);
+
return filter;
};
/**
* Deserializes a filter
*
* @param {Object} obj map of serialized properties and their values
* @return {Filter} filter or null if the filter couldn't be created
« lib/elemHide.js ('K') | « lib/elemHide.js ('k') | lib/filterListener.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld