Issue 5395 - Make sure element hiding emulation doesn't degrade website performance too much

chrome/content/elemHideEmulation.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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/>.
  */
 
 /* globals filterToRegExp */
 
 "use strict";
 
+const MIN_INVOCATION_INTERVAL = 3000;
 const abpSelectorRegexp = /:-abp-([\w-]+)\(/i;
 
 let reportError = () => {};
 
 function splitSelector(selector)
 {
   if (selector.indexOf(",") == -1)
     return [selector];
 
   let selectors = [];
@@ skipping 160 matching lines @@
 const incompletePrefixRegexp = /[\s>+~]$/;
 
 function HasSelector(selectors)
 {
   this._innerSelectors = selectors;
 }
 
 HasSelector.prototype = {
   requiresHiding: true,
 
+  get dependsOnStyles()
+  {
+    return this._innerSelectors.some(selector => selector.dependsOnStyles);
+  },
+
   *getSelectors(prefix, subtree, styles)
   {
     for (let element of this.getElements(prefix, subtree, styles))
       yield [makeSelector(element, ""), element];
   },
 
   /**
    * Generator function returning selected elements.
    * @param {string} prefix the prefix for the selector.
    * @param {Node} subtree the subtree we work on.
@@ skipping 28 matching lines @@
       .replace("\\x7B ", "{").replace("\\x7D ", "}");
   }
   else
     regexpString = filterToRegExp(propertyExpression);
 
   this._regexp = new RegExp(regexpString, "i");
 }
 
 PropsSelector.prototype = {
   preferHideWithSelector: true,
+  dependsOnStyles: true,
 
   *findPropsSelectors(styles, prefix, regexp)
   {
     for (let style of styles)
       if (regexp.test(style.style))
         for (let subSelector of style.subSelectors)
           yield prefix + subSelector;
   },
 
   *getSelectors(prefix, subtree, styles)
@@ skipping 75 matching lines @@
 
     let suffix = this.parseSelector(selector.substr(content.end + 1));
     if (suffix == null)
       return null;
 
     selectors.push(...suffix);
 
     return selectors;
   },
 
+  _lastInvocation: 0,
+
   addSelectors(stylesheets)

[kzar, 2017/07/10 14:50:17]

Nit: IMO this function could do with a comment to explain the behaviour when the
stylesheets parameter is passed / omitted. It's true I'm not familiar with this
code but that aspect took me a while to grok when it's actually kinda simple.

[Wladimir Palant, 2017/07/11 08:18:11]

Done.

[kzar, 2017/07/11 11:05:14]

Nice, thanks.

   {
+    this._lastInvocation = Date.now();
+
     let selectors = [];
     let selectorFilters = [];
 
     let elements = [];
     let elementFilters = [];
 
     let cssStyles = [];
 
+    let stylesheetOnlyChange = !!stylesheets;
+    if (!stylesheets)
+      stylesheets = this.window.document.styleSheets;
+
     for (let stylesheet of stylesheets)
     {
       // Explicitly ignore third-party stylesheets to ensure consistent behavior
       // between Firefox and Chrome.
       if (!this.isSameOrigin(stylesheet))
         continue;
 
       let rules = stylesheet.cssRules;
       if (!rules)
         continue;
 
       for (let rule of rules)
       {
         if (rule.type != rule.STYLE_RULE)
           continue;
 
         cssStyles.push(stringifyStyle(rule));
       }
     }
 
     let {document} = this.window;
     for (let pattern of this.patterns)
     {
+      if (stylesheetOnlyChange &&
+          !pattern.selectors.some(selector => selector.dependsOnStyles))
+      {
+        continue;
+      }
+
       for (let selector of evaluate(pattern.selectors,
                                     0, "", document, cssStyles))
       {
         if (pattern.selectors.some(s => s.preferHideWithSelector) &&
             !pattern.selectors.some(s => s.requiresHiding))
         {
           selectors.push(selector);
           selectorFilters.push(pattern.text);
         }
         else
         {
           for (let element of document.querySelectorAll(selector))
           {
             elements.push(element);
             elementFilters.push(pattern.text);
           }
         }
       }
     }
 
     this.addSelectorsFunc(selectors, selectorFilters);
     this.hideElemsFunc(elements, elementFilters);
   },
 
+  _stylesheetQueue: null,
+
   onLoad(event)
   {
     let stylesheet = event.target.sheet;
     if (stylesheet)
-      this.addSelectors([stylesheet]);
+    {
+      if (!this._stylesheetQueue &&
+          Date.now() - this._lastInvocation < MIN_INVOCATION_INTERVAL)
+      {
+        this._stylesheetQueue = [];
+        this.window.setTimeout(() =>
+        {
+          let stylesheets = this._stylesheetQueue;
+          this._stylesheetQueue = null;
+          this.addSelectors(stylesheets);

[Felix Dahlke, 2017/07/10 14:37:19]

Nit: I'm probably rusty, but couldn't the above three lines be rewritten as:

  this.addSelectors(this._stylesheetQueue);
  this._stylesheetQueue = null;

[Wladimir Palant, 2017/07/10 14:38:52]

The trouble here is reentrance - if addSelector() call somehow ends up calling
onLoad() we'll have an issue.

[Felix Dahlke, 2017/07/10 14:52:14]

Makes sense.

+        }, MIN_INVOCATION_INTERVAL - (Date.now() - this._lastInvocation));
+      }
+
+      if (this._stylesheetQueue)
+        this._stylesheetQueue.push(stylesheet);
+      else
+        this.addSelectors([stylesheet]);
+    }
   },
 
   apply()
   {
     this.getFiltersFunc(patterns =>
     {
       this.patterns = [];
       for (let pattern of patterns)
       {
         let selectors = this.parseSelector(pattern.selector);
         if (selectors != null && selectors.length > 0)
           this.patterns.push({selectors, text: pattern.text});
       }
 
       if (this.patterns.length > 0)
       {
         let {document} = this.window;
-        this.addSelectors(document.styleSheets);
+        this.addSelectors();
         document.addEventListener("load", this.onLoad.bind(this), true);
       }
     });
   }
 };