Issue 7452 - Do not cache element hiding filter objects by default

lib/elemHide.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 /**
  * @fileOverview Element hiding implementation.
  */
 
 const {Filter} = require("./filterClasses");

[Manish Jethani, 2019/04/07 18:14:11]

I have minimized the changes in this file for now so it becomes easier to review
what's actually changed. I'd like to make those changes too (renaming `filter`
to `filterText` in places) but I can do that once the important parts are
reviewed.

In this rebase no logic has changed except for the second argument to
`Filter.fromText()`.

 const {ElemHideExceptions} = require("./elemHideExceptions");
 const {filterNotifier} = require("./filterNotifier");
 const {normalizeHostname, domainSuffixes} = require("./url");
 const {Cache} = require("./caching");
 
 /**
  * The maximum number of selectors in a CSS rule. This is used by
  * <code>{@link createStyleSheet}</code> to split up a long list of selectors
  * into multiple rules.
  * @const {number}
  * @default
  */
 const selectorGroupSize = 1024;
 
 /**
- * Lookup table, active flag, by filter by domain.
- * (Only contains filters that aren't unconditionally matched for all domains.)
- * @type {Map.<string,Map.<Filter,boolean>>}
- */
-let filtersByDomain = new Map();
-
-/**
- * Lookup table, filter by selector. (Only used for selectors that are
+ * Lookup table, selector by filter text by domain.
+ * (Only contains filter text for filters that aren't unconditionally matched
+ * for all domains.)
+ * @type {Map.<string,Map.<string,string>>}
+ */
+let filterTextByDomain = new Map();
+
+/**
+ * Lookup table, filter text by selector. (Only used for selectors that are
  * unconditionally matched for all domains.)
- * @type {Map.<string,Filter>}
- */
-let filterBySelector = new Map();
-
-/**
- * This array caches the keys of filterBySelector table (selectors
+ * @type {Map.<string,string>}
+ */
+let filterTextBySelector = new Map();
+
+/**
+ * This array caches the keys of filterTextBySelector table (selectors
  * which unconditionally apply on all domains). It will be null if the
  * cache needs to be rebuilt.
  * @type {?string[]}
  */
 let unconditionalSelectors = null;
 
 /**
  * The default style sheet that applies on all domains. This is based on the
  * value of <code>{@link unconditionalSelectors}</code>.
  * @type {?string}
@@ skipping 18 matching lines @@
 let styleSheetCache = new Cache(100);
 
 /**
  * Map to be used instead when a filter has a blank domains property.
  * @type {Map.<string,boolean>}
  * @const
  */
 let defaultDomains = new Map([["", true]]);
 
 /**
- * Set containing known element hiding filters
- * @type {Set.<ElemHideFilter>}
- */
-let knownFilters = new Set();
+ * Set containing text of known element hiding filters
+ * @type {Set.<string>}
+ */
+let knownFilterText = new Set();
 
 /**
  * All domains known to occur in exceptions
  * @type {Set.<string>}
  */
 let knownExceptionDomains = new Set();
 
 /**
  * Returns the suffix of the given domain that is known. If no suffix is known,
  * an empty string is returned.
  * @param {?string} domain
  * @returns {string}
  */
 function getKnownSuffix(domain)
 {
-  while (domain && !filtersByDomain.has(domain) &&
+  while (domain && !filterTextByDomain.has(domain) &&
          !knownExceptionDomains.has(domain))
   {
     let index = domain.indexOf(".");
     domain = index == -1 ? "" : domain.substring(index + 1);
   }
 
   return domain;
 }
 
 /**
  * Adds a filter to the lookup table of filters by domain.
  * @param {Filter} filter
  * @param {?Map.<string,boolean>} [domains]
  */
 function addToFiltersByDomain(filter, domains = filter.domains)
 {
   for (let [domain, isIncluded] of domains || defaultDomains)
   {
     // There's no need to note that a filter is generically disabled.
     if (!isIncluded && domain == "")
       continue;
 
-    let filters = filtersByDomain.get(domain);
+    let filters = filterTextByDomain.get(domain);
     if (!filters)
-      filtersByDomain.set(domain, filters = new Map());
+      filterTextByDomain.set(domain, filters = new Map());
     filters.set(filter.text, isIncluded ? filter.selector : null);
   }
 }
 
 /**
  * Returns a list of selectors that apply on each website unconditionally.
  * @returns {string[]}
  */
 function getUnconditionalSelectors()
 {
   if (!unconditionalSelectors)
-    unconditionalSelectors = [...filterBySelector.keys()];
+    unconditionalSelectors = [...filterTextBySelector.keys()];
 
   return unconditionalSelectors;
 }
 
 /**
  * Returns the list of selectors that apply on a given domain from the subset
  * of filters that do not apply unconditionally on all domains.
  *
  * @param {string} domain The domain.
  * @param {boolean} specificOnly Whether selectors from generic filters should
  *   be included.
  *
  * @returns {Array.<string>} The list of selectors.
  */
 function getConditionalSelectors(domain, specificOnly)
 {
   let selectors = [];
 
   let excluded = new Set();
 
   for (let currentDomain of domainSuffixes(domain, !specificOnly))
   {
-    let filters = filtersByDomain.get(currentDomain);
+    let filters = filterTextByDomain.get(currentDomain);
     if (filters)
     {
       for (let [text, selector] of filters)
       {
         if (!selector)
         {
           excluded.add(text);
         }
         else if ((excluded.size == 0 || !excluded.has(text)) &&
                  !ElemHideExceptions.getException(selector, domain))
@@ skipping 21 matching lines @@
  */
 function getConditionalSelectorsWithExceptions(domain, specificOnly)
 {
   let selectors = [];
   let exceptions = [];
 
   let excluded = new Set();
 
   for (let currentDomain of domainSuffixes(domain, !specificOnly))
   {
-    let filters = filtersByDomain.get(currentDomain);
+    let filters = filterTextByDomain.get(currentDomain);
     if (filters)
     {
       for (let [text, selector] of filters)
       {
         if (!selector)
         {
           excluded.add(text);
         }
         else if (excluded.size == 0 || !excluded.has(text))
         {
@@ skipping 68 matching lines @@
       // Note: Once an exception domain is known it never becomes unknown, even
       // when all the exceptions containing that domain are removed. This is a
       // best-case optimization.
       if (domain != "")
         knownExceptionDomains.add(domain);
     }
   }
 
   // If this is the first exception for a previously unconditionally applied
   // element hiding selector we need to take care to update the lookups.
-  let unconditionalFilterForSelector = filterBySelector.get(selector);
+  let unconditionalFilterForSelector = filterTextBySelector.get(selector);
   if (unconditionalFilterForSelector)
   {
     addToFiltersByDomain(Filter.fromText(unconditionalFilterForSelector,
                                          false));
-    filterBySelector.delete(selector);
+    filterTextBySelector.delete(selector);
     unconditionalSelectors = null;
     defaultStyleSheet = null;
   }
 });
 
 /**
  * Container for element hiding filters
  * @class
  */
 exports.ElemHide = {
   /**
    * Removes all known filters
    */
   clear()
   {
     commonStyleSheet = null;
 
-    for (let collection of [styleSheetCache, filtersByDomain, filterBySelector,
-                            knownFilters, knownExceptionDomains])
+    for (let collection of [styleSheetCache, filterTextByDomain,
+                            filterTextBySelector, knownFilterText,
+                            knownExceptionDomains])
     {
       collection.clear();
     }
 
     unconditionalSelectors = null;
     defaultStyleSheet = null;
 
     filterNotifier.emit("elemhideupdate");
   },
 
   /**
    * Add a new element hiding filter
    * @param {ElemHideFilter} filter
    */
   add(filter)
   {
     let {text} = filter;
 
-    if (knownFilters.has(text))
+    if (knownFilterText.has(text))
       return;
 
     styleSheetCache.clear();
     commonStyleSheet = null;
 
     let {domains, selector} = filter;
 
     if (!(domains || ElemHideExceptions.hasExceptions(selector)))
     {
       // The new filter's selector is unconditionally applied to all domains
-      filterBySelector.set(selector, text);
+      filterTextBySelector.set(selector, text);
       unconditionalSelectors = null;
       defaultStyleSheet = null;
     }
     else
     {
       // The new filter's selector only applies to some domains
       addToFiltersByDomain(filter, domains);
     }
 
-    knownFilters.add(text);
+    knownFilterText.add(text);
     filterNotifier.emit("elemhideupdate");
   },
 
   /**
    * Removes an element hiding filter
    * @param {ElemHideFilter} filter
    */
   remove(filter)
   {
     let {text} = filter;
 
-    if (!knownFilters.has(text))
+    if (!knownFilterText.has(text))
       return;
 
     styleSheetCache.clear();
     commonStyleSheet = null;
 
     let {selector} = filter;
 
     // Unconditially applied element hiding filters
-    if (filterBySelector.get(selector) == text)
-    {
-      filterBySelector.delete(selector);
+    if (filterTextBySelector.get(selector) == text)
+    {
+      filterTextBySelector.delete(selector);
       unconditionalSelectors = null;
       defaultStyleSheet = null;
     }
     // Conditionally applied element hiding filters
     else
     {
       let domains = filter.domains || defaultDomains;
       for (let domain of domains.keys())
       {
-        let filters = filtersByDomain.get(domain);
+        let filters = filterTextByDomain.get(domain);
         if (filters)
         {
           filters.delete(text);
 
           if (filters.size == 0)
-            filtersByDomain.delete(domain);
+            filterTextByDomain.delete(domain);
         }
       }
     }
 
-    knownFilters.delete(text);
+    knownFilterText.delete(text);
     filterNotifier.emit("elemhideupdate");
   },
 
   /**
    * @typedef {object} ElemHideStyleSheet
    * @property {string} code CSS code.
    * @property {Array.<string>} selectors List of selectors.
    */
 
   /**
    * Generates a style sheet for a given domain based on the current set of
    * filters.
    *
    * @param {string} domain The domain.
    * @param {boolean} [specificOnly=false] Whether selectors from generic
    *   filters should be included.
    * @param {boolean} [includeSelectors=false] Whether the return value should
    *   include a separate list of selectors.
    * @param {boolean} [includeExceptions=false] Whether the return value should
    *   include a separate list of exceptions.
    *
-   * @returns {ElemHideStyleSheet} An object containing the CSS code with the
-   *   list of selectors and the list of exceptions.
+   * @returns {ElemHideStyleSheet} An object containing the CSS code and the
+   *   list of selectors.
    */
   generateStyleSheetForDomain(domain, specificOnly = false,
                               includeSelectors = false,
                               includeExceptions = false)
   {
     let code = null;
     let selectors = null;
     let exceptions = null;
 
     domain = normalizeHostname(domain);
@@ skipping 133 matching lines @@
 {
   let styleSheet = "";
 
   for (let selectorGroup of splitSelectors(selectors))
     styleSheet += createRule(selectorGroup);
 
   return styleSheet;
 }
 
 exports.createStyleSheet = createStyleSheet;