Issue 6437 - Cache pattern attributes

lib/content/elemHideEmulation.js

Index: lib/content/elemHideEmulation.js
===================================================================
--- a/lib/content/elemHideEmulation.js
+++ b/lib/content/elemHideEmulation.js
@@ -31,16 +31,23 @@
 {
   let {children} = node.parentNode;
   for (let i = 0; i < children.length; i++)
     if (children[i] == node)
       return i + 1;
   return 0;
 }
 
+function getCachedPropertyValue(object, name, defaultValueFunc = () => {})
+{
+  if (!(name in object))
+    Object.defineProperty(object, name, {value: defaultValueFunc()});
+  return object[name];

[hub, 2018/03/07 23:11:30]

here I would rather do something like:

let value = object[name];
if (value == undefined)
{
   value = defaultValueFunc();
   Object.defineProperty(object, name, {value});
}
return value;

The `in` operator is slower than `[]` and then we avoid a second lookup in the
fast path (hitting the cache).

[Manish Jethani, 2018/03/08 15:29:58]

Thanks, this is a good idea.

Done.

(I'm using strict equality to compare the cached value though since the value
could genuinely be null).

+}
+
 function makeSelector(node, selector)
 {
   if (node == null)
     return null;
   if (!node.parentElement)
   {
     let newSelector = ":root";
     if (selector)
@@ -374,66 +381,90 @@
 
   *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 patternDependsOnStyles(pattern)
+function Pattern(selectors, text)
 {
-  return pattern.selectors.some(s => s.dependsOnStyles);
-}
-
-function patternDependsOnDOM(pattern)
-{
-  return pattern.selectors.some(s => s.dependsOnDOM);
-}
-
-function patternDependsOnStylesAndDOM(pattern)
-{
-  return pattern.selectors.some(s => s.dependsOnStyles && s.dependsOnDOM);
+  this.selectors = selectors;
+  this.text = text;
 }
 
-function patternMaybeDependsOnAttributes(pattern)
-{
-  // 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 pattern.selectors.some(
-    selector => selector.maybeDependsOnAttributes ||
-                (selector instanceof HasSelector &&
-                 selector.dependsOnStyles)
-  );
-}
+Pattern.prototype = {
+  isSelectorHidingOnlyPattern()
+  {
+    return getCachedPropertyValue(
+      this, "_selectorHidingOnlyPattern",
+      () => this.selectors.some(s => s.preferHideWithSelector) &&
+            !this.selectors.some(s => s.requiresHiding)
+    );
+  },
+
+  get dependsOnStyles()
+  {
+    return getCachedPropertyValue(
+      this, "_dependsOnStyles", () => this.selectors.some(s => s.dependsOnStyles)
+    );
+  },
+
+  get dependsOnDOM()
+  {
+    return getCachedPropertyValue(
+      this, "_dependsOnDOM", () => this.selectors.some(s => s.dependsOnDOM)
+    );
+  },
+
+  get dependsOnStylesAndDOM()
+  {
+    return getCachedPropertyValue(
+      this, "_dependsOnStylesAndDOM",
+      () => this.selectors.some(s => s.dependsOnStyles && s.dependsOnDOM)
+    );
+  },
 
-function patternDependsOnCharacterData(pattern)
-{
-  // Observe changes to character data only if there's a contains selector in
-  // one of the patterns.
-  return pattern.selectors.some(selector => selector.dependsOnCharacterData);
-}
+  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)
+            )
+    );
+  },
 
-function patternMatchesMutationTypes(pattern, mutationTypes)
-{
-  return mutationTypes.has("childList") ||
-         (mutationTypes.has("attributes") &&
-          patternMaybeDependsOnAttributes(pattern)) ||
-         (mutationTypes.has("characterData") &&
-          patternDependsOnCharacterData(pattern));
-}
+  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)
+    );
+  },
+
+  matchesMutationTypes(mutationTypes)
+  {
+    return mutationTypes.has("childList") ||
+           (mutationTypes.has("attributes") &&
+            this.maybeDependsOnAttributes) ||
+           (mutationTypes.has("characterData") &&
+            this.dependsOnCharacterData);
+  }
+};
 
 function extractMutationTypes(mutations)
 {
   let types = new Set();
 
   for (let mutation of mutations)
   {
     types.add(mutation.type);
@@ -450,30 +481,30 @@
 function filterPatterns(patterns, {stylesheets, mutations})
 {
   if (!stylesheets && !mutations)
     return patterns.slice();
 
   let mutationTypes = mutations ? extractMutationTypes(mutations) : null;
 
   return patterns.filter(
-    pattern => (stylesheets && patternDependsOnStyles(pattern)) ||
-               (mutations && patternDependsOnDOM(pattern) &&
-                patternMatchesMutationTypes(pattern, mutationTypes))
+    pattern => (stylesheets && pattern.dependsOnStyles) ||
+               (mutations && pattern.dependsOnDOM &&
+                pattern.matchesMutationTypes(mutationTypes))
   );
 }
 
 function shouldObserveAttributes(patterns)
 {
-  return patterns.some(patternMaybeDependsOnAttributes);
+  return patterns.some(pattern => pattern.maybeDependsOnAttributes);
 }
 
 function shouldObserveCharacterData(patterns)
 {
-  return patterns.some(patternDependsOnCharacterData);
+  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));
@@ -589,18 +620,20 @@
       stylesheets = this.document.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(patternDependsOnStylesAndDOM))
+    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;
 
@@ -639,17 +672,17 @@
 
         generator = evaluate(pattern.selectors, 0, "",
                              this.document, cssStyles);
       }
       for (let selector of generator)
       {
         if (selector != null)
         {
-          if (isSelectorHidingOnlyPattern(pattern))
+          if (pattern.isSelectorHidingOnlyPattern())
           {
             selectors.push(selector);
             selectorFilters.push(pattern.text);
           }
           else
           {
             for (let element of this.document.querySelectorAll(selector))
             {
@@ -782,17 +815,17 @@
 
   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,
         {