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: Created Oct. 13, 2018, 11:15 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 416 matching lines...) Expand 10 before | Expand all | Expand 10 after
427 if (value != this._lastHit) 427 if (value != this._lastHit)
428 { 428 {
429 let oldValue = this._lastHit; 429 let oldValue = this._lastHit;
430 this._lastHit = value; 430 this._lastHit = value;
431 filterNotifier.emit("filter.lastHit", this, value, oldValue); 431 filterNotifier.emit("filter.lastHit", this, value, oldValue);
432 } 432 }
433 return this._lastHit; 433 return this._lastHit;
434 }, 434 },
435 435
436 /** 436 /**
437 * Internal counter to keep track of the number of times the
438 * {@link ActiveFilter#domains} property is accessed.
439 * @type {number}
440 */
441 _domainsAccessCount: 0,
442
443 /**
444 * String that the domains property should be generated from 437 * String that the domains property should be generated from
445 * @type {?string} 438 * @type {?string}
446 */ 439 */
447 domainSource: null, 440 domainSource: null,
448 441
449 /** 442 /**
450 * Separator character used in domainSource property, must be 443 * Separator character used in domainSource property, must be
451 * overridden by subclasses 444 * overridden by subclasses
452 * @type {string} 445 * @type {string}
453 */ 446 */
454 domainSeparator: null, 447 domainSeparator: null,
455 448
456 /** 449 /**
457 * Map containing domains that this filter should match on/not match 450 * Map containing domains that this filter should match on/not match
458 * on or null if the filter should match on all domains 451 * on or null if the filter should match on all domains
459 * @type {?Map.<string,boolean>} 452 * @type {?Map.<string,boolean>}
460 */ 453 */
461 get domains() 454 get domains()
462 { 455 {
463 // For some filter types this property is accessed only rarely, especially
464 // when the subscriptions are initially loaded. We defer any caching for
465 // such filters.
466 let cacheValue = ++this._domainsAccessCount > 3;
467
468 let domains = null; 456 let domains = null;
469 457
470 if (this.domainSource) 458 if (this.domainSource)
471 { 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
472 let source = this.domainSource.toLowerCase(); 465 let source = this.domainSource.toLowerCase();
473 466
474 let knownMap = knownDomainMaps.get(source); 467 let knownMap = knownDomainMaps.get(source);
475 if (knownMap) 468 if (knownMap)
476 { 469 {
477 domains = knownMap; 470 domains = knownMap;
478 } 471 }
479 else 472 else
480 { 473 {
481 let list = source.split(this.domainSeparator); 474 let list = source.split(this.domainSeparator);
(...skipping 26 matching lines...) Expand all
508 if (!domains) 501 if (!domains)
509 domains = new Map(); 502 domains = new Map();
510 503
511 domains.set(domain, include); 504 domains.set(domain, include);
512 } 505 }
513 506
514 if (domains) 507 if (domains)
515 domains.set("", !hasIncludes); 508 domains.set("", !hasIncludes);
516 } 509 }
517 510
518 if (!domains || cacheValue) 511 if (!domains || cacheDomains)
519 knownDomainMaps.set(source, domains); 512 knownDomainMaps.set(source, domains);
520 } 513 }
521 514
522 if (!domains || cacheValue) 515 if (!domains || cacheDomains)
516 {
523 this.domainSource = null; 517 this.domainSource = null;
524 } 518 Object.defineProperty(this, "domains", {value: domains});
525 519 }
526 if (!domains || cacheValue) 520 }
527 Object.defineProperty(this, "domains", {value: domains});
528 521
529 return domains; 522 return domains;
530 }, 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,
531 533
532 /** 534 /**
533 * 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
534 * @type {?string[]} 536 * @type {?string[]}
535 */ 537 */
536 sitekeys: null, 538 sitekeys: null,
537 539
538 /** 540 /**
539 * Checks whether this filter is active on a domain. 541 * Checks whether this filter is active on a domain.
540 * @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
850 if (contentType == null) 852 if (contentType == null)
851 ({contentType} = RegExpFilter.prototype); 853 ({contentType} = RegExpFilter.prototype);
852 contentType &= ~type; 854 contentType &= ~type;
853 } 855 }
854 else 856 else
855 { 857 {
856 contentType |= type; 858 contentType |= type;
857 859
858 if (type == RegExpFilter.typeMap.CSP) 860 if (type == RegExpFilter.typeMap.CSP)
859 { 861 {
860 if (!value) 862 if (blocking && !value)
861 return new InvalidFilter(origText, "filter_invalid_csp"); 863 return new InvalidFilter(origText, "filter_invalid_csp");
862 csp = value; 864 csp = value;
863 } 865 }
864 } 866 }
865 } 867 }
866 else 868 else
867 { 869 {
868 switch (option.toLowerCase()) 870 switch (option.toLowerCase())
869 { 871 {
870 case "match-case": 872 case "match-case":
(...skipping 291 matching lines...) Expand 10 before | Expand all | Expand 10 after
1162 * @augments ContentFilter 1164 * @augments ContentFilter
1163 */ 1165 */
1164 function ElemHideBase(text, domains, selector) 1166 function ElemHideBase(text, domains, selector)
1165 { 1167 {
1166 ContentFilter.call(this, text, domains, selector); 1168 ContentFilter.call(this, text, domains, selector);
1167 } 1169 }
1168 exports.ElemHideBase = ElemHideBase; 1170 exports.ElemHideBase = ElemHideBase;
1169 1171
1170 ElemHideBase.prototype = extend(ContentFilter, { 1172 ElemHideBase.prototype = extend(ContentFilter, {
1171 /** 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 /**
1172 * CSS selector for the HTML elements that should be hidden 1196 * CSS selector for the HTML elements that should be hidden
1173 * @type {string} 1197 * @type {string}
1174 */ 1198 */
1175 get selector() 1199 get selector()
1176 { 1200 {
1177 // Braces are being escaped to prevent CSS rule injection. 1201 // Braces are being escaped to prevent CSS rule injection.
1178 return this.body.replace("{", "\\7B ").replace("}", "\\7D "); 1202 return this.body.replace("{", "\\7B ").replace("}", "\\7D ");
1179 } 1203 }
1180 }); 1204 });
1181 1205
(...skipping 70 matching lines...) Expand 10 before | Expand all | Expand 10 after
1252 1276
1253 /** 1277 /**
1254 * Script that should be executed 1278 * Script that should be executed
1255 * @type {string} 1279 * @type {string}
1256 */ 1280 */
1257 get script() 1281 get script()
1258 { 1282 {
1259 return this.body; 1283 return this.body;
1260 } 1284 }
1261 }); 1285 });
LEFTRIGHT

Powered by Google App Engine
This is Rietveld