From ed79071315382c87517ca70644848a422ab523de Mon Sep 17 00:00:00 2001
From: Kevin Schaaf <kschaaf@google.com>
Date: Fri, 20 Jul 2018 20:29:52 -0700
Subject: [PATCH] Updates based on code review.

---
 lib/elements/dom-if.html              |  2 ++
 lib/elements/dom-module.html          | 20 ++++++++-----
 lib/legacy/class.html                 | 28 ++++--------------
 lib/mixins/element-mixin.html         | 42 ++++++++++++++++++++-------
 lib/utils/templatize.html             |  3 ++
 test/unit/strict-template-policy.html | 25 ++++++++--------
 6 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/lib/elements/dom-if.html b/lib/elements/dom-if.html
index 0560c450cd..87b8b62760 100644
--- a/lib/elements/dom-if.html
+++ b/lib/elements/dom-if.html
@@ -255,6 +255,8 @@
         if (c$ && c$.length) {
           // use first child parent, for case when dom-if may have been detached
           let parent = c$[0].parentNode;
+          // Instance children may be disconnected from parents when dom-if
+          // detaches if a tree was innerHTML'ed
           if (parent) {
             for (let i=0, n; (i<c$.length) && (n=c$[i]); i++) {
               parent.removeChild(n);
diff --git a/lib/elements/dom-module.html b/lib/elements/dom-module.html
index 9fa8d88e86..e181787651 100644
--- a/lib/elements/dom-module.html
+++ b/lib/elements/dom-module.html
@@ -15,6 +15,12 @@
 
   let modules = {};
   let lcModules = {};
+  function setModule(id, module) {
+    // store id separate from lowercased id so that
+    // in all cases mixedCase id will stored distinctly
+    // and lowercase version is a fallback
+    modules[id] = lcModules[id.toLowerCase()] = module;
+  }
   function findModule(id) {
     return modules[id] || lcModules[id.toLowerCase()];
   }
@@ -124,16 +130,14 @@
     register(id) {
       id = id || this.id;
       if (id) {
-        if (Polymer.strictTemplatePolicy && findModule(id)) {
-          modules[id] = lcModules[id.toLowerCase()] = null;
-          throw new Error(`strictTemplatePolicy: dom-module ${id} registered twice`);
+        // Under strictTemplatePolicy, reject and null out any re-registered
+        // dom-module since it is ambiguous whether first-in or last-in is trusted 
+        if (Polymer.strictTemplatePolicy && findModule(id) !== undefined) {
+          setModule(id, null);
+          throw new Error(`strictTemplatePolicy: dom-module ${id} re-registered`);
         }
         this.id = id;
-        // store id separate from lowercased id so that
-        // in all cases mixedCase id will stored distinctly
-        // and lowercase version is a fallback
-        modules[id] = this;
-        lcModules[id.toLowerCase()] = this;
+        setModule(id, this);
         styleOutsideTemplateCheck(this);
       }
     }
diff --git a/lib/legacy/class.html b/lib/legacy/class.html
index b0b00c8aef..377146df72 100644
--- a/lib/legacy/class.html
+++ b/lib/legacy/class.html
@@ -152,29 +152,6 @@
           return info.observers;
         }
 
-        /**
-         * @return {HTMLTemplateElement} template for this class
-         */
-        static get template() {
-          // get template first from any imperative set in `info._template`
-          if (info._template !== undefined) {
-            return info._template;
-          }
-          const template = info._template ||
-            // next look in dom-module associated with this element's is.
-            Polymer.DomModule && Polymer.DomModule.import(this.is, 'template') ||
-            // next look for superclass template (note: use superclass symbol
-            // to ensure correct `this.is`)
-            Base.template ||
-            // finally fall back to `_template` in element's prototype.
-            this.prototype._template ||
-            null;
-          if (Polymer.strictTemplatePolicy && !template) {
-            throw new Error(`strictTemplatePolicy: expecting dom-module or null _template for ${this.is}`);
-          }
-          return template;
-        }
-
         /**
          * @return {void}
          */
@@ -373,6 +350,11 @@
         Polymer.LegacyElementMixin(HTMLElement));
       // decorate klass with registration info
       klass.is = info.is;
+      // if user provided template on info, make sure the static _template
+      // is set so the static template getter uses it
+      if (info._template !== undefined) {
+        klass._template = info._template;
+      }
       return klass;
     };
 
diff --git a/lib/mixins/element-mixin.html b/lib/mixins/element-mixin.html
index c51aceb3cd..d17a151342 100644
--- a/lib/mixins/element-mixin.html
+++ b/lib/mixins/element-mixin.html
@@ -275,6 +275,27 @@
       }
     }
 
+    /**
+     * Look up template from dom-module for element
+     * 
+     * @param {!string} is Element name to look up
+     * @return {!HTMLTemplateElement} Template found in dom module, or
+     *   undefined if not found
+     * @private
+     */
+     function getTemplateFromDomModule(is) {
+      let template = null;
+      if (is && Polymer.DomModule) {
+        template = Polymer.DomModule.import(is, 'template');
+        // Under strictTemplatePolicy, require any element with an `is`
+        // specified to have a dom-module
+        if (Polymer.strictTemplatePolicy && !template) {
+          throw new Error(`strictTemplatePolicy: expecting dom-module or null template for ${is}`);
+        }
+      }
+      return template;
+    }
+
     /**
      * @polymer
      * @mixinClass
@@ -373,7 +394,7 @@
        *   class MySubClass extends MySuperClass {
        *     static get template() {
        *       if (!memoizedTemplate) {
-       *         memoizedTemplate = super.template.cloneNode(true);
+       *         memoizedTemplate = MySuperClass.template.cloneNode(true);
        *         let subContent = document.createElement('div');
        *         subContent.textContent = 'This came from MySubClass';
        *         memoizedTemplate.content.appendChild(subContent);
@@ -386,16 +407,15 @@
        */
       static get template() {
         if (!this.hasOwnProperty(JSCompiler_renameProperty('_template', this))) {
-          const template = Polymer.DomModule && Polymer.DomModule.import(
-            /** @type {PolymerElementConstructor}*/ (this).is, 'template') ||
-            // note: implemented so a subclass can retrieve the super
-            // template; call the super impl this way so that `this` points
-            // to the superclass.
-            Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.template;
-          if (Polymer.strictTemplatePolicy && this.is && !template) {
-            throw new Error(`strictTemplatePolicy: expecting dom-module or null template for ${this.is}`);
-          }
-          this._template = template;
+          this._template =
+            // Look in dom-module associated with this element's is
+            getTemplateFromDomModule(/** @type {PolymerElementConstructor}*/ (this).is) ||
+            // Next look for superclass template (call the super impl this
+            // way so that `this` points to the superclass)
+            Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.template ||
+            // Finally, fall back to any _template set on prototype, e.g.
+            // via registered callback
+            this.prototype._template;
         }
         return this._template;
       }
diff --git a/lib/utils/templatize.html b/lib/utils/templatize.html
index 314613cc41..9235220c18 100644
--- a/lib/utils/templatize.html
+++ b/lib/utils/templatize.html
@@ -500,6 +500,9 @@
        * @suppress {invalidCasts}
        */
       templatize(template, owner, options) {
+        // Under strictTemplatePolicy, the templatized element must be owned
+        // by a (trusted) Polymer element, indicated by existence of _methodHost;
+        // e.g. for dom-if & dom-repeat in main document, _methodHost is null
         if (Polymer.strictTemplatePolicy && !owner._methodHost) {
           throw new Error('strictTemplatePolicy: template owner not trusted');
         }
diff --git a/test/unit/strict-template-policy.html b/test/unit/strict-template-policy.html
index 5720829f7f..0dfb32915d 100644
--- a/test/unit/strict-template-policy.html
+++ b/test/unit/strict-template-policy.html
@@ -68,6 +68,7 @@
       });
     </script>
   </dom-module>
+
   <div id="target"></div>
 
   <script>
@@ -93,24 +94,22 @@
         // Catch uncaught errors; note when running in iframe sometimes
         // Safari errors are thrown on the top window, sometimes not, so
         // catch in both places
-        let uncaughtError = null;
         window.onerror = window.top.onerror = function(err) {
-          if (!uncaughtError) {
-            uncaughtError = new Error(err);
+          if (!window.uncaughtError) {
+            window.uncaughtError = new Error(err);
           }
         };
         assert.throws(function() {
           fn();
           // Re-throw any uncaughtErrors
-          let err;
-          if ((err = (uncaughtError || window.uncaughtError))) {
-            throw new Error(err.message);
+          if (window.uncaughtError) {
+            throw new Error(window.uncaughtError.message);
           }
           // Force polyfill reactions and/or async template stamping
           Polymer.flush();
           // Re-throw any uncaughtErrors
-          if ((err = (uncaughtError || window.uncaughtError))) {
-            throw new Error(err.message);
+          if (window.uncaughtError) {
+            throw new Error(window.uncaughtError.message);
           }
         }, re);
       }
@@ -160,7 +159,7 @@
             '  </template>`' +
             '</dom-module>' +
             '<trusted-element></trusted-element>';
-        }, /trusted-element registered twice/);
+        }, /trusted-element re-registered/);
         const el = document.querySelector('trusted-element');
         assert.notOk(el && el.shadowRoot && el.shadowRoot.querySelector('#injected'));
       });
@@ -191,7 +190,7 @@
             '  </template>`' +
             '</dom-module>' +
             '<trusted-element-legacy></trusted-element-legacy>';
-        }, /trusted-element-legacy registered twice/);
+        }, /trusted-element-legacy re-registered/);
         const el = document.querySelector('trusted-element-legacy');
         assert.notOk(el && el.shadowRoot && el.shadowRoot.querySelector('#injected'));
       });
@@ -207,9 +206,9 @@
           is: 'has-no-template-legacy',
           _template: null
         });
-        let el = document.createElement('has-no-template');
+        let el = document.createElement('has-no-template-legacy');
         document.getElementById('target').appendChild(el);
-        assert.notOk(document.querySelector('has-no-template').shadowRoot);
+        assert.notOk(document.querySelector('has-no-template-legacy').shadowRoot);
       });
 
       test('element without explicit template throws', function() {
@@ -230,7 +229,7 @@
           });
           var el = document.createElement('has-no-template-throws-legacy');
           document.getElementById('target').appendChild(el);
-        }, /expecting dom-module or null _template/);
+        }, /expecting dom-module or null template/);
       });
 
     });