Merge pull request #1570 from Polymer/x-style-fixes

X style fixes

src/lib/style-cache.html

@@ -18,26 +18,25 @@
 
     MAX: 100,
 
-    store: function(is, styleData, keyProperties, keyStyles) {
-      styleData.properties = keyProperties;
-      styleData.styles = keyStyles;
+    store: function(is, data, keyValues, keyStyles) {
+      data.keyValues = keyValues;
+      data.styles = keyStyles;
       var s$ = this.cache[is] = this.cache[is] || [];
-      s$.push(styleData);
+      s$.push(data);
       if (s$.length > this.MAX) {
         s$.shift();
       }
     },
 
-    retrieve: function(is, keyProperties, keyStyles) {
+    retrieve: function(is, keyValues, keyStyles) {
       var cache = this.cache[is];
       if (cache) {
         // look through cache backwards as most recent push is last.
-        for (var i=cache.length-1, s; i >= 0; i--) {
-          s = cache[i];
-          key = s;
-          if (this._objectsEqual(keyProperties, key.properties) &&
-            keyStyles === key.styles) {
-            return s;
+        for (var i=cache.length-1, data; i >= 0; i--) {
+          data = cache[i];
+          if (keyStyles === data.styles &&
+              this._objectsEqual(keyValues, data.keyValues)) {
+            return data;
           }
         }
       }
@@ -47,13 +46,18 @@
       this.cache = {};
     },
 
-    // note, intentionally a simple check here, should be good enough
+    // note, this is intentially limited to support just the cases we need
+    // right now. The objects we're checking here are either objects that must 
+    // always have the same keys OR arrays.
     _objectsEqual: function(target, source) {
       for (var i in target) {
         if (target[i] !== source[i]) {
           return false;
         }
       }
+      if (Array.isArray(target)) {
+        return target.length === source.length;
+      }
       return true;
     }
 

src/lib/style-defaults.html

@@ -25,6 +25,7 @@
 
       applyCss: function(cssText) {
         this.style.textContent += cssText;
+        styleUtil.clearStyleRules(this.style);
         this._properties = null;
       },
 
@@ -33,7 +34,6 @@
       get _styleProperties() {
         if (!this._properties) {
           // force rules to reparse since they may be out of date
-          styleUtil.clearStyleRules(this.style);
           styleProperties.decorateStyles(this._styles);
           // NOTE: reset cache for own properties; it may have been set when
           // an element in an import applied styles (e.g. custom-style)

src/lib/style-properties.html

@@ -175,8 +175,10 @@
 
       // Test if the rules in these styles matche the given `element` and if so,
       // collect any custom properties into `props`.
-      propertiesFromStyles: function(styles, element) {
+      propertyDataFromStyles: function(styles, element) {
         var props = {}, self = this;
+        // generates a unique key for these matches
+        var o = [], i = 0;
         styleUtil.forRulesInStyles(styles, function(rule) {
           // TODO(sorvell): we could trim the set of rules at declaration 
           // time to only include ones that have properties
@@ -186,9 +188,12 @@
           if (element && rule.propertyInfo.properties &&
               matchesSelector.call(element, rule.selector)) {
             self.collectProperties(rule, props);
+            // produce numeric key for these matches for lookup
+            addToBitMask(i, o);
           }
+          i++;
         });
-        return props;
+        return {properties: props, key: o};
       },
 
       // Test if a rule matches root crteria (:host or :root) and if so,
@@ -312,6 +317,12 @@
 
     };
 
+    function addToBitMask(n, bits) {
+      var o = parseInt(n / 32);
+      var v = 1 << (n % 32);
+      bits[o] = (bits[o] || 0) | v;
+    }
+
   })();
 
 </script>

src/standard/x-styling.html

@@ -59,10 +59,10 @@
         if (!scope._styleCache) {
           scope._styleCache = new Polymer.StyleCache();
         }
-        var scopeProps = propertyUtils
-          .propertiesFromStyles(scope._styles, this);
+        var scopeData = propertyUtils
+          .propertyDataFromStyles(scope._styles, this);
         // look in scope cache
-        info = scope._styleCache.retrieve(this.is, scopeProps, 
+        info = scope._styleCache.retrieve(this.is, scopeData.key, 
           this._styles);
         // compute style properties (fast path, if cache hit)
         var scopeCached = Boolean(info);
@@ -71,7 +71,7 @@
           // scope cache because they are only for this scope.
           this._styleProperties = info._styleProperties;
         } else {
-          this._computeStyleProperties(scopeProps);  
+          this._computeStyleProperties(scopeData.properties);  
         }
         this._computeOwnStyleProperties();
         // cache miss, do work!
@@ -93,7 +93,7 @@
             _scopeSelector: this._scopeSelector,
             _styleProperties: this._styleProperties
           }
-          scope._styleCache.store(this.is, info, scopeProps, this._styles);
+          scope._styleCache.store(this.is, info, scopeData.key, this._styles);
           if (!globalCached) {
             // save in global cache
             styleCache.store(this.is, Object.create(info), this._ownStyleProperties,
@@ -114,7 +114,7 @@
         var props = Object.create(scope._styleProperties);
         // mixin properties matching this element in scope
         scopeProps = scopeProps || 
-          propertyUtils.propertiesFromStyles(scope._styles, this);
+          propertyUtils.propertyDataFromStyles(scope._styles, this).properties;
         this.mixin(props, scopeProps);
         // finally mixin properties inherent to this element
         this.mixin(props, propertyUtils.rootPropertiesFromStyles(this._styles));

test/unit/styling-cross-scope-var.html

@@ -76,7 +76,7 @@
       :host {
         display: block;
         padding: 8px;
-        --gc3-scope: 5px solid green;
+        --gc4-scope: 5px solid green;
       }
 
       #me {
@@ -87,8 +87,8 @@
         --grand-child-scope-var: 4px solid seagreen;
       }
 
-      #gc3 {
-        --grand-child-scope-var: var(--gc3-scope);
+      #gc4 {
+        --grand-child-scope-var: var(--gc4-scope);
       }
     </style>
 
@@ -97,6 +97,7 @@
       <x-grand-child-scope id="gc1"></x-grand-child-scope>
       <x-grand-child-scope id="gc2"></x-grand-child-scope>
       <x-grand-child-scope id="gc3"></x-grand-child-scope>
+      <x-grand-child-scope id="gc4"></x-grand-child-scope>
     </template>
     <script>
     HTMLImports.whenReady(function() {
@@ -236,6 +237,11 @@
      assertStylePropertyValue(gc2._styleProperties, '--scope-var', '1px');
      assertStylePropertyValue(gc2._styleProperties, '--child-scope-var', '2px');
      assertStylePropertyValue(gc2._styleProperties, '--grand-child-scope-var', '4px');
+     //
+     var gc3 = child.$.gc3;
+     assertStylePropertyValue(gc3._styleProperties, '--scope-var', '1px');
+     assertStylePropertyValue(gc3._styleProperties, '--child-scope-var', '2px');
+     assertStylePropertyValue(gc3._styleProperties, '--grand-child-scope-var', '3px');
 
   });
 
@@ -251,9 +257,9 @@
   });
 
   test('variable can be set to another variable', function() {
-    var gc3 = styled.$.child.$.gc3;
-    assertStylePropertyValue(gc3._styleProperties, '--grand-child-scope-var', '5px');
-    assertComputed(gc3.$.me, '5px');
+    var gc4 = styled.$.child.$.gc4;
+    assertStylePropertyValue(gc4._styleProperties, '--grand-child-scope-var', '5px');
+    assertComputed(gc4.$.me, '5px');
   });
 
   test('variable default values can be assigned to other variables', function() {