From 819652eb9f37166744a3bcb7628696a0ea0f6e90 Mon Sep 17 00:00:00 2001
From: Steven Orvell <sorvell@google.com>
Date: Thu, 19 Oct 2017 12:56:30 -0700
Subject: [PATCH] Allow style elements to be separate in the element template.

Previously Polymer collapsed any included shared styles into 1 style element. This prevents the browser from recognizing shared style elements as similar and optimizing them. With this change, we no longer collapse styles. Note, included styles are still currently collapsed by ShadyCSS but the polyfill will likely not do this in the future.
---
 lib/mixins/dir-mixin.html     |   4 +-
 lib/mixins/element-mixin.html |  55 +++++++---
 lib/utils/style-gather.html   | 192 ++++++++++++++++++++++++++++------
 test/runner.html              |   3 +-
 test/unit/multi-style.html    | 115 ++++++++++++++++++++
 5 files changed, 315 insertions(+), 54 deletions(-)
 create mode 100644 test/unit/multi-style.html

diff --git a/lib/mixins/dir-mixin.html b/lib/mixins/dir-mixin.html
index 8773d83faa..075da5f581 100644
--- a/lib/mixins/dir-mixin.html
+++ b/lib/mixins/dir-mixin.html
@@ -104,8 +104,8 @@
          * @override
          * @suppress {missingProperties} Interfaces in closure do not inherit statics, but classes do
          */
-        static _processStyleText(is, template, baseURI) {
-          let cssText = super._processStyleText(is, template, baseURI);
+        static _processStyleText(cssText, baseURI) {
+          cssText = super._processStyleText(cssText, baseURI);
           cssText = this._replaceDirInCssText(cssText);
           return cssText;
         }
diff --git a/lib/mixins/element-mixin.html b/lib/mixins/element-mixin.html
index 2ed3e29b0d..842aacad21 100644
--- a/lib/mixins/element-mixin.html
+++ b/lib/mixins/element-mixin.html
@@ -399,6 +399,37 @@
       }
     }
 
+    /**
+     * Process all style elements in the element template. Styles with the
+     * `include` attribute are processed such that any styles in
+     * the associated "style modules" are included in the element template.
+     * @param {PolymerElementConstructor} klass Element class
+     * @param {!HTMLTemplateElement} template Template to process
+     * @param {string} is Name of element
+     * @param {string} ext Name of native element
+     * @param {string} baseURI Base URI for element
+     * @private
+     */
+    function processElementStyles(klass, template, is, ext, baseURI) {
+      const styles = Polymer.StyleGather.stylesFromModuleImports(is).concat(
+        Polymer.StyleGather.stylesFromTemplate(template));
+      let templateStyles = template.content.querySelectorAll('style');
+      let lastStyle = templateStyles[templateStyles.length-1];
+      for (let i=0; i < styles.length; i++) {
+        let s = styles[i];
+        // if the style is not in this template, it's been "included" and
+        // we put a clone of it in the template.
+        if (s.getRootNode() != template.content) {
+          s = s.cloneNode(true);
+          template.content.insertBefore(s, lastStyle);
+        }
+        s.textContent = klass._processStyleText(s.textContent, baseURI);
+      }
+      if (window.ShadyCSS) {
+        window.ShadyCSS.prepareTemplate(template, is, ext);
+      }
+    }
+
     /**
      * @polymer
      * @mixinClass
@@ -578,17 +609,15 @@
       }
 
       /**
-       * Gather style text for the template
+       * Gather style text for a style element in the template.
        *
-       * @param {string} is Tag name for this element
-       * @param {!HTMLTemplateElement} template Template to process
+       * @param {string} cssText Text containing styling to process
        * @param {string} baseURI Base URI to rebase CSS paths against
-       * @return {string} The combined CSS text
+       * @return {string} The processed CSS text
        * @protected
        */
-      static _processStyleText(is, template, baseURI) {
-        return Polymer.StyleGather.cssFromModuleImports(is) +
-          Polymer.StyleGather.cssFromTemplate(template, baseURI);
+      static _processStyleText(cssText, baseURI) {
+        return Polymer.ResolveUrl.resolveCss(cssText, baseURI);
       }
 
       /**
@@ -607,16 +636,8 @@
           template.__polymerFinalized = true;
           const importPath = this.importPath;
           const baseURI = importPath ? Polymer.ResolveUrl.resolveUrl(importPath) : '';
-          // support `include="module-name"`
-          let cssText = this._processStyleText(is, template, baseURI);
-          if (cssText) {
-            let style = document.createElement('style');
-            style.textContent = cssText;
-            template.content.insertBefore(style, template.content.firstChild);
-          }
-          if (window.ShadyCSS) {
-            window.ShadyCSS.prepareTemplate(template, is, ext);
-          }
+          // e.g. support `include="module-name"`, and ShadyCSS
+          processElementStyles(this, template, is, ext, baseURI);
           this.prototype._bindTemplate(template);
         }
       }
diff --git a/lib/utils/style-gather.html b/lib/utils/style-gather.html
index 8e9f61c65a..62694530e0 100644
--- a/lib/utils/style-gather.html
+++ b/lib/utils/style-gather.html
@@ -39,15 +39,154 @@
   const StyleGather = {
 
     /**
+     * Returns a list of <style> elements in a space-separated list of `dom-module`s.
+     *
+     * @memberof Polymer.StyleGather
+     * @param {string} moduleIds List of dom-module id's within which to
+     * search for css.
+     * @return {array} Array of contained <style> elements
+     * @this {StyleGather}
+     */
+     stylesFromModules(moduleIds) {
+      const modules = moduleIds.trim().split(/\s+/);
+      const styles = [];
+      for (let i=0; i < modules.length; i++) {
+        styles.push(...this.stylesFromModule(modules[i]));
+      }
+      return styles;
+    },
+
+    /**
+     * Returns a list of <style> elements in a given `dom-module`.
+     * Styles in a `dom-module` can come either from `<style>`s within the
+     * first `<template>`, or else from one or more
+     * `<link rel="import" type="css">` links outside the template.
+     *
+     * @memberof Polymer.StyleGather
+     * @param {string} moduleId dom-module id to gather styles from
+     * @return {array} Array of contained styles.
+     * @this {StyleGather}
+     */
+    stylesFromModule(moduleId) {
+      const m = importModule(moduleId);
+      if (m && m._styles === undefined) {
+        const styles = [];
+        // module imports: <link rel="import" type="css">
+        styles.push(...this._stylesFromModuleImports(m));
+        // include css from the first template in the module
+        const template = m.querySelector('template');
+        if (template) {
+          styles.push(...this.stylesFromTemplate(template,
+            /** @type {templateWithAssetPath} */(m).assetpath));
+        }
+        m._styles = styles;
+      }
+      if (!m) {
+        console.warn('Could not find style data in module named', moduleId);
+      }
+      return m ? m._styles : [];
+    },
+
+    /**
+     * Returns the `<style>` elements within a given template.
+     *
+     * @memberof Polymer.StyleGather
+     * @param {HTMLTemplateElement} template Template to gather styles from
+     * @return {array} Array of styles
+     * @this {StyleGather}
+     */
+    stylesFromTemplate(template, baseURI) {
+      if (!template._styles) {
+        const styles = [];
+        // if element is a template, get content from its .content
+        const e$ = template.content.querySelectorAll('style');
+        for (let i=0; i < e$.length; i++) {
+          let e = e$[i];
+          // support style sharing by allowing styles to "include"
+          // other dom-modules that contain styling
+          let include = e.getAttribute(INCLUDE_ATTR);
+          if (include) {
+            styles.push(...this.stylesFromModules(include));
+          }
+          if (window.ShadyDOM && e.hasAttribute(SHADY_UNSCOPED_ATTR)) {
+            e.textContent = baseURI ?
+              Polymer.ResolveUrl.resolveCss(e.textContent, baseURI) : e.textContent;
+            document.head.insertBefore(e, document.head.firstChild);
+          } else {
+            styles.push(e);
+          }
+        }
+        template._styles = styles;
+      }
+      return template._styles;
+    },
+
+    /**
+     * Returns a list of <style> elements  from stylesheets loaded via `<link rel="import" type="css">` links within the specified `dom-module`.
+     *
+     * @memberof Polymer.StyleGather
+     * @param {string} moduleId Id of `dom-module` to gather CSS from
+     * @return {array} Array of contained styles.
+     * @this {StyleGather}
+     */
+     stylesFromModuleImports(moduleId) {
+      let m = importModule(moduleId);
+      return m ? this._stylesFromModuleImports(m) : [];
+    },
+
+    /**
+     * @memberof Polymer.StyleGather
+     * @this {StyleGather}
+     * @param {!HTMLElement} module dom-module element that could contain `<link rel="import" type="css">` styles
+     * @return {array} Array of contained styles
+     */
+    _stylesFromModuleImports(module) {
+      const styles = [];
+      const p$ = module.querySelectorAll(MODULE_STYLE_LINK_SELECTOR);
+      for (let i=0; i < p$.length; i++) {
+        let p = p$[i];
+        if (p.import) {
+          const importDoc = p.import;
+          if (!importDoc._style) {
+            // NOTE: polyfill affordance.
+            // under the HTMLImports polyfill, there will be no 'body',
+            // but the import pseudo-doc can be used directly.
+            let container = importDoc.body ? importDoc.body : importDoc;
+            const importCss = Polymer.ResolveUrl.resolveCss(container.textContent,
+              importDoc.baseURI);
+            const style = document.createElement('style');
+            if (p.hasAttribute(SHADY_UNSCOPED_ATTR)) {
+              style.setAttribute(SHADY_UNSCOPED_ATTR, '');
+            }
+            style.textContent = importCss;
+            importDoc._style = style;
+          }
+          // support the shady-unscoped promoting styles to main document
+          if (window.ShadyDOM &&
+            importDoc._style.hasAttribute(SHADY_UNSCOPED_ATTR) &&
+            !importDoc._style.parentNode) {
+            document.head.insertBefore(importDoc._style, document.head.firstChild);
+          } else {
+            styles.push(importDoc._style);
+          }
+        }
+      }
+      return styles;
+    },
+
+    /**
+     *
      * Returns CSS text of styles in a space-separated list of `dom-module`s.
+     * Note: This method is deprecated, use `stylesFromModules` instead.
      *
+     * @deprecated
      * @memberof Polymer.StyleGather
      * @param {string} moduleIds List of dom-module id's within which to
      * search for css.
      * @return {string} Concatenated CSS content from specified `dom-module`s
      * @this {StyleGather}
      */
-    cssFromModules(moduleIds) {
+     cssFromModules(moduleIds) {
       let modules = moduleIds.trim().split(/\s+/);
       let cssText = '';
       for (let i=0; i < modules.length; i++) {
@@ -63,7 +202,9 @@
      * template.
      *
      * Any `<styles>` processed are removed from their original location.
+     * Note: This method is deprecated, use `styleFromModule` instead.
      *
+     * @deprecated
      * @memberof Polymer.StyleGather
      * @param {string} moduleId dom-module id to gather styles from
      * @return {string} Concatenated CSS content from specified `dom-module`
@@ -91,7 +232,9 @@
      * Returns CSS text of `<styles>` within a given template.
      *
      * Any `<styles>` processed are removed from their original location.
+     * Note: This method is deprecated, use `styleFromTemplate` instead.
      *
+     * @deprecated
      * @memberof Polymer.StyleGather
      * @param {HTMLTemplateElement} template Template to gather styles from
      * @param {string} baseURI Base URI to resolve the URL against
@@ -100,17 +243,13 @@
      */
     cssFromTemplate(template, baseURI) {
       let cssText = '';
+      const e$ = this.stylesFromTemplate(template, baseURI);
       // if element is a template, get content from its .content
-      let e$ = template.content.querySelectorAll('style');
       for (let i=0; i < e$.length; i++) {
         let e = e$[i];
-        // support style sharing by allowing styles to "include"
-        // other dom-modules that contain styling
-        let include = e.getAttribute(INCLUDE_ATTR);
-        if (include) {
-          cssText += this.cssFromModules(include);
+        if (e.parentNode) {
+          e.parentNode.removeChild(e);
         }
-        e.parentNode.removeChild(e);
         const styleCss = baseURI ?
           Polymer.ResolveUrl.resolveCss(e.textContent, baseURI) : e.textContent;
         if (window.ShadyDOM && e.hasAttribute(SHADY_UNSCOPED_ATTR)) {
@@ -127,6 +266,10 @@
      * Returns CSS text from stylesheets loaded via `<link rel="import" type="css">`
      * links within the specified `dom-module`.
      *
+     * Note: This method is deprecated, use `stylesFromModuleImports` instead.
+     *
+     * @deprecated
+     *
      * @memberof Polymer.StyleGather
      * @param {string} moduleId Id of `dom-module` to gather CSS from
      * @return {string} Concatenated CSS content from links in specified `dom-module`
@@ -136,41 +279,22 @@
       let m = importModule(moduleId);
       return m ? this._cssFromModuleImports(m) : '';
     },
+
     /**
+     * @deprecated
      * @memberof Polymer.StyleGather
      * @this {StyleGather}
      * @param {!HTMLElement} module dom-module element that could contain `<link rel="import" type="css">` styles
      * @return {string} Concatenated CSS content from links in the dom-module
      */
-    _cssFromModuleImports(module) {
+     _cssFromModuleImports(module) {
       let cssText = '';
-      let p$ = module.querySelectorAll(MODULE_STYLE_LINK_SELECTOR);
-      for (let i=0; i < p$.length; i++) {
-        let p = p$[i];
-        if (p.import) {
-          let importDoc = p.import;
-          // NOTE: polyfill affordance.
-          // under the HTMLImports polyfill, there will be no 'body',
-          // but the import pseudo-doc can be used directly.
-          let container = importDoc.body ? importDoc.body : importDoc;
-          const importCss = Polymer.ResolveUrl.resolveCss(container.textContent,
-            importDoc.baseURI);
-          // support the shady-unscoped promoting styles to main document
-          if (window.ShadyDOM && p.hasAttribute(SHADY_UNSCOPED_ATTR)) {
-            if (!unscopedStyleImportsMap.has(importDoc)) {
-              unscopedStyleImportsMap.set(importDoc);
-              const style = document.createElement('style');
-              style.setAttribute(SHADY_UNSCOPED_ATTR, '');
-              style.textContent = importCss;
-              document.head.insertBefore(style, document.head.firstChild);
-            }
-          } else {
-            cssText += importCss;
-          }
-        }
+      let styles = this._stylesFromModuleImports(module);
+      for (let i=0; i < styles.length; i++) {
+        cssText += styles[i].textContent;
       }
       return cssText;
-    }
+    },
   };
 
   Polymer.StyleGather = StyleGather;
diff --git a/test/runner.html b/test/runner.html
index bc8e1f6217..4ed70fb204 100644
--- a/test/runner.html
+++ b/test/runner.html
@@ -74,7 +74,8 @@
       'unit/mixin-behaviors.html',
       'unit/render-status.html',
       'unit/dir.html',
-      'unit/shady-unscoped-style.html'
+      'unit/shady-unscoped-style.html',
+      'unit/multi-style.html'
     ];
 
     // http://eddmann.com/posts/cartesian-product-in-javascript/
diff --git a/test/unit/multi-style.html b/test/unit/multi-style.html
new file mode 100644
index 0000000000..8afe922b47
--- /dev/null
+++ b/test/unit/multi-style.html
@@ -0,0 +1,115 @@
+<!doctype html>
+<html>
+<head>
+  <script src="../../../web-component-tester/browser.js"></script>
+  <link rel="import" href="../../polymer.html">
+</head>
+<body>
+  <dom-module id="shared-styles">
+    <template>
+      <style>
+        #zot {
+          border: 1px solid black;
+        }
+      </style>
+    </template>
+  </dom-module>
+
+  <dom-module id="my-element">
+
+    <template>
+      <style>
+        #foo {
+          border: 2px solid green;
+        }
+      </style>
+      <style>
+        #bar {
+          border: 3px solid orange;
+        }
+        </style>
+      <div id="foo">foo</div>
+      <div id="bar">bar</div>
+    </template>
+  </dom-module>
+
+  <dom-module id="my-element2">
+
+    <template>
+      <style include="shared-styles">
+        #zug {
+          border: 4px solid red;
+        }
+      </style>
+      <div id="zug">zug</div>
+      <div id="zot">zot</div>
+    </template>
+
+    <script>
+
+
+    </script>
+
+  </dom-module>
+
+  <script>
+  suite('elements including multiple styles', function() {
+
+    suiteSetup(function() {
+      const skip = window.ShadyDOM || !window.Polymer;
+      if (skip) {
+        this.skip();
+      } else {
+        class MyElement extends Polymer.Element {
+          static get is() { return 'my-element'; }
+        }
+        customElements.define(MyElement.is, MyElement);
+
+        class MyElement2 extends MyElement {
+          static get is() { return 'my-element2'; }
+          static get template() {
+            Object.getPrototypeOf(this.prototype).constructor.template;
+            const t = Polymer.DomModule.import(MyElement2.is, 'template');
+            const superT = MyElement.template;
+            t.content.insertBefore(superT.content.cloneNode(true), t.content.firstElementChild.nextElementSibling);
+            return t;
+          }
+        }
+        customElements.define(MyElement2.is, MyElement2);
+      }
+    });
+
+    function assertComputed(element, value, property, pseudo) {
+      var computed = getComputedStyle(element, pseudo);
+      property = property || 'border-top-width';
+      if (Array.isArray(value)) {
+        assert.oneOf(computed[property], value, 'computed style incorrect for ' + property);
+      } else {
+        assert.equal(computed[property], value, 'computed style incorrect for ' + property);
+      }
+    }
+
+    test('multiple styles preserved separately in element root and apply', function() {
+      const e = document.createElement('my-element');
+      document.body.appendChild(e);
+      const styles = e.shadowRoot.querySelectorAll('style');
+      assert.equal(styles.length, 2);
+      assertComputed(e.$.foo, '2px');
+      assertComputed(e.$.bar, '3px');
+    });
+
+    test('super class and included styles preserved separately in element root and apply', function() {
+      const e = document.createElement('my-element2');
+      document.body.appendChild(e);
+      const styles = e.shadowRoot.querySelectorAll('style');
+      assert.equal(styles.length, 4);
+      assertComputed(e.$.foo, '2px');
+      assertComputed(e.$.bar, '3px');
+      assertComputed(e.$.zug, '4px');
+      assertComputed(e.$.zot, '1px');
+    });
+
+  });
+  </script>
+</body>
+</html>
\ No newline at end of file