Do not scope cache elements with media rules, :host(), or :host-conte…

…xt() selectors Mark style rules in parsing rather than runtime to make sure scope'd bitmask is consistent. Add tests for cache coherency. Fixes #3696
Polymer · Jun 9, 2016 · 5c3b917 · 5c3b917
1 parent b9c874e
commit 5c3b917
Show file tree

Hide file tree

Showing 7 changed files with 288 additions and 64 deletions.
diff --git a/src/lib/css-parse.html b/src/lib/css-parse.html
@@ -19,7 +19,11 @@
     // given a string of css, return a simple rule tree
     parse: function(text) {
       text = this._clean(text);
-      return this._parseCss(this._lex(text), text);
+      var info = {
+        styleIndex: 0,
+        mediaIndex: 0
+      };
+      return this._parseCss(this._lex(text), text, info);
     },
 
     // remove stuff we don't care about that may hinder parsing
@@ -54,7 +58,7 @@
     },
 
     // add selectors/cssText to node tree
-    _parseCss: function(node, text) {
+    _parseCss: function(node, text, info) {
       var t = text.substring(node.start, node.end-1);
       node.parsedCssText = node.cssText = t.trim();
       if (node.parent) {
@@ -71,6 +75,7 @@
         if (node.atRule) {
           if (s.indexOf(this.MEDIA_START) === 0) {
             node.type = this.types.MEDIA_RULE;
+            node.index = info.mediaIndex++;
           } else if (s.match(this._rx.keyframesRule)) {
             node.type = this.types.KEYFRAMES_RULE;
             node.keyframesName =
@@ -81,13 +86,14 @@
             node.type = this.types.MIXIN_RULE;
           } else {
             node.type = this.types.STYLE_RULE;
+            node.index = info.styleIndex++;
           }
         }
       }
       var r$ = node.rules;
       if (r$) {
         for (var i=0, l=r$.length, r; (i<l) && (r=r$[i]); i++) {
-          this._parseCss(r, text);
+          this._parseCss(r, text, info);
         }
       }
       return node;

diff --git a/src/lib/style-properties.html b/src/lib/style-properties.html
@@ -27,10 +27,28 @@
 
       // decorates styles with rule info and returns an array of used style
       // property names
-      decorateStyles: function(styles) {
+      decorateStyles: function(styles, scope) {
         var self = this, props = {}, keyframes = [];
-        styleUtil.forRulesInStyles(styles, function(rule) {
+        // assume all styles are cacheable
+        for (var idx = 0; i < styles.length; i++) {
+          styles[idx]._cacheable = true;
+        }
+        styleUtil.forRulesInStyles(styles, function(rule, style) {
           self.decorateRule(rule);
+          self.walkHostAndRootProperties(scope, rule, style, function(info) {
+            // we can't cache styles with :host and :root props in @media rules
+            if (rule.parent.type === styleUtil.ruleTypes.MEDIA_RULE) {
+              style._cacheable = false;
+            }
+            if (info.isHost) {
+              // check if the selector is in the form of `:host-context()` or `:host()`
+              // if so, this style is not cacheable
+              var hostContextOrFunction = info.selector.split(' ').some(function(s) {
+                return s.indexOf(scope.is) === 0 && s.length !== scope.is.length;
+              });
+              style._cacheable = style._cacheable && !hostContextOrFunction;
+            }
+          });
           self.collectPropertiesInCssText(rule.propertyInfo.cssText, props);
         }, function onKeyframesRule(rule) {
           keyframes.push(rule);
@@ -231,7 +249,7 @@
       propertyDataFromStyles: function(styles, element) {
         var props = {}, self = this;
         // generates a unique key for these matches
-        var o = [], i = 0;
+        var o = [];
         // note: active rules excludes non-matching @media rules
         styleUtil.forActiveRulesInStyles(styles, function(rule) {
           // TODO(sorvell): we could trim the set of rules at declaration
@@ -247,71 +265,80 @@
             if (matchesSelector.call(element, selectorToMatch)) {
               self.collectProperties(rule, props);
               // produce numeric key for these matches for lookup
-              addToBitMask(i, o);
+              addToBitMask(rule.index, o);
             }
           }
-          i++;
         });
         return {properties: props, key: o};
       },
 
+      walkHostAndRootProperties: function(scope, rule, style, callback) {
+        if (!rule.propertyInfo) {
+          self.decorateRule(rule);
+        }
+        if (!rule.propertyInfo.properties) {
+          return;
+        }
+        var hostScope = scope.is ?
+        styleTransformer._calcHostScope(scope.is, scope.extends) :
+        'html';
+        var parsedSelector = rule.parsedSelector;
+        var isRoot = parsedSelector === ':root';
+        var isHost = parsedSelector.indexOf(':host') === 0;
+        // build info is either in scope (when scope is an element) or in the style
+        // when scope is the default scope; note: this allows default scope to have
+        // mixed mode built and unbuilt styles.
+        var cssBuild = scope.__cssBuild || style.__cssBuild;
+        if (cssBuild === 'shady') {
+          // :root -> x-foo > *.x-foo for elements and html for custom-style
+          isRoot = parsedSelector === (hostScope + '> *.' + hostScope) || parsedSelector.indexOf('html') !== -1;
+          // :host -> x-foo for elements, but sub-rules have .x-foo in them
+          isHost = !isRoot && parsedSelector.indexOf(hostScope) === 0;
+        }
+        if (cssBuild === 'shadow') {
+          isRoot = parsedSelector === ':host > *' || parsedSelector === 'html';
+          isHost = isHost && !isRoot;
+        }
+        if (!isRoot && !isHost) {
+          return;
+        }
+        var selectorToMatch = hostScope;
+        if (isHost) {
+          // need to transform :host under ShadowDOM because `:host` does not work with `matches`
+          if (settings.useNativeShadow && !rule.transformedSelector) {
+            // transform :host into a matchable selector
+            rule.transformedSelector =
+            styleTransformer._transformRuleCss(
+              rule,
+              styleTransformer._transformComplexSelector,
+              scope.is,
+              hostScope
+            );
+          }
+          selectorToMatch = rule.transformedSelector || hostScope;
+        }
+        callback({
+          selector: selectorToMatch,
+          isHost: isHost,
+          isRoot: isRoot
+        });
+      },
+
       hostAndRootPropertiesForScope: function(scope) {
         var hostProps = {}, rootProps = {}, self = this;
         // note: active rules excludes non-matching @media rules
         styleUtil.forActiveRulesInStyles(scope._styles, function(rule, style) {
-          if (!rule.propertyInfo) {
-            self.decorateRule(rule);
-          }
-          if (!rule.propertyInfo.properties) {
-            return;
-          }
-          var hostScope = scope.is ?
-            styleTransformer._calcHostScope(scope.is, scope.extends) :
-             'html';
-          var parsedSelector = rule.parsedSelector;
-          var isRoot = parsedSelector === ':root';
-          var isHost = parsedSelector.indexOf(':host') === 0;
-          // build info is either in scope (when scope is an element) or in the style
-          // when scope is the default scope; note: this allows default scope to have
-          // mixed mode built and unbuilt styles.
-          var cssBuild = scope.__cssBuild || style.__cssBuild;
-          if (cssBuild === 'shady') {
-            // :root -> x-foo > *.x-foo for elements and html for custom-style
-            isRoot = parsedSelector === (hostScope + '> *.' + hostScope) || parsedSelector.indexOf('html') !== -1;
-            // :host -> x-foo for elements, but sub-rules have .x-foo in them
-            isHost = !isRoot && parsedSelector.indexOf(hostScope) === 0;
-          }
-          if (cssBuild === 'shadow') {
-            isRoot = parsedSelector === ':host > *' || parsedSelector === 'html';
-            isHost = isHost && !isRoot;
-          }
-          if (!isRoot && !isHost) {
-            return;
-          }
-          var selectorToMatch = hostScope;
-          if (isHost) {
-            // need to transform :host under ShadowDOM because `:host` does not work with `matches`
-            if (settings.useNativeShadow && !rule.transformedSelector) {
-              // transform :host into a matchable selector
-              rule.transformedSelector =
-                styleTransformer._transformRuleCss(
-                  rule,
-                  styleTransformer._transformComplexSelector,
-                  scope.is,
-                  hostScope
-                );
-            }
-            selectorToMatch = rule.transformedSelector || hostScope;
-          }
           // if scope is StyleDefaults, use _element for matchesSelector
-          var element = scope._element || scope;
-          if (matchesSelector.call(element, selectorToMatch)) {
-            if (isHost) {
-              self.collectProperties(rule, hostProps);
-            } else {
-              self.collectProperties(rule, rootProps);
+          self.walkHostAndRootProperties(scope, rule, style, function(info) {
+            var element = scope._element || scope;
+            if (matchesSelector.call(element, info.selector)) {
+              if (info.isHost) {
+                self.collectProperties(rule, hostProps);
+              } else {
+                self.collectProperties(rule, rootProps);
+              }
             }
-          }
+          });
         });
         return {rootProps: rootProps, hostProps: hostProps};
       },

diff --git a/src/standard/x-styling.html b/src/standard/x-styling.html
@@ -39,7 +39,7 @@
           }
         } else {
           this._ownStylePropertyNames = this._styles && this._styles.length ?
-            propertyUtils.decorateStyles(this._styles) :
+            propertyUtils.decorateStyles(this._styles, this) :
             null;
         }
       },
@@ -144,9 +144,18 @@
             _scopeSelector: this._scopeSelector,
             _styleProperties: this._styleProperties
           };
-          scopeData.key.customStyle = {};
-          this.mixin(scopeData.key.customStyle, this.customStyle);
-          scope._styleCache.store(this.is, info, scopeData.key, this._styles);
+          // the scope cache does not evaluate if @media rules, :host(), or :host-context() rules defined in this element have changed
+          // therefore, if we detect those rules, we opt-out of the scope cache
+          var scopeCacheable = this._styles.every(function(s) {
+            return s._cacheable;
+          });
+          if (scopeCacheable) {
+            scopeData.key.customStyle = {};
+            this.mixin(scopeData.key.customStyle, this.customStyle);
+            scope._styleCache.store(this.is, info, scopeData.key, this._styles);
+          }
+          // global cache key is all property values consumed in this element,
+          // we _can_ use the global cache with @media, :host(), and :host-context() rules, as _computeStyleProperties will determine if those properties have changed
           if (!globalCached) {
             // save in global cache
             styleCache.store(this.is, Object.create(info), this._ownStyleProperties,

diff --git a/test/.eslintrc.json b/test/.eslintrc.json
@@ -5,6 +5,7 @@
   "globals": {
     "assert": true,
     "sinon": true,
-    "WCT": true
+    "WCT": true,
+    "fixture": true
   }
 }
diff --git a/test/runner.html b/test/runner.html
@@ -64,6 +64,7 @@
       'unit/styling-cross-scope-apply.html?dom=shadow',
       'unit/styling-cross-scope-apply.html?dom=shadow&lazyRegister=true&useNativeCSSProperties=true',
       'unit/styling-cross-scope-unknown-host.html',
+      'unit/style-cache.html',
       'unit/custom-style.html',
       'unit/custom-style.html?lazyRegister=true&useNativeCSSProperties=true',
       'unit/custom-style.html?dom=shadow',

diff --git a/test/smoke/media-query-cache.html b/test/smoke/media-query-cache.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <meta charset="utf-8">
+  <title>Media Query and Caching</title>
+  <link rel="import" href="../../polymer.html">
+</head>
+
+<body>
+  <dom-module id="my-element">
+    <template>
+      <style>
+        :root {
+          --color: orange;
+        }
+
+        @media(max-width: 800px) {
+          :root {
+            --color: green;
+          }
+        }
+
+        div {
+          height: 50px;
+          width: 50px;
+          background: var(--color);
+          color: var(--text-color);
+        }
+
+        :host {
+          display: block;
+          background: blue;
+          height: 100px;
+          --text-color: white;
+        }
+
+        @media(max-width: 800px) {
+          :host {
+            background: red;
+          }
+        }
+
+        :host([foo]) {
+          --text-color: black;
+        }
+
+        :host-context([bar]) {
+          --text-color: lightgray;
+        }
+      </style>
+      <div>text</div>
+    </template>
+    <script>
+      Polymer({
+        is: 'my-element'
+      });
+    </script>
+  </dom-module>
+  <div bar>
+    <my-element foo></my-element>
+  </div>
+</body>
+
+</html>