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

Delta Between Two Patch Sets: lib/filterClasses.js

Issue 29909555: Issue 7046 - Defer caching of domain maps (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Left Patch Set: Update JSDoc Created Oct. 13, 2018, 11:41 p.m.
Right Patch Set: Change shouldCacheDomain function to cacheDomain property Created Oct. 16, 2018, 6 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « lib/elemHide.js ('k') | test/filterClasses.js » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 429 matching lines...) Expand 10 before | Expand all | Expand 10 after
440 domainSource: null, 440 domainSource: null,
441 441
442 /** 442 /**
443 * Separator character used in domainSource property, must be 443 * Separator character used in domainSource property, must be
444 * overridden by subclasses 444 * overridden by subclasses
445 * @type {string} 445 * @type {string}
446 */ 446 */
447 domainSeparator: null, 447 domainSeparator: null,
448 448
449 /** 449 /**
450 * Number of times {@link ActiveFilter#domains} has been accessed.
451 * @type {number}
452 * @private
453 */
454 _domainsAccessCount: 0,
455
456 /**
457 * Maximum number of times {@link ActiveFilter#domains} can be accessed
458 * before the value is cached.
459 * @type {number}
460 * @private
461 */
462 _domainsAccessCacheThreshold: 3,
463
464 /**
465 * Map containing domains that this filter should match on/not match 450 * Map containing domains that this filter should match on/not match
466 * on or null if the filter should match on all domains 451 * on or null if the filter should match on all domains
467 * @type {?Map.<string,boolean>} 452 * @type {?Map.<string,boolean>}
468 */ 453 */
469 get domains() 454 get domains()
470 { 455 {
471 // For some filter types this property is accessed only rarely, especially
472 // when the subscriptions are initially loaded. We defer any caching for
473 // such filters.
474 let cacheValue = ++this._domainsAccessCount >
475 this._domainsAccessCacheThreshold;
476
477 let domains = null; 456 let domains = null;
478 457
479 if (this.domainSource) 458 if (this.domainSource)
480 { 459 {
460 // For some filter types this property is accessed only rarely,
461 // especially when the subscriptions are initially loaded. We defer any
462 // caching for such filters.
463 let {cacheDomains} = this;
464
481 let source = this.domainSource.toLowerCase(); 465 let source = this.domainSource.toLowerCase();
482 466
483 let knownMap = knownDomainMaps.get(source); 467 let knownMap = knownDomainMaps.get(source);
484 if (knownMap) 468 if (knownMap)
485 { 469 {
486 domains = knownMap; 470 domains = knownMap;
487 } 471 }
488 else 472 else
489 { 473 {
490 let list = source.split(this.domainSeparator); 474 let list = source.split(this.domainSeparator);
(...skipping 26 matching lines...) Expand all
517 if (!domains) 501 if (!domains)
518 domains = new Map(); 502 domains = new Map();
519 503
520 domains.set(domain, include); 504 domains.set(domain, include);
521 } 505 }
522 506
523 if (domains) 507 if (domains)
524 domains.set("", !hasIncludes); 508 domains.set("", !hasIncludes);
525 } 509 }
526 510
527 if (!domains || cacheValue) 511 if (!domains || cacheDomains)
Manish Jethani 2018/10/15 15:36:59 There is a change in the semantics here in that we
528 knownDomainMaps.set(source, domains); 512 knownDomainMaps.set(source, domains);
529 } 513 }
530 514
531 if (!domains || cacheValue) 515 if (!domains || cacheDomains)
516 {
532 this.domainSource = null; 517 this.domainSource = null;
533 } 518 Object.defineProperty(this, "domains", {value: domains});
534 519 }
535 if (!domains || cacheValue) 520 }
536 Object.defineProperty(this, "domains", {value: domains});
537 521
538 return domains; 522 return domains;
539 }, 523 },
524
525 /**
526 * Whether the value of {@link ActiveFilter#domains} should be cached.
527 * Defaults to <code>true</code>, but may be overridden by subclasses that
528 * don't want the value to be cached (for better memory usage).
529 * @type {boolean}
530 * @protected
531 */
532 cacheDomains: true,
540 533
541 /** 534 /**
542 * Array containing public keys of websites that this filter should apply to 535 * Array containing public keys of websites that this filter should apply to
543 * @type {?string[]} 536 * @type {?string[]}
544 */ 537 */
545 sitekeys: null, 538 sitekeys: null,
546 539
547 /** 540 /**
548 * Checks whether this filter is active on a domain. 541 * Checks whether this filter is active on a domain.
549 * @param {string} [docDomain] domain name of the document that loads the URL 542 * @param {string} [docDomain] domain name of the document that loads the URL
(...skipping 309 matching lines...) Expand 10 before | Expand all | Expand 10 after
859 if (contentType == null) 852 if (contentType == null)
860 ({contentType} = RegExpFilter.prototype); 853 ({contentType} = RegExpFilter.prototype);
861 contentType &= ~type; 854 contentType &= ~type;
862 } 855 }
863 else 856 else
864 { 857 {
865 contentType |= type; 858 contentType |= type;
866 859
867 if (type == RegExpFilter.typeMap.CSP) 860 if (type == RegExpFilter.typeMap.CSP)
868 { 861 {
869 if (!value) 862 if (blocking && !value)
870 return new InvalidFilter(origText, "filter_invalid_csp"); 863 return new InvalidFilter(origText, "filter_invalid_csp");
871 csp = value; 864 csp = value;
872 } 865 }
873 } 866 }
874 } 867 }
875 else 868 else
876 { 869 {
877 switch (option.toLowerCase()) 870 switch (option.toLowerCase())
878 { 871 {
879 case "match-case": 872 case "match-case":
(...skipping 291 matching lines...) Expand 10 before | Expand all | Expand 10 after
1171 * @augments ContentFilter 1164 * @augments ContentFilter
1172 */ 1165 */
1173 function ElemHideBase(text, domains, selector) 1166 function ElemHideBase(text, domains, selector)
1174 { 1167 {
1175 ContentFilter.call(this, text, domains, selector); 1168 ContentFilter.call(this, text, domains, selector);
1176 } 1169 }
1177 exports.ElemHideBase = ElemHideBase; 1170 exports.ElemHideBase = ElemHideBase;
1178 1171
1179 ElemHideBase.prototype = extend(ContentFilter, { 1172 ElemHideBase.prototype = extend(ContentFilter, {
1180 /** 1173 /**
1174 * @see ActiveFilter#domains
1175 * @type {?Map.<string,boolean>}
1176 */
1177 get domains()
1178 {
1179 let {get} = Object.getOwnPropertyDescriptor(ActiveFilter.prototype,
1180 "domains");
1181 let value = get.call(this);
1182 this.cacheDomains = true;
1183 return value;
1184 },
1185
1186 /**
1187 * Initially <code>false</code>, but set to <code>true</code> after
1188 * {@link ActiveFilter#domains} has been accessed once.
1189 * @see ActiveFilter#cacheDomains
1190 * @type {boolean}
1191 * @protected
1192 */
1193 cacheDomains: false,
1194
1195 /**
1181 * CSS selector for the HTML elements that should be hidden 1196 * CSS selector for the HTML elements that should be hidden
1182 * @type {string} 1197 * @type {string}
1183 */ 1198 */
1184 get selector() 1199 get selector()
1185 { 1200 {
1186 // Braces are being escaped to prevent CSS rule injection. 1201 // Braces are being escaped to prevent CSS rule injection.
1187 return this.body.replace("{", "\\7B ").replace("}", "\\7D "); 1202 return this.body.replace("{", "\\7B ").replace("}", "\\7D ");
1188 } 1203 }
1189 }); 1204 });
1190 1205
(...skipping 70 matching lines...) Expand 10 before | Expand all | Expand 10 after
1261 1276
1262 /** 1277 /**
1263 * Script that should be executed 1278 * Script that should be executed
1264 * @type {string} 1279 * @type {string}
1265 */ 1280 */
1266 get script() 1281 get script()
1267 { 1282 {
1268 return this.body; 1283 return this.body;
1269 } 1284 }
1270 }); 1285 });
LEFTRIGHT

Powered by Google App Engine
This is Rietveld