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

Unified Diff: lib/filterClasses.js

Issue 29909555: Issue 7046 - Defer caching of domain maps (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Change shouldCacheDomain function to cacheDomain property Created Oct. 16, 2018, 6 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/elemHide.js ('k') | test/filterClasses.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
@@ -452,16 +452,21 @@
* @type {?Map.<string,boolean>}
*/
get domains()
{
let domains = null;
if (this.domainSource)
{
+ // For some filter types this property is accessed only rarely,
+ // especially when the subscriptions are initially loaded. We defer any
+ // caching for such filters.
+ let {cacheDomains} = this;
+
let source = this.domainSource.toLowerCase();
let knownMap = knownDomainMaps.get(source);
if (knownMap)
{
domains = knownMap;
}
else
@@ -498,28 +503,40 @@
domains.set(domain, include);
}
if (domains)
domains.set("", !hasIncludes);
}
- if (domains)
+ if (!domains || cacheDomains)
knownDomainMaps.set(source, domains);
}
- this.domainSource = null;
+ if (!domains || cacheDomains)
+ {
+ this.domainSource = null;
+ Object.defineProperty(this, "domains", {value: domains});
+ }
}
- Object.defineProperty(this, "domains", {value: domains});
- return this.domains;
+ return domains;
},
/**
+ * Whether the value of {@link ActiveFilter#domains} should be cached.
+ * Defaults to <code>true</code>, but may be overridden by subclasses that
+ * don't want the value to be cached (for better memory usage).
+ * @type {boolean}
+ * @protected
+ */
+ cacheDomains: true,
+
+ /**
* Array containing public keys of websites that this filter should apply to
* @type {?string[]}
*/
sitekeys: null,
/**
* Checks whether this filter is active on a domain.
* @param {string} [docDomain] domain name of the document that loads the URL
@@ -1149,16 +1166,38 @@
function ElemHideBase(text, domains, selector)
{
ContentFilter.call(this, text, domains, selector);
}
exports.ElemHideBase = ElemHideBase;
ElemHideBase.prototype = extend(ContentFilter, {
/**
+ * @see ActiveFilter#domains
+ * @type {?Map.<string,boolean>}
+ */
+ get domains()
+ {
+ let {get} = Object.getOwnPropertyDescriptor(ActiveFilter.prototype,
+ "domains");
+ let value = get.call(this);
+ this.cacheDomains = true;
+ return value;
+ },
+
+ /**
+ * Initially <code>false</code>, but set to <code>true</code> after
+ * {@link ActiveFilter#domains} has been accessed once.
+ * @see ActiveFilter#cacheDomains
+ * @type {boolean}
+ * @protected
+ */
+ cacheDomains: false,
+
+ /**
* CSS selector for the HTML elements that should be hidden
* @type {string}
*/
get selector()
{
// Braces are being escaped to prevent CSS rule injection.
return this.body.replace("{", "\\7B ").replace("}", "\\7D ");
}
« no previous file with comments | « lib/elemHide.js ('k') | test/filterClasses.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld