From 5c3b91754405b3cd68489852b1a02595a686b770 Mon Sep 17 00:00:00 2001 From: Daniel Freedman Date: Thu, 9 Jun 2016 13:54:43 -0700 Subject: [PATCH] Do not scope cache elements with media rules, :host(), or :host-context() selectors Mark style rules in parsing rather than runtime to make sure scope'd bitmask is consistent. Add tests for cache coherency. Fixes #3696 --- src/lib/css-parse.html | 12 ++- src/lib/style-properties.html | 139 ++++++++++++++++++------------ src/standard/x-styling.html | 17 +++- test/.eslintrc.json | 3 +- test/runner.html | 1 + test/smoke/media-query-cache.html | 65 ++++++++++++++ test/unit/style-cache.html | 115 ++++++++++++++++++++++++ 7 files changed, 288 insertions(+), 64 deletions(-) create mode 100644 test/smoke/media-query-cache.html create mode 100644 test/unit/style-cache.html diff --git a/src/lib/css-parse.html b/src/lib/css-parse.html index d760d7557f..7bee03bbf5 100644 --- 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 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 index 6f94bc9a6a..94c7c76bf7 100644 --- 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 index b210fb8fe4..f6ee2a64fc 100644 --- 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 index 92d6c6b139..c7280c2b63 100644 --- 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 new file mode 100644 index 0000000000..fb9f522d41 --- /dev/null +++ b/test/smoke/media-query-cache.html @@ -0,0 +1,65 @@ + + + + + + Media Query and Caching + + + + + + + + +
+ +
+ + + diff --git a/test/unit/style-cache.html b/test/unit/style-cache.html new file mode 100644 index 0000000000..9ca1243b6d --- /dev/null +++ b/test/unit/style-cache.html @@ -0,0 +1,115 @@ + + + + + + + + + + + + + + + + + + + + + +