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");
 const {ElemHideExceptions} = require("./elemHideExceptions");
 const {filterNotifier} = require("./filterNotifier");
 
 /**
  * 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.

[Manish Jethani, 2018/11/03 20:01:25]

Throughout this file, "filter" now means filter text rather than filter object.

[hub, 2018/11/22 15:59:33]

I'd update the comment to say `by filter text`

[Manish Jethani, 2018/11/23 02:03:33]

Done.

  * (Only contains filters that aren't unconditionally matched for all domains.)
- * @type {Map.<string,Map.<Filter,boolean>>}
+ * @type {Map.<string,Map.<string,boolean>>}
  */
 let filtersByDomain = new Map();
 
 /**
  * Lookup table, filter by selector. (Only used for selectors that are
  * unconditionally matched for all domains.)
- * @type {Map.<string,Filter>}
+ * @type {Map.<string,string>}
  */
 let filterBySelector = new Map();
 
 /**
  * This array caches the keys of filterBySelector table (selectors
  * which unconditionally apply on all domains). It will be null if the
  * cache needs to be rebuilt.
  * @type {?string[]}
  */
 let unconditionalSelectors = null;
@@ skipping 16 matching lines @@
 
 /**
  * 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>}
+ * @type {Set.<string>}
  */
 let knownFilters = new Set();
 
 /**
  * All domains known to occur in exceptions
  * @type {Set.<string>}
  */
 let knownExceptionDomains = new Set();
 
 /**
@@ skipping 23 matching lines @@
 {
   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);
     if (!filters)
       filtersByDomain.set(domain, filters = new Map());
-    filters.set(filter, isIncluded);
+    filters.set(filter.text, isIncluded ? filter.selector : null);

[Manish Jethani, 2018/11/03 20:01:25]

Instead of true/false, we now keep the selector or null. This means we never
have to access the filter object again.

   }
 }
 
 /**
  * Returns a list of selectors that apply on each website unconditionally.
  * @returns {string[]}
  */
 function getUnconditionalSelectors()
 {
   if (!unconditionalSelectors)
@@ skipping 22 matching lines @@
   // This code is a performance hot-spot, which is why we've made certain
   // micro-optimisations. Please be careful before making changes.
   while (true)
   {
     if (specificOnly && currentDomain == "")
       break;
 
     let filters = filtersByDomain.get(currentDomain);
     if (filters)
     {
-      for (let [filter, isIncluded] of filters)
+      for (let [text, selector] of filters)
       {
-        if (!isIncluded)
+        if (selector == null)
         {
-          excluded.add(filter);
+          excluded.add(text);
         }
-        else
+        else if ((excluded.size == 0 || !excluded.has(text)) &&
+                 !ElemHideExceptions.getException(selector, domain))
         {
-          let {selector} = filter;
-          if ((excluded.size == 0 || !excluded.has(filter)) &&
-              !ElemHideExceptions.getException(selector, domain))
-          {
-            selectors.push(selector);
-          }
+          selectors.push(selector);
         }
       }
     }
 
     if (currentDomain == "")
       break;
 
     let nextDot = currentDomain.indexOf(".");
     currentDomain = nextDot == -1 ? "" : currentDomain.substr(nextDot + 1);
   }
@@ skipping 42 matching lines @@
       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);
   if (unconditionalFilterForSelector)
   {
-    addToFiltersByDomain(unconditionalFilterForSelector);
+    addToFiltersByDomain(Filter.fromText(unconditionalFilterForSelector));

[Manish Jethani, 2018/11/03 20:01:25]

This is the one time where we have to get the filter object. This is relatively
rare, there aren't that many exceptions that apply to _generic_ filters.

     filterBySelector.delete(selector);
     unconditionalSelectors = null;
     defaultStyleSheet = null;
   }
 });
 
 /**
  * Container for element hiding filters
  * @class
  */
@@ skipping 16 matching lines @@
 
     filterNotifier.emit("elemhideupdate");
   },
 
   /**
    * Add a new element hiding filter
    * @param {ElemHideFilter} filter
    */
   add(filter)
   {
-    if (knownFilters.has(filter))
+    if (knownFilters.has(filter.text))
       return;
 
     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, filter);
+      filterBySelector.set(selector, filter.text);
       unconditionalSelectors = null;
       defaultStyleSheet = null;
     }
     else
     {
       // The new filter's selector only applies to some domains
       addToFiltersByDomain(filter, domains);
     }
 
-    knownFilters.add(filter);
+    knownFilters.add(filter.text);
     filterNotifier.emit("elemhideupdate");
   },
 
   /**
    * Removes an element hiding filter
    * @param {ElemHideFilter} filter
    */
   remove(filter)
   {
-    if (!knownFilters.has(filter))
+    if (!knownFilters.has(filter.text))
       return;
 
     commonStyleSheet = null;
 
     let {selector} = filter;
 
     // Unconditially applied element hiding filters
-    if (filterBySelector.get(selector) == filter)
+    if (filterBySelector.get(selector) == filter.text)
     {
       filterBySelector.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);
         if (filters)
         {
-          filters.delete(filter);
+          filters.delete(filter.text);
 
           if (filters.size == 0)
             filtersByDomain.delete(domain);
         }
       }
     }
 
-    knownFilters.delete(filter);
+    knownFilters.delete(filter.text);
     filterNotifier.emit("elemhideupdate");
   },
 
   /**
    * @typedef {object} ElemHideStyleSheet
    * @property {string} code CSS code.
    * @property {Array.<string>} selectors List of selectors.
    */
 
   /**
@@ skipping 124 matching lines @@
 {
   let styleSheet = "";
 
   for (let selectorGroup of splitSelectors(selectors))
     styleSheet += createRule(selectorGroup);
 
   return styleSheet;
 }
 
 exports.createStyleSheet = createStyleSheet;