Issue 6382 - Catch InvalidAccessError exception when checking styles.

lib/content/elemHideEmulation.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";
 
 const {textToRegExp, filterToRegExp, splitSelector} = require("../common");
+const {indexOf} = require("../coreUtils");
 
 let MIN_INVOCATION_INTERVAL = 3000;
 const MAX_SYNCHRONOUS_PROCESSING_TIME = 50;
 const abpSelectorRegexp = /:-abp-([\w-]+)\(/i;
+
+function getCachedPropertyValue(object, name, defaultValueFunc = () => {})
+{
+  let value = object[name];
+  if (typeof value == "undefined")
+    Object.defineProperty(object, name, {value: value = defaultValueFunc()});
+  return value;
+}
 
 /** Return position of node from parent.
  * @param {Node} node the node to find the position of.
  * @return {number} One-based index like for :nth-child(), or 0 on error.
  */
 function positionInParent(node)
 {
-  let {children} = node.parentNode;
-  for (let i = 0; i < children.length; i++)
-    if (children[i] == node)
-      return i + 1;
-  return 0;
-}
-
-function makeSelector(node, selector)
+  return indexOf(node.parentNode.children, node) + 1;
+}
+
+function makeSelector(node, selector = "")
 {
   if (node == null)
     return null;
   if (!node.parentElement)
   {
     let newSelector = ":root";
     if (selector)
       newSelector += " > " + selector;
     return newSelector;
   }
@@ skipping 187 matching lines @@
 
 const incompletePrefixRegexp = /[\s>+~]$/;
 
 function HasSelector(selectors)
 {
   this._innerSelectors = selectors;
 }
 
 HasSelector.prototype = {
   requiresHiding: true,
+  dependsOnDOM: true,
 
   get dependsOnStyles()
   {
     return this._innerSelectors.some(selector => selector.dependsOnStyles);
   },
 
   get dependsOnCharacterData()
   {
     return this._innerSelectors.some(
       selector => selector.dependsOnCharacterData
     );
   },
 
   get maybeDependsOnAttributes()
   {
     return this._innerSelectors.some(
       selector => selector.maybeDependsOnAttributes
     );
   },
 
   *getSelectors(prefix, subtree, styles)
   {
     for (let element of this.getElements(prefix, subtree, styles))
-      yield [makeSelector(element, ""), element];
+      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.
    * @param {StringifiedStyle[]} styles the stringified style objects.
    */
   *getElements(prefix, subtree, styles)
   {
@@ skipping 18 matching lines @@
   }
 };
 
 function ContainsSelector(textContent)
 {
   this._regexp = makeRegExpParameter(textContent);
 }
 
 ContainsSelector.prototype = {
   requiresHiding: true,
+  dependsOnDOM: true,
   dependsOnCharacterData: true,
 
   *getSelectors(prefix, subtree, styles)
   {
     for (let element of this.getElements(prefix, subtree, styles))
-      yield [makeSelector(element, ""), subtree];
+      yield [makeSelector(element), subtree];
   },
 
   *getElements(prefix, subtree, styles)
   {
     let actualPrefix = (!prefix || incompletePrefixRegexp.test(prefix)) ?
         prefix + "*" : prefix;
 
     let elements = scopedQuerySelectorAll(subtree, actualPrefix);
     if (elements)
     {
@@ skipping 45 matching lines @@
         }
   },
 
   *getSelectors(prefix, subtree, styles)
   {
     for (let selector of this.findPropsSelectors(styles, prefix, this._regexp))
       yield [selector, subtree];
   }
 };
 
-function isSelectorHidingOnlyPattern(pattern)
-{
-  return pattern.selectors.some(s => s.preferHideWithSelector) &&
-    !pattern.selectors.some(s => s.requiresHiding);
+function Pattern(selectors, text)
+{
+  this.selectors = selectors;
+  this.text = text;
+}
+
+Pattern.prototype = {
+  isSelectorHidingOnlyPattern()
+  {
+    return getCachedPropertyValue(
+      this, "_selectorHidingOnlyPattern",
+      () => this.selectors.some(selector => selector.preferHideWithSelector) &&
+            !this.selectors.some(selector => selector.requiresHiding)
+    );
+  },
+
+  get dependsOnStyles()
+  {
+    return getCachedPropertyValue(
+      this, "_dependsOnStyles",
+      () => this.selectors.some(selector => selector.dependsOnStyles)
+    );
+  },
+
+  get dependsOnDOM()
+  {
+    return getCachedPropertyValue(
+      this, "_dependsOnDOM",
+      () => this.selectors.some(selector => selector.dependsOnDOM)
+    );
+  },
+
+  get dependsOnStylesAndDOM()
+  {
+    return getCachedPropertyValue(
+      this, "_dependsOnStylesAndDOM",
+      () => this.selectors.some(selector => selector.dependsOnStyles &&
+                                            selector.dependsOnDOM)
+    );
+  },
+
+  get maybeDependsOnAttributes()
+  {
+    // Observe changes to attributes if either there's a plain selector that
+    // looks like an ID selector, class selector, or attribute selector in one
+    // of the patterns (e.g. "a[href='https://example.com/']")
+    // or there's a properties selector nested inside a has selector
+    // (e.g. "div:-abp-has(:-abp-properties(color: blue))")
+    return getCachedPropertyValue(
+      this, "_maybeDependsOnAttributes",
+      () => this.selectors.some(
+              selector => selector.maybeDependsOnAttributes ||
+                          (selector instanceof HasSelector &&
+                           selector.dependsOnStyles)
+            )
+    );
+  },
+
+  get dependsOnCharacterData()
+  {
+    // Observe changes to character data only if there's a contains selector in
+    // one of the patterns.
+    return getCachedPropertyValue(
+      this, "_dependsOnCharacterData",
+      () => this.selectors.some(selector => selector.dependsOnCharacterData)
+    );
+  }
+};
+
+function filterPatterns(patterns, {stylesheets, mutations})
+{
+  if (!stylesheets && !mutations)
+    return patterns.slice();
+
+  return patterns.filter(
+    pattern => (stylesheets && pattern.dependsOnStyles) ||
+               (mutations && pattern.dependsOnDOM)
+  );
 }
 
 function shouldObserveAttributes(patterns)
 {
-  // Observe changes to attributes if either there's a plain selector that
-  // looks like an ID selector, class selector, or attribute selector in one of
-  // the patterns (e.g. "a[href='https://example.com/']")
-  // or there's a properties selector nested inside a has selector
-  // (e.g. "div:-abp-has(:-abp-properties(color: blue))")
-  return patterns.some(
-    pattern => pattern.selectors.some(
-      selector => selector.maybeDependsOnAttributes ||
-                  (selector instanceof HasSelector &&
-                   selector.dependsOnStyles)
-    )
-  );
+  return patterns.some(pattern => pattern.maybeDependsOnAttributes);
 }
 
 function shouldObserveCharacterData(patterns)
 {
-  // Observe changes to character data only if there's a contains selector in
-  // one of the patterns.
-  return patterns.some(
-    pattern => pattern.selectors.some(
-      selector => selector.dependsOnCharacterData
-    )
-  );
+  return patterns.some(pattern => pattern.dependsOnCharacterData);
 }
 
 function ElemHideEmulation(addSelectorsFunc, hideElemsFunc)
 {
   this.document = document;
   this.addSelectorsFunc = addSelectorsFunc;
   this.hideElemsFunc = hideElemsFunc;
   this.observer = new MutationObserver(this.observe.bind(this));
+  this.useInlineStyles = true;
 }
 
 ElemHideEmulation.prototype = {
   isSameOrigin(stylesheet)
   {
     try
     {
       return new URL(stylesheet.href).origin == this.document.location.origin;
     }
     catch (e)
@@ skipping 66 matching lines @@
     return selectors;
   },
 
   /**
    * 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.
+   * @param {MutationRecord[]} [mutations]
+   *    The list of DOM mutations that have been applied to the document and
+   *    made reprocessing necessary. This parameter shouldn't be passed in for
+   *    the initial processing, the entire document will be considered
+   *    then and all rules, including the ones not dependent on the DOM.
    * @param {function} [done]
    *    Callback to call when done.
    */
-  _addSelectors(stylesheets, done)
-  {
+  _addSelectors(stylesheets, mutations, done)
+  {
+    let patterns = filterPatterns(this.patterns, {stylesheets, mutations});
+
     let selectors = [];
     let selectorFilters = [];
 
     let elements = [];
     let elementFilters = [];
 
     let cssStyles = [];
 
-    let stylesheetOnlyChange = !!stylesheets;
-    if (!stylesheets)
+    // If neither any style sheets nor any DOM mutations have been specified,
+    // do full processing.
+    if (!stylesheets && !mutations)
       stylesheets = this.document.styleSheets;
 
-    for (let stylesheet of stylesheets)
+    // If there are any DOM mutations and any of the patterns depends on both
+    // style sheets and the DOM (e.g. -abp-has(-abp-properties)), find all the
+    // rules in every style sheet in the document, because we need to run
+    // querySelectorAll afterwards. On the other hand, if we only have patterns
+    // that depend on either styles or DOM both not both
+    // (e.g. -abp-properties or -abp-contains), we can skip this part.
+    if (mutations && patterns.some(pattern => pattern.dependsOnStylesAndDOM))
+      stylesheets = this.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;
       try
       {
-        let rules = stylesheet.cssRules;

[kzar, 2018/03/19 15:52:07]

If this is the only line that could cause the exception to be thrown, perhaps we
should only wrap this line in the try?

[hub, 2018/03/20 14:53:53]

Done.

-        if (!rules)
-          continue;
-
-        for (let rule of rules)
-        {
-          if (rule.type != rule.STYLE_RULE)
-            continue;
-
-          cssStyles.push(stringifyStyle(rule));
-        }
+        rules = stylesheet.cssRules;
       }
       catch (e)
       {
         // On Firefox, there is a chance that an InvalidAccessError

[kzar, 2018/03/19 15:52:07]

How come this happens? Is there a Firefox bug / did you file one?

[hub, 2018/03/20 16:16:02]

It's not a bug.

https://searchfox.org/mozilla-central/source/layout/style/StyleSheet.cpp#732

The stylesheet haven't been completely loaded yet. AreRulesAvailable() is called
when GetCssRules() is called.

I'm not sure if there is anything we can do to ensure rules are available, but
catching the exception allow to not cancel the processing for the rest.

[kzar, 2018/04/18 16:12:30]

Acknowledged.

In fact that link you gave is pretty useful, perhaps add that instead of the
link to our issue tracker? (Well a permalink for the right line like
https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388f...
)

[hub, 2018/04/18 18:48:23]

Done.

         // get thrown when accessing cssRules. Just skip the stylesheet
         // in that case.
-        // See https://issues.adblockplus.org/ticket/6382
+        // See https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/layout/style/StyleSheet.cpp#699
         continue;
       }
-    }
-
-    let patterns = this.patterns.slice();
+
+      if (!rules)
+        continue;
+
+      for (let rule of rules)
+      {
+        if (rule.type != rule.STYLE_RULE)
+          continue;
+
+        cssStyles.push(stringifyStyle(rule));
+      }
+    }
+
     let pattern = null;
     let generator = null;
 
     let processPatterns = () =>
     {
       let cycleStart = performance.now();
 
       if (!pattern)
       {
         if (!patterns.length)
         {
-          this.addSelectorsFunc(selectors, selectorFilters);
-          this.hideElemsFunc(elements, elementFilters);
+          if (selectors.length > 0)
+            this.addSelectorsFunc(selectors, selectorFilters);
+          if (elements.length > 0)
+            this.hideElemsFunc(elements, elementFilters);
           if (typeof done == "function")
             done();
           return;
         }
 
         pattern = patterns.shift();
 
-        if (stylesheetOnlyChange &&
-            !pattern.selectors.some(selector => selector.dependsOnStyles))
-        {
-          pattern = null;
-          return processPatterns();
-        }
         generator = evaluate(pattern.selectors, 0, "",
                              this.document, cssStyles);
       }
       for (let selector of generator)
       {
         if (selector != null)
         {
-          if (isSelectorHidingOnlyPattern(pattern))
+          if (!this.useInlineStyles ||
+              pattern.isSelectorHidingOnlyPattern())
           {
             selectors.push(selector);
             selectorFilters.push(pattern.text);
           }
           else
           {
             for (let element of this.document.querySelectorAll(selector))
             {
               elements.push(element);
               elementFilters.push(pattern.text);
@@ skipping 26 matching lines @@
   },
 
   _filteringInProgress: false,
   _lastInvocation: -MIN_INVOCATION_INTERVAL,
   _scheduledProcessing: null,
 
   /**
    * Re-run filtering either immediately or queued.
    * @param {CSSStyleSheet[]} [stylesheets]
    *    new stylesheets to be processed. This parameter should be omitted
-   *    for DOM modification (full reprocessing required).
+   *    for full reprocessing.
+   * @param {MutationRecord[]} [mutations]
+   *    new DOM mutations to be processed. This parameter should be omitted
+   *    for full reprocessing.
    */
-  queueFiltering(stylesheets)
+  queueFiltering(stylesheets, mutations)
   {
     let completion = () =>
     {
       this._lastInvocation = performance.now();
       this._filteringInProgress = false;
       if (this._scheduledProcessing)
       {
-        let newStylesheets = this._scheduledProcessing.stylesheets;
+        let params = Object.assign({}, this._scheduledProcessing);
         this._scheduledProcessing = null;
-        this.queueFiltering(newStylesheets);
+        this.queueFiltering(params.stylesheets, params.mutations);
       }
     };
 
     if (this._scheduledProcessing)
     {
-      if (!stylesheets)
-        this._scheduledProcessing.stylesheets = null;
-      else if (this._scheduledProcessing.stylesheets)
-        this._scheduledProcessing.stylesheets.push(...stylesheets);
+      if (!stylesheets && !mutations)
+      {
+        this._scheduledProcessing = {};
+      }
+      else if (this._scheduledProcessing.stylesheets ||
+               this._scheduledProcessing.mutations)
+      {
+        if (stylesheets)
+        {
+          if (!this._scheduledProcessing.stylesheets)
+            this._scheduledProcessing.stylesheets = [];
+          this._scheduledProcessing.stylesheets.push(...stylesheets);
+        }
+        if (mutations)
+        {
+          if (!this._scheduledProcessing.mutations)
+            this._scheduledProcessing.mutations = [];
+          this._scheduledProcessing.mutations.push(...mutations);
+        }
+      }
     }
     else if (this._filteringInProgress)
     {
-      this._scheduledProcessing = {stylesheets};
+      this._scheduledProcessing = {stylesheets, mutations};
     }
     else if (performance.now() - this._lastInvocation < MIN_INVOCATION_INTERVAL)
     {
-      this._scheduledProcessing = {stylesheets};
+      this._scheduledProcessing = {stylesheets, mutations};
       setTimeout(() =>
       {
-        let newStylesheets = this._scheduledProcessing.stylesheets;
+        let params = Object.assign({}, this._scheduledProcessing);
         this._filteringInProgress = true;
         this._scheduledProcessing = null;
-        this._addSelectors(newStylesheets, completion);
+        this._addSelectors(params.stylesheets, params.mutations, completion);
       },
       MIN_INVOCATION_INTERVAL - (performance.now() - this._lastInvocation));
     }
     else if (this.document.readyState == "loading")
     {
-      this._scheduledProcessing = {stylesheets};
+      this._scheduledProcessing = {stylesheets, mutations};
       let handler = () =>
       {
-        document.removeEventListener("DOMContentLoaded", handler);
-        let newStylesheets = this._scheduledProcessing.stylesheets;
+        this.document.removeEventListener("DOMContentLoaded", handler);
+        let params = Object.assign({}, this._scheduledProcessing);
         this._filteringInProgress = true;
         this._scheduledProcessing = null;
-        this._addSelectors(newStylesheets, completion);
+        this._addSelectors(params.stylesheets, params.mutations, completion);
       };
-      document.addEventListener("DOMContentLoaded", handler);
+      this.document.addEventListener("DOMContentLoaded", handler);
     }
     else
     {
       this._filteringInProgress = true;
-      this._addSelectors(stylesheets, completion);
+      this._addSelectors(stylesheets, mutations, completion);
     }
   },
 
   onLoad(event)
   {
     let stylesheet = event.target.sheet;
     if (stylesheet)
       this.queueFiltering([stylesheet]);
   },
 
   observe(mutations)
   {
-    this.queueFiltering();
+    this.queueFiltering(null, mutations);
   },
 
   apply(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});
+        this.patterns.push(new Pattern(selectors, pattern.text));
     }
 
     if (this.patterns.length > 0)
     {
       this.queueFiltering();
       this.observer.observe(
         this.document,
         {
           childList: true,
           attributes: shouldObserveAttributes(this.patterns),
           characterData: shouldObserveCharacterData(this.patterns),
           subtree: true
         }
       );
       this.document.addEventListener("load", this.onLoad.bind(this), true);
     }
   }
 };
 
 exports.ElemHideEmulation = ElemHideEmulation;