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: Use boolean flag instead of counter Created Oct. 15, 2018, 3:28 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 cacheValue = this.shouldCacheDomains();
+
let source = this.domainSource.toLowerCase();
let knownMap = knownDomainMaps.get(source);
if (knownMap)
{
domains = knownMap;
}
else
@@ -498,25 +503,41 @@
domains.set(domain, include);
}
if (domains)
domains.set("", !hasIncludes);
}
- if (domains)
+ if (!domains || cacheValue)
knownDomainMaps.set(source, domains);
}
- this.domainSource = null;
+ if (!domains || cacheValue)
+ {
+ 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. This
+ * is meant to be overridden by subclasses that don't want the value to be
+ * cached (for better memory usage).
+ * @return {boolean} Always <code>true</code>, but may be overriden by
+ * subclasses.
+ * @protected
+ */
+ shouldCacheDomains()
+ {
+ return true;
},
/**
* Array containing public keys of websites that this filter should apply to
* @type {?string[]}
*/
sitekeys: null,
@@ -1149,16 +1170,45 @@
function ElemHideBase(text, domains, selector)
{
ContentFilter.call(this, text, domains, selector);
}
exports.ElemHideBase = ElemHideBase;
ElemHideBase.prototype = extend(ContentFilter, {
/**
+ * Whether {@link ActiveFilter#domains} has been accessed at least once.
+ * @type {boolean}
+ * @private
+ */
+ _domainsAccessed: false,
+
+ /**
+ * See ActiveFilter.domains
+ * @inheritdoc
+ */
+ get domains()
+ {
+ let {get} = Object.getOwnPropertyDescriptor(ActiveFilter.prototype,
+ "domains");
+ let value = get.call(this);
+ this._domainsAccessed = true;
+ return value;
+ },
+
+ /**
+ * See ActiveFilter.shouldCacheDomains()
+ * @inheritdoc
+ */
+ shouldCacheDomains()
+ {
+ return this._domainsAccessed;
+ },
+
+ /**
* 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