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
@@ skipping 333 matching lines @@
     if (suffix == null)
       return null;
 
     selectors.push(...suffix);
 
     return selectors;
   },
 
   _lastInvocation: 0,
 
+  /**
+   * Processes the current document and applies all rules to it.
+   * @param {CSSStyleSheet[]} [stylesheets]
+   *    The list of new stylesheets that have been added to the document and
+   *    made reprocessing necessary. This parameter shouldn't be passed in for
+   *    the initial processing, all of document's stylesheets will be considered
+   *    then and all rules, including the ones not dependent on styles.
+   */
   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 = [];
@@ skipping 63 matching lines @@
     if (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]);
     }
   },
 
@@ skipping 11 matching lines @@
 
       if (this.patterns.length > 0)
       {
         let {document} = this.window;
         this.addSelectors();
         document.addEventListener("load", this.onLoad.bind(this), true);
       }
     });
   }
 };